From 5262d31e336cb26960c4890e281e03793ba210c7 Mon Sep 17 00:00:00 2001 From: smillmorel Date: Wed, 16 Sep 2026 20:33:30 -0400 Subject: [PATCH] fix: stop the Plotly hover handler redrawing on every mouse move plotly_hover/plotly_unhover fire continuously while the mouse moves, and the unhover handler restyled the snap marker unconditionally - even with snapping off and nothing visible. That meant a full Plotly.restyle plus redraw per mouse event, which re-fired hover until the page died with 'RangeError: Maximum call stack size exceeded' and the canvas froze (reported live after opening a model). The marker is now updated only when its visible state actually changes: no marker when snapping is off, and a restyle only when the snapped point differs from the one already shown. Deliberately synchronous - timers are throttled to about a second by WebEngine when the page is not compositing, which stalled the preview. Measured: 200 mouse-move events went from 200 restyles to 0; 200 hovers on one grid dot cost a single restyle. --- src/otko/views/canvas_plotly/html.py | 43 ++++++++++--- tests/gui/test_plotly_hover.py | 93 ++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+), 9 deletions(-) create mode 100644 tests/gui/test_plotly_hover.py diff --git a/src/otko/views/canvas_plotly/html.py b/src/otko/views/canvas_plotly/html.py index 2afa289..46e145e 100644 --- a/src/otko/views/canvas_plotly/html.py +++ b/src/otko/views/canvas_plotly/html.py @@ -77,16 +77,36 @@ _PAGE = """ 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; } - function setHover(x, y, z) { - if (hoverIndex < 0) return; - Plotly.restyle('plot', { x: [[x]], y: [[y]], z: [[z]] }, [hoverIndex]); + // --- 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; + + function samePoint(a, b) { + return !!a && !!b && a[0] === b[0] && a[1] === b[1] && a[2] === b[2]; } - function clearHover() { + 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; - Plotly.restyle('plot', { x: [[]], y: [[]], z: [[]] }, [hoverIndex]); + 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]); } // --- view preservation ------------------------------------------------- @@ -198,12 +218,17 @@ _PAGE = """ var pts = ev.points || []; if (!pts.length) return; var p = pts[0]; - if (kindOf(p) !== 'snap') { clearHover(); return; } + if (kindOf(p) !== 'snap') { setHoverMarker(null); return; } var c = p.customdata; - setHover(c[0], c[1], c[2]); + if (c) setHoverMarker([c[0], c[1], c[2]]); }); - gd.on('plotly_unhover', function () { clearHover(); }); + 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); + }); } window.otkoSetCamera = function (cameraJson) { @@ -212,7 +237,7 @@ _PAGE = """ window.otkoSetSnapEnabled = function (on) { snapEnabled = !!on; - if (!snapEnabled) clearHover(); + if (!snapEnabled) setHoverMarker(null); }; })(); diff --git a/tests/gui/test_plotly_hover.py b/tests/gui/test_plotly_hover.py new file mode 100644 index 0000000..78d5160 --- /dev/null +++ b/tests/gui/test_plotly_hover.py @@ -0,0 +1,93 @@ +"""Regression tests for the Plotly hover/snap-marker contract. + +``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. +""" + +from __future__ import annotations + +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" + +#: 200 mouse-move events over a *non-snap* point with snapping armed. +_CHURN_PROBE = """(function(){ + var gd = document.getElementById('plot'); + var n = 0; var original = Plotly.restyle; + Plotly.restyle = function () { n++; return original.apply(Plotly, arguments); }; + var point = { data: gd.data[0], customdata: [0, 0, 0] }; + try { + for (var i = 0; i < 200; i++) { + gd.emit('plotly_hover', { points: [point] }); + gd.emit('plotly_unhover', {}); + } + return n; + } finally { Plotly.restyle = original; } +})()""" + +_HOVER_MARKER_X = ( + "JSON.stringify((document.getElementById('plot').data" + ".find(function(d){return d.meta && d.meta.kind === 'hover';}) || {}).x)" +) + +_EMIT_SNAP_HOVER = """(function(){ + var gd = document.getElementById('plot'); + var snap = gd.data.filter(function(d){return d.meta && d.meta.kind === 'snap';})[0]; + var point = { data: snap, customdata: [snap.x[0], snap.y[0], snap.z[0]] }; + gd.emit('plotly_hover', { points: [point] }); + return true; +})()""" + + +def _open_canvas(qtbot, name: str): # type: ignore[no-untyped-def] + from otko.views.canvas_plotly import PlotlyCanvas + + canvas = PlotlyCanvas() + qtbot.addWidget(canvas) + canvas.show_project(load_project(EXAMPLES / name)) + qtbot.waitUntil(lambda: canvas._ready, timeout=30000) + qtbot.wait(1500) # let the first Plotly.react settle + return canvas + + +def _run_js(canvas, qtbot, script: str, timeout: int = 15000): # type: ignore[no-untyped-def] + box: dict[str, object] = {} + canvas._web.page().runJavaScript(script, lambda value: box.update(value=value)) + qtbot.waitUntil(lambda: "value" in box, timeout=timeout) + return box["value"] + + +@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.""" + canvas = _open_canvas(qtbot, "space_frame_3d.osmodel") + + restyles = _run_js(canvas, qtbot, _CHURN_PROBE) + + assert restyles == 0, f"mouse movement caused {restyles} plot restyles" + + +@pytest.mark.gui +def test_snap_marker_shows_and_clears(qtbot) -> None: # type: ignore[no-untyped-def] + canvas = _open_canvas(qtbot, "basic_truss.osmodel") + assert _run_js(canvas, qtbot, _HOVER_MARKER_X) == "[]" + + canvas.set_snap_preview_enabled(True) + qtbot.wait(100) + _run_js(canvas, qtbot, _EMIT_SNAP_HOVER) + qtbot.wait(250) + assert _run_js(canvas, qtbot, _HOVER_MARKER_X) != "[]", "snap target not shown" + + _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"