Rethinking RedrawRequested

[Osspial]

The current RedrawRequested event is currently hard to understand, which makes it difficult to implement and difficult to use. This issue will outline why I believe that is the case, and how I think we should fix it.

The current problems

RedrawRequested is extremely inconsistent with the rest of our API. This is exhibited in a couple ways:

1. It's inconsistent with itself, since it sometimes provides ordering guarantees.
   • It's explicitly guaranteed to be the last event emitted if request_redraw is called, but isn't explicitly guaranteed to be the last event if the OS requested the redraw.
2. It's inconsistent with WindowEvent, since no other window event provides any sort of ordering guarantees.
3. It's inconsistent with EventsCleared, since it can be dispatched after EventsCleared if queued with request_redraw during the EventsCleared handler.

Some of that inconsistency is intentional (point 3), and some isn't (point 1), but the end result is that the API is difficult to explain and difficult to implement. Even beyond the complexity those inconsistencies introduce, the current model can result in duplication of RedrawRequested events when using the recommended event loop structure described in the root docs. Consider the following chain of events, with an application using the linked event loop structure:

1. The user resizes the window, resulting in the OS triggering a RequestRedraw event.
2. The event loop dispatches RequestRedraw between NewEvents and EventsCleared, as part of the standard control flow.
3. EventsCleared gets dispatched, upon which the user updates the application's state and calls request_redraw.
4. RequestRedraw gets dispatched immediately after EventsCleared.

The above situation has a couple problems: the first RedrawRequested is dispatched before the application has a chance to update its internal state, and the application goes through the expensive process of rendering a frame twice in a single iteration of the event loop. Ideally, our API wouldn't exhibit either issue, allowing the user to update their application state before all redraws occur and allowing Winit to unify redraw requests from the user and OS into a single event.

Proposed changes

• Rename EventsCleared to MainEventsCleared
• Remove RedrawRequested from WindowEvent.
• Add RedrawRequested(WindowId) to Event.
  • Guarantee that RedrawRequested get dispatched between MainEventsCleared and RedrawEventsCleared.
• Add RedrawEventsCleared to Event.

The rough event loop would end up behaving something like this (adapted from @icefoxen's example in #1032):

let event_loop_windows = /* event loop windows */;
let mut control_flow = ControlFlow::Poll; 

event_handler(NewEvents(StartCause::Init), &event_loop_windows, &mut control_flow);
while control_flow != ControlFlow::Exit {
    event_handler(NewEvents(/* get start cause */), &event_loop_windows, &mut control_flow);

    for e in (window events, user events, device events) {
        event_handler(e, &event_loop_windows, &mut control_flow);
    }
    event_handler(MainEventsCleared, &event_loop_windows, &mut control_flow);

    for window_id in (redraw_windows) {
        event_handler(RedrawRequested(window_id));
    }
    event_handler(RedrawEventsCleared, &event_loop_windows, &mut control_flow);

    /* wait or poll, as the control_flow dictates */
}
event_handler(LoopDestroyed, &event_loop_windows, &mut control_flow);

This results in numerous usability improvements over the current model:

1. RedrawRequested's status as a special event is made more clear, as it's structurally distinct from standard window events.
2. RedrawRequested is now guaranteed to be dispatched after event processing and application update code has been run, fitting it more naturally into the application update/render loop.
3. Winit can now unify all OS-requested and user-requested redraw events, avoiding redundant redrawing.

---

Feedback is encouraged on the above designs, especially WRT naming - I'm not entirely happy with the names I've provided, and mainly included them because I can't think of anything better right now. There may be a better solution to the current API's problems, but I'm presently unable to think of one and we should certainly address them before 0.20's full release.

[icefoxen]

My gut impulse is that you're trying to make winit do more than it needs to here. It seems like, from the application's perspective, the right thing to do is set a flag if you get a resize event, and then when you get to the point when you decide whether or not to draw, you draw if your application needs to redraw, or if the OS has told you to redraw via a resize event. Either way, from the application writer's perspective, your drawing only happens in one place and only when it's actually necessary.

I will have to think about this more, since I still haven't internalized the new event loop as much as you have.

[aloucks]

Part of the confusion with RedrawRequested is that it combines separate concerns: window damage/refresh and the users request to turn the event loop. I think it also interacts with Wait and WaitUntil but I'm getting confused thinking about it while looking at the example (why do I need to request_redraw and use WaitUntil with a timeout?)

Have you considered decoupling these?

I think ideally there would be only one "meta" event that indicates that the event loop has come full circle, but this proposal is adding more. What if EventsCleared was eliminated? Would that solve the problem with the (lack of) ordering guarantee? Could you just call request_redraw from NewEvents and run all your app logic there as well?

[Osspial]

It seems like, from the application's perspective, the right thing to do is set a flag if you get a resize event, and then when you get to the point when you decide whether or not to draw, you draw if your application needs to redraw, or if the OS has told you to redraw via a resize event.

@icefoxen That's actually not entirely correct! Doing that works fine for games, but it leads to visual issues when resizing the window (at least on Windows) which isn't great for desktop applications. If you want to avoid those visual issues, you must perform all redrawing inside of RedrawRequested, and this proposal lets you do that more easily.

There's a bit of vagueness in my description there, since it's harder to concretely describe what happens without concrete examples. Luckily, I've prepared some! Check out this repository and run the examples to (hopefully) see what I'm talking about - they illustrate my point nicely on my Windows machine, and I'd be curious to see what they look like on other platforms.

resize_smooth looks like this, since it handles resizing where the OS wants it to: https://gfycat.com/belatedperfumedjanenschia
The window's content in resize_laggy lags behind the window's resize, since it redraws after the OS has requested a redraw: https://gfycat.com/fancypassionatecaudata

---

Part of the confusion with RedrawRequested is that it combines separate concerns: window damage/refresh and the users request to turn the event loop. I think it also interacts with Wait and WaitUntil but I'm getting confused thinking about it while looking at the example (why do I need to request_redraw and use WaitUntil with a timeout?)

Have you considered decoupling these?

@aloucks Wait, what? They are decoupled. RedrawRequested is intended exclusively for redrawing the window, not turning the event loop - notifying the user of a event loop's completion is EventsCleared's job. When functioning properly, it shouldn't interact with Wait or WaitUntil at all. If there's confusion here, that's a documentation deficit.

I think ideally there would be only one "meta" event that indicates that the event loop has come full circle, but this proposal is adding more. What if EventsCleared was eliminated? Would that solve the problem with the (lack of) ordering guarantee? Could you just call request_redraw from NewEvents and run all your app logic there as well?

Here's my rationale behind all the meta-events, for clarity:

• NewEvents notifies the user why the event loop woke back up. This doesn't do a whole lot when control_flow is set to Poll, but when the user is using WaitUntil it helps them figure out if the loop was woken up because more events were received or because their timer expired.
• EventsCleared (or MainEventsCleared) notifies the user that all events that affect application state have been dispatched, and that it's safe to aggregate the events they've received into an application update. This does not always imply that the application needs to be redrawn. Desktop applications are able to be extremely conservative about when they redraw, and for the sake of resource usage should only redraw when absolutely necessary. Exposing this separately from redrawing makes that significantly easier.
• RedrawEventsCleared... well, I wouldn't expect it to do a whole lot. It is genuinely useful in some circumstances (this simplifies an async event loop's internal logic significantly, for example), but the primary purpose for most people is to provide an obvious book-end for the redraw events and clarify that they happen after MainEventsCleared.

[aloucks]

What is request_redraw needed for? I thought it was basically the equivalent of glfwPostEmptyEvent() but triggers as window refresh. I can see how this would be useful for waking up the event loop from another thread, but I'm trying to understand why it's needed in the single-threaded scenario.

After thinking more about my initial suggestion (single meta event), I can see where it might be problematic for the non-game-loop style app.

I think the problem though is still centered around request_redraw and trying to use RedrawRequested as the sole place for drawing (and how it fits in the overall API). It makes sense in the UI paint-on-window-damage app, but maybe not as much on the traditional game loop.

Here's another thought:

1. Remove request_redraw
2. Rename RedrawRequsted to Refresh (now this is only triggered from OS messages)

And adjust the API usage recommendation to something more like:

1. Traditional game loops should put app logic and rendering in the EventsCleared handler.
2. UI apps probably want to handle Refresh and only re-paint when received.

And finally (some bikeshedding)...

Rename:

NewEvents to EventsBegin
EventsCleared to EventsEnd

I think this naming makes them stand out more as being very closely related.

EDIT 1:

I definitely think request_redraw is what's creating this wrinkle in the API. Game loops and UI apps are just fundamentally different when it comes to their repaint frequency. I think the attempt to make a game loop react to paint messages is what's making things a little more complicated than they need to be.

EDIT 2:

I just changed one of my projects and could definitely see that resizing wasn't as nice 😢. Back to the drawing board I guess.

match event {
    Event::EventsCleared => {
        if Instant::now() - min_duration >= self.last_frame_time {
            self.window.request_redraw();
        }
    }
    Event::WindowEvent {
        event: WindowEvent::RedrawRequested,
        ..
    } => {
        self.last_frame_time = Instant::now();
        *control_flow = ControlFlow::WaitUntil(self.last_frame_time + min_duration);
        for event_handler in event_handlers.iter_mut() {
            event_handler.on_frame(&mut self);
        }
        on_frame(&mut self).expect("on_frame error");
    }
    _ => {}
}

to

match event {
    Event::EventsCleared => {
        if Instant::now() - min_duration >= self.last_frame_time {
            self.last_frame_time = Instant::now();
            *control_flow = ControlFlow::WaitUntil(self.last_frame_time + min_duration);
            for event_handler in event_handlers.iter_mut() {
                event_handler.on_frame(&mut self);
            }
            on_frame(&mut self).expect("on_frame error");
        }
    }
    Event::WindowEvent {
        event: WindowEvent::RedrawRequested,
        ..
    } => {}
    _ => {}
}

So I'm guessing that repainting from RedrawRequested is required on Windows so that it's a reaction to WM_PAINT?

[Osspial]

@aloucks request_redraw is used so that all redraw logic can be placed in the RedrawRequested handler, unifying application-requested and OS-requested redraws into a single event that can cleanly handle both cases without the need for additional complexity. The intention is, if you do redrawing in there, you don't have to think about how to handle OS-requested redrawing differently from application-requested redrawing, or where to place application update code in relation to application redraw code, or how to avoid redrawing twice if the OS requests a redraw and you're redrawing in EventsCleared. All of that complexity gets abstracted away, all rendering processing is treated identically, and everything Just Works without having to think about the underlying semantics of the event loop or the platform. It isn't strictly necessary, but exposing it properly makes everything so much nicer and easy-to-handle, and I don't want to throw all of that away.

The issue is, the current API can't do that because it doesn't always unify application-requested and OS-requested redraws. The redesign proposed here fixes that.

So I'm guessing that repainting from RedrawRequested is required on Windows so that it's a reaction to WM_PAINT?

That is correct. I don't know if it has the exact same semantics on other platforms, but I'd be extremely surprised if no other platform made similar assumptions that resulted in similar behavior when RedrawRequested is handled improperly.

[aloucks]

3. It's inconsistent with EventsCleared, since it can be dispatched after EventsCleared if queued with request_redraw during the EventsCleared handler.

Could this be fixed by injecting another NewEvents ?

[Osspial]

Could this be fixed by injecting another NewEvents ?

Injecting another NewEvents implies dispatching another EventsCleared, calling request_redraw again, dispatching NewEvents again, ad infinitum. That breaks Wait and WaitUntil since it creates an always-running loop, and also means that redrawing doesn't logically occur in the same event loop iteration that the application update did, which is needlessly confusing.

[goddessfreya]

@Osspial Can we also pass a vector of damage reports with RedrawRequested? Unless you got another idea on how to expose them?

[Osspial]

@zegentzy It's definitely worthwhile to expose the damage report. We may want to expose it via some sort of platform-specific UpdateRegion struct, which would allow us to more easily expose things like the region bounding rectangle, but that's a design I haven't put much thought into and may or may not be the correct way to go about exposing it.

[goddessfreya]

Well, for Linux, we got these three things:
https://tronche.com/gui/x/xlib/events/exposure/expose.html
https://people.freedesktop.org/~whot/wayland-doxygen/wayland/Server/structwl__surface__interface.html#ad3cff78b33a03a17c9c148edc44d14ea
https://people.freedesktop.org/~whot/wayland-doxygen/wayland/Server/structwl__surface__interface.html#a922bb3cc09d0819c04cd02f79e79b17f

Afaik, on windows, you just need to call this function here: https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-getupdaterect

And macOS seams to pass a rect:

winit/src/platform_impl/macos/view.rs

Line 283 in a195ce8

extern "C" fn draw_rect(this: &Object, _sel: Sel, rect: id) {

Given the similarities, I propose the following:

1. We should create the new type DamageRegion, which contains the size and position of the damaged rect.
2. DamageRegion should expose some basic functions, like union and difference.
3. request_redraw should take a DamageRegion
4. RedrawRequested should also contain a vector of DamageRegions.

I think this solution is simple enough.

Unresolved issues:

1. Should the size and position be logical or physical?
2. What if two DamageRegions partially overlap?
3. What if two DamageRegions completely overlap?

[elinorbgr]

Note that (as often), the Wayland semantics are quite different. The wayland server is supposed to keep an internal copy of the contents of the client windows, and as such the client never receives any damage information. Instead, tne client sends damage information to the server to notify which part of its contents it modified, so that the server can only update this part for better performance. This is automatically handled by Mesa, so winit does not need to worry about it.

[felixrabe]

@zegentzy I wonder why you added the "good first issue" label, to me it looks like the opposite.

[goddessfreya]

Implementing the solution as suggested by the OP I think is simple enough. You just* need to shuffle some code around to make sure RedrawRequested happens at the right time.

Handling damage information, I'd think, isn't a "good first issue", but, well, I hadn't mentioned that when I labeled it =p.

*Maybe a slight understatement.

[goddessfreya]

@murarth Do you have time to implement this for X? Possibly with damage regions as proposed?

[murarth]

@zegentzy: Yes, I'll see what I can do.

[Osspial]

I'd like to handle damage regions in a separate PR, since it's logically a separate issue than what's brought up here.

As to the DamageRegion proposal, I've got a few questions:

• Why would request_redraw take a single damage region when RedrawRequested take a vector of damage regions?
• What exactly is the underlying type that DamageRegion wraps around?
  • It sounds like it's wrapping around individual damage rects, which need to be combined to get the full damage region. If that's the case, why not expose the PhysicalRects directly?
• If the above model is correct, how do functions like union and difference make any sense?

[goddessfreya]

I'd like to handle damage regions in a separate PR, since it's logically a separate issue than what's brought up here.

That is a good idea. I'll draft something more detailed later this week.

[Osspial]

I've made a branch for handling this: https://github.com/rust-windowing/winit/tree/redraw-requested-2.0

[goddessfreya]

Hey, @ jlogandavison, I noticed you said you wanted to gain some experience with wayland. I think this issue would be a great one to dip your toes in.

Also, please be sure to add yourself to this table: https://github.com/rust-windowing/winit/wiki/Testers-and-Contributors

[goddessfreya]

@jlogandavison

[goddessfreya]

@jlogandavison Thanks for contacting me.

jlogandavison:
@gentz Thanks for suggesting issue #1041 , looks like something I should be able to sink my teeth into. As it stands though, I'm afk for the next 2/3 weeks. I'd absolutely be happy to take this on, but I don't want progress to stall in my absence. Is there a time constraint?
gentz:
There is no time constraint, however, the quicker it is done, like all issues, the better :)
jlogandavison:
Okay. Shall we say, refrain from assigning me for now? That way if somebody can pick it up in the meantime and it may get done sooner. I'll touch base as soon as I'm back at the desk

[mtak-]

If you want to avoid those visual issues, you must perform all redrawing inside of RedrawRequested

So I'm guessing that repainting from RedrawRequested is required on Windows so that it's a reaction to WM_PAINT?

That is correct. I don't know if it has the exact same semantics on other platforms, but I'd be extremely surprised if no other platform made similar assumptions that resulted in similar behavior when RedrawRequested is handled improperly.

If I understand correctly, the new API expects that other platforms with that assumption will also send RedrawRequested out after MainEventsCleared. I haven't done much native UIKit or AppKit programming, but os-driven redraw events do occur during the main event loop. Macos enters a modal event loop during a resize which means EventsCleared is only sent out when the user finishes the resize (when mouse1 is released). I think the ordering of RedrawRequested in the new API is a quirk of Windows more than anything - which is OK if macos/other platforms don't have the same redrawing assumptions as windows and/or do have the same event ordering as windows.

[goddessfreya]

Adding Blocking a Release, as I think these changes should be implemented before we finalize 0.20.

[Osspial]

I haven't done much native UIKit or AppKit programming, but os-driven redraw events do occur during the main event loop.

Really? I've looked through the cocoa documentation, and it seems to suggest otherwise:

NSView defines the methods setNeedsDisplay: and setNeedsDisplayInRect: for marking portions of your view as dirty. Cocoa collects the dirty rectangles and saves them until the top of your run loop is reached, at which point your view is told to redraw itself.

---

Macos enters a modal event loop during a resize which means EventsCleared is only sent out when the user finishes the resize (when mouse1 is released).

I'd consider that a major bug that should be fixed. The Windows API exhibits similar behavior by default, and we do some fairly significant hoop-jumping to avoid exposing that behavior publicly.

I think the ordering of RedrawRequested in the new API is a quirk of Windows more than anything - which is OK if macos/other platforms don't have the same redrawing assumptions as windows and/or do have the same event ordering as windows.

Admittedly, this redesign assumes that that's the case. However, since it's pretty much the only sensible way to dispatch redraw events in the context of the broader application, I'd be extremely surprised if other platforms didn't enforce similar ordering.

[mtak-]

Sorry, the apple situation isn't as bad as I made it out to be.

iOS's docs say a similar thing as macos's:

When the contents of your view change, you do not redraw those changes directly. Instead, you invalidate the view using either the setNeedsDisplay or setNeedsDisplayInRect: method. These methods tell the system that the contents of the view changed and need to be redrawn at the next opportunity. The system waits until the end of the current run loop before initiating any drawing operations.

I wasn't considering redraw events triggered by setNeedsDisplay as being os-driven. I've seen drawRect: in between other events 1) when a UIView is first displayed and 2) on background/foreground.

I haven't yet tested macos, but it sounds like they typically send redraws at the top of the eventloop? I guess that can be worked around somehow.