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

Translated backend from SDL2 to SDL3 #6146

Closed
wants to merge 2 commits into from

Conversation

dovker
Copy link

@dovker dovker commented Feb 4, 2023

SDL 3 Backend

I literally just today wanted to update my engine to SDL3 and found that ImGui backend no longer works, therefore I translated the backend according to SDL3 rules, and as far as I've tested things, everything seems to work.

Screenshots:

image

@ocornut
Copy link
Owner

ocornut commented Feb 4, 2023

Hello. Thanks for the PR.
Since the code is so similar i think it should be kept in the same existing file so we don’t need to maintain two backends.

EDIT my bad, more changes than I saw at first glance.

@ocornut
Copy link
Owner

ocornut commented Feb 4, 2023

Also it seems like you copied the file but also based your work on a very old version of the SDL2 backend. We unfortunately cannot make use of any of it. You would need to start from latest version and make change in the same file.

Thank you.
EDIT used as reference when re-porting latest backend.

@dovker
Copy link
Author

dovker commented Feb 4, 2023

Also it seems like you copied the file but also based your work on a very old version of the SDL2 backend. We unfortunately cannot make use of any of it. You would need to start from latest version and make change in the same file.

Thank you.

Yeah, I just now realised that it's an old backend. I could translate the current SDL backend to SDL3, since I still need it myself and could be of some use to others.

The question now becomes, how do I fit both SDL2 and SDL3 backends in the same file, since a lot of enums and functions were updated in the 3.0 version. Should I use some sort of a macro for that, such as IMGUI_SDL_3?

@ocornut
Copy link
Owner

ocornut commented Feb 4, 2023

I don’t have an answer to your question since I don’t know what has changed yet, since we don’t have a diff. I’ll investigate later.

@ocornut
Copy link
Owner

ocornut commented Feb 6, 2023

I don’t have an answer to your question since I don’t know what has changed yet, since we don’t have a diff. I’ll investigate later.

Looks like more than I expected has changed. I'll be taking over this, thanks for the base PR!

@dovker
Copy link
Author

dovker commented Feb 6, 2023

No problem! (Should I close this PR?, I don't know how this works haha)

@ocornut
Copy link
Owner

ocornut commented Feb 6, 2023

I close when done, i just want to check if some of the workarounds for older SDL may be removed/simplified now.

ocornut added a commit that referenced this pull request Feb 7, 2023
…sdl2.cpp/.h. (#6146)

+ CI: Update Windows CI to update SDL 2.26.3 instead of 2.0.10
@ocornut
Copy link
Owner

ocornut commented Feb 7, 2023

Tangential: pushed 6e976e5 to rename everything from "sdl" to "sdl2" in filenames that didn't have that previously.

ocornut added a commit that referenced this pull request Feb 7, 2023
…sdl2.cpp/.h. (#6146)

+ CI: Update Windows CI to update SDL 2.26.3 instead of 2.0.10
ocornut added a commit that referenced this pull request Feb 7, 2023
…replaced strings (1/2). (#6146)

NO OTHER CHANGES. This WILL NOT compile with SDL3.
This intermediate commit designed to make it easier to visualize the meaningful channges commit in the next commit.
ocornut added a commit that referenced this pull request Feb 7, 2023
…e. Remove some version checks. (#6146)

More update upcoming in docking branch.
@ocornut
Copy link
Owner

ocornut commented Feb 7, 2023

  • 6e976e5 Rename everything from "sdl" to "sdl2" in filenames that didn't have that previously.
  • e816bc6 Merge misc changes from docking branch to reduce small drift. (before forking)
  • d9bf80f Backends: SDL3: copied imgui_impl_sdl2 into imgui_impl_sdl3 and ONLY replaced strings: NO OTHER CHANGES. This WILL NOT compile with SDL3. This intermediate commit designed to make it easier to visualize the meaningful changes in the next commit.
  • 13fbd99 Backends: SDL3: update to run with SDL3. simplify where possible. + Examples: Add SDL3+Gl example.
  • e4233c6 Merge.
  • a526ff8: Backends: SDL3: Added multi-viewports support.

I can see a new issue that SDL2 backend didn't have: newly created and destroyed Windows are now subject to a very short appear/disappear "animation" in Windows 11, which didn't happen with the SDL2. It makes dragging backends feel quite bad.
I haven't yet investigated which change in SDL is causing this.

It's something somewhere in
https://github.com/libsdl-org/SDL/tree/main/src/video/windows
My first intuition was that it was caused by this change
libsdl-org/SDL@b4ebb3b
But trying to revert it didn't fix the issue for me.

Closing this as merged but I expect SDL3 backend to require further changes.

@ocornut ocornut closed this Feb 7, 2023
@ocornut
Copy link
Owner

ocornut commented Feb 7, 2023

My first intuition was that it was caused by this change
libsdl-org/SDL@b4ebb3b
But trying to revert it didn't fix the issue for me.

My bad, it IS exactly this commit (my build scripts weren't setup ideally for SDL3 and my mod wasn't reflected when I attempted the change. I will open an issue on SDL3 repo.

@ocornut
Copy link
Owner

ocornut commented Feb 7, 2023

Posted libsdl-org/SDL#6659 (comment)

ocornut added a commit that referenced this pull request Feb 7, 2023
ocornut pushed a commit that referenced this pull request Oct 24, 2024
…WINDOW_HIGH_PIXEL_DENSITY from the main window. (#8098, #2306)

Amend a526ff8 (#6146)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants