diff --git a/AGENTS.md b/AGENTS.md index 9bf672e..bbe6d81 100644 Binary files a/AGENTS.md and b/AGENTS.md differ diff --git a/src/otko/views/canvas3d/style.py b/src/otko/views/canvas3d/style.py index a55d43b..6051fd9 100644 --- a/src/otko/views/canvas3d/style.py +++ b/src/otko/views/canvas3d/style.py @@ -56,7 +56,6 @@ STYLE_FIELDS: tuple[tuple[str, str, str], ...] = ( ("extrusion_opacity", "Extrusion opacity", "float"), ("background_top", "Background (top)", "color"), ("background_bottom", "Background (bottom)", "color"), - ("show_axis_outline", "Axis outline (grid + ticks)", "bool"), ("label_font_size", "Label font size", "int"), ) @@ -97,11 +96,6 @@ class RenderStyle: extrusion_opacity: float = 0.22 label_font_size: int = 12 - #: Draw the plotly scene grid + tick marks around the model (the "outline" - #: of opstool's scene recipe). Off by default to keep the SAP2000-like - #: clean viewport; the coloured X/Y/Z axis lines always stay visible. - show_axis_outline: bool = False - #: Diverging scale for scalar response overlays (force diagrams today, #: nodal / element response plots later): blue → red, evenly spaced. #: This is opstool's ``default_cmap`` (RdYlBu reversed). diff --git a/src/otko/views/canvas_plotly/html.py b/src/otko/views/canvas_plotly/html.py index e7bb27f..278abba 100644 --- a/src/otko/views/canvas_plotly/html.py +++ b/src/otko/views/canvas_plotly/html.py @@ -122,26 +122,8 @@ _PAGE = """ gd.on('plotly_unhover', function () { clearHover(); }); } - window.otkoUpdate = function (payloadJson, preserveView) { + window.otkoUpdate = function (payloadJson) { 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 d5c0ff0..1e771d5 100644 --- a/src/otko/views/canvas_plotly/plotly_canvas.py +++ b/src/otko/views/canvas_plotly/plotly_canvas.py @@ -5,13 +5,10 @@ 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: 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. +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. """ from __future__ import annotations @@ -141,7 +138,7 @@ class PlotlyCanvas(QWidget): self._default_selection_enabled = True self._working_plane: tuple[str, float] | None = None self._camera = _CameraShim(self) - self._framing_dirty = True + self._camera_dirty = True self._renderer = _PlotlyRendererFacade(self) self._ready = False @@ -176,7 +173,7 @@ class PlotlyCanvas(QWidget): self.set_project(project) if project is not None and project.nodes: self._view_preset = "iso" - self._framing_dirty = True + self._camera_dirty = True self.render() def clear_model(self) -> None: @@ -209,34 +206,34 @@ class PlotlyCanvas(QWidget): def set_parallel_projection(self, on: bool) -> None: self._parallel = bool(on) - self._framing_dirty = True + self._camera_dirty = True def reset_camera(self) -> None: self._view_preset = "iso" - self._framing_dirty = True + self._camera_dirty = True self.render() def view_isometric(self) -> None: self._view_preset = "iso" - self._framing_dirty = True + self._camera_dirty = True self.render() def view_xy(self) -> None: """Top view: the eye sits on +Z looking down.""" self._view_preset = "xy" - self._framing_dirty = True + self._camera_dirty = True self.render() def view_xz(self) -> None: """Front view: the eye sits on -Y.""" self._view_preset = "xz" - self._framing_dirty = True + self._camera_dirty = True self.render() def view_yz(self) -> None: """Right view: the eye sits on +X.""" self._view_preset = "yz" - self._framing_dirty = True + self._camera_dirty = True self.render() # ── working plane ──────────────────────────────────────────────── @@ -275,14 +272,9 @@ class PlotlyCanvas(QWidget): self.render() 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). - """ + """Swap the visual style and repaint the figure.""" 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: @@ -299,32 +291,18 @@ 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) - 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 + if self._camera_dirty: + layout["scene"] = { + **layout["scene"], + "camera": self._camera_dict(self._scene), + } + self._camera_dirty = False + if not self._ready: + return payload = json.dumps({"data": self._scene.data, "layout": layout}) - self._eval(f"window.otkoUpdate({json.dumps(payload)}, {_js_bool(not re_framed)})") + self._web.page().runJavaScript(f"window.otkoUpdate({json.dumps(payload)})") def _camera_dict(self, scene: Scene) -> dict[str, Any]: cx, cy, cz = scene.center diff --git a/src/otko/views/canvas_plotly/trace_builder.py b/src/otko/views/canvas_plotly/trace_builder.py index fa5fc94..40ae710 100644 --- a/src/otko/views/canvas_plotly/trace_builder.py +++ b/src/otko/views/canvas_plotly/trace_builder.py @@ -114,25 +114,11 @@ class Scene: 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) def to_payload(self) -> dict[str, Any]: """Figure dict without the camera — camera is owned by the widget.""" return {"data": self.data, "layout": self.layout} - def axis_overrides(self) -> dict[str, Any]: - """Nested ``scene`` range pins for the framed data window. - - Only pushed together with the camera (view presets / new project), so - the user's own zoom and pan survive ordinary data updates. The padding - comes from opstool's scene recipe (``pad_ratio`` 0.15). - """ - return { - f"{name}axis": {"range": [low, high], "autorange": False} - for name, (low, high) in self.axis_bounds.items() - } - @dataclass class _Mesh: @@ -187,34 +173,6 @@ def _diag_of_points(pts: np.ndarray | None) -> float: return d if d > 0 else 1.0 -#: Padding fraction added around the model when framing the view. -#: From opstool's scene recipe (``pad_ratio=0.15`` for model views). -_PAD_RATIO = 0.15 - - -def _padded_axis_bounds(pts: np.ndarray | None) -> dict[str, tuple[float, float]]: - """Padded ``(min, max)`` per axis; degenerate axes get unit slack.""" - if pts is None or len(pts) == 0: - return {name: (-1.0, 1.0) for name in ("x", "y", "z")} - lower = np.asarray(pts, dtype=float).min(axis=0) - upper = np.asarray(pts, dtype=float).max(axis=0) - pad = (upper - lower) * _PAD_RATIO - bounds: dict[str, tuple[float, float]] = {} - for index, name in enumerate(("x", "y", "z")): - low = float(lower[index] - pad[index]) - high = float(upper[index] + pad[index]) - if high - low < 1e-9: # planar / single-point model - low, high = low - 1.0, high + 1.0 - bounds[name] = (low, high) - return bounds - - -def _diag_of_bounds(bounds: dict[str, tuple[float, float]]) -> float: - """Padded bounding-box diagonal — the framing distance yardstick.""" - diagonal = float(np.linalg.norm([high - low for low, high in bounds.values()])) - return diagonal if diagonal > 0 else 1.0 - - def _frame_basis(el: Any, x_local: np.ndarray) -> tuple[np.ndarray, np.ndarray]: """Local (y, z) basis — mirrors ``ModelRenderer._frame_basis``.""" x = x_local / float(np.linalg.norm(x_local)) @@ -340,31 +298,25 @@ class PlotlyTraceBuilder: candidates.append(grid_pts) all_pts = np.vstack(candidates) if candidates else np.empty((0, 3)) center = tuple(np.mean(all_pts, axis=0)) if len(all_pts) else (0.0, 0.0, 0.0) - bounds = _padded_axis_bounds(all_pts) return Scene( data=data, layout=self._layout(), center=(float(center[0]), float(center[1]), float(center[2])), - diagonal=_diag_of_bounds(bounds), + diagonal=_diag_of_points(all_pts), hover_trace=hover_trace, - axis_bounds=bounds, ) # ── layout ─────────────────────────────────────────────────────── def _layout(self) -> dict[str, Any]: style = self._style - outline = style.show_axis_outline def axis(color: str, title: str) -> dict[str, Any]: - # Coloured axis lines + titles stay visible as the orientation cue - # (plotly has no corner triad); the grid/ticks are the optional - # "outline" from opstool's scene recipe. return { "title": {"text": title, "font": {"color": color, "size": 12}}, - "showgrid": outline, + "showgrid": False, "showbackground": False, - "zeroline": outline, - "showticklabels": outline, + "zeroline": False, + "showticklabels": False, "showspikes": False, "visible": True, "linecolor": color, @@ -550,9 +502,7 @@ class PlotlyTraceBuilder: }, "customdata": list(node_ids), "meta": {"kind": "node"}, - # Engineering-notation hover (opstool's trace recipe) so a - # hover identifies the entity instead of showing the raw id. - "hovertemplate": "Node #%{customdata}", + "hoverinfo": "skip", "name": "nodes", "showlegend": False, } @@ -622,7 +572,7 @@ class PlotlyTraceBuilder: }, "customdata": customdata, "meta": {"kind": "element"}, - "hovertemplate": "Element #%{customdata}", + "hoverinfo": "skip", "name": "elements", "showlegend": False, } diff --git a/src/otko/views/dialogs/plot_properties.py b/src/otko/views/dialogs/plot_properties.py index ae249ef..5240546 100644 --- a/src/otko/views/dialogs/plot_properties.py +++ b/src/otko/views/dialogs/plot_properties.py @@ -11,7 +11,6 @@ from __future__ import annotations from PySide6.QtCore import Signal from PySide6.QtGui import QColor from PySide6.QtWidgets import ( - QCheckBox, QColorDialog, QDialog, QDialogButtonBox, @@ -76,7 +75,6 @@ class PlotPropertiesDialog(QDialog): self._base = style self._colors: dict[str, _ColorButton] = {} self._numbers: dict[str, QDoubleSpinBox | QSpinBox] = {} - self._checks: dict[str, QCheckBox] = {} layout = QVBoxLayout(self) form = QFormLayout() @@ -104,7 +102,6 @@ class PlotPropertiesDialog(QDialog): """ updates: dict[str, object] = {name: w.color() for name, w in self._colors.items()} updates.update({name: w.value() for name, w in self._numbers.items()}) - updates.update({name: w.isChecked() for name, w in self._checks.items()}) return self._base.with_updates(**updates) # ── internals ─────────────────────────────────────────────────── @@ -123,12 +120,6 @@ class PlotPropertiesDialog(QDialog): spin.valueChanged.connect(self._emit_changed) self._numbers[name] = spin return spin - if kind == "bool": - check = QCheckBox() - check.setChecked(bool(value)) - check.toggled.connect(self._emit_changed) - self._checks[name] = check - return check int_spin = QSpinBox() int_spin.setRange(6, 32) int_spin.setValue(int(value)) # type: ignore[call-overload] @@ -145,6 +136,4 @@ class PlotPropertiesDialog(QDialog): button.set_color(getattr(defaults, name)) for name, spin in self._numbers.items(): spin.setValue(getattr(defaults, name)) - for name, check in self._checks.items(): - check.setChecked(getattr(defaults, name)) self._emit_changed() diff --git a/tests/gui/test_plotly_view_preservation.py b/tests/gui/test_plotly_view_preservation.py deleted file mode 100644 index c748163..0000000 --- a/tests/gui/test_plotly_view_preservation.py +++ /dev/null @@ -1,97 +0,0 @@ -"""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" diff --git a/tests/unit/test_plotly_trace_builder.py b/tests/unit/test_plotly_trace_builder.py index 45b2b21..ae960a9 100644 --- a/tests/unit/test_plotly_trace_builder.py +++ b/tests/unit/test_plotly_trace_builder.py @@ -206,57 +206,3 @@ def test_frame_trace_meta_marks_elements_pickable() -> None: # None separators break the line into per-element segments. assert None in frames["x"] assert len(frames["customdata"]) == len(frames["x"]) - - -# ── scene recipe (opstool-derived framing) ─────────────────────────────── -def test_axis_bounds_are_padded_around_the_model() -> None: - project = _load("cantilever") - scene = PlotlyTraceBuilder().build(project, SceneOptions()) - points = np.array([node.coords for node in project.nodes], dtype=float) - for index, axis in enumerate(("x", "y", "z")): - low, high = scene.axis_bounds[axis] - assert low < points[:, index].min() or low <= points[:, index].min() - assert high > points[:, index].max() or high >= points[:, index].max() - assert scene.diagonal > 0 - - -def test_planar_model_gets_unit_slack_on_the_flat_axis() -> None: - project = _load("basic_truss") - scene = PlotlyTraceBuilder().build(project, SceneOptions()) - low, high = scene.axis_bounds["z"] - assert high - low > 0 # a degenerate axis must not collapse the view - - -def test_axis_overrides_pin_ranges_with_autorange_off() -> None: - scene = PlotlyTraceBuilder().build(_load("cantilever"), SceneOptions()) - overrides = scene.axis_overrides() - assert set(overrides) == {"xaxis", "yaxis", "zaxis"} - for axis, override in overrides.items(): - assert override["autorange"] is False - assert len(override["range"]) == 2 - assert override["range"] == list(scene.axis_bounds[axis[0]]) - - -def test_hover_templates_identify_entities() -> None: - scene = PlotlyTraceBuilder().build(_load("basic_truss"), SceneOptions()) - (nodes,) = _traces(scene, "nodes") - (frames,) = _traces(scene, "elements") - assert nodes["hovertemplate"] == "Node #%{customdata}" - assert frames["hovertemplate"] == "Element #%{customdata}" - # Hover must not fall back to the raw-id "skip" mode. - assert "hoverinfo" not in nodes - - -def test_axis_outline_flag_toggles_grid_and_ticks() -> None: - plain = RenderStyle() - outlined = RenderStyle(show_axis_outline=True) - project = _load("cantilever") - scene_plain = PlotlyTraceBuilder(plain).build(project, SceneOptions()) - scene_outlined = PlotlyTraceBuilder(outlined).build(project, SceneOptions()) - - off_axis = scene_plain.layout["scene"]["xaxis"] - on_axis = scene_outlined.layout["scene"]["xaxis"] - assert off_axis["showgrid"] is False and off_axis["showticklabels"] is False - assert on_axis["showgrid"] is True and on_axis["showticklabels"] is True - # The coloured axis lines stay visible either way (orientation cue). - assert off_axis["visible"] is True and on_axis["visible"] is True diff --git a/tests/unit/test_render_style.py b/tests/unit/test_render_style.py index c179eb7..32b027b 100644 --- a/tests/unit/test_render_style.py +++ b/tests/unit/test_render_style.py @@ -49,7 +49,7 @@ def test_editable_subset_matches_the_field_table() -> None: kinds = {name: kind for name, _label, kind in STYLE_FIELDS} for name in EDITABLE_FIELDS: assert hasattr(style, name), name - assert kinds[name] in {"color", "float", "int", "bool"} + assert kinds[name] in {"color", "float", "int"} def test_response_colorscale_is_an_evenly_spaced_mapping() -> None: