Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add FrameThrottled event and a way to request it #2883

Closed
wants to merge 3 commits into from

Conversation

kchibisov
Copy link
Member

Add a way to request a frame throttling hint from the windowing system. This is essential for platforms like Wayland where you don't have real vsync in OpenGL/Vulkan and you're advised to use 'frame callbacks' to throttle your rendering.

Window::request_frame_throttling_hint is a an end point for users to request FrameThrottled event, which returns error when it can't be done, so the users could fallback to their own scheduling strategies.

The API doesn't try to address all sort of frame presentation cases, like how much the frame took, etc. Such cases could be covered later, with different events in addition to the throttling hint.

For now implemented only for Wayland.

Fixes #2412.

--

cc @daxpedda for web.

@kchibisov
Copy link
Member Author

I'm not sure that I like this naming, but I want to make it clear that it's about throttling and not a Vblank timing in particular, given that minimized windows on Web/Wayland might not ever get this event.

Add a way to request a frame throttling hint from the windowing system.
This is essential for platforms like Wayland where you don't have real
vsync in OpenGL/Vulkan and you're advised to use 'frame callbacks' to
throttle your rendering.

`Window::request_frame_throttling_hint` is a an end point for users
to request `FrameThrottled` event, which returns error when it
can't be done, so the users could fallback to their own scheduling
strategies.

The API doesn't try to address all sort of frame presentation cases,
like how much the frame took, etc. Such cases could be covered later,
with different events in addition to the throttling hint.

For now implemented only for Wayland.

Fixes rust-windowing#2412.
Copy link
Member

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Web is done.

src/window.rs Outdated Show resolved Hide resolved
Co-authored-by: daxpedda <[email protected]>
@rib
Copy link
Contributor

rib commented Jun 17, 2023

We already have RedrawRequested events which could be used for this.

Currently RedrawRequested is emitted inconsistently and that's a problem in itself which would be good to address.

This is already problematic because on macOS we emit RedrawRequested via drawRect and after MainEventsCleared and applications just don't really know when they should render.

I would really like us to emit RedrawRequested something like this:

  1. On Web RedrawRequested can be driven by requestAnimationFrame
  2. On Wayland RedrawRequested can be driven by frame callbacks
  3. On macOS RedrawRequested can be driven by drawRect
  4. On Windows RedrawRequested can be driven by WM_PAINT

There shouldn't be any need to opt into frame throttling and this shouldn't require any new API, we can instead address the backend inconsistencies we have so that we can rely on the existing API.

@kchibisov
Copy link
Member Author

Currently RedrawRequested is emitted inconsistently and that's a problem in itself which would be good to address.

The issue is that some clients don't want to draw once they got the throttling event, like alacritty, for example, when nothing changes. However RedrawRequested is used to indicate that OS really wants a buffer from you back, so regardless of whatever state you have you must submit a buffer back. The FrameThrottled event could throttle the Window state updates, so we get perfect frames, but we can't replace one with another without a rework of the RedrawRequested.

@rib
Copy link
Contributor

rib commented Jun 17, 2023

Currently RedrawRequested is emitted inconsistently and that's a problem in itself which would be good to address.

The issue is that some clients don't want to draw once they got the throttling event, like alacritty, for example, when nothing changes. However RedrawRequested is used to indicate that OS really wants a buffer from you back, so regardless of whatever state you have you must submit a buffer back. The FrameThrottled event could throttle the Window state updates, so we get perfect frames, but we can't replace one with another without a rework of the RedrawRequested.

I'd say those semantics for Winit are too poorly defined / inconsistent right now and there is wiggle room to document clear semantics as part of any work to fix how RedrawRequested is emitted consistently..

In terms of the window system being able to demand that you redraw one of the things that's missing for that is being able to report expose rectangles. If the window system really requires a window to be redrawn then that'll likely either be because it's a legacy system, like X11 non-composited responding to Expose events, or e.g. on some systems because the window got resized.

Those cases that require the window to be redrawn should be honored by alacritty.

Those special cases are the exception not the rule for when RedrawRequested should be emitted though.

Most of the time RedrawRequested would follow a redraw_request for a Window.

Maybe I should have been clearer about that in the illustration.

I didn't meant to say that RedrawRequested should happen unconditionally / continuously for each of those per-OS mechanism. RedrawRequested should happen after the application has requested a redraw but e.g. triggered via requestAnimationFrame on Web or a frame callback on Wayland etc.

There may be times that it's triggered for the (exceptional) window-system specific cases that you're referring too but normally it would only trigger if alacritty specifically requested it - so either way alacritty should be fine to redraw whenever it fires and it can still handle incremental redrawing correctly. If nothing has changed alacritty wouldn't request a redraw.

@kchibisov
Copy link
Member Author

In terms of the window system being able to demand that you redraw one of the things that's missing for that is being able to report expose rectangles. If the window system really requires a window to be redrawn then that'll likely either be because it's a legacy system, like X11 non-composited responding to Expose events, or e.g. on some systems because the window got resized.

There's a protocol in works for Wayland to request a new buffer to handle GPU resents, so the compositor could re upload stuff. Or, for example, when client side decorations change the state, and they require a commit from the primary surface, so the sync frame perfect rendering could be achieved. In this case RedrawRequested is required, though, it could still be throttled by the frame callbacks.

Those cases that require the window to be redrawn should be honored by alacritty

That's true, we do request a redraw, but sometimes we winit asks us to redraw to sync with its internal stuff, like subsurfaces.

Most of the time RedrawRequested would follow a redraw_request for a Window

That's true, but the event has established certain behavior, we can't really change the semantics, because it might not be followed.

I didn't meant to say that RedrawRequested should happen unconditionally / continuously for each of those per-OS mechanism. RedrawRequested should happen after the application has requested a redraw but e.g. triggered via requestAnimationFrame on Web or a frame callback on Wayland etc.

There may be times that it's triggered for the (exceptional) window-system specific cases that you're referring too but normally it would only trigger if alacritty specifically requested it - so either way alacritty should be fine to redraw whenever it fires and it can still handle incremental redrawing correctly. If nothing has changed alacritty wouldn't request a redraw.

That's good, but how we'd handle macOS/Windows now? How the event should behave on them? Should we throttle by the timer, how should we indicate the fallback? What should we do about sync requests, like resize on macOS? I agree that it's messy right now, but I don't see how we can merge them without removing RedrawRequsted entirely. If you have a suggestion how it should work I'd like to hear it.

@rib
Copy link
Contributor

rib commented Jun 17, 2023

In terms of the window system being able to demand that you redraw one of the things that's missing for that is being able to report expose rectangles. If the window system really requires a window to be redrawn then that'll likely either be because it's a legacy system, like X11 non-composited responding to Expose events, or e.g. on some systems because the window got resized.

There's a protocol in works for Wayland to request a new buffer to handle GPU resents, so the compositor could re upload stuff. Or, for example, when client side decorations change the state, and they require a commit from the primary surface, so the sync frame perfect rendering could be achieved. In this case RedrawRequested is required, though, it could still be throttled by the frame callbacks.

yeah, that's ok - I'm not saying there aren't reasons for the window system to request redraws, there are.

In my view though there's no reason for these to require a completely separate mechansim for notifying the app when to redraw.

The app should be able to drive redraws off of one event, and it's a separate detail that sometimes that will be triggered for window system specific reasons and most of the time it will just be triggered because the app requested it.

@kchibisov
Copy link
Member Author

The app should be able to drive redraws off of one event, and it's a separate detail that sometimes that will be triggered for window system specific reasons and most of the time it will just be triggered because the app requested it.

But it simply means that in one case the user must do a redraw and in other we want to draw right away? If what you want is RedrawRequested(Reason::User/System), then we could try to merge them, but what should we do with timer based rendering in such case? Should we indicate that scheduling is instant somehow?

@rib
Copy link
Contributor

rib commented Jun 17, 2023

But it simply means that in one case the user must do a redraw and in other we want to draw right away?

This feels like a detail we're not seeing eye to eye on. I'm not sure what this means.

From the app pov it shouldn't care about Reason::User/System, so no I'm not suggesting that needs to be exposed in the event.

I'm trying to not complicate the discussion but I do think that RedrawRequested should include a redraw region (either a single rectangle or a list of rectangles) but I think we can just ignore that as a separate topic for now.

E.g on macOS all redraw events should happen via a drawRect callback. So I don't see any special cases like "in one case the user must do a redraw and in other we want to draw right away"

It's possible that macOS might call drawRect when we didn't request it and so the app might get a RedrawRequested because macOS requires the app to redraw but otherwise it will get a drawRect because the winit backend requested a drawRect callback in response to the application requesting a redraw.

Similarly for Windows. It's possible the app might sometimes get a WM_PAINT because of a window resize that could trigger a RedrawRequested event, but otherwise we should just get WM_PAINT events because the backend has asked for a WM_PAINT in response to the application requesting a redraw.

In all cases here though the application can just get a RedrawRequested event and it doesn't need to care whether it's a result of something like a window resize or a result of a request_redraw.

If the app isn't e.g. resizing it's window to trigger extra drawRect or WM_PAINT events then it will only be receiving RedrawRequested events after it requested to redraw, but these will still come off the back of drawRect or WM_PAINT (it's just that the backend will make a special request for these).

@daxpedda
Copy link
Member

daxpedda commented Jun 20, 2023

From the app pov it shouldn't care about Reason::User/System, so no I'm not suggesting that needs to be exposed in the event.

As far as I understood, this is the big discrepancy here. In some cases when it comes from the system the user MUST redraw, but in other cases the user DOES NOT NEED TO redraw.

If I understand this correctly, we have the following cases we want to deal with:

  • Tell the user they MUST redraw, because the system requires it.
  • Tell the user they requested a redraw, with Window::request_redraw().
  • Tell the user they requested a throttled redraw, with Window::request_frame_throttling_hint(). (maybe rename to request_throttled_redraw() at that point?)

I absolutely believe there is a use-case for all three to be differentiated by the user, this is important information!

I would be in favor of merging these events and adding a field to RedrawRequested, it makes sense to me. Maybe instead of Reason::User/System, something like Reason::Requested/Forced/Throttled. At that point maybe we should also consider renaming RedrawRequested to Redraw.

@kchibisov
Copy link
Member Author

@daxpedda I think that could work #2412 (comment) just fine, I'm just not sure if it'll really keep at the monitor frame rate on anything other than Wayland, and how to deal with X11, but we can basically throttle transparently for the user, given that if they want to draw as fast as they can, they don't need any request_redraw in the first place.

@daxpedda
Copy link
Member

[..], I'm just not sure if it'll really keep at the monitor frame rate [..]

[..], but we can basically throttle transparently for the user, [..]

I don't think we should. This is a bigger issue that we shouldn't be handling here. Drawing at the monitor frame rate requires some sort of frame pacing, which isn't what we are doing here. Even the whole frame throttling thing is just "frame pacing provided by the system", it's not accurate and far from ideal.

[..], given that if they want to draw as fast as they can, they don't need any request_redraw in the first place.

I thought they did, e.g. MacOS requires you to draw inside drawRect.

If this isn't the case and users can draw as fast as they can without Window::request_redraw(), then we could certainly remove it, in which case forget my arguments above and we could simplify more things as well.

@kchibisov
Copy link
Member Author

I don't think we should. This is a bigger issue that we shouldn't be handling here. Drawing at the monitor frame rate requires some sort of frame pacing, which isn't what we are doing here. Even the whole frame throttling thing is just "frame pacing provided by the system", it's not accurate and far from ideal.

But I don't see an issue here? For example on Wayland we simply request the callbacks behind the user, and once they do request_redraw we align the delivery with the requested frame callback or send it back immediately. That will do exactly what macOS with drawRect does from what I can see, and we could do the same for web, and Windows.

I'd wait for @rib input here, but I don't see an issue with such approach.

If we don't want the throttling merged with RedrawRequested, we'd have some sort of timer based renderer, but we generally want the users to draw on RedrawRequested, or when they don't want to do so due to their scheduling, they could simply ignore the event.

Be aware that we don't force the re-drawings on users, they ask for them and we use windowing system mechanisms to 'start rendering at the right time'.

@daxpedda
Copy link
Member

daxpedda commented Jun 20, 2023

After some IRC talk and clearing up some of the misconceptions I had, I arrived at the following conclusion:

There's two use-cases/scenarios Winit needs to handle:

  • Frame-throttling.
  • System notifies the user they MUST redraw.

Instant redrawing doesn't require any interaction with Winit. To the API I would suggest is:

  • We only need one event, it needs a field to tell the user if it's "forced" by the system or it comes from a frame throttling request.
  • Window only needs one method, requesting a frame throttling event.

In the future we might also want to offer more time-based approaches to frame throttling, which I alluded to here: #2412 (comment). This is unrelated to this PR though.

@kchibisov
Copy link
Member Author

Closed in favor of #2896.

@kchibisov kchibisov closed this Jun 22, 2023
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.

Implement VSync source
3 participants