Change focus indicator to polling #959
Merged
Conversation
This file contains 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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #959 +/- ##
==========================================
+ Coverage 81.03% 81.06% +0.03%
==========================================
Files 274 274
Lines 11503 11523 +20
Branches 1291 1293 +2
==========================================
+ Hits 9321 9341 +20
Misses 2008 2008
Partials 174 174 ☔ View full report in Codecov by Sentry. |
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.
Previously, the focus indicator plugin would show and hide based on events from Whim's core. However, this had the downside that the focus indicator could suffer from flickering. This was a result of the multi-threading work for #859, as all events were being processed (as opposed to relying on the STA always providing the latest event).
The focus indicator has now switched to a polling model running on a long-running task (i.e., a different thread). There is a hardcoded sleep for 16ms between each poll (roughly corresponding to 60fps). Polling avoids the continual handling of each event Windows emits and instead relies on the current state of Whim. This should resolve #944.
The polling has the downside of significantly increasing the logs which are generated. Some of the more common logs have been moved to
Verboseto ameliorate this.The polling has the additional benefit of having the indicator follow the window as it moves around the screen. This was not feasible with the event-based model. The focus indicator also now uses the actual window size, rather than the size which Whim stores. This is good for naughty windows (like Logi Options+) which prevent Whim from resizing them.
This PR also changes the focus indicator to only apply the indicator color to the border of the indicator window, resolving #669. This does not use an actual border for each window - it still uses the prior approach of using a Whim-managed window. From the brief research I did, it did not seem that Windows really supports custom borders without potentially messing with the window itself. This does not resolve #908.