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

Use EGL over GLX #15286

Merged
merged 3 commits into from
Oct 22, 2024
Merged

Use EGL over GLX #15286

merged 3 commits into from
Oct 22, 2024

Conversation

sfan5
Copy link
Collaborator

@sfan5 sfan5 commented Oct 13, 2024

only affects the legacy CIrrDeviceLinux.
GLX is quite old and EGL is the "new" standard for this.
this change could fix some problems people have been having.

To do

This PR is technically ready.

Note: this should be tested on Linux/Intel, Linux/AMD and Linux/NVIDIA before merge!

@sfan5 sfan5 added Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements Bugfix 🐛 PRs that fix a bug Waiting (on dependency) Waiting on another PR or external circumstances (not for rebases/changes requested) labels Oct 13, 2024
@sfan5 sfan5 added Linux and removed Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements labels Oct 13, 2024
@okias
Copy link
Contributor

okias commented Oct 13, 2024

No-one should use GLX,.. I just had talk about GLX/EGL subject on XDC24 few days ago.

I assume the SDL2 revert is not meant to remain at time of merge?

@sfan5
Copy link
Collaborator Author

sfan5 commented Oct 14, 2024

I assume the SDL2 revert is not meant to remain at time of merge?

When we're using SDL2 it will handle the GL context for us (and I bet it already uses EGL).
This fix is specifically for the impending release, where we will temporarily disable SDL2 again.

@lhofhansl
Copy link
Contributor

I wonder if FSAA will still work with this.
On my machine that works with GLX, but not SDLx.

@sfan5
Copy link
Collaborator Author

sfan5 commented Oct 14, 2024

On my system FSAA doesn't work with this (just like SDL). I'm not sure why and couldn't quickly figure it out. 🤷
probably something from this guide missing https://learnopengl.com/Advanced-OpenGL/Anti-Aliasing

@sfan5 sfan5 removed the Waiting (on dependency) Waiting on another PR or external circumstances (not for rebases/changes requested) label Oct 14, 2024
@lhofhansl
Copy link
Contributor

I had looked briefly in the past. While it's logically not that hard to add, I found it incredibly tedious and obtuse in Irrlicht.

@sfan5
Copy link
Collaborator Author

sfan5 commented Oct 16, 2024

@lhofhansl actually I think the root cause is just #13397 and it's just a weird accident that it still worked with GLX.
we could possibly introduce multisample textures to bring back FSAA. not sure if worth it.

@lhofhansl
Copy link
Contributor

I don't mean to say that that should block this PR.

That said, with some larger viewing ranges the experience without FSAA is really shitty and SSAA is just too slow (and FXAA does not deal with geometry aliasing at all).

@okias
Copy link
Contributor

okias commented Oct 17, 2024

Tested on Linux, works well for me (disabled SDL2, Wayland + Xwayland)

@sfan5 sfan5 added this to the 5.10.0 milestone Oct 19, 2024
@sfan5
Copy link
Collaborator Author

sfan5 commented Oct 19, 2024

Adding to milestone since this is at the very least a maintenance improvement and could potentially fix the game not launching.

@lhofhansl
Copy link
Contributor

lhofhansl commented Oct 19, 2024

could possibly introduce multisample textures to bring back FSAA. not sure if worth it

Am I the only one who uses large viewing_ranges and finds the geometry aliasing effects causing moire patterns grating? It looks like a cheap, bad game from the early 2000s.
FXAA or any other pixel space anti-aliasing will not fix this. SSAA does by just rendering at a higher resolution, but it's too slow.

IMHO there is no alternative really... Well other than rendering at a lower geometry resolution at a distance, perhaps with some LOD.

Oh, and for #13397 I added the ability to disable post-processing. FSAA does work with shaders, just not with post-processing (unless said PP takes multi-sampled textures into account)

Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

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

Tested on Linux non-SDL X11
Enabled settings: shadows + post processing + debanding enabled.

Intel HD, i915, OpenGL 4.6. Values of antialiasing:

  • none: works
  • fsaa (16): "works" but looks like none
  • ssaa produces a black screen. No errors. [1]
  • fxaa: antialiasing works

AMD Polaris, amdgpu, OpenGL 4.6. Values of antialiasing:

  • none: works
  • fsaa (16): "works" but looks like none
  • ssaa produces a black screen. No errors. [1]
  • fxaa: antialiasing works

[1] Disabling post-processing or shaders altogether can get it to "work", but it looks like none.

Furthermore (not relevant for this PR), I noticed that the Debian package libgl1-mesa-dev is now a placeholder for transition. libegl-dev is now required. libopengl-dev is technically optional, but CMake fails to find libOpenGL.so.0 on its own if that's missing.

@SmallJoker
Copy link
Member

I also found an interesting performance issue when using DRI_PRIME=1 vblank_mode=0 ./minetest after switching the display cable to the dGPU. The dGPU is used but the FPS jumps all over the place. I don't think it's relevant for this PR but leaving it here just in case.

test_pr_15286.mp4

@lhofhansl
Copy link
Contributor

fsaa (16): "works" but looks like none

FSAA definitely works for me, on Intel integrated graphics and NVidia, and it make a huge difference.
Note that you won't see a big difference with smaller viewing_ranges (less than, say, 500). Beyond that you'll get pretty bad geometry (not texture) aliasing.

FSAA and SSAA both handle geometry aliasing.
FXAA and SSAA both handle texture aliasing.

(And so FSAA does not help with texture aliasing, and FXAA does not help with geometry aliasing).

SSAA of course is best, but it's also by far the slowest - unusable even with my NVidia card.

@sfan5 sfan5 merged commit aa27311 into luanti-org:master Oct 22, 2024
15 checks passed
@sfan5 sfan5 deleted the yesegl branch October 22, 2024 21:04
@grorp
Copy link
Member

grorp commented Oct 22, 2024

Now I'm getting these warnings at startup:

libEGL warning: egl: failed to create dri2 screen
MESA-INTEL: warning: Haswell Vulkan support is incomplete

@grorp
Copy link
Member

grorp commented Oct 22, 2024

FSAA still has an effect for me (as long as post-processing) is disabled, but font rendering is somehow worse now:

screenshot with 16x FSAA, before
screenshot 1

screenshot with 16x FSAA, after
screenshot 2

screenshot with 16x FSAA, before, this time in the mainmenu
screenshot 3

screenshot with 16x FSAA, after, mainmenu
screenshot 4

@grorp
Copy link
Member

grorp commented Oct 22, 2024

And now it's hanging at startup with a black window and this backtrace:

#0  0x00007eff5da0a87d in poll () from /lib64/libc.so.6
#1  0x00007eff5d5710b2 in _xcb_conn_wait.part.0 () from /lib64/libxcb.so.1
#2  0x00007eff5d573090 in xcb_wait_for_special_event () from /lib64/libxcb.so.1
#3  0x00007eff4e27a551 in dri3_wait_for_event_locked () from /lib64/libEGL_mesa.so.0
#4  0x00007eff4e27a75b in dri3_find_back () from /lib64/libEGL_mesa.so.0
#5  0x00007eff4e27cc91 in dri3_get_buffer.isra () from /lib64/libEGL_mesa.so.0
#6  0x00007eff4e27d028 in loader_dri3_get_buffers () from /lib64/libEGL_mesa.so.0
#7  0x00007eff2ee385a5 in dri_image_drawable_get_buffers () from /usr/lib64/dri/crocus_dri.so
#8  0x00007eff2ee38653 in dri2_allocate_textures () from /usr/lib64/dri/crocus_dri.so
#9  0x00007eff2ee3bb86 in dri_st_framebuffer_validate () from /usr/lib64/dri/crocus_dri.so
#10 0x00007eff2ef18e1c in st_framebuffer_validate () from /usr/lib64/dri/crocus_dri.so
#11 0x00007eff2ef19d22 in st_manager_validate_framebuffers () from /usr/lib64/dri/crocus_dri.so
#12 0x00007eff2f1a095b in st_update_framebuffer_state () from /usr/lib64/dri/crocus_dri.so
#13 0x00007eff2f1a5b4f in st_Clear () from /usr/lib64/dri/crocus_dri.so
#14 0x0000000000c50880 in irr::video::COpenGLDriver::clearBuffers(unsigned short, irr::video::SColor, float, unsigned char) ()
#15 0x0000000000c4fa80 in irr::video::COpenGLDriver::beginScene(unsigned short, irr::video::SColor, float, unsigned char, irr::video::SExposedVideoData const&, irr::core::rect<int>*) ()
#16 0x000000000068f583 in GUIEngine::run() ()
#17 0x0000000000690160 in GUIEngine::GUIEngine(JoystickController*, irr::gui::IGUIElement*, RenderingEngine*, IMenuManager*, MainMenuData*, bool&) ()
#18 0x00000000004ff0bf in ClientLauncher::main_menu(MainMenuData*) ()
#19 0x000000000050086b in ClientLauncher::launch_game(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, bool, GameStartData&, Settings const&) ()
#20 0x000000000050335f in ClientLauncher::run(GameStartData&, Settings const&) ()
#21 0x00000000004cfe58 in main ()

Commit d52e4cd, and it's fixed when I revert aa27311

It only happens if I start it via the desktop file after make install, starting it from terminal works as expected

@lhofhansl
Copy link
Contributor

lhofhansl commented Oct 22, 2024

I get an immediate Segmentation fault now with the NVidia driver (not Intel)

I thought we get an option to switch back to GLX. My fault I did not actually check, but no option to keep using GLX is not acceptable. @sfan5

@lhofhansl
Copy link
Contributor

lhofhansl commented Oct 23, 2024

2024-10-22 17:03:01: ERROR[Main]: Irrlicht: Could not create EGL surface.

Thread 1 "minetest" received signal SIGSEGV, Segmentation fault.
0x0000000000c60471 in irr::video::COpenGLDriver::~COpenGLDriver() ()
(gdb) bt
#0  0x0000000000c60471 in irr::video::COpenGLDriver::~COpenGLDriver() ()
#1  0x0000000000c60723 in virtual thunk to irr::video::COpenGLDriver::~COpenGLDriver() ()
#2  0x0000000000c6ce03 in irr::video::createOpenGLDriver(irr::SIrrlichtCreationParameters const&, irr::io::IFileSystem*, irr::video::IContextManager*) ()
#3  0x0000000000c0165f in irr::CIrrDeviceLinux::createDriver() ()
#4  0x0000000000c0c6fe in irr::CIrrDeviceLinux::CIrrDeviceLinux(irr::SIrrlichtCreationParameters const&) ()
#5  0x0000000000bfff30 in createDeviceEx ()
#6  0x00000000005f9e12 in RenderingEngine::RenderingEngine(MyEventReceiver*) ()
#7  0x00000000004fea74 in ClientLauncher::init_engine() ()
#8  0x0000000000503d0f in ClientLauncher::run(GameStartData&, Settings const&) ()
#9  0x00000000004d13d8 in main ()

Works fine with aa27311 reverted.

@sfan5
Copy link
Collaborator Author

sfan5 commented Oct 23, 2024

Ok then let's not invest more time into legacy shit.

I thought we get an option to switch back to GLX. My fault I did not actually check, but no option to keep using GLX is not acceptable. @sfan5

There is absolutely no reason to ever use GLX anymore.

@okias
Copy link
Contributor

okias commented Oct 24, 2024

For the xcb, looks very similar to https://gitlab.freedesktop.org/xorg/lib/libxcb/-/issues/38 .

Going back to GLX seriously doesn't make sense, just workarounding buggy X11 should be enough

@sfan5
Copy link
Collaborator Author

sfan5 commented Oct 25, 2024

It doesn't matter, because when we finally enable SDL (permanently) all this legacy code that directly deals with X11 or GLX can be deleted.

robertkirkman added a commit to robertkirkman/tur that referenced this pull request Nov 24, 2024
I am pretty sure upstream Luanti has mostly already done this on **all its X11-based
official targets**, but I did not yet do this
to Luanti-on-TUR because I have not had time to finish fixing + testing
this mode.

In particular I believe that this mode has a unique potential to
possibly engage with twaik's `termux-wsi-layer` and possibly benefit
from an extremely huge performance boost on some specific devices, if it
can be made to work with it. termux/termux-packages#22353

However, unfortunately right now when I try to use this PR with
`termux-wsi-layer` installed on a device where it works with
`glmark2-es2`, I see this error

```
Thread 1 "luanti" received signal SIGSEGV, Segmentation fault.
0x0000007fad9782b0 in XGetVisualInfo ()
   from /data/data/com.termux/files/usr/lib/libX11.so
(gdb) bt
   from /data/data/com.termux/files/usr/lib/libX11.so
    config=0xb400007d1ca54f00, attribute=12334, value=0x7fffffd464)
    at /data/data/com.termux/files/home/termux-packages/packages/termux-wsi-layer/egl.c:133
    this=0xb400007daca1c9b0)
    at /data/data/com.termux/files/home/.termux-build/luanti/src/irr/src/CEGLManager.cpp:185
    this=this@entry=0xb400007ddca220d0)
    at /data/data/com.termux/files/home/.termux-build/luanti/src/irr/src/CIrrDeviceLinux.cpp:414
    this=0xb400007ddca220d0, param=...)
    at /data/data/com.termux/files/home/.termux-build/luanti/src/irr/src/CIrrDeviceLinux.cpp:166
    at /data/data/com.termux/files/home/.termux-build/luanti/src/irr/src/Irrlicht.cpp:66
    at /data/data/com.termux/files/home/.termux-build/luanti/src/src/client/renderingengine.cpp:151
    receiver=0xb400007d9ca1d630)
    at /data/data/com.termux/files/home/.termux-build/luanti/src/src/client/renderingengine.cpp:206
    at /data/data/com.termux/files/home/.termux-build/luanti/src/src/client/clientlauncher.cpp:290
    cmd_args=...)
    at /data/data/com.termux/files/home/.termux-build/luanti/src/src/client/clientlauncher.cpp:98
    at /data/data/com.termux/files/home/.termux-build/luanti/src/src/main.cpp:264
(gdb)
```

`0001-force-egl-priority-over-glx.patch`: this is
luanti-org/luanti#15286 but applied to
Luanti-on-TUR

`0005-toggle-on-the-recent-migration-away-from-sdl2.patch`: this is
luanti-org/luanti#15284 but applied to
Luanti-on-TUR

`0006-fix-incompatible-type-ekey-code.patch`: I wrote this, this is necessary to fix a
minor build error when SDL2 is disabled in Luanti
robertkirkman added a commit to robertkirkman/tur that referenced this pull request Nov 24, 2024
I am pretty sure upstream Luanti has mostly already done this on **all its X11-based
official targets**, but I did not yet do this
to Luanti-on-TUR because I have not had time to finish fixing + testing
this mode.

In particular I believe that this mode has a unique potential to
possibly engage with twaik's `termux-wsi-layer` and possibly benefit
from an extremely huge performance boost on some specific devices, if it
can be made to work with it. termux/termux-packages#22353

However, unfortunately right now when I try to use this PR with
`termux-wsi-layer` installed on a device where it works with
`glmark2-es2`, I see this error

```
Thread 1 "luanti" received signal SIGSEGV, Segmentation fault.
0x0000007fad9782b0 in XGetVisualInfo ()
   from /data/data/com.termux/files/usr/lib/libX11.so
(gdb) bt
   from /data/data/com.termux/files/usr/lib/libX11.so
    config=0xb400007d1ca54f00, attribute=12334, value=0x7fffffd464)
    at /data/data/com.termux/files/home/termux-packages/packages/termux-wsi-layer/egl.c:133
    this=0xb400007daca1c9b0)
    at /data/data/com.termux/files/home/.termux-build/luanti/src/irr/src/CEGLManager.cpp:185
    this=this@entry=0xb400007ddca220d0)
    at /data/data/com.termux/files/home/.termux-build/luanti/src/irr/src/CIrrDeviceLinux.cpp:414
    this=0xb400007ddca220d0, param=...)
    at /data/data/com.termux/files/home/.termux-build/luanti/src/irr/src/CIrrDeviceLinux.cpp:166
    at /data/data/com.termux/files/home/.termux-build/luanti/src/irr/src/Irrlicht.cpp:66
    at /data/data/com.termux/files/home/.termux-build/luanti/src/src/client/renderingengine.cpp:151
    receiver=0xb400007d9ca1d630)
    at /data/data/com.termux/files/home/.termux-build/luanti/src/src/client/renderingengine.cpp:206
    at /data/data/com.termux/files/home/.termux-build/luanti/src/src/client/clientlauncher.cpp:290
    cmd_args=...)
    at /data/data/com.termux/files/home/.termux-build/luanti/src/src/client/clientlauncher.cpp:98
    at /data/data/com.termux/files/home/.termux-build/luanti/src/src/main.cpp:264
(gdb)
```

`0001-force-egl-priority-over-glx.patch`: this is
luanti-org/luanti#15286 but applied to
Luanti-on-TUR

`0005-toggle-on-the-recent-migration-away-from-sdl2.patch`: this is
luanti-org/luanti#15284 but applied to
Luanti-on-TUR

`0006-fix-incompatible-type-ekey-code.patch`: I wrote this, this is necessary to fix a
minor build error when SDL2 is disabled in Luanti
robertkirkman added a commit to robertkirkman/tur that referenced this pull request Nov 24, 2024
…l its X11-based

official targets**, but I did not yet do this
to Luanti-on-TUR because I have not had time to finish fixing + testing
this mode.

`0001-force-egl-priority-over-glx.patch`: this is
luanti-org/luanti#15286 but applied to
Luanti-on-TUR

`0005-toggle-on-the-recent-migration-away-from-sdl2.patch`: this is
luanti-org/luanti#15284 but applied to
Luanti-on-TUR

`0006-fix-incompatible-type-ekey-code.patch`: I wrote this, this is necessary to fix a
minor build error when SDL2 is disabled in Luanti
robertkirkman added a commit to robertkirkman/tur that referenced this pull request Nov 24, 2024
I am pretty sure upstream Luanti has mostly already done this on **all its X11-based official targets**, but I did not yet do this to Luanti-on-TUR because I have not had time to finish fixing + testing
this mode.

`0001-force-egl-priority-over-glx.patch`: this is
luanti-org/luanti#15286 but applied to
Luanti-on-TUR

`0005-toggle-on-the-recent-migration-away-from-sdl2.patch`: this is
luanti-org/luanti#15284 but applied to
Luanti-on-TUR

`0006-fix-incompatible-type-ekey-code.patch`: I wrote this, this is necessary to fix a
minor build error when SDL2 is disabled in Luanti
robertkirkman added a commit to robertkirkman/tur that referenced this pull request Nov 24, 2024
I am pretty sure upstream Luanti has mostly already done this on **all its X11-based official targets**, but I did not yet do this to Luanti-on-TUR because I have not had time to finish fixing + testing
this mode.

`0001-force-egl-priority-over-glx.patch`: this is
luanti-org/luanti#15286 but applied to
Luanti-on-TUR

`0005-toggle-on-the-recent-migration-away-from-sdl2.patch`: this is
luanti-org/luanti#15284 but applied to
Luanti-on-TUR

`0006-fix-incompatible-type-ekey-code.patch`: I wrote this, this is necessary to fix a
minor build error when SDL2 is disabled in Luanti
robertkirkman added a commit to robertkirkman/tur that referenced this pull request Nov 24, 2024
I am pretty sure upstream Luanti has mostly already done this on **all its X11-based official targets**, but I did not yet do this to Luanti-on-TUR because I have not had time to finish fixing + testing
this mode.

`0001-force-egl-priority-over-glx.patch`: this is
luanti-org/luanti#15286 but applied to
Luanti-on-TUR

`0005-toggle-on-the-recent-migration-away-from-sdl2.patch`: this is
luanti-org/luanti#15284 but applied to
Luanti-on-TUR

`0006-fix-incompatible-type-ekey-code.patch`: I wrote this, this is necessary to fix a
minor build error when SDL2 is disabled in Luanti
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants