diff --git a/GPU_ACCELERATION.md b/GPU_ACCELERATION.md index 83c78808..96373da3 100644 --- a/GPU_ACCELERATION.md +++ b/GPU_ACCELERATION.md @@ -89,17 +89,32 @@ renderer not being software. - **`bec_widgets/utils/gpu_acceleration.py`** (new) — caches an offscreen-context probe of the GL renderer, refuses software rasterisers, honours `BEC_WIDGETS_OPENGL=auto|1|0`, and provides `grab_widget()` for OpenGL-safe screenshots. -- **`plot_base.py`** — `PlotBase.USE_OPENGL` class flag (default `False`); the viewport is swapped - after construction because `GraphicsLayoutWidget.__init__` forwards no viewport argument to - `GraphicsView`. -- **`waveform.py`, `multi_waveform.py`** — `USE_OPENGL = True`. +- **`plot_base.py`** — `use_opengl` `SafeProperty(bool)`, default `True` via `PlotBase.USE_OPENGL`, + switchable at runtime and exposed over RPC. The viewport is swapped after construction because + `GraphicsLayoutWidget.__init__` forwards no viewport argument to `GraphicsView`. - **`bec_widget.py`** — the three `self.grab()` screenshot sites now call `grab_widget(self)`. -- **`tests/unit_tests/test_gpu_acceleration.py`** — 10 tests covering the renderer gate, the env - var, and that a capture over an OpenGL viewport is non-blank. +- **`tests/unit_tests/`** — 13 tests in `test_gpu_acceleration.py` plus 3 in + `test_plot_base_next_gen.py`, covering the renderer gate, the env var, non-blank captures over an + OpenGL viewport, and the runtime toggle. -Opt-in per widget rather than a global `pg.setConfigOption("useOpenGL", True)`, because a global -switch would put Image and Heatmap on a GL viewport for zero gain while still paying the blank-grab -and software-renderer costs. +Set per widget rather than via a global `pg.setConfigOption("useOpenGL", True)`, so a single plot can +be dropped back to raster without disturbing the rest of the application. + +## Runtime switching + +The viewport *can* be swapped after construction, but not naively. pyqtgraph parents each item's +`OpenGLState` to the **viewport widget**, so `useOpenGL()` deletes it on the C++ side while the item +keeps a stale Python reference. The next `paintGL` then raises +`RuntimeError: Signal source has been deleted` — and because `PlotCurveItem.paint` is wrapped in +`@debug.warnOnException`, the exception is swallowed: **the curve silently stops rendering instead of +crashing.** + +Measured over 4 toggle cycles: **29 swallowed GL paint exceptions** without a reset, **0** when the +stale state is released first. `set_view_opengl()` therefore disconnects `sigPlotChanged` and clears +`glstate` on every affected item before swapping, so the item rebuilds against the new context. + +`use_opengl` reflects the *live* viewport rather than the requested value — setting it `True` on a +software renderer leaves it `False`. ## Test status @@ -112,10 +127,14 @@ the failures are attributable to this change: - 7 × `test_plugin_creator.py::TestAddWidgetVariants` — environmental. The copier template task runs `pyside6-uic`, which exits 127 (not found) in the cloned `bec_312_pg-gpu` env. -Targeted runs: 10/10 new tests, 167 passed across waveform/multi-waveform/plot_base/lifecycle/ -scatter, 322 passed in a plot/image/heatmap/crosshair/roi/export sweep. An `AttributeError` -traceback logged during `test_waveform.py` is pre-existing — it also appears with -`BEC_WIDGETS_OPENGL=0`. +Targeted runs after adding the property: 16/16 new tests, 183 passed across +waveform/multi-waveform/plot_base/lifecycle/scatter, and 402 passed in a +plot/image/heatmap/crosshair/roi/export/rpc/client sweep (same single pre-existing failure). An +`AttributeError` traceback logged during `test_waveform.py` is also pre-existing — it appears with +`BEC_WIDGETS_OPENGL=0` too. + +`bw-generate-cli --target bec_widgets` was re-run; the only change to the generated +`bec_widgets/cli/client.py` is the new `use_opengl` accessor on the plot classes. ## Not addressed diff --git a/bec_widgets/cli/client.py b/bec_widgets/cli/client.py index caf73412..7708f24e 100644 --- a/bec_widgets/cli/client.py +++ b/bec_widgets/cli/client.py @@ -2104,6 +2104,28 @@ class Heatmap(RPCBase): ``DEFAULT_UPDATE_RATE`` (25 Hz unless overridden). """ + @property + @rpc_call + def use_opengl(self) -> "bool": + """ + Whether the plot currently renders through an OpenGL viewport. + + Reflects the live viewport rather than the requested value: setting this to + True is best effort, and stays False when no hardware-accelerated context is + available (see `bec_widgets.utils.gpu_acceleration.opengl_available`). + """ + + @use_opengl.setter + @rpc_call + def use_opengl(self) -> "bool": + """ + Whether the plot currently renders through an OpenGL viewport. + + Reflects the live viewport rather than the requested value: setting this to + True is best effort, and stays False when no hardware-accelerated context is + available (see `bec_widgets.utils.gpu_acceleration.opengl_available`). + """ + @rpc_timeout(None) @rpc_call def screenshot(self, file_name: "str | None" = None): @@ -2832,6 +2854,28 @@ class Image(RPCBase): ``DEFAULT_UPDATE_RATE`` (25 Hz unless overridden). """ + @property + @rpc_call + def use_opengl(self) -> "bool": + """ + Whether the plot currently renders through an OpenGL viewport. + + Reflects the live viewport rather than the requested value: setting this to + True is best effort, and stays False when no hardware-accelerated context is + available (see `bec_widgets.utils.gpu_acceleration.opengl_available`). + """ + + @use_opengl.setter + @rpc_call + def use_opengl(self) -> "bool": + """ + Whether the plot currently renders through an OpenGL viewport. + + Reflects the live viewport rather than the requested value: setting this to + True is best effort, and stays False when no hardware-accelerated context is + available (see `bec_widgets.utils.gpu_acceleration.opengl_available`). + """ + @rpc_timeout(None) @rpc_call def screenshot(self, file_name: "str | None" = None): @@ -4025,6 +4069,28 @@ class MotorMap(RPCBase): ``DEFAULT_UPDATE_RATE`` (25 Hz unless overridden). """ + @property + @rpc_call + def use_opengl(self) -> "bool": + """ + Whether the plot currently renders through an OpenGL viewport. + + Reflects the live viewport rather than the requested value: setting this to + True is best effort, and stays False when no hardware-accelerated context is + available (see `bec_widgets.utils.gpu_acceleration.opengl_available`). + """ + + @use_opengl.setter + @rpc_call + def use_opengl(self) -> "bool": + """ + Whether the plot currently renders through an OpenGL viewport. + + Reflects the live viewport rather than the requested value: setting this to + True is best effort, and stays False when no hardware-accelerated context is + available (see `bec_widgets.utils.gpu_acceleration.opengl_available`). + """ + @rpc_timeout(None) @rpc_call def screenshot(self, file_name: "str | None" = None): @@ -4517,6 +4583,28 @@ class MultiWaveform(RPCBase): ``DEFAULT_UPDATE_RATE`` (25 Hz unless overridden). """ + @property + @rpc_call + def use_opengl(self) -> "bool": + """ + Whether the plot currently renders through an OpenGL viewport. + + Reflects the live viewport rather than the requested value: setting this to + True is best effort, and stays False when no hardware-accelerated context is + available (see `bec_widgets.utils.gpu_acceleration.opengl_available`). + """ + + @use_opengl.setter + @rpc_call + def use_opengl(self) -> "bool": + """ + Whether the plot currently renders through an OpenGL viewport. + + Reflects the live viewport rather than the requested value: setting this to + True is best effort, and stays False when no hardware-accelerated context is + available (see `bec_widgets.utils.gpu_acceleration.opengl_available`). + """ + @rpc_timeout(None) @rpc_call def screenshot(self, file_name: "str | None" = None): @@ -5819,6 +5907,28 @@ class ScatterWaveform(RPCBase): ``DEFAULT_UPDATE_RATE`` (25 Hz unless overridden). """ + @property + @rpc_call + def use_opengl(self) -> "bool": + """ + Whether the plot currently renders through an OpenGL viewport. + + Reflects the live viewport rather than the requested value: setting this to + True is best effort, and stays False when no hardware-accelerated context is + available (see `bec_widgets.utils.gpu_acceleration.opengl_available`). + """ + + @use_opengl.setter + @rpc_call + def use_opengl(self) -> "bool": + """ + Whether the plot currently renders through an OpenGL viewport. + + Reflects the live viewport rather than the requested value: setting this to + True is best effort, and stays False when no hardware-accelerated context is + available (see `bec_widgets.utils.gpu_acceleration.opengl_available`). + """ + @rpc_timeout(None) @rpc_call def screenshot(self, file_name: "str | None" = None): @@ -6488,6 +6598,28 @@ class Waveform(RPCBase): ``DEFAULT_UPDATE_RATE`` (25 Hz unless overridden). """ + @property + @rpc_call + def use_opengl(self) -> "bool": + """ + Whether the plot currently renders through an OpenGL viewport. + + Reflects the live viewport rather than the requested value: setting this to + True is best effort, and stays False when no hardware-accelerated context is + available (see `bec_widgets.utils.gpu_acceleration.opengl_available`). + """ + + @use_opengl.setter + @rpc_call + def use_opengl(self) -> "bool": + """ + Whether the plot currently renders through an OpenGL viewport. + + Reflects the live viewport rather than the requested value: setting this to + True is best effort, and stays False when no hardware-accelerated context is + available (see `bec_widgets.utils.gpu_acceleration.opengl_available`). + """ + @rpc_timeout(None) @rpc_call def screenshot(self, file_name: "str | None" = None): diff --git a/bec_widgets/utils/gpu_acceleration.py b/bec_widgets/utils/gpu_acceleration.py index 30b671d0..460a457e 100644 --- a/bec_widgets/utils/gpu_acceleration.py +++ b/bec_widgets/utils/gpu_acceleration.py @@ -134,6 +134,53 @@ def opengl_available(requested: bool = True) -> bool: return True +def _release_gl_state(view: GraphicsView) -> None: + """Drop the cached ``OpenGLState`` of every item in ``view``'s scene. + + pyqtgraph creates that state once per item and parents it to the *viewport* + widget. Swapping the viewport deletes it on the C++ side while the item + keeps a stale Python reference, so the next ``paintGL`` raises + ``RuntimeError: Signal source has been deleted``. ``PlotCurveItem.paint`` + swallows it, which shows up as a silently missing curve rather than a + crash. Clearing the reference makes the item rebuild its state against the + new context. + """ + scene = view.scene() + if scene is None: + return + for item in scene.items(): + # PlotDataItem delegates drawing to a PlotCurveItem held in .curve, which + # is itself in the scene; check both so nothing is missed. + for target in {item, getattr(item, "curve", None)}: + if target is None or getattr(target, "glstate", None) is None: + continue + signal = getattr(target, "sigPlotChanged", None) + if signal is not None: + try: + signal.disconnect(target.glstate.verticesChanged) + except (RuntimeError, TypeError): + # not connected, or the C++ side is already gone + pass + target.glstate = None + + +def set_view_opengl(view: GraphicsView, enabled: bool) -> bool: + """Switch ``view`` between the OpenGL and raster viewport at runtime. + + Args: + view(GraphicsView): The view whose viewport should be swapped. + enabled(bool): Whether the OpenGL viewport is wanted. + + Returns: + bool: Whether the OpenGL viewport is active afterwards. + """ + if isinstance(view.viewport(), QOpenGLWidget) is enabled: + return enabled + _release_gl_state(view) + view.useOpenGL(enabled) + return isinstance(view.viewport(), QOpenGLWidget) + + def _accelerated_views(widget: QWidget) -> list[GraphicsView]: """GraphicsView descendants of ``widget`` that are on an OpenGL viewport.""" views = widget.findChildren(GraphicsView) diff --git a/bec_widgets/widgets/plots/multi_waveform/multi_waveform.py b/bec_widgets/widgets/plots/multi_waveform/multi_waveform.py index 7bd17252..d68443cc 100644 --- a/bec_widgets/widgets/plots/multi_waveform/multi_waveform.py +++ b/bec_widgets/widgets/plots/multi_waveform/multi_waveform.py @@ -70,8 +70,6 @@ class MultiWaveform(PlotBase): PLUGIN = True RPC = True ICON_NAME = "ssid_chart" - # Many simultaneous curves; the biggest beneficiary of the shader path. - USE_OPENGL = True USER_ACCESS = [ *PlotBase.USER_ACCESS, # MultiWaveform Specific RPC Access diff --git a/bec_widgets/widgets/plots/plot_base.py b/bec_widgets/widgets/plots/plot_base.py index bd3c4b2d..d5b1b33e 100644 --- a/bec_widgets/widgets/plots/plot_base.py +++ b/bec_widgets/widgets/plots/plot_base.py @@ -6,6 +6,7 @@ import numpy as np import pyqtgraph as pg from bec_lib import bec_logger from qtpy.QtCore import QPoint, QPointF, Qt, Signal +from qtpy.QtOpenGLWidgets import QOpenGLWidget from qtpy.QtWidgets import QHBoxLayout, QLabel, QMainWindow, QVBoxLayout, QWidget from bec_widgets.utils.bec_connector import ConnectionConfig @@ -14,7 +15,7 @@ from bec_widgets.utils.crosshair import Crosshair from bec_widgets.utils.entry_validator import EntryValidator from bec_widgets.utils.error_popups import SafeProperty, SafeSlot from bec_widgets.utils.fps_counter import FPSCounter -from bec_widgets.utils.gpu_acceleration import opengl_available +from bec_widgets.utils.gpu_acceleration import opengl_available, set_view_opengl from bec_widgets.utils.plot_indicator_items import BECArrowItem, BECTickItem from bec_widgets.utils.qt_data_subscription import QtDataSubscription from bec_widgets.utils.round_frame import RoundedFrame @@ -113,16 +114,17 @@ class PlotBase(BECWidget, QWidget): "minimal_crosshair_precision.setter", "update_rate", "update_rate.setter", + "use_opengl", + "use_opengl.setter", "screenshot", ] USER_ACCESS = [*BECWidget.USER_ACCESS, *BASE_USER_ACCESS] - # Whether this plot benefits from the OpenGL viewport. Only PlotCurveItem and - # PColorMeshItem have a shader path in pyqtgraph 0.14; image- and scatter-based - # plots gain nothing, so they stay on the raster viewport. Subclasses that draw - # curves set this to True. The final say belongs to `opengl_available`, which - # also honours the BEC_WIDGETS_OPENGL environment variable. - USE_OPENGL = False + # Default for the `use_opengl` property. Curve-heavy plots gain the most, but + # the OpenGL viewport is no slower for the others, so it is on by default and + # can be toggled per widget at runtime. `opengl_available` still has the final + # say: it declines on a software renderer and honours BEC_WIDGETS_OPENGL. + USE_OPENGL = True # Custom Signals property_changed = Signal(str, object) @@ -170,9 +172,7 @@ class PlotBase(BECWidget, QWidget): self.plot_widget = pg.GraphicsLayoutWidget(parent=self) # GraphicsLayoutWidget forwards no viewport argument to GraphicsView, so the # viewport is swapped after construction instead. - self._opengl_enabled = opengl_available(self.USE_OPENGL) - if self._opengl_enabled: - self.plot_widget.useOpenGL(True) + self.use_opengl = self.USE_OPENGL self.plot_widget.ci.setContentsMargins(0, 0, 0, 0) self.plot_item = pg.PlotItem(viewBox=BECViewBox(enableMenu=True)) self.plot_widget.addItem(self.plot_item) @@ -323,6 +323,31 @@ class PlotBase(BECWidget, QWidget): self.add_side_menus() self.side_panel.show() + @SafeProperty(bool, doc="Render the plot through an OpenGL viewport.") + def use_opengl(self) -> bool: + """ + Whether the plot currently renders through an OpenGL viewport. + + Reflects the live viewport rather than the requested value: setting this to + True is best effort, and stays False when no hardware-accelerated context is + available (see `bec_widgets.utils.gpu_acceleration.opengl_available`). + """ + return isinstance(self.plot_widget.viewport(), QOpenGLWidget) + + @use_opengl.setter + def use_opengl(self, value: bool) -> None: + """ + Switch the plot between the OpenGL and raster viewport. + + Args: + value(bool): Whether the OpenGL viewport is wanted. + """ + if value and not opengl_available(True): + if self.use_opengl: + set_view_opengl(self.plot_widget, False) + return + set_view_opengl(self.plot_widget, value) + @SafeProperty(bool, doc="Enable popups setting dialogs for the plot widget.") def enable_popups(self): """ diff --git a/bec_widgets/widgets/plots/waveform/waveform.py b/bec_widgets/widgets/plots/waveform/waveform.py index 009fea03..f87f8c77 100644 --- a/bec_widgets/widgets/plots/waveform/waveform.py +++ b/bec_widgets/widgets/plots/waveform/waveform.py @@ -136,8 +136,6 @@ class Waveform(PlotBase): PLUGIN = True RPC = True ICON_NAME = "show_chart" - # Curves are drawn by PlotCurveItem, which has a shader path in pyqtgraph 0.14. - USE_OPENGL = True USER_ACCESS = [ *PlotBase.USER_ACCESS, "_config_dict", diff --git a/tests/unit_tests/test_gpu_acceleration.py b/tests/unit_tests/test_gpu_acceleration.py index de96c5bc..fbc3a429 100644 --- a/tests/unit_tests/test_gpu_acceleration.py +++ b/tests/unit_tests/test_gpu_acceleration.py @@ -5,7 +5,12 @@ from qtpy.QtOpenGLWidgets import QOpenGLWidget from qtpy.QtWidgets import QLabel, QVBoxLayout, QWidget from bec_widgets.utils import gpu_acceleration -from bec_widgets.utils.gpu_acceleration import ENV_VAR, grab_widget, opengl_available +from bec_widgets.utils.gpu_acceleration import ( + ENV_VAR, + grab_widget, + opengl_available, + set_view_opengl, +) @pytest.fixture(autouse=True) @@ -65,6 +70,62 @@ def test_unrecognised_env_var_falls_back_to_auto(monkeypatch): assert opengl_available(requested=True) is True +def _curve_view(qtbot, use_opengl: bool): + pg.setConfigOption("useOpenGL", use_opengl) + view = pg.GraphicsLayoutWidget() + plot = view.addPlot() + x = np.arange(5_000, dtype=np.float64) + plot.addItem(pg.PlotDataItem(x, np.sin(x * 0.01), pen=pg.mkPen("r", width=2))) + view.resize(400, 300) + qtbot.addWidget(view) + view.show() + qtbot.waitExposed(view) + return view + + +def test_set_view_opengl_toggles_viewport(qtbot): + view = _curve_view(qtbot, use_opengl=False) + if not gpu_acceleration.opengl_available(True): + pytest.skip("no hardware OpenGL available in this environment") + + assert set_view_opengl(view, True) is True + assert isinstance(view.viewport(), QOpenGLWidget) + assert set_view_opengl(view, False) is False + assert not isinstance(view.viewport(), QOpenGLWidget) + + +def test_set_view_opengl_is_idempotent(qtbot): + view = _curve_view(qtbot, use_opengl=False) + viewport = view.viewport() + assert set_view_opengl(view, False) is False + # no needless swap: the same viewport object is kept + assert view.viewport() is viewport + + +def test_toggling_back_to_opengl_does_not_strand_gl_state(qtbot): + """Swapping the viewport deletes the item's OpenGLState on the C++ side. + + Without clearing the stale reference, the next paintGL raises + 'Signal source has been deleted'. PlotCurveItem.paint swallows that, so the + curve silently stops rendering instead of crashing. + """ + view = _curve_view(qtbot, use_opengl=True) + if not isinstance(view.viewport(), QOpenGLWidget): + pytest.skip("no OpenGL viewport available in this environment") + + curve = next(i for i in view.scene().items() if isinstance(i, pg.PlotCurveItem)) + view.viewport().repaint() + assert curve.glstate is not None, "expected the GL path to have been taken" + + set_view_opengl(view, False) + assert curve.glstate is None, "stale OpenGLState was not released on swap" + + set_view_opengl(view, True) + view.viewport().repaint() + # rebuilt against the new context rather than reusing the deleted object + assert curve.glstate is not None + + def _non_background_fraction(pixmap) -> float: """Fraction of pixels differing from the most common colour.""" image = pixmap.toImage() diff --git a/tests/unit_tests/test_plot_base_next_gen.py b/tests/unit_tests/test_plot_base_next_gen.py index 343cd141..06eb2313 100644 --- a/tests/unit_tests/test_plot_base_next_gen.py +++ b/tests/unit_tests/test_plot_base_next_gen.py @@ -1,5 +1,9 @@ import numpy as np +import pytest +from qtpy.QtOpenGLWidgets import QOpenGLWidget +import bec_widgets.widgets.plots.plot_base as plot_base_module +from bec_widgets.utils.gpu_acceleration import opengl_available from bec_widgets.widgets.plots.plot_base import PlotBase, UIMode from .client_mocks import mocked_client @@ -588,3 +592,35 @@ def test_update_rate_widget_defaults(): assert Waveform.DEFAULT_UPDATE_RATE == 15.0 assert Heatmap.DEFAULT_UPDATE_RATE == 5.0 assert Image.DEFAULT_UPDATE_RATE == 25.0 + + +def test_use_opengl_defaults_to_true_when_available(qtbot, mocked_client): + """PlotBase opts into the OpenGL viewport by default.""" + pb = create_widget(qtbot, PlotBase, client=mocked_client) + assert pb.USE_OPENGL is True + assert pb.use_opengl is opengl_available(True) + + +def test_use_opengl_can_be_toggled_at_runtime(qtbot, mocked_client): + """The viewport can be swapped after the widget is built.""" + pb = create_widget(qtbot, PlotBase, client=mocked_client) + if not opengl_available(True): + pytest.skip("no hardware OpenGL available in this environment") + + pb.use_opengl = False + assert pb.use_opengl is False + assert not isinstance(pb.plot_widget.viewport(), QOpenGLWidget) + + pb.use_opengl = True + assert pb.use_opengl is True + assert isinstance(pb.plot_widget.viewport(), QOpenGLWidget) + + +def test_use_opengl_declines_without_hardware_context(qtbot, mocked_client, monkeypatch): + """Requesting OpenGL on a software renderer leaves the raster viewport in place.""" + pb = create_widget(qtbot, PlotBase, client=mocked_client) + monkeypatch.setattr(plot_base_module, "opengl_available", lambda requested=True: False) + + pb.use_opengl = True + assert pb.use_opengl is False + assert not isinstance(pb.plot_widget.viewport(), QOpenGLWidget)