diff --git a/AGENTS.md b/AGENTS.md index bbe6d81..9bf672e 100644 Binary files a/AGENTS.md and b/AGENTS.md differ 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"