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.

Sign up for GitHub

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

Jump to bottom

Using SDL_RenderSetScale with sdl_sdlrenderer backend #6065

Open
sjmeagher opened this issue Jan 9, 2023 · 6 comments · May be fixed by #7178
Open

Using SDL_RenderSetScale with sdl_sdlrenderer backend #6065

sjmeagher opened this issue Jan 9, 2023 · 6 comments · May be fixed by #7178
Labels
backends dpi

Comments

@sjmeagher
Copy link

sjmeagher commented Jan 9, 2023 •
edited

Version/Branch of Dear ImGui:

Version: 1.89.2
Branch: master, commit d7c8516

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_sdl.cpp + imgui_impl_sdl_sdlrenderer.cpp
Compiler: Apple clang version 12.0.0 (clang-1200.0.32.29)
Operating System: macOS 10.15.7

My Issue/Question:

If SDL_RenderSetScale sets scale to anything other than 1, this ruins mouse coordinates.

Standalone, minimal, complete and verifiable example:
insert SDL_RenderSetScale(renderer, 2.0f, 2.0f); after line 43 of example_sdl_sdlrenderer/main.cpp.

Partial solution

Following patch to imgui_impl_sdl.cpp restores correct mouse coordinates for purposes of clicking a button, but leaves window repositioning wrong.

diff --git a/backends/imgui_impl_sdl.cpp b/backends/imgui_impl_sdl.cpp
index a5151f3c..aeaf5bc0 100644
--- a/backends/imgui_impl_sdl.cpp
+++ b/backends/imgui_impl_sdl.cpp
@@ -459,7 +459,12 @@ static void ImGui_ImplSDL2_UpdateMouseData()
             int window_x, window_y, mouse_x_global, mouse_y_global;
             SDL_GetGlobalMouseState(&mouse_x_global, &mouse_y_global);
             SDL_GetWindowPosition(bd->Window, &window_x, &window_y);
-            io.AddMousePosEvent((float)(mouse_x_global - window_x), (float)(mouse_y_global - window_y));
+            
+            float scaleX, scaleY = 1.0f;
+            if (bd->Renderer) {
+	          SDL_RenderGetScale(bd->Renderer, &scaleX, &scaleY);
+            }
+            io.AddMousePosEvent((float)(mouse_x_global - window_x) / scaleX, (float)(mouse_y_global - window_y) / scaleY);
         }
     }
 }

UPDATE : Complete fix?

Following patch addresses both window movement and button clicking

diff --git a/backends/imgui_impl_sdl.cpp b/backends/imgui_impl_sdl.cpp
index a5151f3c..75bddde4 100644
--- a/backends/imgui_impl_sdl.cpp
+++ b/backends/imgui_impl_sdl.cpp
@@ -255,7 +255,11 @@ bool ImGui_ImplSDL2_ProcessEvent(const SDL_Event* event)
     {
         case SDL_MOUSEMOTION:
         {
-            io.AddMousePosEvent((float)event->motion.x, (float)event->motion.y);
+	        	float scaleX = 1.0f;
+            float scaleY = 1.0f;
+            if (bd->Renderer)
+	    				SDL_RenderGetScale(bd->Renderer, &scaleX, &scaleY);
+            io.AddMousePosEvent((float)event->motion.x / scaleX, (float)event->motion.y / scaleY);
             return true;
         }
         case SDL_MOUSEWHEEL:
@@ -459,7 +463,13 @@ static void ImGui_ImplSDL2_UpdateMouseData()
             int window_x, window_y, mouse_x_global, mouse_y_global;
             SDL_GetGlobalMouseState(&mouse_x_global, &mouse_y_global);
             SDL_GetWindowPosition(bd->Window, &window_x, &window_y);
-            io.AddMousePosEvent((float)(mouse_x_global - window_x), (float)(mouse_y_global - window_y));
+            
+            float scaleX = 1.0f;
+            float scaleY = 1.0f;
+            if (bd->Renderer)
+	    				SDL_RenderGetScale(bd->Renderer, &scaleX, &scaleY);
+            
+            io.AddMousePosEvent((float)(mouse_x_global - window_x) / scaleX, (float)(mouse_y_global - window_y) / scaleY);
         }
     }
 }
@olinorwell
Copy link

I have experienced this same issue this week as well, I just assumed it was a known limitation. If that fix works then that would be very useful to include.

@sjmeagher
Copy link
Author

sjmeagher commented Jan 12, 2023 •
edited

Ok. I'll make a PR in the next couple of days.

@ocornut
Copy link
Owner

ocornut commented Jan 13, 2023

Hello, thanks for opening the issue. No need for a PR if it is the same change.
I'll need to investigate this. The problem is that there are many different ways to handle scaling/DPI/multi-DPI and many of those scaling-relating changes are pushing toward one way and preventing others from being used. This is why there are so many variations of "scaling fix" in issues and pr at the moment, in particularly for Mac. I'll need to sit down with a Mac for a longer while and tackle a shared design for that.

@sjmeagher
Copy link
Author

Thanks for update.
Yes PR would have been patch in original post, so I won't submit a PR.

(Maybe not the right place, but thanks for making and maintaining this awesome piece of software).

@ocornut
Copy link
Owner

ocornut commented Jan 31, 2023

I am not sure I understand this.

Adding SDL_RenderSetScale(renderer, io.DisplayFramebufferScale.x, io.DisplayFramebufferScale.y); in the main loop of our demo app, e.g. right before SDL_SetRenderDrawColor(...), seems to be one way of FIXING #5931 / #6121 for me, and then inputs are matching display.

With this setup io.DisplaySize is the "half resolution" version (1 point = 1 imgui unit = 2 pixels).
Which is not the ideal setup but it does work.
AFAIK in hi-dpi mode if we don't call SDL_RenderSetScale(renderer, 2, 2) then we get the result of #5931 / #6121.

Your message however suggest the exact opposite thing.

ocornut added a commit that referenced this issue Jan 31, 2023
@ocornut
Examples: SDL+SDL_Renderer: Added call to SDL_RenderSetScale() to dis…
5a3f82e 
…play is correct on a Retina display (albeit lower-res as our other unmodified examples). (#6121, #6065, #5931).
@sjmeagher
Copy link
Author

sjmeagher commented Jan 31, 2023 •
edited

(This is in reference to commit 2efebe3)
That placement of
SDL_RenderSetScale(renderer, io.DisplayFramebufferScale.x, io.DisplayFramebufferScale.y);
visually undoes the
SDL_RenderSetScale(renderer, 2.0f, 2.0f);
at line 43 of the example I gave.
The effect is that the renderer is not visually scaled.

E.g. here it is without SDL_RenderSetScale(renderer, io.DisplayFramebufferScale.x, io.DisplayFramebufferScale.y); (i.e comment out line 146 of demo app).
image

and here it is with (i.e. keep line 146 of demo app).
image

kjblanchard pushed a commit to kjblanchard/imgui that referenced this issue May 5, 2023
@ocornut @kjblanchard
Examples: SDL+SDL_Renderer: Added call to SDL_RenderSetScale() to dis…
6101f82 
…play is correct on a Retina display (albeit lower-res as our other unmodified examples). (ocornut#6121, ocornut#6065, ocornut#5931).
@dacobi dacobi mentioned this issue Jun 12, 2023
Open
bog-dan-ro added a commit to bog-dan-ro/imgui that referenced this issue Dec 29, 2023
@bog-dan-ro
Fixed SDL(3)_SetRenderScale handling
a182970 
Fixed ocornut#6065
@bog-dan-ro bog-dan-ro linked a pull request Dec 29, 2023 that will close this issue
Open
bog-dan-ro added a commit to bog-dan-ro/imgui that referenced this issue Dec 29, 2023
@bog-dan-ro
Fixed SDL(3)_SetRenderScale handling
293bc5a 
Fixed ocornut#6065
bog-dan-ro added a commit to bog-dan-ro/imgui that referenced this issue Dec 29, 2023
@bog-dan-ro
Fixed SDL(3)_SetRenderScale handling
77948bb 
Fixed ocornut#6065
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
backends dpi
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixed SDL(3)_SetRenderScale handling bog-dan-ro/imgui
3 participants
@olinorwell @ocornut @sjmeagher