-
Notifications
You must be signed in to change notification settings - Fork 904
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
Modified repaint system on Mac OS to use the OS-mediated system instead of doing it entirely in winit to better support non-GPU-accelerated rendering #2189
Conversation
this currently does break gpu rendering in my wgpu project, unfortunately there's a not in the documentation for setNeedsDisplay that it has no effect in OpenGl views, which probably also means it has no effect in Metal views as well |
Sorry for the delay, I got access to a slightly newer mac that can run Metal to iterate with and I tried to fix this and that took some time to do cheaply. Unfortunately, it did not go ideally. What I did was enable both the old non-OS-mediated system and the new OS-mediated system, which did allow both GPU and non-GPU drawing to take place. Unfortunately, in the non-GPU case it fired RedrawRequested twice for each repaint request, thus causing duplication of CPU time and possibly misbehaving according to winit's documentation. I see several ways to move forward, but none are ideal. I'm willing to try to implement these, although I'd likely have much more success implementing those that don't require changing other platforms to support new APIs.
|
In general you either use window in hardware acceleration context or not. So as a solution if macOS requires something else in software acceleration context you could add a window builder option for macOS like |
Having a separate API on different platforms seems questionable at best. Ideally, users of softbuffer could just see that the platforms that they care about are supported and then not care about any platform-specific concerns any further than that. |
Is there any consensus or feedback from the winit maintainers over the best way to handle this? I'm thinking option 5 from my (not immediately) previous comment, as it's the only option that doesn't rely on heuristics and includes no new APIs. It might be a good idea to open an issue after so someone who is looking for issues to work on later can go and add such an API, or at least to make note of the non-ideal resolution, but by implementing option 5 now everything works with Softbuffer (and I can add a warning to its documentation about efficiency — it might be prudent to add it to the winit rustdoc on request_redraw as well) and the behavior of GPU-accelerated rendering is unchanged. |
…ccelerated repaint requests
In order to get softbuffer fully working on mac os as quickly as possible, iI've re-added the non-OS-mediated repaint system, and tested against both softbuffer and wgpu and both work. After this is merged, I'll push a softbuffer update warning about not doing anything heavy in RedrawRequested's handler and to cache, due to the double call when not using GPU-accelerated rendering on Mac OS. EDIT: It has been brought to my attention that I should clarify this: only non-GPU-accelerated applications (which presently means only softbuffer to the best of my knowledge) receive two |
Requesting repaint twice sounds like a bad idea especially for GPU-using games, because it halves their possible render performance (due to duplicate drawing) unless they have some way of picking out only one of the two events, and most such programs don't have any concept of “nothing needs redraw, so don't” since they're doing continuous animation. So, making that change would complicate (what I assume is) the typical case of winit's usage. Considering the issues, it seems to me that a better option would be to, somewhat as mentioned above but in reverse, add a builder option to opt-in to cooperating-with-the-system repaints (which might also be useful for other platforms in more complex cases). (Or perhaps use cases like softbuffer should actually be creating their own custom views (in macOS terminology)/widgets with normal-for-the-platform redraw callbacks.) |
@kpreid The OS-mediated system is a no-op when using GPU-based games, so it won't affect that use case. That's a huge part of why I think that this is the best solution for now. As far as I am aware, there are no non-GPU-mediated uses of winit currently, as all I'm aware of after my work on Softbuffer is some other people who attempted the same thing but didn't get nearly as far, and thus it was never suitable for production use. |
I understood your description as that all applications, GPU-accelerated or not, would receive two |
That is indeed not the case. To confirm, only non-GPU-accelerated applications (which presently means only softbuffer to the best of my knowledge) receive two I just re-read my description and I see now that is very unclear. I'll edit it to make it more clear. |
CHANGELOG.md
if knowledge of this change could be valuable to usersUpdated documentation to reflect any user-facing changes, including notes of platform-specific behaviorCreated or updated an example program if it would help users understand this functionalityUpdated feature matrix, if new features were added or implementedThere has been prior discussion of this issue and how I found out that this is the needed fix:
The short version of the context for this change is that in working on softbuffer, my new crate to allow GPU-less 2D display to a raw window handle window I encountered a bug where once something was shown to a Mac OS window (during a resize) then nothing more could be shown until the window resized. After much digging, it turned out that the cause for this is that winit is not properly notifying Mac OS of the repaints in order to cause it to allow it to push these repaints to the screen. This is because on Mac OS winit handles repaints entirely within itself and does not notify the OS or allow it to mediate them. This pull request changes that by making request_repaint() notify the OS instead of simply tracking it internally. It also removes the internal tracking system as it is no longer used.
My biggest concern with this pull request is that it might break GPU-accelerated rendering. I can't test this myself as the only Mac computer that I have access to is too old to support Metal and this Wgpu. I tried to find some opengl crates with usable examples for testing, but what I found didn't work properly before the change so the results from after the change are not meaningful. As such, before this is merged it is vital that someone who has access to a more modern Mac computer test this with wgpu and other GPU-accelerated rendering systems to ensure that they are not broken.