Skip to content

win32: fix multi-window redraw starvation - #4653

Open
Niketion wants to merge 2 commits into
rust-windowing:masterfrom
Niketion:fix/win32-multi-window-redraw-3648
Open

win32: fix multi-window redraw starvation#4653
Niketion wants to merge 2 commits into
rust-windowing:masterfrom
Niketion:fix/win32-multi-window-redraw-3648

Conversation

@Niketion

@Niketion Niketion commented Jul 31, 2026

Copy link
Copy Markdown
  • Tested on all platforms changed (Windows)
  • Added an entry to the changelog module
  • Updated the internal dispatch documentation; there is no user-facing API change
  • Created or updated an example program
    (not needed for this backend-internal fix; regression coverage was added instead)

Fixes #3648.

Problem

Win32 does not guarantee FIFO delivery of WM_PAINT between windows.

The previous implementation stopped native-message dispatch after one RedrawRequested and 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 RedrawRequested handler could also be selected repeatedly before the other invalid windows.

Consequences included:

  • starvation of other windows;
  • missing NewEvents and AboutToWait callbacks;
  • ControlFlow::WaitUntil deadlines not being observed;
  • pump_app_events returning after one window rather than one redraw batch.

Approach

Treat pending Win32 paints as a redraw batch.

After the first RedrawRequested in a native-message dispatch:

  • only WM_PAINT messages are drained;
  • each HWND emits at most one public RedrawRequested;
  • redraws requested by a serviced window are deferred until the batch completes;
  • duplicate native paints are processed without emitting another public event, allowing Win32 to validate their update region;
  • a further repeat bounds dispatch for callers invalidating windows through raw Win32 APIs.

The event loop continues to execute its normal:

NewEvents -> AboutToWait -> wait

phase between batches.

The old persistent interrupt_msg_dispatch state is no longer needed.

The implementation intentionally does not use PM_NOREMOVE as a pure look-ahead.

PeekMessage may still remove WM_PAINT messages whose update region is null, and Window::request_redraw uses RDW_INTERNALPAINT, which posts a paint even when no region is invalid.

Validation

Added unit tests for:

  • dispatching distinct HWNDs once per batch;
  • suppressing and bounding repeated paints;
  • continuing to other windows after a duplicate;
  • ensuring dispatch-state mutations also occur in release builds.

Added an interactive Windows regression test covering:

  • five deliberately overlapping windows;
  • redraw requests from AboutToWait;
  • continuous redraw requested from each window's own handler;
  • the stronger case where each handler requests all windows;
  • NewEvents, AboutToWait, and WaitUntil cadence;
  • run_app_on_demand and pump_app_events;
  • at most one redraw per HWND per pump call.

Representative release results

current window:
  redraws=[20, 20, 20, 20, 20]

all windows requested by every handler:
  redraws=[54, 54, 54, 54, 55]
  NewEvents=55
  AboutToWait=54

pump_app_events:
  redraws=[20, 20, 20, 20, 20]

Validation was repeated in debug and release, including Rust 1.86 and 32-bit compilation

@Niketion
Niketion force-pushed the fix/win32-multi-window-redraw-3648 branch from 0c06743 to bcc3130 Compare July 31, 2026 09:42
@Niketion
Niketion force-pushed the fix/win32-multi-window-redraw-3648 branch from f7373b9 to 846ea56 Compare August 5, 2026 12:00
@Niketion Niketion changed the title win32: fix multi-window redraw starvation win32: fix redraw starvation across windows and continuous input Aug 5, 2026
@nicoburns

Copy link
Copy Markdown
Contributor

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.

@Niketion

Niketion commented Aug 5, 2026

Copy link
Copy Markdown
Author

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

@Niketion Niketion changed the title win32: fix redraw starvation across windows and continuous input win32: fix multi-window redraw starvation Aug 5, 2026
@kchibisov

Copy link
Copy Markdown
Member

does it look good to you now @nicoburns ?

@nicoburns

nicoburns commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

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 Analysis

The 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:

  1. Continuous-redraw apps never reach the wait phase. If the app calls request_redraw() from inside RedrawRequested (the standard way to animate), the WM_PAINT handler re-invalidates the window after DefWindowProcW, the interrupt flag is set, the wait is skipped, and PeekMessage immediately hands back the same window's WM_PAINT. For as long as the app keeps animating, AboutToWait and NewEvents are never emitted on Windows. That means ControlFlow::WaitUntil deadlines never fire (they're only checked in wait_for_messages_impl / call_new_events), any per-frame work in about_to_wait stops running, and pump_app_events no longer emits a NewEvents…AboutToWait pair per call (the trailing wakeup() becomes a no-op because we never left HandlingMainEvents). It also diverges from the other backends, where the run loop still goes idle between frames.
  2. I don't think the multi-window + continuous-redraw case is actually fixed. The second trace in Not receiving RedrawRequested events for all windows that .request_redraw() #3648 shows Windows returning window 4's WM_PAINT twice in a row while 0–3 are also invalid, so paint dispatch isn't FIFO — it looks z-/creation-ordered. With this PR, after window 4 paints and re-invalidates itself, the next PeekMessage should pick 4 again, now with no AboutToWait in between at all. Could you post the actual events=[…] counts for the variant where each window calls request_redraw() from its own RedrawRequested handler? I'd expect [0, 0, 0, 0, N].
  3. The flag now means two things ("return to the external loop ASAP" and "there's more to drain, don't wait"), and in run_app_on_demand it just adds an extra trip round the loop.

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.

@Niketion

Niketion commented Sep 2, 2026

Copy link
Copy Markdown
Author

@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 PeekMessage returns the same HWND again, it would immediately terminate the frame and still leave the other windows pending

Also, since request_redraw uses RDW_INTERNALPAINT, the documented handling of NULL-update-region WM_PAINT messages with PeekMessage / PM_NOREMOVE seems worth checking

@Niketion

Niketion commented Sep 3, 2026

Copy link
Copy Markdown
Author

I reworked the implementation around a per-dispatch redraw batch instead of preserving interrupt_msg_dispatch across event-loop iterations.

The normal NewEvents -> AboutToWait -> wait phase now always runs between batches. After the first RedrawRequested, the dispatcher drains pending WM_PAINT messages while allowing at most one public redraw event per HWND. If a window requests another redraw from inside its own handler, that redraw is deferred until the current batch completes, so the same window cannot immediately jump ahead of the other pending windows.

I also avoided using PM_NOREMOVE as a pure look-ahead, since PeekMessage may still remove null-region WM_PAINT messages, while request_redraw() uses RDW_INTERNALPAINT.

Representative release results:

current window:
[20, 20, 20, 20, 20]

all windows requested by every handler:
[54, 54, 54, 54, 55]

NewEvents=55
AboutToWait=54

pump_app_events:
[20, 20, 20, 20, 20]

The regression test also verifies that WaitUntil deadlines are reached, NewEvents/AboutToWait continue during continuous redraw, and each HWND emits at most one public RedrawRequested per batch.

Does this match the frame boundary you had in mind?

@Niketion
Niketion force-pushed the fix/win32-multi-window-redraw-3648 branch from 02652e3 to ab970ac Compare September 3, 2026 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Not receiving RedrawRequested events for all windows that .request_redraw()

3 participants