silx.gui.plot: Added pygfx/WGPU rendering backend for 2D plotting#4665
silx.gui.plot: Added pygfx/WGPU rendering backend for 2D plotting#4665physwkim wants to merge 24 commits into
Conversation
| if os.environ.get("XDG_SESSION_TYPE", "") == "wayland": | ||
| raise RuntimeError( | ||
| "pygfx backend is not available: " | ||
| "Wayland sessions are not supported" |
There was a problem hiding this comment.
Just a quick comment, do you have some link to an issue to track this upstream?
Since wayland is used in recent versions of linux and qt6 is picking it if available, this is quite critical.
There was a problem hiding this comment.
Yes, it's being tracked upstream here:
pygfx/rendercanvas#36
pygfx/wgpu-py#688
pygfx/rendercanvas#66
There was a problem hiding this comment.
I verified that rendering works on native Wayland when using present_method="bitmap"
There was a problem hiding this comment.
Thanks for the references!
So with wayland, pygfx is missing some system pointer to bypass qt and access the window system.
PySide6 looks to have recently added QWaylandApplication.
It was requested in pyqt6 some time ago.
So hopefully this will move forward.
There was a problem hiding this comment.
Closing the loop: this is now implemented in the backend -- native Wayland sessions automatically use the bitmap present method, while X11/XWayland keep the faster screen present. The upstream issue (rendercanvas#36) still tracks native screen support on Wayland.
0c03f74 to
133331d
Compare
t20100
left a comment
There was a problem hiding this comment.
Thanks for your contribution and your patience! Sorry for the delay in reviewing this.
First round of general comments and partial review.
Do not hesitate to add your name and/or employer as copyright owner of the pygfx backend part! As long as it is under the MIT license, it is welcomed!
We will certainly need help to maintain this backend (new features, bug fixes,...), is it OK for you to help us over time?
_PlotFrameCore.py is mostly a copy of GLPlotFrame.py and BackendPygfx.py has a lot in common with BackendOpenGL.py which completely makes sense for a first integration.
We'll refactor this after merging this PR to avoid maintaining duplicated code. To ease this refactoring, it would be best to keep the duplicated code as close as possible to the one of the OpenGL backend so the parts to put in common can be clearly identified (e.g. _ensureAspectRatio).
| indices = numpy.linspace(0, 255, 255).astype(numpy.int32) | ||
| newLut[1:] = lut[indices] | ||
|
|
||
| lut[:] = newLut |
There was a problem hiding this comment.
lut is modifed in-place, this would be more readable if not. Also why compacting the LUT ratehr than extending it to add the NaN color?
There was a problem hiding this comment.
Reworked: scalar images are now colormapped on the CPU via Colormap.applyToData and uploaded as an RGBA texture (the same path as RGBA images), so the in-place LUT and the compacting are both gone. This reuses the shared, tested colormap code.
| snapshot = self._renderer.snapshot() | ||
|
|
||
| # snapshot is (H, W, 4) RGBA uint8 | ||
| from PIL import Image as PILImage |
There was a problem hiding this comment.
Best to avoid lazy import unless necessary.
I propose for now to do as in the OpenGL backend and use silx.gui.plot.backends.glutils.PlotImageFile.saveImageToFile so there is basic SVG support.
In another PR, we can consider using pillow (since it is already a dependency of fabio which silx depends on, so it is OK to use it).
There was a problem hiding this comment.
Done -- saveGraph now uses silx.gui.plot.backends.glutils.PlotImageFile.saveImageToFile, like the OpenGL backend (no more lazy PIL import). It also gives basic SVG export. Happy to consider pillow separately.
| y2Min = y2Center - 0.5 * dataH | ||
| y2Max = y2Center + 0.5 * dataH | ||
| else: | ||
| raise RuntimeError("Unsupported dimension to keep: %s" % keepDim) |
There was a problem hiding this comment.
why changing it compared to the implementation in BackendOpenGL?
There was a problem hiding this comment.
No good reason -- reverted to the shared helpers: _ensureAspectRatio now uses findDimToKeep / ensureAspectRatio from backends.utils, matching the OpenGL backend.
| import os | ||
| import sys | ||
|
|
||
| if sys.platform.startswith("linux"): | ||
| if not os.environ.get("DISPLAY", "") and not os.environ.get( | ||
| "WAYLAND_DISPLAY", "" | ||
| ): | ||
| raise RuntimeError( | ||
| "pygfx backend is not available: " | ||
| "neither DISPLAY nor WAYLAND_DISPLAY is set" | ||
| ) | ||
| # Both XWayland (xcb) and the native "wayland" Qt platform are | ||
| # supported: BackendPygfx uses the fast "screen" present on | ||
| # X11/XWayland and falls back to "bitmap" present on native | ||
| # Wayland, where "screen" would conflict with Qt's compositing. | ||
|
|
There was a problem hiding this comment.
There is no need checking DISPLAY, it should already be an issue before reaching this point.
Is checking WAYLAND_DISPLAY really needed since there is a fallback to bitmap?
There was a problem hiding this comment.
The guard now accepts WAYLAND_DISPLAY in addition to DISPLAY and no longer rejects native Wayland: BackendPygfx detects the native Wayland Qt platform and falls back from the screen to the bitmap present method there, while X11/XWayland keep screen. I kept a minimal linux display check for a clearer early error, but I'm happy to drop it entirely if you'd prefer.
There was a problem hiding this comment.
Why having this tests?
if sys.platform.startswith("linux"):
if not os.environ.get("DISPLAY", "") and not os.environ.get(
"WAYLAND_DISPLAY", ""
):
It should be the case, or I would just let the app fail, the other backends would not work as well if DISPLAY is not defined.
There was a problem hiding this comment.
Good call — dropped it entirely. The pygfx branch now looks like the other two.
The Wayland note that comment carried already lives in BackendPygfx.__init__, where the present method actually gets picked.
|
Thank you for the thorough review, and no worries about the delay! silx has been a huge help for us at PAL, so of course I'm happy to help maintain this backend over time and to contribute wherever I can be useful. On copyright: I've added Pohang Accelerator Laboratory as a copyright holder alongside ESRF on the pygfx backend, and credited you and @loichuder in the authors list. I've pushed a round of changes addressing the inline comments below: dropped the |
The pygfx backend guard refused to start whenever XDG_SESSION_TYPE was "wayland". That over-blocks: rendercanvas runs on Linux through X11 and, on a Wayland session, forces QT_QPA_PLATFORM=xcb to render via XWayland. XDG_SESSION_TYPE stays "wayland" in that case, so the guard rejected a configuration that actually works. Check the real Qt platform instead. Only the native "wayland" platform is unsupported; XWayland (xcb) is allowed, and the error now points users at QT_QPA_PLATFORM=xcb.
The pygfx backend defaults to "screen" present (direct GPU rendering). On native Wayland this shares Qt's wl_display connection and bypasses Qt's compositing, causing Wayland protocol errors (invalid wl_callback) and a fatal connection crash (cf. wgpu-py#688). BackendPygfx now detects a native Wayland platform at construction and falls back to the robust "bitmap" present (CPU readback) there, keeping the faster "screen" present on X11/XWayland. The PlotWidget guard no longer rejects the native "wayland" platform (and accepts WAYLAND_DISPLAY in addition to DISPLAY), so native Wayland sessions work out of the box.
Reviewer noted "wgpu" is too generic since other WGPU wrappers may exist; select the backend with the single "pygfx" id.
…kend The WGSL min/max reduction and histogram compute shaders and their _WgpuComputeHelper/_AsyncCompute helpers were never wired into the backend (no callers of _computeGpuDataStats/_computeGpuHistogram). Drop the dead code along with the now-unused wgpu and threading imports, as requested in review.
The image streaming update path is handled by the reusable-item pool in addImage() via _build(); updateData() had no callers. Drop it.
_ensureAspectRatio reimplemented the findDimToKeep/ensureAspectRatio math inline. Reuse the shared backends.utils helpers as BackendOpenGL does, so the duplicated code stays aligned for later factoring.
Qt can report an unreliable screen DPI. Mirror the handling in silx.gui._glutils.OpenGLWidget.getDotsPerInch: use physicalDotsPerInch and clamp values below 55 or above 1000 to a sane default (logged once).
Use silx.gui.plot.backends.glutils.PlotImageFile.saveImageToFile as the OpenGL backend does, instead of a lazy PIL import. This adds the basic SVG export that was previously raising NotImplementedError.
Replace the GPU colormap pipeline (scalar texture + LUT texture + clim/ gamma, plus the CPU normalization pre-processing it needed) with a CPU RGBA computed by silx.gui.colors.Colormap.applyToData, uploaded as an RGBA texture through the same path as RGBA images. This reuses the existing, tested colormap code and drops the duplicated machinery (_fastColormapRange, _colormapToLUT, _prepareScalarForGPU, _handleNaN, _getOrCreateCmapTexture, _initGPUColormap). It also fixes non-linear normalizations (log/sqrt/gamma) and NaN color for scalar images, which the LUT path only handled for linear normalization.
Add Pohang Accelerator Laboratory as copyright holder alongside ESRF (both dated 2026, when the backend was created), and credit the review participants T. Vincent and L. Huder in __authors__ of the pygfx backend.
The "~3x faster" figure for the "screen" present method is unsubstantiated; drop it from the docstring.
9e450c2 to
d8ee520
Compare
pygfx 0.16 assigns ndarray.shape in-place (pygfx.resources._texture and _buffer), which NumPy 2.5 deprecates. silx tests promote warnings to errors and importing pygfx happens in the session-scoped test_options fixture, so this warning cascaded into ~1270 setup errors across the whole suite on CI. Scope the ignore to the pygfx module so silx own shape-setting is still caught.
The Linux runner has no GPU, so pygfx wgpu device probe failed and all pygfx backend tests were skipped there (they already run on macOS via Metal). Install mesa-vulkan-drivers (lavapipe) and libgl1-mesa-dri to provide a software Vulkan/GL device, mirroring the existing software-GL setup used for the OpenGL backend. WITH_PYGFX_TEST already defaults to True, so no workflow change is needed.
The pygfx tests now run on the Linux runner (lavapipe provides a wgpu device) but the first render aborts the pytest subprocess with SIGABRT and no message. Enable PYTHONFAULTHANDLER (dump traceback on fatal signal), unbuffered output, and Rust backtrace/logs to capture the cause. To be reverted once the crash is understood.
The pygfx backend tests aborted the pytest process with SIGABRT on the Linux runner. Root cause: pytest-xvfb defaults to a 16-bit virtual display, on which lavapipe's Vulkan surface advertises an empty list of supported formats. The pygfx/wgpu "screen" present path then calls wgpuSurfaceConfigure with Bgra8UnormSrgb, which fails validation and triggers a fatal wgpu-native panic (uncatchable, aborts the process) at the first pygfx render. Set xvfb_colordepth to 24 (matching real desktops and the macOS runner) so BGRA8 is available and the swapchain configures cleanly. Revert the temporary diagnostics env vars added to surface the crash.
run_tests() runs pytest against the installed silx package, so the source pyproject.toml is not the pytest configfile and its xvfb_colordepth ini option was silently ignored (rootdir = site-packages/silx, no configfile), leaving the lavapipe SIGABRT in place. Pass the override via PYTEST_ADDOPTS in the Test step so it reaches the actual pytest run, and drop the ineffective pyproject.toml entry.
…pygfx SIGABRT Switching an existing PlotWidget from the "gl" backend to "pygfx" aborted the process (SIGABRT, exit 250) on Linux CI. With a Qt OpenGL context current on the thread, wgpu's GL/EGL backend panics with EGL_BAD_ACCESS while enumerating adapters / creating the surface -- an uncatchable Rust abort, so testSwitchBackend's try/except could not intercept it. macOS never caught this: testSwitchBackend is gated by use_opengl (WITH_GL_TEST=False there) and macOS wgpu uses Metal, not EGL. Restrict the wgpu instance to the "Primary" backends (Vulkan/Metal/DX12) that the pygfx backend targets, excluding the fragile GL/EGL path. This must run before the wgpu instance is created, so route both first-touch sites through one helper restrictWgpuToPrimaryBackends(): - BackendPygfx import (product path) - silx/test/utils.py pygfx availability probe (test path, which creates the wgpu instance early during test-options setup, before BackendPygfx is imported)
Windows CI runners have no Vulkan/DX12 software driver (unlike Linux's lavapipe or macOS's Metal), so wgpu adapter enumeration in the pygfx availability probe (silx.test.utils -> gfx...get_shared().device) hangs indefinitely instead of raising. The try/except around the probe cannot intercept a hang, so the Test step stalls until the job timeout. Add a per-OS with_pygfx_test matrix flag (true on ubuntu/macOS, false on Windows) wired to WITH_PYGFX_TEST, which short-circuits the pygfx gate in silx.test.utils before the hanging probe runs.
t20100
left a comment
There was a problem hiding this comment.
Thanks al lot for addressing the review!
I'll have a second close look at it.
silx has been a huge help for us at PAL
Good to hear! Happy if it can help!
so of course I'm happy to help maintain this backend over time and to contribute wherever I can be useful.
Excellent!
| import os | ||
| import sys | ||
|
|
||
| if sys.platform.startswith("linux"): | ||
| if not os.environ.get("DISPLAY", "") and not os.environ.get( | ||
| "WAYLAND_DISPLAY", "" | ||
| ): | ||
| raise RuntimeError( | ||
| "pygfx backend is not available: " | ||
| "neither DISPLAY nor WAYLAND_DISPLAY is set" | ||
| ) | ||
| # Both XWayland (xcb) and the native "wayland" Qt platform are | ||
| # supported: BackendPygfx uses the fast "screen" present on | ||
| # X11/XWayland and falls back to "bitmap" present on native | ||
| # Wayland, where "screen" would conflict with Qt's compositing. | ||
|
|
There was a problem hiding this comment.
Why having this tests?
if sys.platform.startswith("linux"):
if not os.environ.get("DISPLAY", "") and not os.environ.get(
"WAYLAND_DISPLAY", ""
):
It should be the case, or I would just let the app fail, the other backends would not work as well if DISPLAY is not defined.
…selection Other backends do not check for a display server, and a missing one already fails earlier. Backend selection for pygfx now matches matplotlib and gl. The Wayland present-method rationale the removed comment carried is already documented in BackendPygfx.__init__.
Split out of #4520 to provide the 2D plot backend only, as suggested in the
review (one feature per PR). #4520 stays open for the 2D/3D demo; the 3D backend
will follow in a separate PR.
This adds a pygfx/WGPU rendering backend for
silx.gui.plot, enablingnative Metal (macOS), Vulkan (Linux) and DirectX (Windows).
It is a drop-in alternative to the OpenGL backend, selected with
PlotWidget(backend="pygfx").Colormap(applyToData), so allnormalizations (linear, log, sqrt, gamma, arcsinh) and the NaN color reuse
the shared, tested code path
bitmappresent methodOptional dependency:
pip install silx[pygfx](pygfx >=0.16,<0.17 · rendercanvas >=2.6.1).
Updates addressing the first review round
pygfx(dropped thewgpualias)Colormap.applyToDatainstead of a GPU LUTpipeline (this also fixes log/sqrt/gamma and the NaN color for scalar images)
findDimToKeep/ensureAspectRatio)saveImageToFile, which adds basic SVG exportsilx.gui._glutils.OpenGLWidgetbitmappresent method,so the
DISPLAY/Wayland availability guards are no longer neededAI disclosure
This PR was prepared with Opus 4.8.