win32: fix multi-window redraw starvation - #4653
Conversation
0c06743 to
bcc3130
Compare
f7373b9 to
846ea56
Compare
|
I believe this PR currently has a problem where it will schedule a redraw for every single input event if the handler every event requests redraw, with no coalescing at all. I think we need some kind of middle ground where input events are processed in a batch up until some deadline or similar. |
|
Thanks, this is a valid concern. Addressing this correctly would require a more deliberate batching or deadline policy, together with broader testing across run_app, pump_app_events, multiple windows, and native modal loops. I think that goes beyond the original scope of this PR. I’m going to revert the continuous-input/posted-message changes for now and keep this PR focused on the original multi-window starvation issue in #3648 |
|
does it look good to you now @nicoburns ? |
|
This is getting a little bit out of my area of expertise, but my LLM thinks this still isn't right (although the issue I raised above was fixed): Click to reveal LLM AnalysisThe change makes interrupt_msg_dispatch survive across dispatch_peeked_messages, and while it's set we skip prepare_wait / MsgWaitForMultipleObjectsEx / wakeup entirely. That fixes the literal reproducer in #3648 (requests only from about_to_wait), but:
What I'd suggest instead is to keep the per-frame wait phase and make the dispatch fair: treat all WM_PAINTs that are pending at the start of a dispatch as one frame. Concretely, in dispatch_peeked_messages track the HWNDs that have received RedrawRequested during this call, and only honour the interrupt when the next pending paint (PeekMessageW(.., PM_NOREMOVE | PM_QS_PAINT)) is for a window that's already in that set. That gives exactly one RedrawRequested per requesting window per AboutToWait, fixes both scenarios in the issue, keeps the WaitUntil / AboutToWait cadence intact, and keeps pump_app_events returning after one frame rather than one window. |
|
@nicoburns Thanks, I think the continuous-redraw concern is valid. My current Scenario B only re-requests once from the first I’m less certain that the suggested look-ahead guarantees fairness: if Also, since |
|
I reworked the implementation around a per-dispatch redraw batch instead of preserving The normal I also avoided using Representative release results: The regression test also verifies that Does this match the frame boundary you had in mind? |
02652e3 to
ab970ac
Compare
changelogmodule(not needed for this backend-internal fix; regression coverage was added instead)
Fixes #3648.
Problem
Win32 does not guarantee FIFO delivery of
WM_PAINTbetween windows.The previous implementation stopped native-message dispatch after one
RedrawRequestedand preserved that interruption across event-loop iterations.This fixed requests made from
AboutToWait, but caused continuous-redraw applications to skip the normal wait phase.A window requesting another redraw from its
RedrawRequestedhandler could also be selected repeatedly before the other invalid windows.Consequences included:
NewEventsandAboutToWaitcallbacks;ControlFlow::WaitUntildeadlines not being observed;pump_app_eventsreturning after one window rather than one redraw batch.Approach
Treat pending Win32 paints as a redraw batch.
After the first
RedrawRequestedin a native-message dispatch:WM_PAINTmessages are drained;HWNDemits at most one publicRedrawRequested;The event loop continues to execute its normal:
NewEvents -> AboutToWait -> waitphase between batches.
The old persistent
interrupt_msg_dispatchstate is no longer needed.The implementation intentionally does not use
PM_NOREMOVEas a pure look-ahead.PeekMessagemay still removeWM_PAINTmessages whose update region is null, andWindow::request_redrawusesRDW_INTERNALPAINT, which posts a paint even when no region is invalid.Validation
Added unit tests for:
Added an interactive Windows regression test covering:
AboutToWait;NewEvents,AboutToWait, andWaitUntilcadence;run_app_on_demandandpump_app_events;Representative release results
Validation was repeated in debug and release, including Rust 1.86 and 32-bit compilation