search

rust-windowing / winit

Window handling library in pure Rust
https://docs.rs/winit/
Apache License 2.0
4.9k stars 914 forks source link

Fix redraws not being scheduled correctly on macOS and iOS #2640

Open madsmtm opened 1 year ago

madsmtm commented 1 year ago

As the title says, RedrawRequested occurs without a corresponding MainEventsCleared (and RedrawEventsCleared on macOS). Fundamentally, drawing inside kCFRunLoopBeforeWaiting is just plain incorrect.

Possible solutions:

Some of the previous discussion:

And some references to myself:

rib commented 1 year ago

I just came to essentially ask "why does the macos backend emit RedrawRequested events in two different ways?", so glad to find this existing issue.

I was looking at implementing the run, run_ondemand and pump_events APIs discussed in https://github.com/rust-windowing/winit/issues/2706 and (although it's not exactly related) I just found it a bit surprising that RedrawRequested events were dispatched as part of the RunLoop observer and via View drawRect callbacks.

I think I was only expecting redraw events to happen via the drawRect callbacks. I would then imagine that Window::request_redraw() would then somehow effectively use [view setNeedsDisplay: true]; to drive redraw requests through the standard drawRect callback.

More generally it seems like the idea that Winit dispatches events in phases from NewEvents, through MainEventsCleared and then through to RedrawEventsCleared isn't really a good match for several platforms, including macos and wonder if this design should be reconsidered.

It doesn't seem like a portable design to try and promise that RedrawRequested + RedrawEventsCleared events must be dispatched after MainEventsCleared. It's at least a bad fit for macos + iOS, and I guess it also doesn't really make sense for Web, assuming redraws should be driven off the back of requestAnimationFrame callbacks. On Android we can currently honor the ordering requirements for these events but we may also need to think about how to support scheduling rendering via a Choreographer which could be another example of wanting to drive rendering via callbacks.

If we want to preserve something similar to NewEvents and MainEventsCleared then the way they are supported in the macos backend is perhaps the most portable interpretation of these - i.e. they indicate when the underlying event loop wakes up and indicate when the loop is about to go back to waiting/polling. (still though, even that interpretation doesn't really apply to Windows and Web in quite the same way as it does for other platforms)

kchibisov commented 1 year ago

More generally it seems like the idea that Winit dispatches events in phases from NewEvents, through MainEventsCleared and then through to RedrawEventsCleared isn't really a good match for several platforms, including macos and wonder if this design should be reconsidered.

I think the thing what winit is doing is not entirely wrong, and I guess most platforms don't require sync events. Though, I have my own issues with the winit events policy (resize confirmation in multithreaded environments, and so on).

I think the issue with the RedrawRequested is that it's behavior is inconsistant, and having a single event for all of the platform different logic is showing that it's not enough.

Winit has its own event loop of redraw_requested() and then getting it after all input/user events for the given window, which sounds fine, however then we have redraw events from the OS, which sometimes sync(macOS), sometimes not sync(Wayland), and so on.

and I guess it also doesn't really make sense for Web, assuming redraws should be driven off the back of requestAnimationFrame callbacks.

Isn't it a more or less hint? Also, I don't see how RedrawRequested and RedrawEventsCleared are not portable in a single window context for example. I think the only issue for them arises from the multi-window nature of them, and the fact that RedrawEventsCleared is a global event.

I think we should have a concept of About to start polling again, and woke up, however what I think is that we shouldn't try to make redraws for all the windows shared.

However, the RedrawRequested is a good scheduling mechanism for the users, what I think we can do is to add an extra event, which tells that sync drawing was requested by the OS as in, it'll be requested for now only on macOS, and it'll be clear, that you can't post pone that event.

rib commented 1 year ago

I think the thing what winit is doing is not entirely wrong, and I guess most platforms don't require sync events. Though, I have my own issues with the winit events policy (resize confirmation in multithreaded environments, and so on).

Even though most platforms don't need strictly sync redraw callback I think most window systems still have a natural way of driving redraws as events of some form and that makes it awkward to have ordering relationships between so-called 'main events' and whatever drives rendering (because it's likely that redraws are basically driven by a "main" event too)

E.g.

MacOS:

iOS:

Web:

Android:

Windows

Wayland:

X11

Winit has its own event loop of redraw_requested() and then getting it after all input/user events for the given window, which sounds fine, however then we have redraw events from the OS, which sometimes sync(macOS), sometimes not sync(Wayland), and so on.

If the window system has a standard way of driving redraws then imho it should probably be the responsibility of each backend to implement request_redraw() in a way that will service the app's redraw requests via that standard mechanism. E.g. for MacOS that could be via [view setNeedsDisplay: true]; which will result in drawRect callback in a finite amount of time.

The fact that some systems require synchronous handling of render requests can be quite easily nomalized by documenting the same requirement at the Winit level (i.e. expect apps to redraw synchronously when they get a redraw event).

and I guess it also doesn't really make sense for Web, assuming redraws should be driven off the back of requestAnimationFrame callbacks.

Isn't it a more or less hint? Also, I don't see how RedrawRequested and RedrawEventsCleared are not portable in a single window context for example. I think the only issue for them arises from the multi-window nature of them, and the fact that RedrawEventsCleared is a global event.

With Web I'm pretty sure you can render whenever you like but you almost certainly don't want to do that - same as with Wayland where you should probably generally drive rendering via frame callbacks - even though you don't technically have to.

I'm not exactly sure what you're referring to regarding being multi-window or not. For web you have a fairly simple frame callback mechanism that can help to sync/throttle your rendering with the browser compositor, which should be equally relevant for single or multi-canvas web applications?

I think we should have a concept of About to start polling again, and woke up

Yeah, I tend to think it would make sense to redefine NewEvents and MainEventsCleared with these terms.

The main issue I see with that is that the Web backend never knows when the browser polls/wakes up and the Windows backend would need to emulate these through some combination of PeekMessage and GetMessage to infer when it is about to "wait".

however what I think is that we shouldn't try to make redraws for all the windows shared.

Right, I don't think we would want this either - we should be keeping something like RedrawRequested(WindowId) - it's just that the events should consistently, only be driven by the window system's standard protocol for scheduling redraws.

However, the RedrawRequested is a good scheduling mechanism for the users, what I think we can do is to add an extra event, which tells that sync drawing was requested by the OS as in, it'll be requested for now only on macOS, and it'll be clear, that you can't post pone that event.

The request_redraw() API is a useful scheduling mechanism and I think all backends should be able to make that Just Work(tm) based on the standard redraw mechanisms without needing to introduce a way of dispatching redraw requests as anything special.

By default apps shouldn't have any awareness that MacOS and iOS expect redrawing to be serviced synchronously - Winit can just document the exact same expectation that apps should render synchronously when they get a RedrawRequested callback/event.

If applications don't want to be portable to MacOS or iOS (and also don't want sensible, default frame scheduling across all other platforms) there could be an extension that lets apps know that they can render outside of redraw events, but I feel like this should be the special case because the purpose of Winit is to provide a portable window system abstraction and ideally apps should be portable by default.

kchibisov commented 1 year ago

frame callback

On Wayland, it's a throttling hint, you don't have to draw on it or anything like that. Though, it's common to draw from the frame callback.

If the window system has a standard way of driving redraws then imho it should probably be the responsibility of each backend to implement request_redraw() in a way that will service the app's redraw requests via that standard mechanism. E.g. for MacOS that could be via [view setNeedsDisplay: true]; which will result in drawRect callback in a finite amount of time.

The API is miss-used, we can't use it for the RedrawRequested anyway, I think you remember my issue about the Frame event, in general, the throttling hint should be separate, and the Sync redraw should be separate as well. Though, it's hard for me to give a real answer how it should be, there're lots of caveats on Wayland alone how it should be done...

I'm not exactly sure what you're referring to regarding being multi-window or not. For web you have a fairly simple frame callback mechanism that can help to sync/throttle your rendering with the browser compositor, which should be equally relevant for single or multi-canvas web applications?

I mean that current winit is sane with single window, but with multi windows, you have a global RedrawEventsCleared, and some clients draw on it. So the API should change here, as it's not practical, I'd prefer have events separate, and have a callback before loop goes back to polling.

The request_redraw() API is a useful scheduling mechanism and I think all backends should be able to make that Just Work(tm) based on the standard redraw mechanisms without needing to introduce a way of dispatching redraw requests as anything special.

By default apps shouldn't have any awareness that MacOS and iOS expect redrawing to be serviced synchronously - Winit can just document the exact same expectation that apps should render synchronously when they get a RedrawRequested callback/event.

I think that's fine to approach, it's just the issue is way more complicated than it should. For example, on really should resize your window only once per frame callback(insane, yeah?), and I'd really like to somehow model it that way. The thing is that RedrawRequested right now is a must draw, but what we need is a should draw, though, maybe we can pass some enum whether the event could be ignored or the actual redraw should be performed?

If applications don't want to be portable to MacOS or iOS (and also don't want sensible, default frame scheduling across all other platforms) there could be an extension that lets apps know that they can render outside of redraw events, but I feel like this should be the special case because the purpose of Winit is to provide a portable window system abstraction and ideally apps should be portable by default.

App can draw whenever, we don't need an extension for that, what we want is a recommended place to perform drawing operations.

Be aware that frame callbacks on Wayland require a request(I'd assume the same on the Web), and to do that request you need to actually submit a buffer right after that and commit a surface, you can ask for frame hints with empty commits, but it's really discouraged, given that you can have visual glitches. I'm not sure how to model that with just a callback based approach, or how all of that should interact with the resize event, given that EGL has buffer latching on Wayland and ideally we want to avoid that for users automatically, without requiring them an extra research into EGL platform extensions(It wasn't even in extension, unless I've send a patch into the spec).

Ideally a resize should be sent once per frame callback, then frame callback should tell that a redraw must be done or something like that, and user should ask for a new frame hint once they draw. I don't see a way how to do that automatically though, because we don't hold any of the user buffers or graphics stack, we don't really know whether the user drawn or something like that.

The above is one of the reasons I think that RedrawRequested can't really be merged with the frame callbacks on Wayland. Having a separate hint requests on the other side, is likely fine.

rib commented 1 year ago

Be aware that frame callbacks on Wayland require a request(I'd assume the same on the Web), and to do that request you need to actually submit a buffer right after that and commit a surface,

If you're dealing with a UI app that isn't rendering continuously then it's possible that you've woken up spontaneously due to some input and want to render a new frame immediately - not via a requestAnimationFrame / frame callback, and so there should be a special case to consider there.

I see this as a platform/backend-specific detail. Wayland frame callbacks are a fairly simple ACK you get back from the compositor after a frame has been handled by the compositor - so yeah, you wouldn't want to submit a dummy frame with an empty buffer just for the sake of getting a frame callback.

The frame callbacks effectively just throttle the next frame, either during an animation in a UI app or when continuously rendering in a game.

So you would have a special case essentially for any spontaneous request_redraw that would dispatch a RedrawRequested asap (not throttled by a frame callback) and any subsequent request_redraw will be throttled until you get your frame callback (and your throttle can be reset/cleared when you get the callback too if nothing has requested a redraw). I think the backend state tracking for this should be relatively simple. If you have a pending frame callback then redraw_requests are throttled, otherwise dispatch RedrawRequested immediately.

I think it's a very similar situation with the Web requestAnimationFrame (again I think I'd just treat this as a backend implementation detail). For Web if you have just got a spontaneous request_redraw e.g. because it's a UI that doesn't render continuously then in that case you can dispatch a RedrawRequested immediately and pretty much just like with Wayland you can then throttle any further redraw_requests until you get an animation frame callback.

In both cases you can get throttling for back-to-back queuing of frames and you can respond immediately, without waiting for a round-trip/callback for spontaneous frames - e.g. when a user interacts with a UI.

rib commented 1 year ago

I mean that current winit is sane with single window, but with multi windows, you have a global RedrawEventsCleared, and some clients draw on it. So the API should change here, as it's not practical, I'd prefer have events separate, and have a callback before loop goes back to polling.

okey, right. yeah I currently tend to think RedrawEventsCleared should just be removed entirely - the concept doesn't really make sense to me.

The idea of redraw events (plural) seems pretty odd in the first place (I'd only expect there to be one event for redrawing each frame.

I'd guess that anything that's currently doing stuff off the back of RedrawEventsCleared can probably either be adapted to work off the back of a redraw event or else off the back of the "About to start polling again" event.

rib commented 1 year ago

App can draw whenever, we don't need an extension for that, what we want is a recommended place to perform drawing operations.

This isn't always exactly true. Mobile OSs have life cycles that say when you can/can't render - so it's going to be a very strong recommendation in that case.

Hoodad commented 1 year ago

The idea of redraw events (plural) seems pretty odd in the first place (I'd only expect there to be one event for redrawing each frame.

RedrawEventsCleared Emitted after all RedrawRequested events have been processed and control flow is about to be taken away from the program. If there are no RedrawRequested events, it is emitted immediately after MainEventsCleared.

Feels like this is there to ensure you get a clear signal for when all windows have been rendered. That's why its in plural? Not applicable to too many applications I would assume, but definitely relevant for the once with multiple windows.

Focusing in on the other events related to rendering....

MainEventsCleared Emitted when all of the event loop’s input events have been processed and redraw processing is about to begin.

If the documentation is anything to go by I think this should be called InputEventsProcessed. If not, at least the documentation should try and define what "main" events are.

RedrawRequested([WindowId]) Emitted after MainEventsCleared when a window should be redrawn.

I think this should be emitted by the OS and not tied to MainEventsCleared (which is a bit odd of a name since what even are "main" events). Though you can probably argue that the OS should want to have a new frame when there have been new user input. So maybe its not as bad as it might appear. Though in the case of no user input, to keep the animations smooth this event should still be triggered.

rib commented 1 year ago

The idea of redraw events (plural) seems pretty odd in the first place (I'd only expect there to be one event for redrawing each frame.

RedrawEventsCleared Emitted after all RedrawRequested events have been processed and control flow is about to be taken away from the program. If there are no RedrawRequested events, it is emitted immediately after MainEventsCleared.

Feels like this is there to ensure you get a clear signal for when all windows have been rendered. That's why its in plural? Not applicable to too many applications I would assume, but definitely relevant for the once with multiple windows.

yeah it's a little bit ambiguous to me. in the past I've also seen it as an implication that winit might queue your redraw requests so that if you make 5 requests then you might get 5 RedrawRequested events (in the past I think the android backend did actually queue redraw requests so it might have even worked like that in the past but it certainly doesn't do that any more).

Having multiple redraw events for separate windows would make sense but I don't really see that winit can promise to order how all windows redraw with respect to all other events.

MainEventsCleared Emitted when all of the event loop’s input events have been processed and redraw processing is about to begin.

If the documentation is anything to go by I think this should be called InputEventsProcessed. If not, at least the documentation should try and define what "main" events are.

I think "main" events are basically any and all events except a few special cases including NewEvents, RedrawRequested and LoopDestroyed, and not just input events. E.g. they could include window resize events or suspend/resume events.

Like redraw events, I don't think winit would be able to promise any special order for delivering input events with respect to other events unless it enforced internally buffering of events - which wouldn't generally be desirable.

With the way event loops work across most platforms I think the best re-interpretation of MainEventsCleared would be "the event loop is about to wait for new events" (which is how the macos backend implements it but it's not exactly what other backends do)

In practice the OS won't generally offer fine-grained control of the order that events will be dispatched without requiring you to add your own buffering + filtering abstraction for events. An event loop is generally just a thin wrapper over some operating-system-specific API for efficiently pausing a program (without burning the CPU) and waiting for new I/O events.

The Winit design (with MainEventsCleared and RedrawEventsCleared) gives a misleading impression that it can neatly control the ordering between render events and all other events but I don't think that's really portable. (especially for macos/ios, but I think the design is also getting in the way of more sensible redraw events for other platforms including Wayland, Web and Windows)

RedrawRequested([WindowId]) Emitted after MainEventsCleared when a window should be redrawn.

I think this should be emitted by the OS and not tied to MainEventsCleared (which is a bit odd of a name since what even are "main" events). Though you can probably argue that the OS should want to have a new frame when there have been new user input. So maybe its not as bad as it might appear. Though in the case of no user input, to keep the animations smooth this event should still be triggered.

I'd maybe say that it would be the backends responsibility to dispatch the RedrawRequested event with sensible defaults that make sense for the OS. In the case of Wayland that would leverage frame callbacks, in the case of Web it would leverage requestAnimationFrame and on MacOS/iOS they would be directly dispatched by the OS via View drawRect.

It makes sense that apps (UIs or game engines) would want to defer input processing until they are ready to render (so they can incorporate the latest input state into what they render) but this is a higher level concern for the UI framework or engine I think. The low-level event loop should just deliver input events as and when they come physically and UI frameworks / engines will generally have their own intermediate input state that will be accounted for when they start rendering.

Hoodad commented 1 year ago

I think "main" events are basically any and all events except a few special cases including NewEvents, RedrawRequested and LoopDestroyed, and not just input events. E.g. they could include window resize events or suspend/resume events.

Exactly, this is the problem, having a loose definition of what the event signals will cause speculation and advanced guessing.

Like redraw events, I don't think winit would be able to promise any special order for delivering input events with respect to other events unless it enforced internally buffering of events - which wouldn't generally be desirable.

Yeah the documentation tries to paint a picture that things are ordered and structured, which is either not desirable (in the case of buffering the events) or not true as it may not know when all events have been received or the order of them.

rib commented 1 year ago

Adding to this...

I realized yesterday that my corresponding Windows changes for run, run_ondemand and pump_events weren't correct and I had to take a second pass over those and found that the MainEventsCleared/RedrawRequested/RedrawEventsCleared ordering requirements are also a pretty big pain to manage in the Windows backend currently.

In my second attempt at updating the Windows backend I've currently just gone ahead and "broken" the ordering for Windows too so that Windows can just dispatch RedrawRequested directly off the back of WM_PAINT which is more in line with how the macos backend drives RedrawRequested via drawRect callbacks.

Overall this seems to result in some nice simplifications and looks like it's going to be a more portable behaviour too.

madsmtm commented 3 months ago

I've looked into this some more over the past few months; turns out that it is actually wgpu's fault, they are overriding the layer of our NSView, which makes the view "layer-hosting" (see wantsLayer), and prevents the usual mechanisms from working.

I've submitted a fix for this in https://github.com/gfx-rs/wgpu/pull/6107, we'll have to wait until a version of wgpu is released with this change (since fixing this properly in Winit will mean that the old versions of wgpu that doesn't have this fix will no longer render).

madsmtm commented 1 week ago

Update: I have submitted PRs/issues to prevent views being layer-hosting on a bunch of major raw-window-handle "consumer" crates in the ecosystem:

Let me know if you know anything missing here! In any case, I'll try to move forwards with the actual fix in Winit (and verify that it works with glutin before merging).