-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[linux] Added EGL headless backend support #7007
Conversation
@brunoabinader, thanks for your PR! By analyzing the history of the files in this pull request, we identified @kkaefer, @tmpsantos and @jfirebaugh to be potential reviewers. |
6a1847e
to
54997f1
Compare
5971ac1
to
8dc0033
Compare
- &clang35_packages [ 'clang-3.5', 'libstdc++-4.9-dev', 'libstdc++6' ] | ||
- &gcc5_packages [ 'gcc-5', 'g++-5' ] | ||
- &egl_packages [ 'libgles2-mesa-dev', 'libgbm-dev' ] | ||
- &glfw_packages [ 'libxrandr-dev', 'libxcursor-dev', 'libxinerama-dev' ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😍
PRIVATE platform/linux/src/headless_backend_egl.cpp | ||
PRIVATE platform/linux/src/headless_display_egl.cpp | ||
) | ||
# XXX: Provide surface-less EGL mesa for CI builds. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remaining todo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah - #7020
I'll add that link to the comment 👍
EGLDisplay display_ = display->attribute<EGLDisplay>(); | ||
EGLConfig& config = display->attribute<EGLConfig&>(); | ||
|
||
eglBindAPI(EGL_OPENGL_API); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing error handling
EGL_ALPHA_SIZE, 0, | ||
EGL_DEPTH_SIZE, 1, | ||
EGL_RENDERABLE_TYPE, EGL_OPENGL_BIT, | ||
EGL_NONE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a note that the configuration here doesn't matter since we're rendering to a framebuffer anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea 👍
b9de0b3
to
e9f3dce
Compare
EGLDisplay display_ = display->attribute<EGLDisplay>(); | ||
EGLConfig& config = display->attribute<EGLConfig&>(); | ||
|
||
if (!eglBindAPI(EGL_OPENGL_API)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are setting MBGL_USE_GLES2
above, but requesting a desktop OpenGL API here. Did you try requesting EGL_OPENGL_ES_API
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes - I initially used EGL_OPENGL_ES_API
, but then I obtained a crash right after when querying for the shading version string.
I believe this happens because when I attempt to use that enum, querying for the version string of my local EGL implementation returns Version: OpenGL ES-CM 1.1 Mesa 10.1.3
. From the eglBindAPI documentation, it says eglBindAPI and the corresponding EGL_OPENGL_ES_API and EGL_OPENVG_API api parameters are supported only if the EGL version is 1.2 or greater.
@@ -25,6 +26,15 @@ HeadlessDisplay::Impl::Impl() { | |||
throw std::runtime_error("eglInitialize() failed.\n"); | |||
} | |||
|
|||
if (major >= 1 && minor >= 2) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that fails for EGL 2.0
@@ -25,6 +26,15 @@ HeadlessDisplay::Impl::Impl() { | |||
throw std::runtime_error("eglInitialize() failed.\n"); | |||
} | |||
|
|||
if (major >= 1 && minor >= 2) { | |||
EGLenum api = minor >= 4 ? EGL_OPENGL_API : EGL_OPENGL_ES_API; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also problematic works for 1.4
, but not for 2.0
? If it works, we /always/ want the OpenGL ES API, since that's what we're mainly targeting. We just happen to be largely compatible with Desktop OpenGL due to the limited featureset we're using.
5f46eb5
to
38d8fd6
Compare
@kkaefer I found the reason why we were unable to compile the shader. Even though we bind to To fix that, we need to pass |
4bb04b0
to
9a4f6cf
Compare
Heh, this worked better than I've expected: Because we really use OpenGL ES 2.0 client API, I had to append the code snippet below to each of the test shaders from
If my assumptions about Android using EGL as backend are correct, then this is probably one step further running core tests on Android. |
9a4f6cf
to
882baa4
Compare
@kkaefer 🚢 ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the only test that runs the render test-suite are the node-bindings, so it seems that we're only testing rendering with OSMesa, but not with EGL or GLX. Can we add on a render test to those as well?
This is going to be provided in #7053 👍 |
Prevents some OpenGL implementations from bailing out.
a06ae5a
to
2247a0e
Compare
@kkaefer I've rebased this after #7053 got merged. There are now 4 new Travis CI builds added:
@mikemorris the added Node builds won't affect the |
addons: *qt4 | ||
before_script: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the GLX - Node - Clang 3.8 - Release
build currently using RUN_XVFB
be switched to these setup scripts instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, forgot about that :)
|
||
export -f mapbox_start_xvfb | ||
|
||
function mapbox_install_mesa_glx { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is needed as a separate script for only the QT builds because Mesa is installed in platform/linux/config.cmake
for Linux builds, right? Is there any way we could consolidate these into a single script that accepts args to install the necessary version of Mesa?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The instructions we set in platform/linux/config.cmake
should be sufficient for all things Mapbox GL-wise. However, Qt libraries don't know about our custom libGL.so
, so we must tell 'em to use it via LD_LIBRARY_PATH
.
Because that's the only reason for having this function, I'm not sure if we should add extra functionality in this. Do we have a need for this elsewhere?
@@ -1,5 +1,5 @@ | |||
mason_use(glfw VERSION 3.2.1) | |||
if (WITH_OSMESA OR IS_CI_BUILD) | |||
if(IS_CI_BUILD AND NOT WITH_EGL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little confused by this - are we installing Mesa somewhere else for EGL CI builds? It appears that we're still setting MASON_MESA_SUFFIX
for WITH_EGL
builds in CMakeLists.txt
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little confused by this - are we installing Mesa somewhere else for EGL CI builds?
Yes - we are using the system's mesa install via libgles2-mesa-dev
.
It appears that we're still setting MASON_MESA_SUFFIX for WITH_EGL builds in CMakeLists.txt.
That should cause no harm - it's going to be used when we get the EGL surface-less mesa build :)
Original author: Tiago Vignatti <tvignatti@gmail.com> Calling X11 window system is superfluous for headless rendering. This patch implements EGL platform using GBM, which is slightly more simple than the GLX path when using X11. In principle there are no big advantages in terms of performance etc. My motivation behind this was to get in touch with the code and the project. For testing I'm using: $ unset DISPLAY && ./build/linux-x86_64/Debug/mbgl-test v2: rebased patch against the new cmake changes; walk through render node to find a valid one; remove EGLSurface completely cause windows are not needed here.
03481a2
to
e95ff7e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to put the breaks on here for a sec. We already have quite a few rows in the Travis matrix, and frequently bump up against our built limits, leading to time waiting for the queue to clear.
Can we think about ways to make the matrix smaller?
e90a379
to
65d541c
Compare
65d541c
to
23de317
Compare
ab4ad53
to
bef0f95
Compare
@jfirebaugh we've reduced the build matrix to 10 items. |
Follow-up to the work started by @tiagovignatti in #5843 - thanks Tiago!
This PR implements EGL headless backend, using
EGL_DEFAULT_DISPLAY
as primary display - thus no need for handling render nodes directly via gbm.Though we use software rendering via LLVMpipe, we still need a valid display for EGL to be properly initialized. On Travis CI, like the Qt builds, we still need to start Xvfb to obtain a valid display.
I've put some effort attempting to find a working setup for Mesa to have its EGL platform working with a native display that doesn't require a window system, but so far no success. The most promising option, though, comes from this recent Mesa extension - MESA_platform_surfaceless - that promises to provide a native display independent of any native window system. Given this solution already works and is testable via CI, we can pursuit this as a follow-up work.