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

SDL3 - Nothing displayed above original size after resize #9029

Closed
LinUwUxCat opened this issue Feb 8, 2024 · 15 comments
Closed

SDL3 - Nothing displayed above original size after resize #9029

LinUwUxCat opened this issue Feb 8, 2024 · 15 comments
Milestone

Comments

@LinUwUxCat
Copy link

LinUwUxCat commented Feb 8, 2024

Hello, I was doing a simple image viewer using SDL3. However, when resizing the window, it seems that anything above the original size (here 800x600) is not displayed.

SSR-2024-02-08_18.29.23.mp4

I'm not sure if i'm missing a call or if this is a bug. The code for this application is available here, it's not complex and all that's displayed is basically a texture made from a surface that was created from loading a BMP.
Running KDE on X11 - SDL version is latest on main at the time of writing, so 1684032.

@madebr
Copy link
Contributor

madebr commented Feb 9, 2024

I can reproduce this behavior with testsprite by stashing the renderer viewport at the start of the frame, and restoring it at the end (this is similar to what imgui does).
Don't forget to start it with --resizable.

For "LIVe", I can workaround the issue by adding a SDL_SetRenderViewPort(renderer, NULL) before the call to SDL_RenderTexture.

--- a/test/testsprite.c
+++ b/test/testsprite.c
@@ -396,7 +396,9 @@ int SDL_AppIterate(void)
     Uint64 now;
     int i;
     int active_windows = 0;
+    SDL_Rect r;
 
+    SDL_GetRenderViewport(state->renderers[0], &r);
     for (i = 0; i < state->num_windows; ++i) {
         if (state->windows[i] == NULL ||
             (suspend_when_occluded && (SDL_GetWindowFlags(state->windows[i]) & SDL_WINDOW_OCCLUDED))) {
@@ -421,6 +423,7 @@ int SDL_AppIterate(void)
         next_fps_check = now + fps_check_delay;
         frames = 0;
     }
+    SDL_SetRenderViewport(state->renderers[0], &r);
 
     return 0;  /* keep going */
 }

@madebr
Copy link
Contributor

madebr commented Feb 9, 2024

Modifying src/imgui/imgui_impl_sdlrenderer3.cpp as follows also works.

--- a/src/imgui/imgui_impl_sdlrenderer3.cpp
+++ b/src/imgui/imgui_impl_sdlrenderer3.cpp
@@ -134,7 +134,7 @@ void ImGui_ImplSDLRenderer3_RenderDrawData(ImDrawData* draw_data)
     };
     BackupSDLRendererState old = {};
     old.ClipEnabled = SDL_RenderClipEnabled(bd->SDLRenderer) == SDL_TRUE;
-    SDL_GetRenderViewport(bd->SDLRenderer, &old.Viewport);
+    //SDL_GetRenderViewport(bd->SDLRenderer, &old.Viewport);
     SDL_GetRenderClipRect(bd->SDLRenderer, &old.ClipRect);
 
        // Will project scissor/clipping rectangles into framebuffer space
@@ -197,7 +197,7 @@ void ImGui_ImplSDLRenderer3_RenderDrawData(ImDrawData* draw_data)
     }
 
     // Restore modified SDL_Renderer state
-    SDL_SetRenderViewport(bd->SDLRenderer, &old.Viewport);
+    //SDL_SetRenderViewport(bd->SDLRenderer, &old.Viewport);
     SDL_SetRenderClipRect(bd->SDLRenderer, old.ClipEnabled ? &old.ClipRect : nullptr);
 }

I'm not sure this is an imgui bug, a deficiency of SDL's api, or something else.

@LinUwUxCat
Copy link
Author

For "LIVe", I can workaround the issue by adding a SDL_SetRenderViewPort(renderer, NULL) before the call to SDL_RenderTexture.

Thank you, this works. I'm not sure either which project is responsible for this, but if the behavior can be replicated in testsprite it might be from SDL3. I guess I'll leave the issue open.

@slouken
Copy link
Collaborator

slouken commented Feb 9, 2024

SDL is simply doing what is being asked of it. If you explicitly set the viewport, as imgui is doing, then that viewport will persist until set to a different value. If you set the viewport you need to respond to resize messages and set the viewport to the new render size.

@slouken slouken closed this as not planned Won't fix, can't repro, duplicate, stale Feb 9, 2024
@madebr
Copy link
Contributor

madebr commented Feb 9, 2024

By deficiency, I meant: "is there a way to detect whether the render viewport is unlimited/uninitialized?"
Is there some way to do what imgui intended to do?
These lines in imgui.

@slouken
Copy link
Collaborator

slouken commented Feb 9, 2024

Ah, good point. Maybe we should always return the actual viewport and document that -1 means unset?

@slouken slouken reopened this Feb 9, 2024
@slouken slouken added this to the 3.2.0 milestone Feb 9, 2024
@madebr
Copy link
Contributor

madebr commented Feb 9, 2024

I'd prefer to keep the meaning of negative return values to mean: error.
How about an extra int/book pointer argument that sets some flag? Can we use properties? Or should we stay away from those in inner game loops?
Or simply return 1 for unlimited and 0 for limited. By using flags, it can be extended later on.

@slouken
Copy link
Collaborator

slouken commented Feb 9, 2024

I meant the rect width and height would be -1. We could also add SDL_RenderHasViewport() or something like that.

@madebr
Copy link
Contributor

madebr commented Feb 9, 2024

SDL3 currently has SDL_SetRenderClipRect, SDL_GetRenderClipRect and SDL_RenderClipEnabled,
but only SDL_SetRenderViewport and SDL_GetRenderViewport.

So the extra function makes sense. What about SDL_RenderHasViewport?
(I think the naming convention fits with SDL3)

@slouken
Copy link
Collaborator

slouken commented Feb 9, 2024

Well, it's a little misleading because you ALWAYS have a viewport and it's implicitly the size of your drawable. I'm kind of inclined to have GetViewport() return the actual rect and let application code do a separate query of the drawable size if they need it.

@madebr
Copy link
Contributor

madebr commented Feb 9, 2024

Yeah, makes sense. As long as it is documented.

@slouken
Copy link
Collaborator

slouken commented Feb 10, 2024

There's lots of code that queries the viewport with that function and expects to have a valid width and height. Hmmm. We could add a function SDL_SaveRenderViewport() / SDL_RestoreRenderViewport(), but someone who doesn't see those will still think it's entirely valid to get the viewport and then set it again.

Alternatively, internally if you set the viewport to the full buffer size, we could "unset" the viewport so it automatically changes. It's probably better to add SDL_RenderHasCustomViewport() and then only save/restore the viewport if that returns true.

@slouken
Copy link
Collaborator

slouken commented Feb 10, 2024

I went ahead and added SDL_RenderViewportSet(), so the imgui code would look like:

    BackupSDLRendererState old = {};
    old.ClipEnabled = SDL_RenderClipEnabled(bd->SDLRenderer) == SDL_TRUE;
    old.ViewportEnabled = SDL_RenderViewportSet(bd->SDLRenderer) == SDL_TRUE;
    SDL_GetRenderViewport(bd->SDLRenderer, &old.Viewport);
    SDL_GetRenderClipRect(bd->SDLRenderer, &old.ClipRect);
...
    // Restore modified SDL_Renderer state
    SDL_SetRenderViewport(bd->SDLRenderer, old.ViewportEnabled ? &old.Viewport : nullptr);
    SDL_SetRenderClipRect(bd->SDLRenderer, old.ClipEnabled ? &old.ClipRect : nullptr);

@madebr
Copy link
Contributor

madebr commented Feb 10, 2024

Let's tag @ocornut as a heads up.

These lines should be replaced by the lines of #9029 (comment)

ocornut added a commit to ocornut/imgui that referenced this issue Feb 12, 2024
…not restore a wrong viewport if none was initially set.

libsdl-org/SDL#9029
@ocornut
Copy link

ocornut commented Feb 12, 2024

Done with ocornut/imgui@2af01ba, thanks for the notice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants