| Summary: | SDL doesn't send SDL_WINDOWEVENT_RESIZED soon enough for Metal fullscreen transition | ||
|---|---|---|---|
| Product: | SDL | Reporter: | foo.null |
| Component: | *don't know* | Assignee: | Ryan C. Gordon <icculus> |
| Status: | RESOLVED WORKSFORME | QA Contact: | Sam Lantinga <slouken> |
| Severity: | normal | ||
| Priority: | P2 | CC: | amaranth72 |
| Version: | 2.0.9 | ||
| Hardware: | x86 | ||
| OS: | macOS 10.13 | ||
| Attachments: | Don't defer window resize/move events | ||
Hm, I guess this may only be throwing off the Metal validator. Not sure the correctness impact otherwise or if SDL is prematurely optimizing this. Ah this isn't for avoiding multiple events, this is just SDL deferring sending resize/move events until the fullscreen animation is done. I don't know why SDL is doing that. Created attachment 3540 [details]
Don't defer window resize/move events
Pinging this as this is SDL doing something clearly wrong (allowing at least one frame to render incorrectly) for an unclear reason. This doesn't necessarily answer your questions about SDL window events, but if you're using Metal on macOS you can use your NSView's resizeWithOldSuperViewSize callback to update the metal layer's drawable size. It should be called immediately. SDL's internal Metal code has an example of it: https://hg.libsdl.org/SDL/file/e1e13e154128/src/video/cocoa/SDL_cocoametalview.m#l77 Thanks for the workaround. I think in my case it'll be easier to just patch SDL than add another resize handler. Not that this is likely specific to Metal, just that there is no GL validator to complain about it. The platform implementations of OpenGL windowing (NSOpenGL in the case of macOS) handle the GL's backbuffer surface internally – so there's something similar to that Metal code already being run for OpenGL. Just using the window size as reported by OpenGL won't let you do high-dpi properly, since it uses DPI-scaled units whereas you'd want to specify the drawable size in pixels. The metal code I linked handles that properly as well. If you were using OpenGL and needed to query the proper drawable size in pixels, you could use SDL_GL_GetDrawableSize. Typically in that case you'll also need to know the DPI scale in order to size things appropriately, which can be obtained via the drawable size divided by the window size. (In reply to Alex Szpakowski from comment #7) > Just using the window size as reported by OpenGL Whoops, I meant: as reported by SDL. (In reply to foo.null from comment #6) > Thanks for the workaround. I think in my case it'll be easier to just patch > SDL than add another resize handler. I'd be wary of doing that without knowing the full reasoning behind why the check was there in the first place, or at least without doing extensive testing of using that event in various fullscreen transition scenarios (including multiple queued transitions) to verify that removing that check doesn't introduce a regression. Maybe it's completely fine to remove that line, but it was put there for a reason and platform windowing-related details are very finicky... [Sorry for the spam, I should've collated my thoughts before posting...] (In reply to Alex Szpakowski from comment #9) > Maybe it's completely fine to remove that line, but it was put there for a > reason and platform windowing-related details are very finicky... For the deferred positioning line in particular, it was added to fix a bug: https://hg.libsdl.org/SDL/rev/05aa6d232dca I'm not sure about the deferred resizing line (it was added when Spaces support was first committed so there isn't a bug description to go along with it), but at the very least we'd need to make sure either the bug the deferred positioning line addressed doesn't happen at all anymore, or there's another way to fix that bug. > Just using the window size as reported by OpenGL won't let you do high-dpi properly, since it uses DPI-scaled units whereas you'd want to specify the drawable size in pixels. The metal code I linked handles that properly as well. > If you were using OpenGL and needed to query the proper drawable size in pixels, you could use SDL_GL_GetDrawableSize. Typically in that case you'll also need to know the DPI scale in order to size things appropriately, which can be obtained via the drawable size divided by the window size. I think this is a bit tangent to the issue here (being rendering a frame using a previous drawable size). For Metal I'm using -[CAMetalLayer drawableSize] and for OpenGL I use SDL_GL_GetDrawableSize(). > For the deferred positioning line in particular, it was added to fix a bug: https://hg.libsdl.org/SDL/rev/05aa6d232dca > I'm not sure about the deferred resizing line (it was added when Spaces support was first committed so there isn't a bug description to go along with it), but at the very least we'd need to make sure either the bug the deferred positioning line addressed doesn't happen at all anymore, or there's another way to fix that bug. Ah, I didn't realize how intricate this could have been :). Maybe it'll be easier for me to create another handler after all. The use case for fixing 3719 is not one particularly relevant to me though (for me, users should be able to resize the window and not have their choice overruled when exiting fullscreen). Reporting back.. I'm adding a custom handler for the Metal renderer. This seems to fix a noticeable lag in animation on one machine. On other Macs it appears to not be that noticeable. If I opt the OpenGL renderer into it too, it seems to cause an animation lag oddly on that machine, so I guess I'm going to keep the OpenGL path the same as it has always been where I haven't seen visible issues. ¯\_(ツ)_/¯ I think this can be closed. I was piggying off of SDL_RenderGetMetalLayer (which I really shouldn't have). I'm now setting up the device/layer/views myself. Being notified of view changes makes more sense in a view size change handler rather than a window size handler as well here. |
In SDL_cocoawindow.m you have: - (void)windowDidResize:(NSNotification *)aNotification { if (inFullscreenTransition) { /* We'll take care of this at the end of the transition */ return; } ... This code sends SDL_WINDOWEVENT_RESIZED window event (as well as some other window events) after the fullscreen transition is done perhaps to avoid sending multiple events. An app on receiving SDL_WINDOWEVENT_RESIZED may update the viewport and any textures that depend on the new drawable size (eg: depth stencil texture, multisample texture). If the app tries to create a render command encoder using the old textures (with improper sizes) for the next frame, Metal's validator will halt the app in the debugger. This appears to be what happens when SDL tries to defer sending the window events from a fullscreen transition. Commenting the bit of code above appears to work around the issue.