libsdl-org
Viewport resets on window move
Since 2.0.22, viewport set by SDL renderer gets reset on window move, which is still the case in current dev builds (some yesterday's commit built manually). Confirmed to happen on Arch X11 and macOS 12.
Another case of viewport getting reset is OpenGL viewport in High DPI-enabled window on macOS. This one seems to happen from at least SDL 2.0.20.
Real-world program that relies on viewport functionality is DOSBox Staging. Described behaviours were first discovered there and are easily reproducible.
Small(ish?) example program, shows a green rectangle. Press V to set viewport, and F to unset. Moving the window with the viewport set resets it, pressing V sets it back.
#include <SDL2/SDL.h>
int main()
{
SDL_Init(SDL_INIT_VIDEO);
SDL_Window* window;
SDL_Renderer* renderer;
const SDL_Rect viewport_rect = {112, 0, 800, 600};
SDL_Event event;
SDL_bool exit = SDL_FALSE;
SDL_SetHint(SDL_HINT_RENDER_VSYNC, "1");
SDL_CreateWindowAndRenderer(1024, 600, SDL_WINDOW_ALLOW_HIGHDPI, &window, &renderer);
SDL_RenderSetViewport(renderer, &viewport_rect); // Ignored?
while (!exit)
{
while (SDL_PollEvent(&event))
switch (event.type)
{
case SDL_QUIT:
exit = SDL_TRUE;
break;
case SDL_KEYDOWN:
if (event.key.keysym.sym == SDLK_f)
SDL_RenderSetViewport(renderer, NULL);
if (event.key.keysym.sym == SDLK_v)
SDL_RenderSetViewport(renderer, &viewport_rect);
break;
}
SDL_SetRenderDrawColor(renderer, 0x00, 0x00, 0x00, 0xff);
SDL_RenderClear(renderer);
SDL_SetRenderDrawColor(renderer, 0x00, 0x7f, 0x00, 0xff);
SDL_RenderFillRect(renderer, NULL);
SDL_RenderPresent(renderer);
}
SDL_DestroyRenderer(renderer);
SDL_DestroyWindow(window);
SDL_Quit();
return 0;
}
Caused by a fix intended for high-DPI: https://github.com/libsdl-org/SDL/commit/cb8163081685fd97deba3c800bb8f10841b555a1
Not sure how we'd accommodate all situations - maybe we just scale current state instead of resetting, not sure.
I'm an idiot - we've had an event for display changes this whole time:
https://github.com/libsdl-org/SDL/blob/main/include/SDL_video.h#L179
Fixing now.
Latest commit should take care of the window movement issue.
Note that display changes will still shake up your viewport since applications have to handle drawable size being different on multiple displays, but this at least fixes what we can fix in 2.0. Definitely look at DISPLAY_CHANGED if you haven't already, I sure didn't know about it until about 15 minutes ago...
Gonna reopen this (and move it to 2.26)...Ethan's fix is good, but we want to try to preserve the existing viewport instead of reset it to the whole window.
Thanks for the fix! On Linux, both OpenGL and SDL renderer keep their viewports now. Unfortunately, macOS-specific case (OpenGL context, HiDPI window flag) is still affected. DISPLAY_CHANGED - will take a look at it, thanks for letting me know!
I can reproduce here, now (and reproduce it on OpenGL and Metal). Digging in.
For OpenGL:
data->drawstate.target = renderer->target;
if (!data->drawstate.target) {
int w, h;
SDL_GL_GetDrawableSize(renderer->window, &w, &h);
if ((w != data->drawstate.drawablew) || (h != data->drawstate.drawableh)) {
data->drawstate.viewport_dirty = SDL_TRUE; // if the window dimensions changed, invalidate the current viewport, etc.
data->drawstate.cliprect_dirty = SDL_TRUE;
data->drawstate.drawablew = w;
data->drawstate.drawableh = h;
}
}
Every time we run the command queue, we query the drawable size, which changes when we move to a different DPI monitor, and force the viewport to reset to what we think it should be, and then everything goes wrong.
Does the drawable size not change on other platforms when moving to a new DPI display...? This bug shouldn't be Mac-specific.
Metal also sets statecache.viewport_dirty = SDL_TRUE; at the start of every command queue (and the GL renderer has an #ifdef MACOSX at the start that always forces this regardless of drawable size, too).
The GL renderer adjusts the viewport Y position based on drawable size, but otherwise does no scaling in relation to it, Metal does neither. I suspect that's the solution, scaling based on these...which I guess is what we were punting on for 2.24 in the first place.
It sounds like the original bug reset the viewport on every window move, and now it still happens, but only when changing monitors (which we'll fix in 2.26.0), is that right?
Ok, yes, this seems to be the case...when there's a display change, we set the viewport to the whole window, regardless of what it was set to by the app, at the higher level in SDL_RendererEventWatch.
It is not happening on a move that doesn't change displays on macOS.
Disregard my commentary about the drawable size changing above, it's unrelated and I was chasing the wrong detail here.
So...I think we're okay for 2.24 here? We do need to deal with this better, but that can wait for 2.26.
It is not happening on a move that doesn't change displays on macOS.
Just to make sure we're on the same page: did you test with SDL_WINDOW_ALLOW_HIGHDPI flag? @kcgen made a recording on his Mac Mini (with SDL built today, 6e007c3), posting it to demonstrate the issue, and also attaching a code sample that was tested. Note that's a single display setup, and the triangle stretches as soon as there's window movement.
(This last issue was unrelated to the renderer issue that started this discussion, as the example code uses OpenGL directly instead of the SDL renderer API.)
Okay, I looked into this, here's what's happening:
Cocoa requires us to call -[NSOpenGLContext update] when the drawable resizes or moves:
https://developer.apple.com/documentation/appkit/nsopenglcontext/1436135-update?language=objc
So we do:
https://github.com/libsdl-org/SDL-historical-archive/commit/7d3f096090fb2920884c3396d529eb7038eb7f71
(In modern times, SDL does some stuff to manage multiple contexts and threads, but eventually, on moving the window, we're going to call -[NSOpenGLContext update].)
And -[NSOpenGLContext update] calls glViewport.
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 3.1
frame #0: 0x000000021a546c88 libGL.dylib`glViewport
libGL.dylib`glViewport:
-> 0x21a546c88 <+0>: mov x4, x3
0x21a546c8c <+4>: mov x3, x2
0x21a546c90 <+8>: mov x2, x1
0x21a546c94 <+12>: mov x1, x0
Target 0: (viewport_test_gl) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 3.1
* frame #0: 0x000000021a546c88 libGL.dylib`glViewport
frame #1: 0x00000001bbb43cd0 AppKit`-[NSOpenGLContext(NSOpenGLContextInternal) _updateOpenGLViewport] + 140
frame #2: 0x00000001bbb44274 AppKit`-[NSOpenGLContext update] + 412
frame #3: 0x00000001005ea53c libSDL2-2.0.0.dylib`-[SDLOpenGLContext explicitUpdate](self=0x0000600002125ef0, _cmd="explicitUpdate") at SDL_cocoaopengl.m:185:9
frame #4: 0x00000001005e9ffc libSDL2-2.0.0.dylib`-[SDLOpenGLContext updateIfNeeded](self=0x0000600002125ef0, _cmd="updateIfNeeded") at SDL_cocoaopengl.m:118:9
frame #5: 0x00000001005ea038 libSDL2-2.0.0.dylib`-[SDLOpenGLContext update](self=0x0000600002125ef0, _cmd="update") at SDL_cocoaopengl.m:127:5
frame #6: 0x00000001005f0868 libSDL2-2.0.0.dylib`ScheduleContextUpdates(data=0x0000600002620ba0) at SDL_cocoawindow.m:282:17
frame #7: 0x00000001005f0580 libSDL2-2.0.0.dylib`-[Cocoa_WindowListener windowDidMove:](self=0x0000600002125ea0, _cmd="windowDidMove:", aNotification=@"NSWindowDidMoveNotification") at SDL_cocoawindow.m:778:5
All of this to say: it might be a Mac-only requirement (or not!), but you either need to call glViewport every frame, or watch for both move and resize events from SDL and call it in both those cases. There isn't anything else we can do here, it's a macOS quirk.
Moving this to the SDL3 milestone since there are still some highdpi/multidisplay details that are still waiting to be considered, I think, but otherwise this is solid for 2.26.0.
Uh-huh. Apple then. Yeah, I ended up setting viewport on move and resize on macOS. Thanks for looking into that.