libsdl-org / SDL

Simple Directmedia Layer
https://libsdl.org
zlib License
9.98k stars 1.84k forks source link

SDL3 on macOS - cursor positions not aligned on HighDpi monitor #10875

Closed DominusExult closed 1 month ago

DominusExult commented 2 months ago

macOS 14 and 15, monitor Apple Studio Display (5120 x 2880) and Apple Thunderbolt Display (2560 x 1440).

When the monitor is not using the full resolution but a scaled one, the system cursor and the in-App cursor are not aligned in windowed mode. Meaning that when you move the system cursor over the app the app cursor is not where it is supposed to be the further you are away from the top left position. When I use the full resolution of the monitor it works correctly.

image

I've screen recorded this with our ported Exult.

Scaled resolution: https://youtu.be/gu8AojWZTmY

Native resolution: https://youtu.be/FJAFQKinc68

When you click in the app you don't notice much as the system cursor is not visible anymore but it still behaves oddly of course when you move the cursor out of the window. And we are also using drag and drop from an outside app and there it is annoying that there is an offset (see https://youtu.be/swUtzi5tAq4 as an example).

This used to work at some point of our porting progress but I can't bisect as we didn't keep track of all our changes mandated by SDL3 changes. And just maybe it is an Exult issue :)

slouken commented 2 months ago

Are you using SDL_ConvertEventToRenderCoordinates()?

Dragon-Baroque commented 2 months ago

Are you using SDL_ConvertEventToRenderCoordinates()?

Yes.

slouken commented 2 months ago

Any chance you can repro with the SDL test apps or a modification of them?

DominusExult commented 2 months ago

I tried with testdropfile and there it is correct.

Another thing I tried in our source was disabling of passing the SDL_WINDOW_HIGH_PIXEL_DENSITY flag to SDL_CreateWindow with the result that it works correctly then.

slouken commented 2 months ago

What if you run testdropfile with --high-pixel-density?

DominusExult commented 2 months ago

Thanks, that is a "success". In the screenshot you can see the offset. The crosshairs should be beneath the cursor not way over there (the offset is reversed to what we see in Exult, but still the crosshairs and system cursor meet in the upper left corner ).

image

Dragon-Baroque commented 2 months ago

Strange that the x and y scaling is one half in testdropfile, but is two in Exult. But testdropfile does not use SDL_ConvertEventToRenderCoordinates, does it ?

Kontrabant commented 2 months ago

The testdropfile behavior with --high-pixel-density replicates on Wayland with a scaled desktop as well, so it's not exclusive to the Mac backend.

DominusExult commented 1 month ago

hmm, seems it may never have worked correctly with SDL3.

Kontrabant commented 1 month ago

This cursor/cross-hairs mismatch in testdropfile is fixed now in e4f987f

Dragon-Baroque commented 1 month ago

This cursor/cross-hairs mismatch in testdropfile is fixed now in https://github.com/libsdl-org/SDL/commit/e4f987f299683a1016dd441c860c6aa4bd03bfa1

I built a similar patch - adding a SDL_ConvertEventToRenderCoordinates to testdropfile - and I can confirm that the crosshair is correct on Wayland with the --scale option. Thank you for highlighting this option, and for highlighting SDL_GetWindowFromEvent in the commit, I did not know about either.

Also @DominusExult confirmed that the crosshair works correctly in testdropfile with or without --high-pixel-density on his High DPI display, with the added SDL_ConvertEventToRenderCoordinates.

On the Exult track, @DominusExult has added a SDL_SetRenderLogicalPresentation to Exult in Windowed mode, and confirmed that the cursor mismatch that triggered the present issue is gone too.

In summary,

Do you agree with the above summary ?

Now,

DominusExult commented 1 month ago

Additionally all other SDL tests that have mouse/touch events need to be examined as well if they need a similar fix when HighDpi is enabled. Testdropfile was just the first I could think of that visibly shows the mouse coordinates.

Kontrabant commented 1 month ago

Additionally all other SDL tests that have mouse/touch events need to be examined as well if they need a similar fix when HighDpi is enabled. Testdropfile was just the first I could think of that visibly shows the mouse coordinates.

The two others that I can think of offhand that may be problematic are testmouse, but it's not currently an issue there, as it doesn't currently support the high-DPI flag, and testaudio, which I just tried and found doesn't work properly on scaled desktops.

DominusExult commented 1 month ago

Tests that show the coordinates but don’t draw a cursor do exist, too, I think. I‘ll take a look later if I remember correct

DominusExult commented 1 month ago

Found a few with HighDpi problems, mostly really a mouse coordinates problem, some have visual, other problems.

testaudio: mouse clicks don't match (you may move something else around than what you clicked on) testaudiostreamdynamicresample: mouse clicks don't match (the slider you want to move doesn't, but maybe another one) testcustomcursor: cursor ok, display bad testhittesting: most noticeable with the right red box, only on the bottom does it register as the red box testintersections: the mouse-click-drag don't register where you drag but elsewhere testmanymouse: only works in the upper left quarter of the screen testoverlay: I think it is misbehaving with the mouse coordinations as well

But overall I tend to agree with what @Dragon-Baroque asked, wouldn't it make more sense for SDL to do SDL_ConvertEventToRenderCoordinates when SDL_WINDOW_HIGH_PIXEL_DENSITY is set (and maybe SDL_SetRenderLogicalPresentation)? It seems to me that this is required anyway, so maybe do it anyway? (just wondering, fine either way)

slouken commented 1 month ago

In summary,

  • Exult did not work because it used SDL_ConvertEventToRenderCoordinates but lacked a SDL_SetRenderLogicalPresentation and
  • testdropfile did not work because it used SDL_SetRenderLogicalPresentation but lacked a SDL_ConvertEventToRenderCoordinates.

Do you agree with the above summary ?

Now,

  • Should a SDL application use SDL_SetRenderLogicalPresentation ?

No, I think this is a bug in SDL. Using SDL_ConvertEventToRenderCoordinates() should work without setting logical presentation. I'll take a look.

slouken commented 1 month ago

It looks like it does work here.

If I run testwm --high-pixel-density with the following patch, I see that if I don't use SDL_ConvertEventToRenderCoordinates(), then the red square does not follow the mouse, but if I do, then it does, which is what I would expect. Note that logical presentation is disabled with this set of command line options. What am I missing?

diff --git a/test/testwm.c b/test/testwm.c
index 068dbe42d..6a57d92a0 100644
--- a/test/testwm.c
+++ b/test/testwm.c
@@ -48,6 +48,7 @@ SDL_COMPILE_TIME_ASSERT(cursorNames, SDL_arraysize(cursorNames) == SDL_SYSTEM_CU
 static int system_cursor = -1;
 static SDL_Cursor *cursor = NULL;
 static const SDL_DisplayMode *highlighted_mode = NULL;
+static float mouse_x, mouse_y;

 /* Draws the modes menu, and stores the mode index under the mouse in highlighted_mode */
 static void
@@ -164,8 +165,16 @@ static void loop(void)
 #endif

     while (SDL_PollEvent(&event)) {
+#if 1
+        SDL_ConvertEventToRenderCoordinates(state->renderers[0], &event);
+#endif
         SDLTest_CommonEvent(state, &event, &done);

+        if (event.type == SDL_EVENT_MOUSE_MOTION) {
+            mouse_x = event.motion.x;
+            mouse_y = event.motion.y;
+            SDL_Log("Mouse moved to %g,%g\n", mouse_x, mouse_y);
+        }
         if (event.type == SDL_EVENT_WINDOW_RESIZED) {
             SDL_Window *window = SDL_GetWindowFromEvent(&event);
             if (window) {
@@ -243,6 +252,17 @@ static void loop(void)
             menurect.h = (float)viewport.h - y;
             draw_modes_menu(window, renderer, menurect);

+            {
+                char text[128];
+                SDL_FRect rect = { mouse_x, mouse_y, 32.0f, 32.0f };
+                SDL_SetRenderDrawColor(renderer, 255, 0, 0, 255);
+                SDL_RenderFillRect(renderer, &rect);
+
+                SDL_snprintf(text, sizeof(text), "(%.2f,%.2f)", mouse_x, mouse_y);
+                SDL_SetRenderDrawColor(renderer, 255, 255, 255, 255);
+                SDLTest_DrawString(renderer, mouse_x + 34.0f, mouse_y + 8.0f, text);
+            }
+
             SDL_Delay(16);
             SDL_RenderPresent(renderer);
         }
Kontrabant commented 1 month ago
* In the port of Exult to SDL 3, every SDL event that provides mouse positions in the window is now fronted with a `SDL_ConvertEventToRenderCoordinates`. Would you consider converting the event coordinates in SDL before the SDL_Event is sent to the application ?

There's really no perfect solution here. The platforms that use scaled desktops still send native pointer events in logical window-relative coordinates, and expect any coordinates given to them to be window-relative as well. If SDL were to start sending pointer coordinates that were transformed to be relative to the backbuffer, they would still have to be transformed back to window-relative for certain operations, and now different parts of the API use different coordinate systems.

Translating coordinates when dealing with DPI-aware windows is just something that client apps need to be deal with, even without SDL in the middle.

Dragon-Baroque commented 1 month ago

Note that logical presentation is disabled with this set of command line options. What am I missing?

Are you claiming that testdropfile or testwm do not perfom SDL_SetRenderLogicalPresentation with the options --high-pixel-density or --scale ? I am afraid not so, the SDL_SetRenderLogicalPresentation is in src/test/SDL_test_common.c at line 1483 and these options let the if at line 1469 pass. And gdb confirms the call to SDL_SetRenderLogicalPresentation. Did I misunderstand you ?

Kontrabant commented 1 month ago

Without the --logical-presentation command line parameter explicitly set to something other than the default value of 'disabled', the test code calls SDL_SetRenderLogicalPresentation with SDL_LOGICAL_PRESENTATION_DISABLED, so the function call is just a no-op in that case.

DominusExult commented 1 month ago

so, to recap, our (Exult) problem is a different one that for some reason is fixed by enabling SDL_SetRenderLogicalPresentation but looks similar to what SDL's test apps had (only these were mostly only missing SDL_ConvertEventToRenderCoordinates)? One hint, in Exult the system cursor "runs behind" while in SDL it "ran in front" (if you get what I mean).

image image

slouken commented 1 month ago

It sounds like maybe Exult is calling SDL_ConvertEventToRenderCoordinates() twice? It should also not need to call SDL_SetRenderLogicalPresentation() unless that's by design for some reason.

Dragon-Baroque commented 1 month ago

It sounds like maybe Exult is calling SDL_ConvertEventToRenderCoordinates() twice?

I thought about that a while ago and I traced Exult under gdb with a breakpoint on SDL_ConvertEventToRenderCoordinates and a command to that breakpoint to get the backtrace. It displayed only one location in Exult where SDL_ConvertEventToRenderCoordinates was called, and that was the expected location. The proof was not conclusive enough, for

It should also not need to call SDL_SetRenderLogicalPresentation() unless that's by design for some reason.

We, Exult developers, are somewhat in the dark here. I am afraid we added a SDL_SetRenderLogicalPresentation for Windowed Exult by analogy with Full Screen Exult. And it fixed the cursor problem, we do not really know why.

The best I can propose, at this point, is to help @DominusExult add printouts of the state of the Logical Presentation before and after the SDL_SetRenderLogicalPresentation we added to Windowed Exult, and printouts to the Mouse events before and after the SDL_ConvertEventToRenderCoordinates. Since, without the added SDL_SetRenderLogicalPresentation, the Render Coordinates look wrong by a scale of two, could the Cocoa driver have set a Logical Presentation wrong by a scale of two - or one half ?

In other words, could you advise us on how to understand why SDL_SetRenderLogicalPresentation works ?

slouken commented 1 month ago

I would disable logical presentation and then print the values of the mouse events as they come out of the SDL event queue, and then after you run SDL_ConvertEventToRenderCoordinates(), and then again where you display the cursor.

I think what's happening is that you are rendering the mouse cursor into the larger back buffer, implicitly scaling everything by 2. SDL_SetRenderLogicalPresentation() would "fix" this by making sure that your application doesn't see the larger back buffer. That may or may not be what you want, depending on how you handle high DPI displays.

DominusExult commented 1 month ago

Hmm, I think we can close the topic here and handle the problem in our issue again. If I get rid of all our SDL_ConvertEventToRenderCoordinates everything works fine as well without SDL_SetRenderLogicalPresentation. So we do something that makes SDL_ConvertEventToRenderCoordinates unnecessary and thus when we use it, we get this offset.

Thanks for all your help and at least we did find bugs in the test apps :)

slouken commented 1 month ago

Didn't you need to add that to handle touch events on iOS properly?

DominusExult commented 1 month ago

oh! yes, for iOS I need the SDL_ConvertEventToRenderCoordinates on iOS. I wasn't checking that, only the windowed macOS part. And I'm not saying that we should get rid of SDL_ConvertEventToRenderCoordinates, just that we do something that makes SDL_ConvertEventToRenderCoordinates offset our cursor

slouken commented 1 month ago

It looks like this is all resolved, and @icculus just switched logical presentation to use the same logic as SDL2 again, so I'll go ahead and close this. Please let us know if you're still seeing issues, and provide repro steps with SDL test programs if possible.

Dragon-Baroque commented 1 month ago

Please let us know if you're still seeing issues...

In Exult : No longer. We spotted today a faulty code change of two months ago in the Exult port to SDL 3 that made us believe we were able to avoid SDL_SetRenderLogicalPresentation while using SDL_ConvertEventToRenderCoordinates in Windowed Exult in the past, whereas not using SDL_SetRenderLogicalPresentation now, caused the present cursor tracking issue.

So our choice is to use SDL_ConvertEventToRenderCoordinates in all cases, since we need it for iOS Exult and for HighDPI displays in macOS Fullscreen Exult, and that implies we use SDL_SetRenderLogicalPresentation in all cases too.

Thank you also for reaching the ABI and the API freeze.

slouken commented 1 month ago

You're welcome, thank you for the update!