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

Replace GLX with EGL for X11 #389

Merged
merged 10 commits into from
May 30, 2024

Conversation

christian-rauch
Copy link
Collaborator

@christian-rauch christian-rauch commented Jul 23, 2018

This PR replaces GLX by EGL for the X11 windowing and fixes potential issues like: #74 #242 #260. Example programs like SimpleDisplay work already.

Before continuing working on this: Will replacing GLX by EGL be potentially merged? This will remove the GLX dependency but adds and dependency on EGL (which is also required by Android).

EDIT: With NVIDIA drivers > 384 (390, 396), the window content does not update and stays black.

Fixes #782.

@christian-rauch christian-rauch force-pushed the x11_egl branch 2 times, most recently from 893c862 to 299be6b Compare July 26, 2018 17:24
@christian-rauch christian-rauch changed the title [WIP] EGL for X11 Replace GLX with EGL for X11 Jul 26, 2018
@stevenlovegrove
Copy link
Owner

This is great! If we can make this reliable enough, I would be very keen to merge. Especially if we can support the use case of a frame buffer without window.

@christian-rauch
Copy link
Collaborator Author

I posted the issue with the blank window with certain nvidia drivers with a minimal example at the nvidia forum: https://devtalk.nvidia.com/default/topic/1037901/x11-with-egl-produces-black-window-for-driver-versions-gt-384 . Since this issue also appears in the official mesa example, I somehow suspect that this is a driver issue.
The other issue that shared EGL contexts are not supported. I.e. at the moment the program simply exits when trying to access a context from another thread. This might be related to the X11+GLX context sharing issue #393.

When this works, going to windowless should be easy. We could even separate the EGL context creation into a class that is used by X11, Wayland and windowless.

@christian-rauch christian-rauch force-pushed the x11_egl branch 3 times, most recently from 4935a80 to d74ffff Compare August 6, 2018 18:04
@christian-rauch
Copy link
Collaborator Author

The X11+EGL window and context creation now works in a threaded environment.
However, I needed to add an new method RemoveCurrent to the WindowInterface to unset the current context. When starting a new thread, you need to call BindToContext() to set the context in this thread, and call RemoveCurrent() to unset it. I added a new example HelloPangolinThreads that demonstrates the procedure.
The threading patches also work on the previous X11+GLX version. I included them here because they are the only way I could make threading with X11+EGL working. But I can create another dedicated PR for the threading patches if requiered.

@christian-rauch christian-rauch force-pushed the x11_egl branch 3 times, most recently from f8dfdc5 to c47722c Compare August 7, 2018 01:20
@stevenlovegrove
Copy link
Owner

So did you manage to work around the NVidia issue that you references?

@christian-rauch
Copy link
Collaborator Author

The nvidia driver issue is unrelated to Pangolin, it also appears for the minimal EGL example that I posted in the nvidia forum https://devtalk.nvidia.com/default/topic/1037901/x11-with-egl-produces-black-window-for-driver-versions-gt-384 .

It seems to only occur with the nvidia drivers > 384 from the official CUDA repository for Ubuntu. E.g. it is working on other distributions or using the nvidia installation script (see post by matthewcmatl in the linked nvidia issue).
I have not tested this issue in many other configurations. It would be nice if someone with Ubuntu 18.04 could test if this occures with the drivers from the official package repo and the nvidia CUDA repo.

@stevenlovegrove
Copy link
Owner

Okay, I'm excited to merge this, but I guess I should wait until the Ubuntu package is fixed.

@christian-rauch
Copy link
Collaborator Author

I can confirm that the X11+EGL feature is working now with nvidia driver versions 390.48 (Ubuntu 18.04 repo) and 410.48 (CUDA repo).

@stevenlovegrove
Copy link
Owner

Are there any downsides to this implementation, or should I now consider this for merge?

@christian-rauch
Copy link
Collaborator Author

I am using this branch for quite a while now on Ubuntu 14.04 and 18.04 with Nvidia drivers from the CUDA repo. I think it would be good if this branch could be tested by others in different configurations to see if this issue with black windows still exists.

@fwcore
Copy link

fwcore commented Jul 25, 2023

Hi all, I tried this PR in a Ubuntu 22.04 docker with X11. HelloPangolin works as expected.

This PR helps me to solve my issue (libtorch+Pangolin segfault, see #884). Thank you very much.

The only thing I think is a little bit verbose is that I have to link explicitly OpenGL::EGL in CMakeLists.txt. I expect to link it implicitly through linking to pango_opengl, as for ${GLEW_LIBRARY}. See

https://github.com/christian-rauch/Pangolin/blob/5bf16e811704f98e12393f2a4ac5f5918e1f8fff/components/pango_opengl/CMakeLists.txt#L57C51-L57C66

Here is my current CMakeLists.txt

find_package(OpenGL REQUIRED COMPONENTS OpenGL EGL)
target_link_libraries(pangolin_libtorch_test
  PUBLIC
  "${TORCH_LIBRARIES}"
  ${Pangolin_LIBRARIES}
  OpenGL::EGL)

The EGL is explicitly added here. If I don't do this, I will have this error:

CMake Error at CMakeLists.txt:33 (add_executable):
  Target "pangolin_libtorch_test" links to target "OpenGL::EGL" but the
  target was not found.  Perhaps a find_package() call is missing for an
  IMPORTED target, or an ALIAS target is missing?

@EXing
Copy link

EXing commented Mar 19, 2024

Thanks to this MR, save me from OpenGL errors

@christian-rauch
Copy link
Collaborator Author

@stevenlovegrove I've tested this successfully on Intel and Nvidia systems natively on X11 and via XWayland Wayland display servers. Since this removes the GLX dependency completely and fixes issues reported by others (#782, ), I would finally like to merge this now.

Copy link
Owner

@stevenlovegrove stevenlovegrove left a comment

Choose a reason for hiding this comment

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

Great, thanks!

@christian-rauch christian-rauch merged commit 661500c into stevenlovegrove:master May 30, 2024
9 checks passed
@christian-rauch christian-rauch deleted the x11_egl branch May 30, 2024 19:47
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 this pull request may close these issues.

Failed to run examples/SimplePlot in ubuntu20.04
6 participants