From 0611153555e80ee3ae913d35c00a1d4ce3f35a6b Mon Sep 17 00:00:00 2001 From: smillmorel Date: Wed, 16 Sep 2026 20:14:05 -0400 Subject: [PATCH] fix: stop the Plotly canvas wedging on rapid or overlapping updates Three defects in the view-preservation path could leave the plot frozen (no updates, no orbit, stale colours): 1. The merge injected raw _fullLayout objects - including undefined when a push landed before the previous react resolved - and plotly validates layouts, so one bad value made every later react fail permanently. currentView() now deep-copies and validates eye/center/up and each range, and mergeView() is wrapped so it can never block an update. 2. Updates were not serialized: overlapping Plotly.react calls on one graph div left it unresponsive. otkoUpdate now queues and coalesces (one react at a time, latest payload wins), and logs instead of failing silently. 3. A style change re-framed the camera, so re-colouring yanked a view the user had orbited. Colour/opacity ride the traces and background/outline ride the layout, so set_style is now a non-framing push. Verified in the browser: a user orbit survives colour changes, 30 rapid preview updates land on the final value and stay responsive, and deleting the live camera no longer wedges the plot. --- src/otko/views/canvas_plotly/html.py | 110 +++++++++++++----- src/otko/views/canvas_plotly/plotly_canvas.py | 10 +- tests/gui/test_plotly_view_preservation.py | 8 +- 3 files changed, 96 insertions(+), 32 deletions(-) diff --git a/src/otko/views/canvas_plotly/html.py b/src/otko/views/canvas_plotly/html.py index e7bb27f..2afa289 100644 --- a/src/otko/views/canvas_plotly/html.py +++ b/src/otko/views/canvas_plotly/html.py @@ -89,6 +89,90 @@ _PAGE = """ Plotly.restyle('plot', { x: [[]], y: [[]], z: [[]] }, [hoverIndex]); } + // --- view preservation ------------------------------------------------- + // `Plotly.react` resets any scene attribute the incoming layout omits, so a + // non-framing update carries the live camera/ranges forward. Only plain, + // validated copies are merged: the raw `_fullLayout` objects are owned by + // plotly (react mutates them in place), and injecting an undefined value + // would make every later react fail, wedging the plot. + function toPlain(value) { + try { return JSON.parse(JSON.stringify(value)); } catch (err) { return null; } + } + + function currentView() { + var gd = document.getElementById('plot'); + var scene = gd && gd._fullLayout ? gd._fullLayout.scene : null; + if (!scene) return null; + var view = { ranges: {} }; + var camera = toPlain(scene.camera); + if (camera && camera.eye && camera.center) { + view.camera = { eye: camera.eye, center: camera.center, up: camera.up }; + if (camera.projection) view.camera.projection = camera.projection; + } + ['xaxis', 'yaxis', 'zaxis'].forEach(function (axis) { + var source = scene[axis]; + if (!source || !Array.isArray(source.range) || source.range.length !== 2) return; + var low = Number(source.range[0]); + var high = Number(source.range[1]); + if (isFinite(low) && isFinite(high) && high > low) view.ranges[axis] = [low, high]; + }); + return view; + } + + function mergeView(fig, view) { + try { + if (!view) return; + fig.layout.scene = fig.layout.scene || {}; + if (view.camera) fig.layout.scene.camera = view.camera; + Object.keys(view.ranges).forEach(function (axis) { + fig.layout.scene[axis] = fig.layout.scene[axis] || {}; + fig.layout.scene[axis].range = view.ranges[axis]; + fig.layout.scene[axis].autorange = false; + }); + } catch (err) { + // Never let view preservation block the figure update. + if (window.console) console.warn('otko: view merge skipped', err); + } + } + + // --- update queue ------------------------------------------------------ + // One react at a time, latest payload wins. Overlapping reacts on the same + // graph div are what left the plot unresponsive before. + var pendingUpdate = null; + var reactBusy = false; + + function runUpdate() { + if (reactBusy || !pendingUpdate) return; + var job = pendingUpdate; + pendingUpdate = null; + reactBusy = true; + + var fig; + try { + fig = JSON.parse(job.payload); + } catch (err) { + reactBusy = false; + if (window.console) console.error('otko: bad payload', err); + return; + } + mergeView(fig, job.preserve ? currentView() : null); + + Plotly.react('plot', fig.data, fig.layout, config).then(function () { + findHover(); + installHandlers(); + }, function (err) { + if (window.console) console.error('otko: react failed', err); + }).then(function () { + reactBusy = false; + runUpdate(); + }); + } + + window.otkoUpdate = function (payloadJson, preserveView) { + pendingUpdate = { payload: payloadJson, preserve: !!preserveView }; + runUpdate(); + }; + function installHandlers() { if (handlersReady) return; var gd = document.getElementById('plot'); @@ -122,32 +206,6 @@ _PAGE = """ gd.on('plotly_unhover', function () { clearHover(); }); } - 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(); - }); - }; - window.otkoSetCamera = function (cameraJson) { Plotly.relayout('plot', { 'scene.camera': JSON.parse(cameraJson) }); }; diff --git a/src/otko/views/canvas_plotly/plotly_canvas.py b/src/otko/views/canvas_plotly/plotly_canvas.py index d5c0ff0..754a779 100644 --- a/src/otko/views/canvas_plotly/plotly_canvas.py +++ b/src/otko/views/canvas_plotly/plotly_canvas.py @@ -277,12 +277,13 @@ class PlotlyCanvas(QWidget): def set_style(self, style: RenderStyle) -> None: """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). + Deliberately *not* a re-frame: colour/opacity are trace props and the + background/outline ride the layout, so a style change (including the + live preview while dragging a colour) updates without snapping the + camera the user has moved. """ 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: @@ -306,7 +307,8 @@ class PlotlyCanvas(QWidget): 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. + ``reset_camera``) sends the computed framing. Style changes are + non-framing: they re-colour in place. """ self._scene = self._builder.build(self._project, self._options) if not self._ready: diff --git a/tests/gui/test_plotly_view_preservation.py b/tests/gui/test_plotly_view_preservation.py index c748163..3250779 100644 --- a/tests/gui/test_plotly_view_preservation.py +++ b/tests/gui/test_plotly_view_preservation.py @@ -85,7 +85,8 @@ def test_view_presets_and_reset_re_frame(qtbot) -> None: # type: ignore[no-unty @pytest.mark.gui -def test_style_change_re_frames_for_layout_only_attrs(qtbot) -> None: # type: ignore[no-untyped-def] +def test_style_change_keeps_the_view_but_updates_the_layout(qtbot) -> None: # type: ignore[no-untyped-def] + """Re-colouring must not snap the camera back to the preset.""" from otko.views.canvas3d.style import RenderStyle canvas, calls = _canvas_with_captured_js(qtbot) @@ -94,4 +95,7 @@ def test_style_change_re_frames_for_layout_only_attrs(qtbot) -> None: # type: i canvas.set_style(RenderStyle(show_axis_outline=True)) - assert calls[-1].endswith(", false)"), "background/outline live in the layout" + assert calls, "a style change must push" + payload, preserve = _parse_call(calls[-1]) + assert preserve is True, "colour changes must not re-frame the view" + assert payload["layout"]["scene"]["xaxis"]["showgrid"] is True # layout applied