feat: support OpenGL EGL and GLX contexts#230
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds explicit OpenGL context selection on Linux (EGL vs GLX) so headless OpenGL rendering works without an X server by default, and updates build/CI wiring to match MapLibre Native’s MLN_WITH_* options. It also introduces a temporary serialization workaround in the C++ bridge to avoid crashes when creating/destroying headless OpenGL renderers concurrently.
Changes:
- Add
egl,glx, andx11Cargo features and map them inbuild.rsto the corresponding MapLibre Native build/link configuration. - Make EGL the default Linux OpenGL context (surfaceless) and split CI into separate
egl(no Xvfb) andglx(Xvfb) jobs. - Serialize OpenGL headless display create/teardown in the bridge (
map_renderer.h) via a global mutex until the upstream fix lands.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/cpp/map_renderer.h |
Adds an OpenGL-only global mutex + forced backend activation to serialize headless display init/teardown. |
README.md |
Documents new Linux OpenGL context/windowing feature flags and updated dependency instructions. |
justfile |
Updates Linux dependency installation to support egl vs glx backends (and Xvfb only for GLX). |
examples/slint/Cargo.toml |
Exposes the new egl/glx/x11 feature passthroughs for the Slint example. |
Cargo.toml |
Defines new egl/glx/x11 features and their relationships to opengl. |
build.rs |
Adds feature-derived OpenGL context selection and adjusts CMake/link flags; emits feature-combination warnings. |
.github/workflows/ci.yml |
Splits Linux OpenGL CI into egl and glx variants, running GLX under Xvfb. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CommanderStorm
left a comment
There was a problem hiding this comment.
I think this needs another quick round, or at least a lot more clarity in the approach 😉
| - `egl`: use EGL context (default) | ||
| - `glx`: use a GLX context. Implies `x11` | ||
| - `x11`: build with X11 support |
There was a problem hiding this comment.
needs much more docs what this is and when to use which.
I am for example not sure if we should have an x11 feature or just an glx feature..
There was a problem hiding this comment.
I dropped the egl and x11 feature flags and kept only glx, by not covering the relatively niche EGL + X11 case for now. The docs are simplified accordingly.
| - `vulkan` (default on Linux/Windows): `cargo build --features vulkan` | ||
| - `opengl` (cross-platform): `cargo build --features opengl` | ||
| - `metal` (default on macOS/iOS): `cargo build --features metal` | ||
| - `opengl` (Linux/Windows): `cargo build --features opengl` |
There was a problem hiding this comment.
Since our dependencys no longer install anything for this, should it remain a feature?
We should remove dead features..
There was a problem hiding this comment.
It looks dead but it's needed as the platform selector.
I actually thought the same and considered opengl-egl / opengl-glx features, but stopped because we still need to select Windows + OpenGL.
| // TODO: Remove this mutex once the upstream fix is released and `MLN_REVISION` is | ||
| // bumped: https://github.com/maplibre/maplibre-native/pull/4332 | ||
| #if MLN_RENDER_BACKEND_OPENGL | ||
| inline std::mutex& headlessDisplayMutex() { | ||
| static std::mutex mutex; | ||
| return mutex; | ||
| } | ||
| #endif | ||
|
|
There was a problem hiding this comment.
If there is a PR out already, I would like to have the fix only in one place unless you have a reason to need this right now.
If that is the case, I am fine with merging.
Having a fix in one place is just a lot less work overall that we are 90% sure to forget 😉
There was a problem hiding this comment.
Agreed. But without it, concurrent renderers crash on both EGL and GLX, and maplibre/maplibre-native#4332 will probably take a while to be merged and released.
So I'm a bit unsure, but I think adding the mutex to MLN-rs too is not so harmful for now.
There was a problem hiding this comment.
will probably take a while to be merged and released
Sounds like a solvable issue. Core is not bound to any of the other release cycles ^^
CC @louwers
CommanderStorm
left a comment
There was a problem hiding this comment.
but before merging, lets see what bart has to say regarding the mutex. seems solvable
|
also, ci is red a bit |
|
I don't think my review above changed.. |
|
Thank you for reviewing! I'll wait for maplibre/maplibre-native#4332. |
# Conflicts: # .github/workflows/ci.yml # Cargo.toml # build.rs # examples/slint/Cargo.toml # justfile
|
I'd like to merge now to avoid further heavy conflicts. I'll file a tracking issue to drop the duplicate thread-safety, blocked on maplibre/maplibre-native#4332. |
Resolves #229.
Linux OpenGL rendering used GLX, which requires X11/Xvfb. This PR adds an EGL context so headless OpenGL works without an X server (beneficial for container environments), and makes EGL the default OpenGL context on Linux.
Feature changes
opengl: OpenGL backend. The Linux default is now EGL (surfaceless, no X server needed)glx: use the GLX (X11) context on Linux (the previous default)CI matrix
egl(no xvfb) andglx(xvfb).Blocker
AI assistance
Claude was used for investigating MLN and coding assistance.