Fix some stutters when dragging maps #4003
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
`ZoneRenderer` debounces repaints, so it isn't necessary to throttle updates just to avoid excessive repaints. By not throttling updates, the next `ZoneRenderer` paint will tend to be more up-to-date than it would be with the throttling.
It can be the callers responsibility instead to clear the image if necessary. As it happens, current usage already fills the buffers completely and does not require a prior clearing: - `renderLumensOverlay` and `renderLightOverlay` fill with a solid colour first. - `renderFog` and `paintComponent` render opaquely to the entire buffer.
The previous implementation would always apply a full delay to any dispatch request, regardless of how long it had been since the last scheduling of a task. This meant that execution A could complete, then execution B could be requested _just before_ the end of the delay time and still be delayed the full time, resulting a double delay. This is noticeable when events are coming in rapid succession (e.g., mouse dragging) as what should be a steady stream of task runs is instead choppy and inconsistent. The new implementation only delays tasks that have come in close succession to a previous task. If a long time has passed, no delay is applied. The new implementation also avoids locks in favour of a lighter weight compare-exchange operation.
cwisniew
approved these changes
Apr 26, 2023
cwisniew
added
the
code-maintenance
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.
Identify the Bug or Feature request
Implements #4002
Description of the Change
In addition to the change requested in #4002, I found these additional things along the way:
NotificationOverlayis unused and has been removed.BufferedImagePool. Callers already either apply a fill colour or completely fill the buffer with rendering results. So the clearing step is just wasted work, and has been removed.DebounceExecutorwould always delay a task by the full amount, even if it's been a while since the last execution. This can create noticable extra delays when the frame rate is capped, similar to the stutters caused byDefaultTool.mouseDragged(). I've put in a new implementation that delays tasks by only enough to get past the last execution's time budget, and runs tasks with no delay if it's been long enough since the last execution.Possible Drawbacks
Should be none.
Documentation Notes
N/A
Release Notes
This change is