From 8f7bc3cf4fb786e40eabc8d404d3ff0f29584c46 Mon Sep 17 00:00:00 2001 From: smillmorel Date: Wed, 16 Sep 2026 20:05:35 -0400 Subject: [PATCH] fix: keep the Plotly camera and axis ranges through data updates Plotly.react resets any scene attribute the incoming layout omits, so every selection change snapped the camera back to the default and re-fit the ranges - the pinned framing was meaningless. Data-only pushes now pass preserveView and html.py merges the live camera and axis ranges into the incoming layout; only an explicit re-frame sends computed framing. Also fixes the ordering bug where the framing flag was consumed before the not-ready early return, which dropped the very first framing on the floor (show_project runs before loadFinished). Verified in the browser: user orbit + zoom survive selection and working plane updates, while reset_camera re-frames. Regression-tested headlessly by asserting the emitted preserveView flag. --- AGENTS.md | Bin 6216 -> 6694 bytes src/otko/views/canvas_plotly/html.py | 20 +++- src/otko/views/canvas_plotly/plotly_canvas.py | 64 ++++++++---- tests/gui/test_plotly_view_preservation.py | 97 ++++++++++++++++++ 4 files changed, 159 insertions(+), 22 deletions(-) create mode 100644 tests/gui/test_plotly_view_preservation.py diff --git a/AGENTS.md b/AGENTS.md index bbe6d81ae9f93827dedba19041d40702ee26143b..9bf672ed364662b78c3069ba6e5aa0ff386b4a4e 100644 GIT binary patch delta 489 ZcmX?Mu*_tGheSQYz=Dx(e8JPhXeo(DFWjF diff --git a/src/otko/views/canvas_plotly/html.py b/src/otko/views/canvas_plotly/html.py index 278abba..e7bb27f 100644 --- a/src/otko/views/canvas_plotly/html.py +++ b/src/otko/views/canvas_plotly/html.py @@ -122,8 +122,26 @@ _PAGE = """ gd.on('plotly_unhover', function () { clearHover(); }); } - window.otkoUpdate = function (payloadJson) { + window.otkoUpdate = function (payloadJson, preserveView) { var fig = JSON.parse(payloadJson); + // `Plotly.react` resets any scene attribute the incoming layout omits, + // which would snap the camera and the padded ranges back on every data + // update. When this is not an explicit re-frame, carry the user's + // current view forward into the incoming layout. + if (preserveView) { + var gd = document.getElementById('plot'); + var scene = gd && gd._fullLayout ? gd._fullLayout.scene : null; + if (scene) { + fig.layout.scene = fig.layout.scene || {}; + fig.layout.scene.camera = scene.camera; + ['xaxis', 'yaxis', 'zaxis'].forEach(function (axis) { + if (!scene[axis]) return; + fig.layout.scene[axis] = fig.layout.scene[axis] || {}; + fig.layout.scene[axis].range = scene[axis].range; + fig.layout.scene[axis].autorange = false; + }); + } + } Plotly.react('plot', fig.data, fig.layout, config).then(function () { findHover(); installHandlers(); diff --git a/src/otko/views/canvas_plotly/plotly_canvas.py b/src/otko/views/canvas_plotly/plotly_canvas.py index 1e771d5..d5c0ff0 100644 --- a/src/otko/views/canvas_plotly/plotly_canvas.py +++ b/src/otko/views/canvas_plotly/plotly_canvas.py @@ -5,10 +5,13 @@ implements the same public surface as :class:`otko.views.canvas3d.ModelCanvas` (signals, selection, working plane, view presets, display toggles) so ``MainWindow`` can swap the two at runtime. -Update strategy: ``Plotly.react`` diffs client-side, and the layout only -carries ``scene.camera`` when a view preset or the projection toggle asks for -it — so re-rendering on a model edit or selection change never yanks the -camera the user is orbiting. +Update strategy: figures are pushed with ``Plotly.react``. Because react +resets any scene attribute the incoming layout omits, a non-framing push is +flagged ``preserveView`` and the JS side carries the live camera and axis +ranges forward — so re-rendering on a model edit or selection change never +yanks the camera the user is orbiting. Only an explicit re-frame (new +project, view preset, ``reset_camera``, style change) sends the computed +camera plus opstool-style padded axis ranges. """ from __future__ import annotations @@ -138,7 +141,7 @@ class PlotlyCanvas(QWidget): self._default_selection_enabled = True self._working_plane: tuple[str, float] | None = None self._camera = _CameraShim(self) - self._camera_dirty = True + self._framing_dirty = True self._renderer = _PlotlyRendererFacade(self) self._ready = False @@ -173,7 +176,7 @@ class PlotlyCanvas(QWidget): self.set_project(project) if project is not None and project.nodes: self._view_preset = "iso" - self._camera_dirty = True + self._framing_dirty = True self.render() def clear_model(self) -> None: @@ -206,34 +209,34 @@ class PlotlyCanvas(QWidget): def set_parallel_projection(self, on: bool) -> None: self._parallel = bool(on) - self._camera_dirty = True + self._framing_dirty = True def reset_camera(self) -> None: self._view_preset = "iso" - self._camera_dirty = True + self._framing_dirty = True self.render() def view_isometric(self) -> None: self._view_preset = "iso" - self._camera_dirty = True + self._framing_dirty = True self.render() def view_xy(self) -> None: """Top view: the eye sits on +Z looking down.""" self._view_preset = "xy" - self._camera_dirty = True + self._framing_dirty = True self.render() def view_xz(self) -> None: """Front view: the eye sits on -Y.""" self._view_preset = "xz" - self._camera_dirty = True + self._framing_dirty = True self.render() def view_yz(self) -> None: """Right view: the eye sits on +X.""" self._view_preset = "yz" - self._camera_dirty = True + self._framing_dirty = True self.render() # ── working plane ──────────────────────────────────────────────── @@ -272,9 +275,14 @@ class PlotlyCanvas(QWidget): self.render() def set_style(self, style: RenderStyle) -> None: - """Swap the visual style and repaint the figure.""" + """Swap the visual style and repaint the figure. + + Treated as a re-frame because the background and axis outline are + layout-level (``Plotly.restyle`` cannot carry them). + """ self._style = style self._builder.set_style(style) + self._framing_dirty = True self.render() def set_display_options(self, *, show_node_labels: bool, show_element_labels: bool) -> None: @@ -291,18 +299,32 @@ class PlotlyCanvas(QWidget): # ── internals ─────────────────────────────────────────────────── def _push_scene(self) -> None: + """Send the figure to plotly.js. + + ``Plotly.react`` resets any scene attribute the incoming layout omits, + which used to snap the camera *and* the padded ranges back on every + selection change. So a non-framing push marks itself + ``preserveView`` and the JS side carries the live camera/ranges + forward; only an explicit re-frame (new project, view preset, + ``reset_camera``, style change) sends the computed framing. + """ self._scene = self._builder.build(self._project, self._options) - layout = dict(self._scene.layout) - if self._camera_dirty: - layout["scene"] = { - **layout["scene"], - "camera": self._camera_dict(self._scene), - } - self._camera_dirty = False if not self._ready: + # Nothing to push yet; crucially this must come *before* the + # framing flag is consumed, or the framing scheduled before + # loadFinished would be dropped on the floor. return + layout = dict(self._scene.layout) + re_framed = self._framing_dirty + if re_framed: + scene_layout = dict(layout["scene"]) + scene_layout["camera"] = self._camera_dict(self._scene) + for axis, override in self._scene.axis_overrides().items(): + scene_layout[axis] = {**scene_layout.get(axis, {}), **override} + layout["scene"] = scene_layout + self._framing_dirty = False payload = json.dumps({"data": self._scene.data, "layout": layout}) - self._web.page().runJavaScript(f"window.otkoUpdate({json.dumps(payload)})") + self._eval(f"window.otkoUpdate({json.dumps(payload)}, {_js_bool(not re_framed)})") def _camera_dict(self, scene: Scene) -> dict[str, Any]: cx, cy, cz = scene.center diff --git a/tests/gui/test_plotly_view_preservation.py b/tests/gui/test_plotly_view_preservation.py new file mode 100644 index 0000000..c748163 --- /dev/null +++ b/tests/gui/test_plotly_view_preservation.py @@ -0,0 +1,97 @@ +"""Regression tests for the Plotly canvas push contract. + +``Plotly.react`` resets any scene attribute the incoming layout omits, so a +data-only push must be flagged ``preserveView`` (the JS side then carries the +live camera and axis ranges forward). Only explicit re-frames may send the +computed framing. These tests capture the JavaScript the canvas emits instead +of driving the browser, so they stay fast and deterministic. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +pytest.importorskip("PySide6") +pytest.importorskip("plotly") + +from otko.services import load_project + +EXAMPLES = Path(__file__).resolve().parents[2] / "examples" + + +def _canvas_with_captured_js(qtbot): # type: ignore[no-untyped-def] + from otko.views.canvas_plotly import PlotlyCanvas + + canvas = PlotlyCanvas() + qtbot.addWidget(canvas) + # Pretend the page finished loading, then capture instead of running JS. + canvas._ready = True + calls: list[str] = [] + canvas._eval = lambda js: calls.append(js) # type: ignore[method-assign] + return canvas, calls + + +def _parse_call(call: str) -> tuple[dict, bool]: + """Split a captured ``window.otkoUpdate(, )`` call.""" + body = call[len("window.otkoUpdate(") : -1] + literal, _sep, flag = body.rpartition(", ") + return json.loads(json.loads(literal)), flag == "true" + + +@pytest.mark.gui +def test_framing_push_is_not_marked_preserve_view(qtbot) -> None: # type: ignore[no-untyped-def] + canvas, calls = _canvas_with_captured_js(qtbot) + + canvas.show_project(load_project(EXAMPLES / "cantilever.osmodel")) + + assert calls, "show_project must push a figure" + payload, preserve = _parse_call(calls[-1]) + assert preserve is False, "initial push must be a re-frame" + # The framing push carries the camera and the padded ranges. + scene = payload["layout"]["scene"] + assert "camera" in scene + assert "range" in scene["xaxis"] + assert scene["xaxis"]["autorange"] is False + + +@pytest.mark.gui +def test_data_update_preserves_the_view(qtbot) -> None: # type: ignore[no-untyped-def] + canvas, calls = _canvas_with_captured_js(qtbot) + + canvas.show_project(load_project(EXAMPLES / "cantilever.osmodel")) + calls.clear() + + canvas.selection.select_node(1) # data-only update + + assert calls, "selection change must push" + payload, preserve = _parse_call(calls[-1]) + assert preserve is True, "data update must preserve the view" + assert "camera" not in payload["layout"]["scene"], "must not re-send the camera" + + +@pytest.mark.gui +def test_view_presets_and_reset_re_frame(qtbot) -> None: # type: ignore[no-untyped-def] + canvas, calls = _canvas_with_captured_js(qtbot) + + canvas.show_project(load_project(EXAMPLES / "cantilever.osmodel")) + + for action in (canvas.view_xy, canvas.view_xz, canvas.view_yz, canvas.reset_camera): + calls.clear() + action() + assert calls[-1].endswith(", false)"), f"{action.__name__} must re-frame" + + +@pytest.mark.gui +def test_style_change_re_frames_for_layout_only_attrs(qtbot) -> None: # type: ignore[no-untyped-def] + from otko.views.canvas3d.style import RenderStyle + + canvas, calls = _canvas_with_captured_js(qtbot) + canvas.show_project(load_project(EXAMPLES / "cantilever.osmodel")) + calls.clear() + + canvas.set_style(RenderStyle(show_axis_outline=True)) + + assert calls[-1].endswith(", false)"), "background/outline live in the layout"