Repository navigation
fix(capture): keep the overlay out of scan captures; release idle HDR sessions - #78
Merged
Merged
Conversation
… sessions - Screen captures include layered windows (verified: GDI CopyFromScreen returns a topmost WS_EX_LAYERED window's pixels), so a tooltip still showing from the previous scan was read back into the next capture, drawn over the very item being re-scanned or its neighbours. The overlay window now uses WDA_EXCLUDEFROMCAPTURE (verified: the same probe then reads the pixels underneath; the running overlay reports affinity 0x11). ToolTip.ExcludeFromCapture in config.cfg (default on) turns this off for users who want tooltips in screenshots/streams. - Opt-in HDR capture kept its desktop-duplication sessions forever after the first scan; an active duplication makes DWM copy every frame. Sessions are now released after 30 s without an HDR capture. Claude-Session: https://claude.ai/code/session_01QFKiA2aswHyp8SnpraLxE2
|
Warning Review limit reachedEnable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Next included review available in 16 minutes. View limit details
📝 Summary
Merge Risk: 🔵 Low · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 18.75% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 32 functions across 10 files. (8 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly summarizes both primary changes: excluding the overlay from capture and releasing idle HDR sessions. Description check The description directly explains the capture issue, HDR session changes, settings behavior, validation, and known limitations. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 18.75% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 32 functions across 10 files. (8 skipped: 8 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
ExcludeFromCapture keeps the overlay out of scans, but it also hides tooltips from OBS and screenshots, and it could only be changed by editing the config file. Add a General settings switch (all locales), and make the overlay re-apply the display affinity on the next tooltip when the setting changes. Claude-Session: https://claude.ai/code/session_01AJBRm45o94V3UvbN5ZbLUD
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…ases - IdleRelease checks the deadline under the caller's lock, so a timer callback queued behind a capture re-arms instead of tearing down the session just used. - A failed SetWindowDisplayAffinity is no longer recorded as applied; the next tooltip retries and the warning is logged once per failure streak. - Tests cover the countdown restart, the queued-callback race, cancel, and the capture exclusion success, failure, and unsupported-OS paths. Claude-Session: https://claude.ai/code/session_01AJBRm45o94V3UvbN5ZbLUD
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
CopyFromScreen(SourceCopy)returns the pixels of a topmostWS_EX_LAYEREDwindow. A tooltip still showing from the previous scan (1.5 s) is therefore read back into the next icon/name capture, drawn over the item being re-scanned or its neighbours. Plausible contributor to "inconsistent icon scans".Related: #34 (repeated icon scans returning the same item at 0.94 while different items were clicked is consistent with the previous tooltip being captured; EFT's own hover tooltip is a separate cause).
What
SetWindowDisplayAffinity(WDA_EXCLUDEFROMCAPTURE)(Win10 2004+; failure is logged and leaves today's behaviour).ToolTip.ExcludeFromCapture(default on) is a Settings → General switch, "Hide tooltips from screenshots and recordings", in all 7 locales. Changing it takes effect the next time a tooltip appears.IdleReleasehelper; HDR sessions are released after 30 s without an HDR capture and recreated on the next one.Validation
0x11affinity → background pixels captured.0x11, main window unchanged; clean exit.IdleReleaseTests(release after idle, countdown restarts on use, no release before use/after cancel), config default/disable test.dotnet test412/412 on this branch.Tooltip_capture_exclusion_can_be_turned_off_and_back_on_from_settingschecks that turning the setting off and back on updates the runtime value and saves it each time. The WebView2 smoke test checks that the switch is visible. All 7 locale files have the same 385 keys.EnableWindowsTargeting) for the app, unit test, and UI test projects with 0 errors, and CSharpier is clean. Tests were not run there because Linux has no WindowsDesktop runtime.IdleReleasetimer callback can still release sessions just after aTouch(). The only cost is one extra session rebuild on the next HDR capture.Decision for review
Not done (needs in-game evidence)
AlwaysOnTopdefault true, layered) over a fullscreen game: its cost lands in DWM/the game's flip model, which the idle benchmark cannot see. No change without a measurement.https://claude.ai/code/session_01QFKiA2aswHyp8SnpraLxE2
https://claude.ai/code/session_01AJBRm45o94V3UvbN5ZbLUD