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 Support #182

Closed
RGPaul opened this issue Feb 28, 2023 · 9 comments
Closed

SDL3 Support #182

RGPaul opened this issue Feb 28, 2023 · 9 comments

Comments

@RGPaul
Copy link
Contributor

RGPaul commented Feb 28, 2023

Hey,
thanks for this great Library.

Is there Support for SDL3 planned?
I have some trouble to get the high dpi setup correctly for iOS (touch points are offset).

@texus
Copy link
Owner

texus commented Feb 28, 2023

Is there Support for SDL3 planned?

It is definitely planned once SDL3 is released, but I'm not sure if I can keep it supported during its development.
I'm not actively following SDL development (I only read the release notes when they make a new release), so I don't actually know if they expect to break a lot of code or not in SDL3. If the API won't change often then I could perhaps allow the cmake script to find SDL3 as well and only occasionally check if my code still compiles.

Edit: It looks like many functions have already been renamed, so I guess the biggest changes that affect TGUI have already been done. I'll try to have a look soon for making the code compatible with both SDL2 and SDL3.

I have some trouble to get the high dpi setup correctly for iOS (touch points are offset).

This will be tricky for me to solve, because I don't have access to any Apple devices myself.
Are you able to get the correct position from SDL itself? If SDL provides incorrect values then there isn't much that I can do about it, but if SDL is providing correct numbers then you can perhaps show me an example SDL code so that I can figure out the difference with what TGUI is doing now.

@RGPaul
Copy link
Contributor Author

RGPaul commented Mar 1, 2023

Edit: It looks like many functions have already been renamed, so I guess the biggest changes that affect TGUI have already been done. I'll try to have a look soon for making the code compatible with both SDL2 and SDL3.

Sounds great.

I have some trouble to get the high dpi setup correctly for iOS (touch points are offset).

This will be tricky for me to solve, because I don't have access to any Apple devices myself. Are you able to get the correct position from SDL itself? If SDL provides incorrect values then there isn't much that I can do about it, but if SDL is providing correct numbers then you can perhaps show me an example SDL code so that I can figure out the difference with what TGUI is doing now.

It seems to be an Issue with SDL2. I had the same problem with my Mac.
I could solve it using this code:

gui.onViewChange([&gui, &window, &renderer] {
        int widthInPixels, heightInPixels;
        SDL_GetWindowSizeInPixels(window, &widthInPixels, &heightInPixels);
        SDL_Log("New Window Size: %d x %d", widthInPixels, heightInPixels);
        SDL_RenderSetLogicalSize(renderer, widthInPixels, heightInPixels);
    });

But that isn't working for iOS.

Edit:
I will check if I can get it working with a plain SDL Application and give feedback or create a pull request if we can adjust TGUI to fix it.

@texus
Copy link
Owner

texus commented Mar 1, 2023

The SDL touch event provides values between 0 and 1, and TGUI is multiplying this with the window size. Since the logical and physical window size are different on high-DPI screens, I'm probably just multiplying with the wrong number.

I'm currently multiplying event.tfinger.x and event.tfinger.y with the values returned by SDL_GetWindowSize. I probably need to multiply them with the values returned by SDL_GetWindowSizeInPixels instead. (The relevant lines are found at https://github.com/texus/TGUI/blob/0.10/src/Backend/Window/SDL/BackendGuiSDL.cpp#L333-L386, with m_windowSize and m_framebufferSize containing the values from SDL_GetWindowSize and SDL_GetWindowSizeInPixels respectively)

While that would explain the wrong scaling of touch events, I don't understand how your fix for macOS would work, so maybe the issue is still more complex than this.

@RGPaul
Copy link
Contributor Author

RGPaul commented Mar 1, 2023

I can try to adjust that in BackendGuiSDL, but I think using SDL_GetWindowSize is correct. SDL_GetWindowSize should return points instead of pixels.

@RGPaul
Copy link
Contributor Author

RGPaul commented Mar 3, 2023

Okay it works correctly (even without SDL_RenderSetLogicalSize mentioned in #182 (comment)) if I change SDL_GetWindowSize to SDL_GetWindowSizeInPixels in BackendGuiSDL.cpp:

void BackendGuiSDL::updateContainerSize()
{
    // We can't do anything yet if we don't have a window
    if (!m_window)
        return;

    SDL_GetWindowSizeInPixels(m_window, &m_windowSize.x, &m_windowSize.y);
    ...

Edit:
macOS still needs the workaround mentioned in #182 (comment)

@texus
Copy link
Owner

texus commented Mar 3, 2023

Great, I'll update the code with a similar change later today so that the touch events will work out of the box on iOS.

If you are willing to help figure out why the macOS workaround is needed, I have a few more questions:

  • Without the workaround on macOS, are the touch events wrong or is the drawing scaled incorrectly (i.e. does the window contents still look the same with or without that code)?
  • Are you passing the SDL_WINDOW_ALLOW_HIGHDPI flag to SDL_CreateWindow?
  • Does it make a difference when you call SDL_SetHint(SDL_HINT_WINDOWS_DPI_SCALING, "1"); before SDL_Init? (I'm assuming it makes no difference)
  • Could you check what the value of event.tfinger.windowID is when you get a touch event? If it differs from 0 on macOS, then perhaps that is the difference with iOS (as I assume it is 0 on iOS)?

@RGPaul
Copy link
Contributor Author

RGPaul commented Mar 3, 2023

If you are willing to help figure out why the macOS workaround is needed, I have a few more questions:

Sure

  • Without the workaround on macOS, are the touch events wrong or is the drawing scaled incorrectly (i.e. does the window contents still look the same with or without that code)?

The drawing is correct - just the mouse hit and mouse hover is incorrect.

  • Are you passing the SDL_WINDOW_ALLOW_HIGHDPI flag to SDL_CreateWindow?

Yes

  • Does it make a difference when you call SDL_SetHint(SDL_HINT_WINDOWS_DPI_SCALING, "1"); before SDL_Init? (I'm assuming it makes no difference)

there is no difference

  • Could you check what the value of event.tfinger.windowID is when you get a touch event? If it differs from 0 on macOS, then perhaps that is the difference with iOS (as I assume it is 0 on iOS)?

on macOS I found:
eventSDL.button.windowID: 1

on iOS I found:
eventSDL.tfinger.windowID: 1

Edit:
I don't know if it makes a difference but on macOS the video driver says it is cocoa and on iOS the video driver name is uikit - determined using SDL_GetCurrentVideoDriver

Edit2:
I don't know if you have seen it, but there is an interesting discussion about high dpi about SDL3 (why I thought I need SDL3 Support for iOS): libsdl-org/SDL#7134

@texus
Copy link
Owner

texus commented Mar 3, 2023

I didn't realize earlier that on macOS you would be using mouse events instead of touch events, that is the difference between the macOS and iOS issues.

The mouse positions apparently need to be scaled. I didn't understand why it would be different on macOS and Windows, so I booted up my Windows, messed with the scaling, and found that the mouse positions where broken there as well. I've committed a change (1b24db4) that should fix the touch events on iOS and fixes the mouse positions on my Windows with high-DPI. So you should check if it also fixes the issue on macOS.

@RGPaul
Copy link
Contributor Author

RGPaul commented Mar 3, 2023

Okay now it works without the workaround! Thanks alot!

@RGPaul RGPaul closed this as completed Mar 3, 2023
texus added a commit that referenced this issue Mar 5, 2023
… choose the version if both SDL2 and SDL3 exist) (#182)
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

2 participants