Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change focus indicator to polling #959

Merged
merged 8 commits into from
Aug 1, 2024
Merged

Conversation

dalyIsaac
Copy link
Owner

@dalyIsaac dalyIsaac commented Jul 31, 2024

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 Verbose to 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.

@dalyIsaac dalyIsaac added bug Something isn't working focus indicator Whim.FocusIndicator labels Jul 31, 2024
@dalyIsaac dalyIsaac linked an issue Jul 31, 2024 that may be closed by this pull request
@dalyIsaac dalyIsaac marked this pull request as ready for review July 31, 2024 21:38
Copy link

codecov bot commented Jul 31, 2024

Codecov Report

Attention: Patch coverage is 90.32258% with 3 lines in your changes missing coverage. Please review.

Project coverage is 81.06%. Comparing base (b777779) to head (f26306c).

Files Patch % Lines
src/Whim/Store/Store.cs 50.00% 2 Missing ⚠️
src/Whim/Native/NativeManager.cs 0.00% 1 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

@dalyIsaac dalyIsaac merged commit 5432198 into main Aug 1, 2024
6 checks passed
@dalyIsaac dalyIsaac deleted the focus-indicator-polling branch August 1, 2024 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working focus indicator Whim.FocusIndicator
Projects
None yet
1 participant