fix(desktop): only repaint the highlighter overlay when something changes - #3891
Open
paulovitin wants to merge 1 commit into
Open
fix(desktop): only repaint the highlighter overlay when something changes#3891paulovitin wants to merge 1 commit into
paulovitin wants to merge 1 commit into
Conversation
…nges The highlighter overlay read accessibility state at the monitor refresh rate and requested a redraw on every read, whether or not anything had moved. Left running, the process rendered, tessellated and submitted a GPU frame roughly 60 times a second forever, which is the loop that the memory reports in Automattic#3874 sampled. Redraws are now demand-driven: `RenderState::set_rects` reports whether the new accessibility read would paint a different frame, and `Window::render` returns the repaint delay egui asked for so animations, hover feedback and popups still get the frames they need. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Addresses #3874 (I'm one of the reporters there).
The problem
The highlighter overlay never stops painting.
WindowManagerApp::about_to_waitreads accessibility state everyread_interval— the fastest monitor's refresh interval, so ~16 ms — andread_rect_updatesthen callsrequest_redraw()on every overlay window unconditionally, whether or not the read produced different rectangles.So for as long as Harper is running — including all night on a machine nobody is touching — the process runs a full egui pass, tessellates, uploads buffers and submits a wgpu queue every 16 ms per monitor. That is the loop the diagnostics on #3874 sampled:
The change
Redraws become demand-driven:
RenderState::set_rectsnow reports whether the new accessibility read would actually paint a different frame (geometry, lint, rule name or source text changed, a highlight was added or removed, or a popup was invalidated).read_rect_updatesonly requests a redraw when it did.Window::renderreturns the repaint delay egui asked for (FullOutput::viewport_output[…].repaint_delay), andWindowManagerAppkeeps the earliest pending deadline in a smallRepaintSchedule.Duration::MAX— egui's "nothing is animating" — schedules nothing at all. Anything shorter wakes the windows up on time, so hover feedback, tooltips, popups closing themselves and any egui animation still get the frames they need.Nothing changes about how often the accessibility tree is read or how quickly a moved highlight follows its text; only frames that would be pixel-identical to the previous one are skipped. An idle overlay now submits zero GPU frames instead of ~60 per second per monitor.
Honesty about scope
I want to be clear about what this does and does not prove. I did not bisect the allocation to a single leaking object — that needs a multi-hour soak on a real Metal surface. What I can say is that this removes the unbounded per-frame renderer work that the reported stacks point at and that runs even when the machine is completely idle, which is exactly the "leave it open overnight" scenario in the issue. If growth persists after this, the next thing to look at is the renderer setup itself.
Related and deliberately not in this PR, to keep it reviewable:
Window::newbuilds oneegui_wgpu::winit::Painter— and therefore one wgpu instance/device — per monitor, while egui-wgpu's contract is a single painter shared across viewports. Any per-submission resource cost is multiplied per display today.Window::renderdropsFullOutput::viewport_outputon the floor apart from the repaint delay this PR reads. There are no child viewports in this UI, so nothing appears to accumulate, but it is not the documented backend contract.Happy to follow up on either if you'd like them addressed.
Tests
11 new unit tests, no GPU or window required (
cargo test -p harper-desktop --lib, 63 passing):render_state: identical reads and empty reads request no repaint; moved rectangles, changed lints, edited source text, added/removed highlights, and a popup invalidated by a shrinking read all do.window_manager:RepaintScheduleignoresDuration::MAX, keeps the soonest of competing requests, fires a due repaint exactly once, and treatsDuration::ZEROas immediately due.🤖 Generated with Claude Code