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: