rust-windowing / winit

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

Implement VSync source #2412

Open kchibisov opened 2 years ago

kchibisov commented 2 years ago

This is a common concept in e.g. browsers to have non blocking frame scheduling at the display refresh rate.

It seems like it's possible on all desktop platforms and the web. For the rest we may indicate that vsync source is not supported.

The way to integrate it would be to add WindowEvent::Frame which must be send when the new frame is ready to be drawn. That will come from the compositor directly and the users must request those event via Window::request_frame() -> Result<()>, where the result will indicate that the event can't be scheduled due to reasons.

The Api that should be used for that on platform are the following.

X11 - Xpresent macOS - DisplayLink Wayland - frame callbacks Windows - DWM get composition timings + a timer. Web - request animation frame.

The Api is essential for Wayland clients, since they must not rely on vsync from the graphics Api.

dhardy commented 2 years ago

Sounds good to me, though it would be worth considering how failures of request_frame should be handled (e.g. requesting a wake in 16ms?).

kchibisov commented 2 years ago

should be handled (e.g. requesting a wake in 16ms?).

I think it's up to application what to do here. Like it clearly knows how that it failed before hand, so can just fallback to something else. I don't want to add timers to every backend, just to provide some hand holding. At least for now. In general this can only fail on X11 if you don't have libXpresent installed, so it's like you should really try to fail...

rib commented 2 years ago

Based on discussing this a bit on IRC I think it could be good to highlight one clear sub-problem that came up which is that it could be really helpful to have just one place where applications are allowed to render.

e.g. in the case of Wayland, if it was guaranteed that apps only rendered during RedrawRequested event callbacks then the backend could potentially register frame callbacks there to get notifications from the compositor for throttling the application.

(so potentially frame throttling might be possible on Wayland with no new API if it were documented that rendering were limited to RedrawRequested events. Similarly this should be possible for X11).

Thinking more generally about high level APIs related to rendering synchronization may open a bigger can of worms, which could be good to try and break down.

One initial concern I have about a request_frame() API that seems inspired by requestAnimationFrame in browsers is that it looks kind of back-to-front compared to typical presentation APIs that would throttle / synchronize you based on a previously submitted frame.

At a lower level then synchronization with the vblank will either be handled somewhat transparently by apis like eglSwapBuffers that can be configured to block while they wait the next vblank to release a new backbuffer or may be done more explicitly with a fence object api of some form that lets the application poll (or get an event) for when work on the gpu has completed. These mechanisms only really apply to non-composited applications though.

In composited environments the norm is to get asynchronous feedback after you give the compositor a frame that may tell you when the frame became visible for the user or at least when the compositor processed the frame. For basic throttling of the application then the exact semantics don't necessarily matter too much, just so long as it's a 1:1 ratio for vblank.

In both cases though the synchronization (or I'd say throttling for a composited environment) comes after you have rendered something and submitted it to be presented.

In general though it's probably good to keep in mind the differences between composited / non-composited applications here (presentation being throttled by a compositor vs synchronized with real hardware) which will mainly affect how the app gets its feedback. In the case of eglSwapBuffers the feedback may just be implicit (the app process may be blocked until a new back buffer is ready)

Off the top of my head I can think of three main points of interest and kinds of synchronization that applications are likely to be interested in for different use cases:

  1. The compositor has finished processing the frame that included your app-rendered frame (this is what wayland frame callbacks are generally about)
  2. The compositor has rendered its own frame that includes your app-rendered frame and that is now visible to the user on a screen (E.g. some X11 compositors can give info about this, and I think there was probably also wayland protocol for this too)
  3. Now is a "good" time to start rendering to have the minimum time between rendering and having your frame be visible to a user. (As in scheduling rendering to minimize latency between what's rendered and seen - important for VR, also called frame pacing, E.g. see https://developer.android.com/games/sdk/frame-pacing)

The above request_frame() API seems like might be more suited to trying to support frame pacing, which is more about trying to use heuristics to predict the best time for an application to start rendering. I'd generally say this is a much harder, and also more subjective, nuanced problem space than the problem of throttling rendering to the VSync, which is what the title of this issue references.

Libraries like Swappy on Android might help with being able to support frame pacing via Winit but it's the kind of thing that needs to use some amount of guess work and heuristics and I think it could be quite challenging to stabilize for lots of general purpose use cases across lots of window systems.

Looking just at Swappy for Android you can see here https://developer.android.com/games/sdk/frame-pacing/opengl/add-functions how the library also depends on graphics driver extensions which will probably complicate the relationship between Winit and APIs like Wgpu.

If there would be real interest in re-considering quite how Winit drives its rendering, such as to help with synchronization/throttling/pacing, it could also be a good opportunity to re-consider what would be ideal for iOS and OSX which I generally recall drive their rendering via delegate callbacks where apps are really expected to do, and finish their rendering synchronously as part of those callbacks whenever they happen (which doesn't fit super well with the winit event model as far as I recall)... just found the issue I was thinking of: https://github.com/rust-windowing/winit/issues/2010

On Android we also want to be very clear about where rendering happens considering that apps must not render if they don't have an associated surface, so I'd definitely be interested in seeing Winit clearly limit/define where applications are allowed to render.

kchibisov commented 1 year ago

I think what I really want here is a frame throttling source, so I can draw at the vsync rate without blocking and with very low chances of missing vblanks.

I've started the work here https://github.com/rust-windowing/winit/pull/2535 for X11 for now, since I can't add Wayland ergonomically here.

rib commented 1 year ago

Assuming that we're dealing with composited applications (i.e. ignoring the possibility of writing compositors using Winit for now) then yeah it seems good to prioritize having a way the throttle client rendering, which may not happen automatically with APIs like egl/glxSwapBuffers.

ah, just realized that the original issue description listed most of the above, sorry

For X11, Wayland and maybe Windows then it seems like there's more capability to track the progress of specific frames (or on Android with the extensions that Swappy uses) where you can get detailed stats/metrics relating to a specific frame.

For macOS/iOS/Android (choreographer)/web you essentially just get a higher-level callback mechanism that will generally throttle your rendering but without any detailed per-frame feedback.

I think it would be nice if Winit could support both of these general models:

  1. a way to get per-frame throttling and timing feedback if supported by the window system or
  2. a general frame pacing callback/event

but I guess that the easiest abstraction to support initially in a portable way would just be (2) to have a frame pacing callback/event across platforms.

Based on some of the earlier discussion it might also be good to consider making such a callback/event be the only place that applications are allowed to render (good for Wayland).

It might also be possible to just define that the existing RedrawRequested event would be that event.

rib commented 1 year ago

It's maybe worth highlighting here that the #2535 discussion has been a good reminder of how much of a mine field this is for X11. This capability was something that was worked on mainly in support of Intel drivers and Gnome 3 way back around 2010-2013 but even to this day there's not really any one standard way to support throttling of x11 clients to vblank/the compositor and it's also a fairly disproportionate amount of work to try and cover all the unique cases, including:

It's pretty much no joke that each of those cases need special consideration and a mixture of different technical solutions that aren't very consistent with each other. Sometimes the ideal solution depends on glx/egl extensions but with inconsistency between glx + egl support (both out of scope of Winit currently). The situation with Vulkan is likely even worse in some cases because since Vulkan has emerged there has been comparatively little development on x11 (the main case of handling full screen games would be the priority)

kchibisov commented 1 year ago

From what I can say, the only option for now would be not support X11 at all. And add support for _NET_WM_FRAME_DRAWN will be adopted by picom/kwin for example. Xpresent shown that it's not really good solution and works only with Xwayland(which is good for games though, but there's no way to detect that we're under XWayland).

So I guess we may just go without X11 for now, it's not really a big deal given that Wayland is where all the cool work is happening and X11 support is more about providing better experience for software that no-one cares about maintaining anymore.

rib commented 1 year ago

Yeah, I'm mostly all for forgetting about X11 at this point ;-p

Unfortunately, in reality Wayland still isn't supported on Nvidia and so a large chunk of Linux desktop users are likely to still be stuck using X11 which does spoil the picture here somewhat.

Maybe one way of starting to think of things could be that classic X11 likely implies that you're running on Nvidia and everyone with open source drivers is probably (or should at least have the option of) running a Wayland compositor / xwayland. Though it's not quite that simple - I have a hybrid Razer laptop with integrated (Intel) graphics and a discrete Nvidia GPU and that's also awkward to run Wayland on.

rib commented 1 year ago

but there's no way to detect that we're under XWayland

I imagine if this would be helpful we can figure out a way of detecting this with a reasonable amount of certainty (though regarding xpresent I think we'd be playing with fire to be trying to use the protocol directly outside of the EGL/GLX driver)

kchibisov commented 1 year ago

Well, the recent nvidia stuff does work on Wayland from what I see and some folks run sway on nvidia binary drivers, so it's changing from what I can say.

rib commented 1 year ago

Yeah actually I just had my mind blown while looking at the mutter source code re: Nvidia support.

I was just looking through some of the mutter code out of curiosity to see if I could find a standard enough atom that would e.g. be set on the "root" window to identify xwayland and I just saw that they have been adding EGLStream support to mutter!

EGLStreams is the extension that Nvidia have always pushed for using as the basis for supporting Wayland which has historically been rejected by the open source xorg/freedesktop community (since it means having a special case for lots of things just to support a single vendor via a vendor-specific EGL extension)

It looks like that was started around december 2021 and there's still more recent work that's been happening for supporting EGLStreams so maybe we're finally going to get to the point where we really can say good riddance to classic X11!

rib commented 1 year ago

That's made my day!

Considering that I bootstrapped the Wayland support in Mutter I was always a bit disappointed that for years afterwards the maintainers took the principled stance against supporting Nvidia's EGLStream extension vs being (imho) more pragmatic about the bigger benefits that could come from being able to move the community forwards to Wayland at least (and just argue about finding a common technical solution to replace EGLStreams + gbm later)

There are actually reasonable pros and cons arguments for EGLStreams vs gbm (which is also not a perfect solution) so I think we could have been more compromising early on here.

kchibisov commented 1 year ago

If you look at comsic-comp (The pop os compositor) it should support EGL streams, since smithay supports EGL streams, but EGL streams are not that required nowadays given that NVIDIA has drm/gbm support now. You just need a recent driver and luck that it'll work with your setup.

rib commented 1 year ago

Some of their work to support drm/kms/gbm looked like it was very limited, and only usable for some very narrow use cases but maybe it's improved. I wouldn't be surprised it's it's still much better to just use EGLStreams for Nvidia for anything but bare bones support. At least in mutter it looks like they are actively working on EGLStreams support so I guess it's still the more practical way of support NVidia vs using their gbm support.

kchibisov commented 1 year ago

I think for now we'll go with just a throttling hint given that frame timings are extra on top of it most of the time and some clients might not provide it. At least this will solve the problem for the web and Wayland backends, and the rest will use fallbacks.

Drawing on such throttling hint is not what applications always want, since they could have a state where nothing should be drawn. The only caveat with such API is that you should do the drawing after requesting, this assumption will be on our users, given that winit doesn't draw or control the drawing itself.

daxpedda commented 1 year ago

I don't know much about the other backends besides Web, but maybe we could draw the line at throttling hints. Wgpu is planning to implement an API to get precise timings for frame pacing: https://github.com/gfx-rs/wgpu/issues/2869.

So if Winit offers throttling hints and Wgpu offers timings for frame pacing, that seems like a good separation of responsibilities to me.

kchibisov commented 1 year ago

So if Winit offers throttling hints and Wgpu offers timings for frame pacing, that seems like a good separation of responsibilities to me.

winit could get this as well(from display server, it's available on at least all desktop platforms), though it's true that graphics drivers also provide such information and some provide extra methods to manipulate it.

Throttling could be added because it's not controversial at least.

daxpedda commented 1 year ago

winit could get this as well(from display server, it's available on at least all desktop platforms)

Ah, I wasn't aware, sounds good to me then.

rib commented 1 year ago

winit could get this as well(from display server, it's available on at least all desktop platforms), though it's true that graphics drivers also provide such information and some provide extra methods to manipulate it.

I think it's unlikely that there's much redundancy for getting the same information on a specific OS.

One of the challenges here is that different OSs expose similar things in rather different ways - this makes it challenging to figure out a neat way of exposing some of these details in a portable way, especially if Winit tries to avoid graphics driver details, and Wgpu tries to avoid window system details.

That said though I agree that helping with throttling would be a good starting point and I think there's relatively low hanging fruit where Winit can help here.

With the pump events branch I made some progress towards making RedrawRequested the event that applications can use to drive their rendering which should naturally throttle rendering for Web, Wayland, Windows, macOS and iOS.

I think the RedrawRequested event is already a reasonable way to expose frame scheduling/throttling for Winit.

  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

We can then clearly document that applications should drive their rendering via RedrawRequested events exclusively.

That seems like it would be a good first goal for Winit to at least drive redraws based on the throttled events that the window systems provide.

There would certainly be ways to improve the scheduling on different OSs but as a baseline that seems like it could be better than the current situation.

We also had some good discussion about some of this stuff here: https://github.com/rust-windowing/winit/issues/2640

kchibisov commented 1 year ago

I think the RedrawRequested event is already a reasonable way to expose frame scheduling/throttling for Winit.

I don't think it's a good idea tbh. We need an event to tell application to redraw, which on web it's not the case, so right now we don't even send this event, because OS never asks you to draw. However on macOS/Wayland you could have this event for sure when the OS simply forces you to redraw, regardless of the frame scheduling. This event is not only about contiguous drawing in the end of the day.

The application may even not use the frame throttling hints and rely on the graphics driver, so it's not that great either here.

However what we can do instead is track when the application uses frame callbacks and throttle other UI events, like squash resizes under the hood and so on, so for example it'll mitigate EGL resizing buffer latching on Wayland, because you'll resize only once per frame callback.

We can then clearly document that applications should drive their rendering via RedrawRequested events exclusively.

My main issue with that is that sometimes you should force the client to submit the buffer regardless of what throttling you're using, it's true that we basically have a way to request a redraw and a way to throttle frames, and those events sound really similar.

I'd give an example on how RedrawRequested is being used in the wild by alacritty. This all happens inside the run.

match event {
  WindowEvent::RedrawRequested => dirty = true;
  WindowEvent::FrameThrottled => has_frame = true;
}

if !has_frame {
  return;
}

if dirty {
  dirty = false;
  do_gl_stuff();
  has_frame = false;
  request_frame().or_schedule_timer();
  swap_buffers();
}

It's true that we could throttle the redrawing via the RedrawRequested by naturally throttling it, however the issue is how should we limit fps on macOS/Windows, what to do with the platforms we can't? The proposed API in https://github.com/rust-windowing/winit/pull/2883 will handle such cases by telling the users that they should do it on their own, with RedrawRequested we can't unless we redesign the entire event.

rib commented 1 year ago

Winit shouldn't need to have separate events to distinguish when a redraw is technically required due to window-system specific details (such as Expose events on non-composited x11) or e.g. resizing of windows, that would be an over-complicated application programming model imho.

It should be ok that there's one event apps can drive redraws from, which 99% of the time arrives because the app has requested it or may come because the OS requires it.

One thing that's probably missing from Winit's design currently though is that request_redraw should probably accept damage rectangles/regions so that RedrawRequested events can echo back the damaged regions.

That would be more consistent e.g. with the design of drawRect that can tell you a region to redraw, as well as X11 Expose events as well as WM_PAINT update regions.

Being able to forward a region that needs to be redrawn lets you just have one mechanism because the OS can simply emit events with a full-window region if it requires you to redraw everything, or else the events can provide a union of the regions that where damaged via request_redraw.

It's true that we could throttle the redrawing via the RedrawRequested by naturally throttling it, however the issue is how should we limit fps on macOS/Windows

We can use drawRect on macOS as suggested which should throttle? macOS is specifically one of the platforms where we really want to remove the emission of RedrawRequested after MainEventsCleared and should pretty much end up implementing the throttling your after out of the box.

and WM_PAINT on windows which should also throttle

kchibisov commented 1 year ago

I'm pretty sure we're talking past each others.

If I as an application request a throttling hint, but I don't want to draw on receiving it, how I should handle that with your idea to merging the events? Why should we care about damage regions in the first place now, given that they are emitted by, for example egl/vulkan/wgpu calls under the hood?

The main point is that my application want to request frames for contiguous animations, and in some cases my application doesn't want to do any drawing in reaction to FrameThrottled but simply wait until its scene changes and do an instant drawing, if we make change the RedrawRequested to a point of DrawNow then ok, sure, but it's not really clear that we really should do the draw? I think on macOS we must draw in drawRect, because the OS calls back to you. And how should we deal with user requested redraws? We can't call drawRect ourselves, so we must call it from other place like it is now.

From the drawRect docs

This method is called when a view is first displayed or when an event occurs that invalidates a visible part of the view. You should never call this method directly yourself. To invalidate part of your view, and thus cause that portion to be redrawn, call the setNeedsDisplay or setNeedsDisplayInRect: method instead.

So it seems like drawRect calling is done by the os, which is not what the event I'm proposing does or could possibly do, it's exactly what RedrawRequested is for, and it means that the sync operation should be done.

rib commented 1 year ago

I'm pretty sure we're talking past each others.

yeah, I guess maybe. I can try and be more specific with how I'm imagining the implementation would work in my mind to see if we can figure how we're talking past each other.

If I as an application request a throttling hint, but I don't want to draw on receiving it,

tbh this isn't a concept I would expose to applications - I think RedrawRequested events can be implicitly throttled because most window systems have mechanisms (such as drawRect, WM_PAINT, frame callbacks, requestAnimationFrame) we can utilize for this without requiring applications to request anything.

The main point is that my application want to request frames for contiguous animations

yeah, and in this case your app would be calling request_redraw while animating which would result in a future RedrawRequested event.

If you aren't animating and nothing has changed you won't request a redraw and so you wont get RedrawRequested events. (except for window-system specific special cases like windows resizes which you need to handle even if you aren't animating)

On macOS after you request a redraw, then the backend will request a callback from drawRect and then the next time that drawRect is called then the application will get a RedrawRequested event which the app can then render within, properly synchronized in the way macOS/iOS wants all rendering to be synchronized.

On Windows after you request a redraw then the backend will request a WM_PAINT event and the next time that it receives a WM_PAINT (throttled by the OS) it will emit a RedrawRequested event for the app.

On Wayland after you request a redraw Winit can dispatch that e.g. after MainEventsCleared (wayland doesn't care when the client renders) but you will never emit RedrawRequested more frequently than one per frame callback, so sometimes you may throttle dispatching the RedrawRequested until the next frame callback arrives.

Similarly on Web after you request a redraw Winit can dispatch a RedrawRequested event asap but never more frequently than once per requestAnimationFrame callback.

kchibisov commented 1 year ago

tbh this isn't a concept I would expose to applications - I think RedrawRequested events can be implicitly throttled because most window systems have mechanisms (such as drawRect, WM_PAINT, frame callbacks, requestAnimationFrame) we can utilize for this without requiring applications to request anything.

So in such case when the application doesn't want to draw anything it simply ignores the event?

kchibisov commented 1 year ago

I mean, if we pass a state on how sent the RedrawRequested back like system or user and if we warn when user request went None we could probably do that, we just can't keep the event as is and change its semantics, and we'd need some indication when there's a must to redraw (it'll send a System in it, on Wayland it'll soon be the case, when the protocol to get new buffers will be present, and on macOS it'll be the case on startup and some internal operations, like Resize).

tronical commented 1 year ago

FWIW, the way @rib describes it is also pretty much the way Qt works.

rib commented 1 year ago

tbh this isn't a concept I would expose to applications - I think RedrawRequested events can be implicitly throttled because most window systems have mechanisms (such as drawRect, WM_PAINT, frame callbacks, requestAnimationFrame) we can utilize for this without requiring applications to request anything.

So in such case when the application doesn't want to draw anything it simply ignores the event?

No you shouldn't be ignoring the event because it's either come because the window system requires it (e.g. window resize) or it's come because your app requested it.

If you don't want to draw anything then you don't need to request to redraw and so you will only get RedrawRequested events for event that you must redraw for.

kchibisov commented 1 year ago

I mean, I know how it sort of works, the issue is that we try to revoke semantics of the established event to completely different semantics. I'm fine with such event if we can somehow indicate via it that redraw must be performed regardless of user decisions. So if we settle with RedrawRequested(User/System) meaning that System forces redraw and User means, that it's your request and you may not draw on it, then it's probably will work just fine. It'll be squashed under the hood with system taking preference over used and being throttled the way it expects to be throttled.

I'm just afraid that we'd need yet another event telling clients that they must resubmit their buffers, which is a real world use case on the present systems.

kchibisov commented 1 year ago

If you don't want to draw anything then you don't need to request to redraw and so you will only get RedrawRequested events for event that you must redraw for.

But that's an issue then? On Wayland you must draw after asking for a frame and the frame event is more of a hint, not a must. You may not know before hand that you don't want to draw, because by the time you wait for the frame back, the state might change and you may want to draw.

rib commented 1 year ago

We have a bit of a zig zag conversation going atm, but just to note that I commented here https://github.com/rust-windowing/winit/pull/2883#issuecomment-1595863774 that I'm not suggesting that the user/system distinction needs to be exposed to apps.

kchibisov commented 1 year ago

What I'm trying to say that I want to request a frame hint, without a need to draw, because I don't know in advance whether I'll draw or not, simply because by the time I wait for a hint, I might get event from e.g. PTY that I must draw, but by the time I drawn into the screen the state was like I don't want to draw, simply because I just draw something.

rib commented 1 year ago

On Wayland you must draw after asking for a frame and the frame event is more of a hint, not a must. You may not know before hand that you don't want to draw, because by the time you wait for the frame back, the state might change and you may want to draw.

You don't request a frame in wayland though, you can submit a frame and you can ask for a frame callback that the compositor will ack when it's finished dealing with the corresponding frame. It's a purely CPU-side feedback mechanism that can be used by clients to avoid submitting to the compositor faster than it's rendering.

You shouldn't need to predict the future.

The wayland backend can always ask for a frame callback when it submits a frame (it almost certainly should do that for feedback)

I think this may be a notable detail that's perhaps being misunderstood about Wayland frame callbacks.

If you want to submit a frame to a wayland compositor while utilizing frame callbacks for throttling you don't need to:

  1. request a callback
  2. wait for a callback
  3. render frame
  4. submit frame

You can just:

  1. render a frame
  2. submit a frame + request a callback (the callback is effectively for the frame your submitting being handled by the compositor)

So if the app requests a redraw, you can immediately let it render a frame and submit the frame with a request for a callback.

You can also then track that you have recently emitted a RedrawRequested event but you have not yet received a callback for that and so if you get any more requests then the backend can now make sure it waits until the frame callback arrives.

rib commented 1 year ago

With the way requestAnimationFrame works for Web the state tracking would be really similar to the Wayland backend I believe.

kchibisov commented 1 year ago

I know the flow, you do

wl_surface.frame();
wl_surface.commit();

The thing is that compositor writers don't really like when you do so without a buffer and other state being attached, so that's why they all suggest the following flow.

loop {
if has_frame {
  wl_sufface.frame();
  eglSwapBuffers();
}

What I simply want is the way to request a hint without drawing in the future on e.g. macOS, because that's how I do for years on Wayland.

  1. Ask for a frame
  2. Submit buffer
  3. wait once the screen is dirty and draw again (other thread marks it as dirty).
rib commented 1 year ago

Yeah the wayland backend shouldn't be submitting dummy frames or anything like that for the sake of getting a callback.

It's an awkward design challenge currently that Winit tries to pretend that it's decoupled from graphics drivers, but really that's a bit of a white lie. The window system and graphics stack are more tightly coupled than winit wants to admit unfortunately and this is an example of the problem with that assumption.

An app that's using the gpu will commit frames to a Wayland compositor via the graphics driver which is not something winit is responsible for.

That's an inherent awkward detail with the combination of the design of Winit currently combined with the design of Wayland.

With what I'm advocating for though RedrawRequested is not something that applications can ignore. They are either redrawing for events the window system requires a redraw for, or they are redrawing because they want to.

I think that means you can also rely on applications committing a frame for the compositor.

If you can't assume that then I think the Wayland backend is pretty much out luck anyway here when it comes to frame callbacks at all. I don't really think the winit wayland backend should ever be trying to submit it's own dummy frames for the sake of getting callbacks.

kchibisov commented 1 year ago

I mean the design works, because you can ask right before drawing, driver can't request frame callbacks(unless you've enabled vsync).

What I want is to ignore drawing when I don't want to, but still request a frame to have a hint if the drawing will change over time, like keyboard input or some other thread decided that I need to draw.

In a way you say it and how it looks, it means that my other thread must do redraw_requested, so the OS calls me back when the time comes, and with Wayland I simply do that right before the previous draw call, so I have the frame scheduled already and I don't want to do anything like that.

Maybe we want to change the request_redraw as well so in some cases it'll be a Hint and in other Draw? So on macOS, you won't have a hint, and on Wayland you could have both simply. And once you can't request a Hint it'll return an error? Both will be merged together and work the way they should? And this will be also indicated in the event itself.

I think such design will account both Wayland/macOS and timer based rendering fallbacks. You'd still be able to merge all of them into single events and so on. Given that we have squashing of the events, it'll mean that asking for Hint and then for Draw will result in getting Draw. The only issue is asking for a Draw on Wayland without doing a commit afterwards won't really work unless you have Hint in flight.

I'm sorry, but I don't see how we could do it otherwise. It just semantics of macOS's drawRect and Wayland frame callbacks is different.

on macOS you get it once you have dirty state, so it'll call to you once the right time has come. On Wayland you just ask for frames and throttle until you get one, you could still set dirty bit in between, but it you'll wait until you get your callback done. web is a bit different and allows asking at any time from what I can see.

rib commented 1 year ago

Checking the implementation of mutter I can see that yeah it buffers all new pending frame callbacks until it gets a surface commit and then merges those pending callbacks into the surface state such that the callbacks will then happen after the next frame.

Technically you could imagine it would be possible for the compositor to respond to frame callbacks independent of commit messages.

But yeah that's a more high-level design challenge for the Wayland backend + Winit I feel.

It wouldn't seem appropriate for the Wayland backend to work around that by committing it's own frames while it doesn't know whether a Winit app has committed frames.

If the wayland backend wants to use frame callbacks at all it seems like you have to know when the app is committing frames, and I feel like a reasonable way to know that is to require apps to draw in response to RedrawRequested events - similar to how macos apps are expected to draw in response to drawRect.

rib commented 1 year ago

On Wayland you just ask for frames and throttle until you get one,

Sorry but I still dispute the notion that on Wayland you ask for frames.

An async window system wouldn't want to pre-throttle apps with a 'can I please render' round trip, unnecessarily

frame callbacks are designed to come after your frame has been handled by the compositor.

If you try and invert that then you're going to be misusing the API imho.

kchibisov commented 1 year ago

If you try and invert that then you're going to be misusing the API imho.

But that's how everyone are using them? If you see any wayland example it simply does the loop I've said. I'm not inverting anything, it's just what I see after looking and writing wayland clients for years. https://emersion.fr/blog/2018/wayland-rendering-loop/ (from wayland maintainer). Application may decide to not render at all, because in the of waiting for the callback it wasn't marked as dirty by some other thread or input event.

What you want to do with RedrawRequested is completely parallel to what I want with FrameThrottled, they could co-exist and one can throttle another on Wayland/Web, but merging will require adding tracking into the event itself.

rib commented 1 year ago

I guess we're maybe talking cross purposes again.

The loop you posted above looks fine, it's not inverted in the way I'm thinking above, it just makes sure that it only requests a frame callback when it knows there will be a commit from calling eglSwapBuffers

Okey, I think maybe i'm seeing how we're talking over each other.

Another reductive way of looking at your FrameThrottled API is as practically-wayland-specific way of making it the application's responsibility to call wl_surface.frame() if they do decide to commit a surface.

And that then means you don't have to assume they will render at some point because you're now effectively requiring apps to always call a winit API when they present a frame.

Another way of looking at it is as a solution for handling the fact that Winit is not integrated with the graphics driver and you need the app to tell Winit when it really presents a frame.

Potentially a more general API could also be something like Window::pre_present_notify(), which would give Winit (the Wayland backend specifically) the information it needs to know that the application is about to commit a frame and so you can request frame callback if you want)

rib commented 1 year ago

Looking at FrameThrottled in that light it feels like it's a solution that's really trying to tackle multiple separate issues at once.

One of the problems is currently wayland-specific (the backend doesn't know when wl_surface.commit() is called) but it also ends up exposing a throttling solution that is a very thin wrapper over how the Wayland protocol works. I can also sort-of see how this could be mapped to Web requestAnimationFrame but it doesn't feel like a good fit for other window systems.

That said I personally think it would be good to break the illusion that Winit and the graphics drivers are entirely orthogonal since I think that's pretty much not the ugly reality of window systems.

If you need a hook for when apps present frames though I would currently tend to think that a more general one like a pre_present_notify that all apps should call could prove to be more useful.

This feels like a somewhat orthogonal problem, separate from 'how can we throttle rendering'.

E.g. If there was a general hook like Window::pre_present_notify() I think that would also then be enough for the Wayland backend to support throttling RedrawRequested events as described above.

Other backends might not strictly need that kind of hook yet but there's still a chance they could take advantage of the extra information as part of how they handle throttling.

kchibisov commented 1 year ago

The idea right how it should work was outlined before and yes, other backends don't really map in a way you describe, however winit could do similar scheduling based on DisplayLink(some dwm API) on macOS/Windows where it'll callback to the user once the frame fires on such interfaces. So it'll basically be a throttling hint that way.

It's true that having pre_present_notify could help, but on the other hand it's not clear that we always should request a frame that way. The main issue I have with your RedrawRequested which I've said multiple times, that my app generally wants to get hints, but it doesn't mean that it wants to draw anything when it gets each of the hint. This maps on Web and Wayland, but doesn't on the macOS, with its drawRect.

How would the rendering loop for macOS looks like? I think if you ask it that you want to present it'll simply call drawRect and you must do the actual drawing (also, I don't see that you must do so from their docs).

It seems to me that what you suggests is more or less:

  1. Add pre_present_notify, so it can schedule frame callbacks transparently for the users. Users won't know about them, but they will throttle things, if pre_present_notify is used, everything is instant.
  2. Deliver the RedrawRequsted for the users if they asked for they do pre_present_notify and asked for request_redraw.
  3. The request_redraw must not batch, at all.
  4. On macOS/Windows pre_present_notify is a no-op, the request_redraw will force the macOS to call drawRect thingy.
  5. On Web, it does the thing similar to what Wayland does.

Is this what you want? So the users simply do request_redraw and throttling is done transparently for them? They'll still must draw on the RedrawRequested, because they've asked for it, and we remove arbitrary asking for such a redraw when resize happened (unless backend wants it for its internal state tracking, like it's on Wayland).

kchibisov commented 1 year ago

As a separate note, we have a cases, where users wake up winit's event loop from other thread with a need to force a redraw operation, given that some main events could be altered (resizes) whether we get a frame callback or not, should we state internally that all users event are handled before all winit events, so if they want to ask for redraw, they'll get what they want?

Edit: get what they want, I mean that winit's internal behaviors like squashing resizes on Wayland, so you have only one resize per frame callback would be hold?

kchibisov commented 1 year ago

Hm, no, throttling resizes is a bad idea, we'd just say that You should resize the actual frame buffer once you get a RedrawRequested event.

kchibisov commented 1 year ago

While that's sounds more or less, what should we do with X11 @rib and backends we fail to apply the model for some specific reason? Should we simply say that such backends will deliver event right away and you may want to throttle everything yourself?

I'm also not sure if everything will work the way it was discussed here in practice (it'll target current monitor refresh rate), so we'll see how it'll go...

kchibisov commented 1 year ago

@rib I guess windows is also done now, to some point?

kchibisov commented 1 year ago

Windows is still not done, https://github.com/rust-windowing/winit/issues/2900#issuecomment-1694637038

fredizzimo commented 11 months ago

Is there any time estimate for the macOS implementation?

I know have a draft PR for Neovide that switches to use the winit Wayland implementation instead of the custom one we used before and that works great.

But macOS is still problematic for us, since the opengl swap_buffers is broken, as reported for example in these issues

So, for now, the only real option for macOS users, is to use a software timer with the refresh rate set to the monitor rate, which is reported to work better than the standard opengl vsync.

We are soonish planning to do a release with all our rendering changes, but I would prefer to not release something that either does no frame sync at all using a lot of power, or something that syncs to the completely wrong rate if possible.

I have tried to find someone to implement the displaylink callbacks in Neovide, but it seems that no macOS developers are available. So, I'm hoping for more luck with winit developers, although that's probably too much of wishful thinking as well.

The Windows implementation is not that important for us, since the DWMFlush based implementation that we have work reasonably well. It's not perfect, sometimes frames are dropped without any visible reason, but I think it has something to do with the GPU throttling down, and otherwise the system not prioritising the graphics display. And I have tried many different things, but waiting for DWMFlush, just before swapping the buffers is the best I found. I also tried IDXGIOutput::WaitForVBlank but it was not really any better. The only way I was able to get it completely smooth was when I was using Direct3D and waitable swapchains, but I decided for now at least to not use that, since it's too different from the other platforms.

kchibisov commented 11 months ago

Is there any time estimate for the macOS implementation?

There're no estimates, it just someone should want to do that. I don't think it's hard though, you just need to hook the callback. Timer isn't that bad as long as it's consistent.

I might allocate my time for that, but no promises. I do only Linux stuff in the end.

fredizzimo commented 11 months ago

Thanks.

Yes, that's what I thought would be the case, we just have to wait and see when someone picks it up. I just wanted to let you know that from the perspective of Neovide it's quite high priority, but we don't expect anything from you. If someone really needs it from our side, they can make the winit implementation and send a pull request here.

It's always tricky to find people to implement something that they don't really need themselves. Especially on macOS, which has much fewer developers in total, compared to what seems to be a quite big userbase for Neovide. As you said, it looks like it's easy to do, and I was even considering doing it blindly myself and just check that it compiles, but I decided against it, since it's very hard to guarantee that it actually works, and not just improves the situation.