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

[macOS][SDL3] SDL_HideCursor() has no effect in fullscreen #8190

Closed
kanjitalk755 opened this issue Sep 2, 2023 · 10 comments · Fixed by #8224
Closed

[macOS][SDL3] SDL_HideCursor() has no effect in fullscreen #8190

kanjitalk755 opened this issue Sep 2, 2023 · 10 comments · Fixed by #8224

Comments

@kanjitalk755
Copy link
Contributor

When you build and run the following source, a white rectangle that moves according to the mouse movement will be displayed in the window.

#include <SDL.h>
#include <stdlib.h>

#if !SDL_VERSION_ATLEAST(3, 0, 0)
#define SDL_EVENT_MOUSE_MOTION		SDL_MOUSEMOTION
#define SDL_EVENT_KEY_DOWN		SDL_KEYDOWN
#define SDL_EVENT_MOUSE_BUTTON_DOWN	SDL_MOUSEBUTTONDOWN
#define SDL_EVENT_QUIT			SDL_QUIT
#define SDL_FillSurfaceRect		SDL_FillRect
#define SDL_RenderTexture		SDL_RenderCopy
#endif

int main(int argc, char *argv[]) {
	if (SDL_Init(SDL_INIT_VIDEO)) exit(1);
	atexit(SDL_Quit);
	const int width = 640, height = 480;
#if SDL_VERSION_ATLEAST(3, 0, 0)
	SDL_Window *window = SDL_CreateWindow("SDL3", width, height, 0);
	if (!window) exit(1);
	SDL_Renderer *renderer = SDL_CreateRenderer(window, NULL, SDL_RENDERER_ACCELERATED | SDL_RENDERER_PRESENTVSYNC);
	SDL_Surface *surface = SDL_CreateSurface(width, height, SDL_PIXELFORMAT_ARGB8888);
	SDL_SetRenderLogicalPresentation(renderer, width, height, SDL_LOGICAL_PRESENTATION_LETTERBOX, SDL_SCALEMODE_LINEAR);
	SDL_HideCursor();
#else
	SDL_Window *window = SDL_CreateWindow("SDL2", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, width, height, 0);
	if (!window) exit(1);
	SDL_Renderer *renderer = SDL_CreateRenderer(window, -1, SDL_RENDERER_ACCELERATED | SDL_RENDERER_PRESENTVSYNC);
	SDL_Surface *surface = SDL_CreateRGBSurfaceWithFormat(0, width, height, 32, SDL_PIXELFORMAT_ARGB8888);
	SDL_ShowCursor(SDL_FALSE);
#endif
	if (!surface) exit(1);
	if (!renderer) exit(1);
	SDL_Texture *texture = SDL_CreateTexture(renderer, SDL_PIXELFORMAT_ARGB8888, SDL_TEXTUREACCESS_STREAMING, width, height);
	if (!texture) exit(1);
//
	int fullscreen = 0, x = 0, y = 0;
	for (int quit = 0; !quit;) {
		SDL_Event e;
		while (SDL_PollEvent(&e)) {
			switch (e.type) {
				case SDL_EVENT_MOUSE_MOTION:
					if (fullscreen) {
						x += e.motion.xrel;
						y += e.motion.yrel;
					}
					else {
						x = e.motion.x;
						y = e.motion.y;
					}
					break;
				case SDL_EVENT_KEY_DOWN:
					fullscreen = !fullscreen;
					SDL_SetWindowFullscreen(window, fullscreen ? SDL_TRUE : SDL_FALSE);
					SDL_SetWindowGrab(window, fullscreen ? SDL_TRUE : SDL_FALSE);
					break;
				case SDL_EVENT_MOUSE_BUTTON_DOWN:
				case SDL_EVENT_QUIT:
					quit = 1;
					break;
			}
		}
		SDL_Rect rect = { 0, 0, width, height }, rect2 = { x, y, 20, 20 };
		SDL_FillSurfaceRect(surface, &rect, 0);
		SDL_FillSurfaceRect(surface, &rect2, 0xffffffff);
		SDL_UpdateTexture(texture, NULL, surface->pixels, surface->pitch);
		SDL_RenderTexture(renderer, texture, NULL, NULL);
		SDL_RenderPresent(renderer);
	}
	return 0;
}

Press any key to switch to full screen.
The mouse cursor is not displayed in SDL2 (expected behavior), but it is displayed in SDL3.

Build with SDL2:

cc -I/Library/Frameworks/SDL2.framework/Headers -F/Library/Frameworks -framework SDL2 main.c

Build with SDL3:

cc -I/Library/Frameworks/SDL3.framework/Headers -F/Library/Frameworks -framework SDL3 main.c
@kanjitalk755
Copy link
Contributor Author

I found the first commit (a077cc8) this behavior occurs.

@sridenour Is there any countermeasure?

@sridenour
Copy link
Contributor

sridenour commented Sep 8, 2023

I'll investigate when I get time, hopefully this weekend.

It's worth pointing out that SDL2 has had the exact same change for a few versions now and, as you pointed out, doesn't exhibit this problem.

Creating the window as fullscreen and then hiding the cursor does work (which is what I tested when I made the change). But changing to/from fullscreen seems to reset the cursor's visibility at the OS level without updating SDL's internal state (calling SDL_CursorVisible() after changing to fullscreen returns FALSE even though it's plainly visible).

@kanjitalk755 As a temporary fix, showing and then re-hiding the cursor works.

SDL_SetWindowFullscreen(window, fullscreen ? SDL_TRUE : SDL_FALSE);
SDL_SetWindowGrab(window, fullscreen ? SDL_TRUE : SDL_FALSE);
SDL_ShowCursor();
SDL_HideCursor();

@kanjitalk755
Copy link
Contributor Author

It's worth pointing out that SDL2 has had the exact same change for a few versions now and, as you pointed out, doesn't exhibit this problem.

No equivalent changes were found in the SDL2 branch.

@kanjitalk755 As a temporary fix, showing and then re-hiding the cursor works.

For the example source, the above workaround worked fine.

But if I insert the following code in the initialization to make fullscreen exclusive:

	SDL_DisplayID displayID = SDL_GetPrimaryDisplay();
	int count;
	const SDL_DisplayMode **displayModes = SDL_GetFullscreenDisplayModes(displayID, &count);
	if (displayModes == NULL || count <= 0 || SDL_SetWindowFullscreenMode(window, *displayModes)) exit(1);

The cursor appear even if using the workaround.

Reverting commit a077cc8 (partly) works fine in both cases.

https://github.com/kanjitalk755/SDL/tree/revert_a077cc8

@sridenour
Copy link
Contributor

sridenour commented Sep 8, 2023

No equivalent changes were found in the SDL2 branch.

Apparently I had a huge brain malfunction and forgot that the pull request for SDL2 I submitted at the same time as the one for SDL3 only had the early-out in SDL_SetCursor() and none of the rest of the SDL3 changes.

Reverting commit a077cc8 (partly) works fine in both cases.

If you haven't already, submit a pull request for your partial revert.

@slouken
Copy link
Collaborator

slouken commented Sep 8, 2023

Actually, I think the early out in SDL_SetCursor() is wrong. See the note above the function:

/* SDL_SetCursor(NULL) can be used to force the cursor redraw,
   if this is desired for any reason.  This is used when setting
   the video mode and when the SDL window gains the mouse focus.
 */

@sridenour
Copy link
Contributor

Since it's been in SDL2 since February with no reported problems, maybe the early out could be adjusted to also check for NULL instead of being removed entirely. When I get home I can check to see if this also fixes the problem in SDL3.

Without the early out, SDL is unusable with Dear ImGui on macOS. Dear ImGui calls SDL_SetCursor() every frame, whether the cursor needs to change or not, and with the old behavior this was making event polling take over a millisecond on macOS, causing frame drops and stuttering.

kanjitalk755 added a commit to kanjitalk755/SDL that referenced this issue Sep 9, 2023
From libsdl-org#7249, reverted the hunks other than libsdl-org#7239.
@slouken
Copy link
Collaborator

slouken commented Sep 9, 2023

@sridenour, I'm not following what needs to be reverted and why. Can you check #8224 to see if it's actually what we want, and whether we need similar changes in SDL2?

@sridenour
Copy link
Contributor

sridenour commented Sep 9, 2023

@slouken #8224 looks good to me. And it matches what's been in SDL2 since February with no reported problems.

What/Why

The way SDL2 handles cursor changes on macOS (and the way SDL3 used to do it, before a077cc8) is by invalidating the window's cursor rectangles (which the OS uses to determine what cursor to use and where), causing the OS to ask the application to create new ones by calling -[resetCursorRects] if it exists, and in SDL's code -[resetCursorRects] sets a new cursor rect that's the same as the old but uses a different cursor. This is the Correct Way, but setting the new cursor rect is slow and happens in the next event loop, causing the next SDL_PollEvent() loop to sometimes take over a millisecond, causing frequent stutter etc in applications that use Dear ImGui (which calls SDL_SetCursor() every frame).

My SDL3 change tried to make it faster by not going the invalidate-reset cursor rects route, but instead change the cursor for the whole screen (which is almost instant), with some guards to catch the cursor going out of the window and change it back. It also added an early out to SDL_SetCursor() so setting the same cursor twice would be a no-op.

My SDL2 change, to keep the change small and avoid unintended behavior changes, just had the SDL_SetCursor() early out. This was enough to keep Dear ImGui from causing stuttering on macOS when the cursor didn't actually change.

Unfortunately, and I'm embarrassed I didn't catch this, my SDL3 change doesn't play nice with changing to fullscreen after the window has been created. #8224 just reverts to the previous way cursor changes were done, but leaves in the early out in SDL_SetCursor(). So the cursor change functionality and behavior in SDL2 and 3 on macOS will match.

@kanjitalk755
Copy link
Contributor Author

@slouken This issue should be fixed to at least the same specification as SDL2 by the 3.2.0 release.
Could you please set it as the milestone?

slouken pushed a commit that referenced this issue Oct 12, 2023
From #7249, reverted the hunks other than #7239.
@slouken
Copy link
Collaborator

slouken commented Oct 12, 2023

Thanks for the investigation guys, I merged #8224.

Kontrabant pushed a commit to Kontrabant/SDL that referenced this issue Oct 22, 2023
From libsdl-org#7249, reverted the hunks other than libsdl-org#7239.
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

Successfully merging a pull request may close this issue.

3 participants