From 1b0620a3921344c060158c48f863c8fbb9022792 Mon Sep 17 00:00:00 2001 From: smillmorel Date: Wed, 16 Sep 2026 20:52:40 -0400 Subject: [PATCH] perf: replace the Plotly snap marker with a projected DOM overlay Orbiting and the snap preview felt far behind the cursor. Measured in the page: a single Plotly.restyle on a gl3d plot costs about 90 ms even for a one-trace figure, so every marker update stalled the scene and the marker trailed the mouse; each one also queued another redraw while the user was dragging. The marker is now a pointer-events-none div positioned by projecting the snapped world point through glplot.cameraParams (validated: the camera centre lands at the canvas centre), updated with one style write and re-projected on plotly_relayout so it stays glued to the target during orbit. No plotly calls at all on the hover path - 200 mouse-move events now cost 0 restyles - and the empty hover trace is gone from the figure. Also logs the WebGL renderer once (via the console bridge) since hardware acceleration decides how smooth orbit feels and is otherwise invisible. --- AGENTS.md | 2 +- src/otko/views/canvas_plotly/html.py | 136 ++++++++++++------ src/otko/views/canvas_plotly/trace_builder.py | 26 ---- tests/gui/test_plotly_hover.py | 64 ++++++--- tests/unit/test_plotly_trace_builder.py | 14 +- 5 files changed, 147 insertions(+), 95 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index babac38..1eb5f24 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -19,7 +19,7 @@ Strict one-way MVVM + services: `views → viewmodels → services → core`. - `core/` (entities: `project.py`, `geometry/`, `materials/`, `sections/`, `loads/`, `analysis/`, `catalog/`): stdlib + numpy + pydantic only. **No Qt, no openseespy. Period.** - `services/` (`opensees_runner.py`, `persistence.py`, `results.py`, ...): may use core + h5py + openseespy. **No Qt.** - `views/`: PySide6/pyvistaqt only. **No direct `import openseespy`** — go through a service. -- `views/canvas3d/` (**PyVista/VTK**, default) and `views/canvas_plotly/` (plotly.js in a `QWebEngineView`) are two backends for the same central 3D view. Both satisfy the `CanvasBackend` protocol in `views/canvas_base.py`, share one `SelectionState` owned by `MainWindow`, and are swapped live via **Options → Canvas Backend** (persisted in `QSettings` under `canvas/backend`). `MainWindow._activate_canvas` keeps both widgets in a `QStackedWidget` — never destroy a canvas mid-session (VTK leaves dangling make-current callbacks). Backend-specific gaps are declared by `CanvasCapabilities` (e.g. Plotly has no force-diagram overlay or video export yet); gate UI on `canvas.capabilities`, never on the backend name. `canvas_plotly/trace_builder.py` is pure (no Qt, no pyvista) and unit-tested headless. `PlotlyCanvas` pushes with `Plotly.react`, which resets scene attributes the layout omits: data-only pushes pass `preserveView` so `html.py` carries the live camera and padded axis ranges forward, and the hover/snap marker is restyled only when its visible state changes (a restyle per hover event redraws per mouse move until the stack blows). Regression tests: `tests/gui/test_plotly_view_preservation.py`, `tests/gui/test_plotly_hover.py`. +- `views/canvas3d/` (**PyVista/VTK**, default) and `views/canvas_plotly/` (plotly.js in a `QWebEngineView`) are two backends for the same central 3D view. Both satisfy the `CanvasBackend` protocol in `views/canvas_base.py`, share one `SelectionState` owned by `MainWindow`, and are swapped live via **Options → Canvas Backend** (persisted in `QSettings` under `canvas/backend`). `MainWindow._activate_canvas` keeps both widgets in a `QStackedWidget` — never destroy a canvas mid-session (VTK leaves dangling make-current callbacks). Backend-specific gaps are declared by `CanvasCapabilities` (e.g. Plotly has no force-diagram overlay or video export yet); gate UI on `canvas.capabilities`, never on the backend name. `canvas_plotly/trace_builder.py` is pure (no Qt, no pyvista) and unit-tested headless. `PlotlyCanvas` pushes with `Plotly.react`, which resets scene attributes the layout omits: data-only pushes pass `preserveView` so `html.py` carries the live camera and padded axis ranges forward, and never re-send `scene.camera`. The snap-hover marker is a `pointer-events-none` DOM overlay positioned by projecting the target through `glplot.cameraParams` — **not** a trace, because a gl3d `Plotly.restyle` costs ~90 ms per call (measured, even for one trace), which made the marker lag behind the cursor and stall orbiting. Regression tests: `tests/gui/test_plotly_view_preservation.py`, `tests/gui/test_plotly_hover.py`. - `views/canvas3d/style.py` (`RenderStyle`) is the single source of truth for colours/sizes on **both** backends; renderers must read it rather than hard-coding a colour. The `STYLE_FIELDS` subset is user-editable via **Options → Plot Properties…** (`views/dialogs/plot_properties.py`), previews live through `MainWindow.set_plot_style(..., persist=False)`, and persists as JSON in `QSettings` under `plot/props`. - `viewmodels/` bridges core↔Qt (signals, `QUndoStack`); `commands/` holds `QUndoCommand` subclasses. - Rules: public functions need type hints + docstring; new domain entities go through Pydantic validation; ops >50 ms run off the GUI thread (`AnalysisWorker` in QThread, cancel via `isInterruptionRequested()`, results cross threads as lightweight `ResultsHandle` to HDF5). diff --git a/src/otko/views/canvas_plotly/html.py b/src/otko/views/canvas_plotly/html.py index 46e145e..12c9bc8 100644 --- a/src/otko/views/canvas_plotly/html.py +++ b/src/otko/views/canvas_plotly/html.py @@ -38,6 +38,12 @@ _PAGE = """ @@ -49,7 +55,6 @@ _PAGE = """ var bridge = null; var snapEnabled = false; var handlersReady = false; - var hoverIndex = -1; var config = { responsive: true, @@ -69,44 +74,73 @@ _PAGE = """ return data && data.meta ? data.meta.kind : null; } - function findHover() { - var gd = document.getElementById('plot'); - hoverIndex = -1; - if (!gd || !gd.data) return; - for (var i = 0; i < gd.data.length; i++) { - var meta = gd.data[i] && gd.data[i].meta; - if (meta && meta.kind === 'hover') { hoverIndex = i; break; } - } - // A freshly pushed figure has an empty hover marker again. - hoverShown = false; - hoverPoint = null; - } - // --- snap-target marker ------------------------------------------------ - // Updated synchronously but only when the visual state actually changes. - // Restyling on every hover/unhover made the plot redraw per mouse move, - // which re-fired hover and ended in a runaway redraw loop (RangeError: - // Maximum call stack size exceeded, frozen canvas). Timers were avoided - // deliberately: WebEngine throttles them to ~1 s when the page is not - // compositing, which made the preview lag. - var hoverShown = false; - var hoverPoint = null; + // A pointer-events-none DOM dot positioned by projecting the snapped world + // point through gl-plot3d's own camera matrices. It deliberately is *not* a + // plotly trace: Plotly.restyle on a gl3d plot costs ~90 ms even for a single + // trace, so a trace marker lagged far behind the cursor and stalled orbiting. + // A projection plus one style write is a few microseconds. + var snapPoint = null; + var snapMarker = null; - function samePoint(a, b) { - return !!a && !!b && a[0] === b[0] && a[1] === b[1] && a[2] === b[2]; + function snapMarkerElement() { + if (!snapMarker) { + snapMarker = document.createElement('div'); + snapMarker.id = 'otko-snap-marker'; + document.body.appendChild(snapMarker); + } + return snapMarker; } - function setHoverMarker(point) { - var want = snapEnabled ? point : null; - if (samePoint(want, hoverPoint) && !!want === hoverShown) return; - if (!want && !hoverShown) { hoverPoint = null; return; } - hoverPoint = want; - hoverShown = !!want; - if (hoverIndex < 0) return; - var x = want ? [want[0]] : []; - var y = want ? [want[1]] : []; - var z = want ? [want[2]] : []; - Plotly.restyle('plot', { x: [x], y: [y], z: [z] }, [hoverIndex]); + function mat4Multiply(a, b) { + var out = new Array(16); + for (var col = 0; col < 4; col++) { + for (var row = 0; row < 4; row++) { + var sum = 0; + for (var k = 0; k < 4; k++) { sum += a[k * 4 + row] * b[col * 4 + k]; } + out[col * 4 + row] = sum; + } + } + return out; + } + + function projectToClient(point) { + var gd = document.getElementById('plot'); + var scene = gd && gd._fullLayout ? gd._fullLayout.scene._scene : null; + var params = scene && scene.glplot ? scene.glplot.cameraParams : null; + if (!params) return null; + var matrix = mat4Multiply( + mat4Multiply(params.projection, params.view), params.model + ); + var x = point[0], y = point[1], z = point[2]; + var clipW = matrix[3] * x + matrix[7] * y + matrix[11] * z + matrix[15]; + if (!isFinite(clipW) || Math.abs(clipW) < 1e-9) return null; + var clipX = matrix[0] * x + matrix[4] * y + matrix[8] * z + matrix[12]; + var clipY = matrix[1] * x + matrix[5] * y + matrix[9] * z + matrix[13]; + var canvas = scene.glplot.canvas; + var ratio = scene.glplot.pixelRatio || 1; + var localX = ((clipX / clipW) * 0.5 + 0.5) * canvas.width / ratio; + var localY = (1 - ((clipY / clipW) * 0.5 + 0.5)) * canvas.height / ratio; + if (localX < 0 || localY < 0 || localX > canvas.clientWidth || localY > canvas.clientHeight) { + return null; // off-screen target + } + var rect = canvas.getBoundingClientRect(); + return [rect.left + localX, rect.top + localY]; + } + + function setSnapMarker(point) { + var marker = snapMarkerElement(); + snapPoint = snapEnabled ? point : null; + if (!snapPoint) { marker.style.display = 'none'; return; } + var position = projectToClient(snapPoint); + if (!position) { marker.style.display = 'none'; return; } + marker.style.left = position[0] + 'px'; + marker.style.top = position[1] + 'px'; + marker.style.display = 'block'; + } + + function refreshSnapMarker() { + if (snapPoint) setSnapMarker(snapPoint); } // --- view preservation ------------------------------------------------- @@ -178,8 +212,8 @@ _PAGE = """ mergeView(fig, job.preserve ? currentView() : null); Plotly.react('plot', fig.data, fig.layout, config).then(function () { - findHover(); installHandlers(); + logRendererOnce(); }, function (err) { if (window.console) console.error('otko: react failed', err); }).then(function () { @@ -188,6 +222,24 @@ _PAGE = """ }); } + // One-off diagnostic: whether WebGL is hardware-accelerated decides how + // smooth orbiting feels, and it is invisible from Python otherwise. + var loggedRenderer = false; + function logRendererOnce() { + if (loggedRenderer) return; + loggedRenderer = true; + try { + var canvas = document.createElement('canvas'); + var gl = canvas.getContext('webgl') || canvas.getContext('experimental-webgl'); + if (!gl) { if (window.console) console.warn('otko: WebGL unavailable'); return; } + var info = gl.getExtension('WEBGL_debug_renderer_info'); + var name = info ? gl.getParameter(info.UNMASKED_RENDERER_WEBGL) : gl.getParameter(gl.RENDERER); + if (window.console) console.warn('otko: WebGL renderer = ' + name); + } catch (err) { + if (window.console) console.warn('otko: WebGL probe failed: ' + err.message); + } + } + window.otkoUpdate = function (payloadJson, preserveView) { pendingUpdate = { payload: payloadJson, preserve: !!preserveView }; runUpdate(); @@ -218,16 +270,20 @@ _PAGE = """ var pts = ev.points || []; if (!pts.length) return; var p = pts[0]; - if (kindOf(p) !== 'snap') { setHoverMarker(null); return; } + if (kindOf(p) !== 'snap') { setSnapMarker(null); return; } var c = p.customdata; - if (c) setHoverMarker([c[0], c[1], c[2]]); + if (c) setSnapMarker([c[0], c[1], c[2]]); }); + // Orbit/zoom moves the world point on screen: re-project so the marker + // stays glued to the target without a plotly redraw. + gd.on('plotly_relayout', function () { refreshSnapMarker(); }); + gd.on('plotly_unhover', function () { // Nothing can be visible unless snapping is armed, so stay out of the // redraw path entirely for ordinary mouse movement. if (!snapEnabled) return; - setHoverMarker(null); + setSnapMarker(null); }); } @@ -237,7 +293,7 @@ _PAGE = """ window.otkoSetSnapEnabled = function (on) { snapEnabled = !!on; - if (!snapEnabled) setHoverMarker(null); + if (!snapEnabled) setSnapMarker(null); }; })(); diff --git a/src/otko/views/canvas_plotly/trace_builder.py b/src/otko/views/canvas_plotly/trace_builder.py index fa5fc94..ebe5e13 100644 --- a/src/otko/views/canvas_plotly/trace_builder.py +++ b/src/otko/views/canvas_plotly/trace_builder.py @@ -112,8 +112,6 @@ class Scene: layout: dict[str, Any] center: tuple[float, float, float] = (0.0, 0.0, 0.0) diagonal: float = 1.0 - #: Trace index (in ``data``) of the empty hover-snap marker, or -1. - hover_trace: int = -1 #: Padded ``(min, max)`` per axis, used to frame the camera deterministically. axis_bounds: dict[str, tuple[float, float]] = field(default_factory=dict) @@ -333,7 +331,6 @@ class PlotlyTraceBuilder: self._build_frames(project, data, opts, points, node_row) self._build_nodes(data, opts, points, node_ids) self._build_labels(project, data, opts, points, node_row) - hover_trace = self._build_hover_marker(data) candidates = [points] if len(points) else [] if grid_pts is not None: @@ -346,7 +343,6 @@ class PlotlyTraceBuilder: layout=self._layout(), center=(float(center[0]), float(center[1]), float(center[2])), diagonal=_diag_of_bounds(bounds), - hover_trace=hover_trace, axis_bounds=bounds, ) @@ -951,28 +947,6 @@ class PlotlyTraceBuilder: ) ) - @staticmethod - def _build_hover_marker(data: list[dict[str, Any]]) -> int: - data.append( - { - "type": "scatter3d", - "mode": "markers", - "x": [], - "y": [], - "z": [], - "marker": { - "color": "#ffd900", - "size": 13, - "line": {"color": "#8a6d00", "width": 1}, - }, - "hoverinfo": "skip", - "name": "snap-hover", - "showlegend": False, - "meta": {"kind": "hover"}, - } - ) - return len(data) - 1 - def _cone_trace( x: list[float], diff --git a/tests/gui/test_plotly_hover.py b/tests/gui/test_plotly_hover.py index 78d5160..3033886 100644 --- a/tests/gui/test_plotly_hover.py +++ b/tests/gui/test_plotly_hover.py @@ -1,10 +1,10 @@ -"""Regression tests for the Plotly hover/snap-marker contract. +"""Regression tests for the Plotly snap-hover marker. -``plotly_hover``/``plotly_unhover`` fire continuously while the mouse moves. -Touching the plot on each one made it redraw per mouse move, re-firing hover -until the stack blew (``RangeError: Maximum call stack size exceeded``) and the -canvas froze. The marker is therefore coalesced and only touched when its -shown/hidden state actually changes. +``Plotly.restyle`` on a gl3d plot costs ~90 ms even for a single trace (measured +in-page), so the marker used to lag far behind the cursor and stall orbiting. +It is now a pointer-events-none DOM overlay positioned by projecting the snapped +world point through gl-plot3d's own camera matrices, which costs microseconds +and issues no plotly calls at all. """ from __future__ import annotations @@ -35,10 +35,12 @@ _CHURN_PROBE = """(function(){ } finally { Plotly.restyle = original; } })()""" -_HOVER_MARKER_X = ( - "JSON.stringify((document.getElementById('plot').data" - ".find(function(d){return d.meta && d.meta.kind === 'hover';}) || {}).x)" -) +_MARKER_STATE = """(function(){ + var marker = document.getElementById('otko-snap-marker'); + if (!marker) return 'missing'; + return JSON.stringify({ display: marker.style.display, left: marker.style.left, + top: marker.style.top }); +})()""" _EMIT_SNAP_HOVER = """(function(){ var gd = document.getElementById('plot'); @@ -48,6 +50,8 @@ _EMIT_SNAP_HOVER = """(function(){ return true; })()""" +_EMIT_UNHOVER = "document.getElementById('plot').emit('plotly_unhover',{}), true" + def _open_canvas(qtbot, name: str): # type: ignore[no-untyped-def] from otko.views.canvas_plotly import PlotlyCanvas @@ -69,7 +73,7 @@ def _run_js(canvas, qtbot, script: str, timeout: int = 15000): # type: ignore[n @pytest.mark.gui def test_ordinary_mouse_movement_does_not_redraw(qtbot) -> None: # type: ignore[no-untyped-def] - """The unfixed version restyled once per hover/unhover event.""" + """The original handler restyled on every hover/unhover event.""" canvas = _open_canvas(qtbot, "space_frame_3d.osmodel") restyles = _run_js(canvas, qtbot, _CHURN_PROBE) @@ -78,16 +82,38 @@ def test_ordinary_mouse_movement_does_not_redraw(qtbot) -> None: # type: ignore @pytest.mark.gui -def test_snap_marker_shows_and_clears(qtbot) -> None: # type: ignore[no-untyped-def] +def test_snap_marker_shows_clears_and_tracks_the_camera(qtbot) -> None: # type: ignore[no-untyped-def] canvas = _open_canvas(qtbot, "basic_truss.osmodel") - assert _run_js(canvas, qtbot, _HOVER_MARKER_X) == "[]" + assert _json_state(_run_js(canvas, qtbot, _MARKER_STATE))["display"] == "none" + + # Nothing shows while snapping is off (the marker belongs to the draw tools). + _run_js(canvas, qtbot, _EMIT_SNAP_HOVER) + qtbot.wait(150) + assert _json_state(_run_js(canvas, qtbot, _MARKER_STATE))["display"] == "none" canvas.set_snap_preview_enabled(True) - qtbot.wait(100) + qtbot.wait(150) _run_js(canvas, qtbot, _EMIT_SNAP_HOVER) - qtbot.wait(250) - assert _run_js(canvas, qtbot, _HOVER_MARKER_X) != "[]", "snap target not shown" + qtbot.wait(200) + shown = _json_state(_run_js(canvas, qtbot, _MARKER_STATE)) + assert shown["display"] == "block", "snap target not shown" + assert float(shown["left"].rstrip("px")) > 0 + assert float(shown["top"].rstrip("px")) > 0 - _run_js(canvas, qtbot, "document.getElementById('plot').emit('plotly_unhover',{})") - qtbot.wait(250) - assert _run_js(canvas, qtbot, _HOVER_MARKER_X) == "[]", "snap target not cleared" + # Orbiting moves the target on screen without any plotly redraw. + _run_js(canvas, qtbot, "Plotly.relayout('plot',{'scene.camera.eye':{x:-9,y:6,z:4}})") + qtbot.wait(600) + moved = _json_state(_run_js(canvas, qtbot, _MARKER_STATE)) + assert moved["display"] == "block" + assert (moved["left"], moved["top"]) != (shown["left"], shown["top"]) + + _run_js(canvas, qtbot, _EMIT_UNHOVER) + qtbot.wait(200) + assert _json_state(_run_js(canvas, qtbot, _MARKER_STATE))["display"] == "none" + + +def _json_state(raw: object) -> dict: + import json + + assert isinstance(raw, str) and raw != "missing", f"marker element missing: {raw!r}" + return json.loads(raw) diff --git a/tests/unit/test_plotly_trace_builder.py b/tests/unit/test_plotly_trace_builder.py index 45b2b21..1d20e4a 100644 --- a/tests/unit/test_plotly_trace_builder.py +++ b/tests/unit/test_plotly_trace_builder.py @@ -26,10 +26,6 @@ def _traces(scene, name: str) -> list[dict]: # type: ignore[no-untyped-def] return [trace for trace in scene.data if trace.get("name") == name] -def _kinds(scene) -> list[str]: # type: ignore[no-untyped-def] - return [trace.get("meta", {}).get("kind", trace["type"]) for trace in scene.data] - - def test_builds_grid_nodes_and_frames() -> None: scene = PlotlyTraceBuilder().build(_load("basic_truss"), SceneOptions()) names = {trace.get("name") for trace in scene.data} @@ -37,8 +33,9 @@ def test_builds_grid_nodes_and_frames() -> None: assert "nodes" in names assert "elements" in names assert scene.diagonal > 0 - # The hover-snap marker is always present so JS can restyle it. - assert scene.data[scene.hover_trace]["meta"]["kind"] == "hover" + # The snap marker is a DOM overlay, not a trace (a gl3d restyle costs + # ~90 ms, which made a trace marker lag behind the cursor). + assert all(trace.get("meta", {}).get("kind") != "hover" for trace in scene.data) def test_nodes_carry_ids_as_customdata() -> None: @@ -187,15 +184,14 @@ def test_payload_is_json_serialisable() -> None: assert scene.layout["scene"]["aspectmode"] == "data" -def test_empty_project_yields_only_the_hover_marker() -> None: +def test_empty_project_yields_no_traces() -> None: scene = PlotlyTraceBuilder().build(None, SceneOptions()) assert scene.data == [] - assert scene.hover_trace == -1 from otko.core import Project empty = PlotlyTraceBuilder().build(Project(ndm=3, ndf=6), SceneOptions()) - assert _kinds(empty) == ["hover"] + assert empty.data == [] def test_frame_trace_meta_marks_elements_pickable() -> None: