Skip to content
This repository has been archived by the owner on May 13, 2024. It is now read-only.

Update CIrrDeviceSDL to support SDL2 #85

Merged
merged 9 commits into from
Jan 16, 2022
Merged

Update CIrrDeviceSDL to support SDL2 #85

merged 9 commits into from
Jan 16, 2022

Conversation

emmauss
Copy link
Contributor

@emmauss emmauss commented Dec 19, 2021

This updates the sdl version CIrrDeviceSDL uses from 1.2 to 2.0. Also, the compiler config header now allows building with only sdl device on windows, x11 and osx.

@sfan5 sfan5 added the enhancement New feature or request label Dec 19, 2021
Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for contributing this, up-to-date SDL support is very useful

source/Irrlicht/CIrrDeviceSDL.cpp Outdated Show resolved Hide resolved
source/Irrlicht/CIrrDeviceSDL.cpp Outdated Show resolved Hide resolved
source/Irrlicht/CIrrDeviceSDL.cpp Show resolved Hide resolved
source/Irrlicht/CIrrDeviceSDL.cpp Outdated Show resolved Hide resolved
source/Irrlicht/CIrrDeviceSDL.cpp Outdated Show resolved Hide resolved
source/Irrlicht/CIrrDeviceSDL.h Outdated Show resolved Hide resolved
source/Irrlicht/CMakeLists.txt Outdated Show resolved Hide resolved
source/Irrlicht/CMakeLists.txt Outdated Show resolved Hide resolved
source/Irrlicht/COpenGLDriver.cpp Show resolved Hide resolved
Copy link
Member

@ShadowNinja ShadowNinja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool, SDL2 is probably our best path forward for native Wayland support. I noted a few suggestions.

source/Irrlicht/CIrrDeviceSDL.cpp Outdated Show resolved Hide resolved
source/Irrlicht/CIrrDeviceSDL.cpp Outdated Show resolved Hide resolved
source/Irrlicht/CIrrDeviceSDL.cpp Outdated Show resolved Hide resolved
source/Irrlicht/CIrrDeviceSDL.cpp Outdated Show resolved Hide resolved
source/Irrlicht/CIrrDeviceSDL.cpp Outdated Show resolved Hide resolved
@emmauss
Copy link
Contributor Author

emmauss commented Dec 21, 2021

Address above reviews

include/IrrCompileConfig.h Outdated Show resolved Hide resolved
@emmauss
Copy link
Contributor Author

emmauss commented Dec 21, 2021

Addressed comments

@ShadowNinja
Copy link
Member

I had to apply the following patch to get it to compile:

diff --git a/source/Irrlicht/COpenGLCommon.h b/source/Irrlicht/COpenGLCommon.h
index d8ecc2e..eff48b6 100644
--- a/source/Irrlicht/COpenGLCommon.h
+++ b/source/Irrlicht/COpenGLCommon.h
@@ -41,8 +41,12 @@
                #define GL_GLEXT_PROTOTYPES 1
                #define GLX_GLXEXT_PROTOTYPES 1
        #endif
-       #include <SDL/SDL_video.h>
-       #include <SDL/SDL_opengl.h>
+       #include <SDL2/SDL_video.h>
+       #include <SDL2/SDL_opengl.h>
+       #include <GL/gl.h>
+       #if defined(_IRR_OPENGL_USE_EXTPOINTER_)
+       #include <GL/glext.h>
+       #endif^M
 #else
        #if defined(_IRR_OPENGL_USE_EXTPOINTER_)
                #define GL_GLEXT_LEGACY 1

After that I'm able to launch Minetest, however there are some issues:

  • Keys aren't read properly. A lot of keys (e.g. shift and the arrow keys) seem to be read as tab.
  • When running under native Wayland (SDL_VIDEODRIVER=wayland) relative mouse movement is bugged (moving the mouse slightly to the left and stopping will cause the view to continuously move left until you move the mouse back to the right)

@emmauss
Copy link
Contributor Author

emmauss commented Dec 23, 2021

I had to apply the following patch to get it to compile:

diff --git a/source/Irrlicht/COpenGLCommon.h b/source/Irrlicht/COpenGLCommon.h
index d8ecc2e..eff48b6 100644
--- a/source/Irrlicht/COpenGLCommon.h
+++ b/source/Irrlicht/COpenGLCommon.h
@@ -41,8 +41,12 @@
                #define GL_GLEXT_PROTOTYPES 1
                #define GLX_GLXEXT_PROTOTYPES 1
        #endif
-       #include <SDL/SDL_video.h>
-       #include <SDL/SDL_opengl.h>
+       #include <SDL2/SDL_video.h>
+       #include <SDL2/SDL_opengl.h>
+       #include <GL/gl.h>
+       #if defined(_IRR_OPENGL_USE_EXTPOINTER_)
+       #include <GL/glext.h>
+       #endif^M
 #else
        #if defined(_IRR_OPENGL_USE_EXTPOINTER_)
                #define GL_GLEXT_LEGACY 1

After that I'm able to launch Minetest, however there are some issues:

* Keys aren't read properly. A lot of keys (e.g. shift and the arrow keys) seem to be read as tab.

* When running under native Wayland (`SDL_VIDEODRIVER=wayland`) relative mouse movement is bugged (moving the mouse slightly to the left and stopping will cause the view to continuously move left until you move the mouse back to the right)

Thanks for testing. I do not readily have a linux system, so testing on it skipped me. The header and keys issues should be fixed. I'm unable to test the mouse issue on wayland since wayland is not supported on the system I got to test linux.

@ShadowNinja
Copy link
Member

It looks like the issue is that Minetest just hides the cursor then warps it back to the center of the window on every frame. Wayland doesn't let you warp the cursor, you have to lock the cursor to the window use relative mouse motion instead (which SDL_SetRelativeMouseMode does).

I remember seeing similar issues before on other platforms (I think it sometimes happened on Windows?) so it's something that we should fix, but fixing it involves making changes to Minetest's mouse handling, so it wouldn't be a part of this PR.

Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unicode input doesn't work, the clipboard is nonfunctional and I doubt this runs on Windows due to the SDL_main mechanism, but it's a start.
I'll merge this later.

@sfan5 sfan5 changed the title Updates CIrrDeviceSDL to SDL2 Update CIrrDeviceSDL to support SDL2 Jan 15, 2022
@v-rob
Copy link
Member

v-rob commented Jan 15, 2022

Glad to see something going through. I still do want to use SDL2 for everything if possible, so I'll likely work on this myself sometime.

@sfan5 sfan5 merged commit 53db262 into minetest:master Jan 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants