diff --git a/apps/studio/src/orisys_studio/app_window.py b/apps/studio/src/orisys_studio/app_window.py index f701223..d126936 100644 --- a/apps/studio/src/orisys_studio/app_window.py +++ b/apps/studio/src/orisys_studio/app_window.py @@ -1204,6 +1204,11 @@ class StudioMainWindow(QMainWindow): # that hangs as well, so it is offered for as long as a session exists at # all — including the moment the worker is still winding down. self.toolbar.set_stop_enabled(self._session_active) + # The tracker reset belongs to a *device session*, so the session state + # decides it and nothing else: not the panel on screen (a view switch used + # to leave a running session's reset unavailable and never restore it) and + # not the transport's checked state (a paused session still owns the device). + self.toolbar.set_reset_enabled(self.session.reset_available) if not self._running: self.toolbar.refresh_labels() self._sync_shortcut_actions() @@ -1516,6 +1521,21 @@ class StudioMainWindow(QMainWindow): def _draw_style(self) -> str: return self.display_ctl.draw_style() + def _arrow_policy(self): + """The one flow-arrow policy the 2D canvas and the 3D surface both sample with. + + The thresholds and the sample grid are display settings (the pane's), so the + policy is built from them here and handed to the 2D renderer; the 3D composer + builds the same value from the same settings object + (``scene3d.composer`` via ``visual_policy.policy_for_settings``). That is what + makes the two figures trigger at the same strength: previously each path built + its own policy *and* the 3D one applied a hysteresis the canvas did not, so the + same frame could carry arrows in one figure and none in the other. + """ + from orisys_app_shared.scene3d.visual_policy import policy_for_settings + + return policy_for_settings(self.mesh3d_panel.display_settings()) + def _on_rotate_arrows(self) -> None: self.display_ctl.on_rotate_arrows() @@ -2087,8 +2107,9 @@ class StudioMainWindow(QMainWindow): self.cal_panel.set_running(True) self.toolbar.set_source_controls_enabled(False) self.sensor_picker.set_picker_enabled(False) - self.toolbar.set_reset_enabled(False) - # Keep the stop affordance consistent with the phase we just entered. + # Keep the stop affordance (and the reset, which this connecting session does + # not own yet) consistent with the phase we just entered: one rule, stated in + # ``_update_run_enabled``. self._update_run_enabled() self._refresh_status_rail() self._refresh_record_state() @@ -2127,9 +2148,9 @@ class StudioMainWindow(QMainWindow): # acquisition thread). self._display_paused = False self._sync_pause_action() - self.toolbar.set_reset_enabled(False) self.toolbar.set_source_controls_enabled(True) self.sensor_picker.set_picker_enabled(True) + # The session is over: the reset goes with it, through the one rule that owns it. self._update_run_enabled() keep_frame = reason in ("ended", "disconnected") if not keep_frame: @@ -2331,8 +2352,10 @@ class StudioMainWindow(QMainWindow): is_new = packet_key != self._last_plot_packet_key if is_new and self.session.note_first_frame(): - self.toolbar.set_reset_enabled(True) - self._sync_shortcut_actions() + # The phase just became ``running``, which is the session state the rail + # reads (the reset keys off it): ask the one owner of that rule rather + # than stating the enablement a second time here. + self._update_run_enabled() # No status-bar line here: the rail's phase chip says 采集中 and the # source slot names the camera, so "运行中(0)" was a second copy of # the same fact that then stayed on screen for the rest of the session. @@ -2561,6 +2584,7 @@ class StudioMainWindow(QMainWindow): depth_range=self._depth_range(), canvas_bgr=hex_to_bgr(color(self._theme, "arrow_canvas", "#0B1830")), domain_mask=boundary, + arrow_policy=self._arrow_policy(), ) if rendered is None: # Never substitute another picture here: a missing depth map that showed diff --git a/apps/studio/src/orisys_studio/scene3d/gl_backend.py b/apps/studio/src/orisys_studio/scene3d/gl_backend.py index d367142..ed4bfae 100644 --- a/apps/studio/src/orisys_studio/scene3d/gl_backend.py +++ b/apps/studio/src/orisys_studio/scene3d/gl_backend.py @@ -263,12 +263,21 @@ def shader_source(profile: Profile, program: str) -> tuple[str, str]: return header + vertex, header + body -def _valid_scalar_range(scalar, valid) -> tuple[float, float] | None: +def _valid_scalar_range( + scalar, valid, *, signed: bool = False +) -> tuple[float, float] | None: """Robust P5/P95 range of *valid* samples, or ``None`` when there are none. Using min/max made empty-field noise paint the whole LUT. A percentile span - with a minimum width keeps weak signals visible without amplifying floor - noise into a full contact. + keeps weak signals visible without amplifying floor noise into a full contact. + + ``signed`` is the *field's* own statement (``FieldDescriptor.signed``), and it is + what anchors the range at the field's zero rather than at the data's current + spread — see :func:`_scale_bounds`. The anchoring is the reason the surface's base + colour is a property of the field rather than of the frame: without it a flat + field was centred on its own midpoint (the middle of the ramp) and the first + press moved it to the first colour of the ramp, re-colouring the resting surface + and leaving it changed after the release. """ import numpy as np @@ -286,13 +295,19 @@ def _valid_scalar_range(scalar, valid) -> tuple[float, float] | None: lower = float(np.percentile(usable, 5.0)) upper = float(np.percentile(usable, 95.0)) # Minimum span: avoid collapsing nearly-constant fields into one colour. - span = upper - lower min_span = max(abs(lower) * 0.05, abs(upper) * 0.05, 1e-3) - if span < min_span: - mid = 0.5 * (lower + upper) - lower = mid - 0.5 * min_span - upper = mid + 0.5 * min_span - return lower, upper + if signed: + # A signed quantity is drawn with the diverging ramp, whose midpoint is the + # field's zero: the range stays symmetric about zero so that zero keeps the + # ramp's own middle colour however strong the response on screen is. + extent = max(abs(lower), abs(upper), 0.5 * min_span) + return -extent, extent + # An unsigned quantity is drawn with the sequential ramp, whose first colour is + # the field's zero ("no response"). Anchoring the lower bound there — rather than + # at this frame's P5 — is what makes the base colour constant across a press; the + # upper bound still follows the measurement. + low = min(0.0, lower) + return low, max(upper, low + min_span) def _blend_scale_range( @@ -321,6 +336,12 @@ def _scale_bounds(scale, automatic: tuple[float, float] | None) -> tuple[float, colour. An automatic range with no valid samples falls back to a fixed 0..1 so the shader always receives a usable interval — never NaN, and never a division by a span of zero. + + The automatic range is anchored at the field's *zero* by the descriptor's own + signedness (see :func:`_valid_scalar_range`): the ramp's first colour is "no + response" for an unsigned field and the diverging ramp's middle is zero for a + signed one. So the surface's base colour is decided by which quantity is drawn — + a property of the field — and not by how strong the current frame's response is. """ if scale is None: fallback = automatic or (0.0, 1.0) @@ -768,7 +789,9 @@ class GLRenderer: GL.glBindBuffer(GL.GL_ARRAY_BUFFER, 0) self._upload_field_textures(field) self._last_upload_ms = (time.perf_counter() - started) * 1000.0 - measured = _valid_scalar_range(field.scalar, field.valid) + measured = _valid_scalar_range( + field.scalar, field.valid, signed=bool(field.descriptor.signed) + ) self._scalar_range = _blend_scale_range(self._scalar_range, measured) self._upload_overlays(frame) diff --git a/apps/studio/src/orisys_studio/scene3d/view.py b/apps/studio/src/orisys_studio/scene3d/view.py index 4971915..ba759d2 100644 --- a/apps/studio/src/orisys_studio/scene3d/view.py +++ b/apps/studio/src/orisys_studio/scene3d/view.py @@ -813,9 +813,12 @@ class Scene3DView(QWidget): Skipped when it is the same descriptor as last time: this runs once per displayed frame, and the label and the settings only change when the claim does. When it does change, the display settings are re-issued so the colour - scale's descriptor — hence the choice between a sequential and a diverging - ramp — follows the live field rather than the one that was on screen when the - operator last touched a control. + scale describes the field actually on screen rather than the one that was + there when the operator last touched a control: the descriptor decides which + ramp is used and where the automatic range is anchored (the field's zero for an + unsigned quantity, zero at the ramp's middle for a signed one), which is what + keeps the surface's base colour a property of the *field* and not of the + frame's current response. """ if descriptor == self._last_descriptor: return diff --git a/apps/studio/src/orisys_studio/session_controller.py b/apps/studio/src/orisys_studio/session_controller.py index a357a11..bc51551 100644 --- a/apps/studio/src/orisys_studio/session_controller.py +++ b/apps/studio/src/orisys_studio/session_controller.py @@ -171,6 +171,19 @@ class SessionController(QObject): worker = self._active_worker return worker is not None and bool(getattr(worker, "is_running", False)) + @property + def reset_available(self) -> bool: + """Whether the tracker reset has a device session to act on. + + Zeroing is a **device** action, so it is offered exactly while a session has + opened the device and is producing frames — the phase ``running``, the same + moment the rail stops saying 连接中. It is deliberately not derived from the + transport's checked state (a paused session still owns its device, so it can + still be zeroed) nor from the view on screen, which is not a fact about the + session at all. + """ + return self._phase == "running" + def set_phase(self, phase: str) -> str: """Track the acquisition phase. An unknown phase is ``idle``, never a new one.""" wanted = str(phase) if str(phase) in SESSION_PHASES else "idle" diff --git a/apps/studio/src/orisys_studio/ui/toolbar.py b/apps/studio/src/orisys_studio/ui/toolbar.py index 74b7e56..3379dad 100644 --- a/apps/studio/src/orisys_studio/ui/toolbar.py +++ b/apps/studio/src/orisys_studio/ui/toolbar.py @@ -468,13 +468,18 @@ class ToolbarWidget(QWidget): The window calls this to *state* what the session is doing (a replay began, a session ended, a recompute pass stopped). An operator's own click goes through the button instead, and is routed by ``_on_run_toggled``. + + Reset is left alone here for the same reason ``_on_run_toggled`` leaves it + alone: unchecked means "paused" as often as it means "stopped", and a paused + session still owns its device. Whether the tracker can be zeroed is the + session's answer (``StudioMainWindow._update_run_enabled``), not this + control's, so a view switch that merely states the transport cannot take a + device action away. """ self.btn_run.blockSignals(True) self.btn_run.setChecked(bool(checked)) self.btn_run.blockSignals(False) self.refresh_labels() - if not checked: - self.btn_reset.setEnabled(False) def set_stop_enabled(self, enabled: bool) -> None: """Stop is offered for as long as there is something to end. diff --git a/apps/studio/tests/test_scene3d_gl_smoke.py b/apps/studio/tests/test_scene3d_gl_smoke.py index dfb204e..b4cada4 100644 --- a/apps/studio/tests/test_scene3d_gl_smoke.py +++ b/apps/studio/tests/test_scene3d_gl_smoke.py @@ -266,6 +266,85 @@ def test_the_rest_frame_is_drawn_neutral_not_as_measurement_zero(gl_widget, qt_a widget.set_settings(DisplaySettings(housing_visible=True)) +def _plate_mode_rgb(pixels: np.ndarray) -> np.ndarray: + """The most common colour of the drawn surface: its *base* colour. + + The resting area is the largest uniform region of a depth picture, so the modal + colour is the colour the ramp gives "no response" — the thing that must not move + when the operator presses. + """ + drawn = _drawn(pixels) + assert drawn.size, "nothing was drawn" + quantised = (drawn // 4) * 4 + values, counts = np.unique(quantised, axis=0, return_counts=True) + return values[int(np.argmax(counts))].astype(float) + + +def test_a_press_does_not_move_the_surfaces_base_colour(gl_widget, qt_app): + """The base colour follows the field, not the frame's current response. + + Reported as "the base color of 3d depth view will change upon pressing". Measured + before the fix on a real recording, with the automatic colour scale: the resting + plate was drawn RGB [248, 0, 240] (the middle of the ramp, because a flat field was + centred on its own midpoint) and the first press moved it to [8, 24, 48] (the first + colour), where it stayed after the release. The automatic range is now anchored at + the field's own zero, so the base colour is the ramp's first colour throughout and + only the upper bound follows the press. + """ + widget, renderer, _ = gl_widget + from orisys_studio.scene3d.gl_backend import lut_rgba + + assets = synthetic.poc_scene() + descriptor = FieldDescriptor("depth_response", "1", "approximate", False) + automatic = DisplaySettings( + housing_visible=False, + color_scale=ColorScale( + descriptor=descriptor, + lower=0.0, + upper=1.0, + lut_id="sequential", + mode="automatic", + ), + ) + widget.set_settings(automatic) + + rest = np.zeros(assets.shape_hw, dtype=np.float32) + widget.present(_frame(assets, rest, frame_id=21)) + rest_base = _plate_mode_rgb(_render(widget, qt_app)) + rest_range = renderer._scalar_range + + # A press on one cell of the 3x3 PoC plate: the rest of the plate is still "no + # response", so the modal colour is still the base colour. + pressed = rest.copy() + pressed[1, 1] = 1.0 + widget.present(_frame(assets, pressed, frame_id=22)) + pressed_base = _plate_mode_rgb(_render(widget, qt_app)) + + table = lut_rgba("sequential")[:, 0, :3].astype(float) + first_colour = table[0] + assert np.abs(rest_base - first_colour).max() <= 6.0, ( + f"the resting plate is not drawn in the ramp's first colour: {rest_base} vs " + f"{first_colour}" + ) + assert np.abs(pressed_base - rest_base).max() <= 6.0, ( + "pressing changed the surface's base colour: " + f"{rest_base} at rest against {pressed_base} pressed" + ) + # The press still has to show: the range's upper bound follows the response. + scale = renderer._last_scale + assert scale is not None and scale[0] == 0.0, ( + f"the automatic range is not anchored at the field's zero: {scale}" + ) + assert ( + renderer._scalar_range is not None + and rest_range is not None + and renderer._scalar_range[1] > rest_range[1] + ), ( + "the press did not reach the colour scale: " + f"{rest_range} at rest against {renderer._scalar_range} pressed" + ) + + def test_a_frame_updates_only_the_sensing_buffers(gl_widget, qt_app): widget, renderer, _ = gl_widget assets = synthetic.poc_scene() diff --git a/apps/studio/tests/test_scene3d_overlays.py b/apps/studio/tests/test_scene3d_overlays.py index 9fa8f6c..4a669df 100644 --- a/apps/studio/tests/test_scene3d_overlays.py +++ b/apps/studio/tests/test_scene3d_overlays.py @@ -3,7 +3,6 @@ from __future__ import annotations import numpy as np -from orisys_app_shared.acquisition.arrows import _policy_from_style from orisys_app_shared.scene3d.composer import SceneComposer from orisys_app_shared.scene3d.contracts import ( DisplaySettings, @@ -12,7 +11,10 @@ from orisys_app_shared.scene3d.contracts import ( freeze, ) from orisys_app_shared.scene3d.synthetic import poc_identity, poc_scene -from orisys_app_shared.scene3d.visual_policy import VisualPolicy, select_flow_samples +from orisys_app_shared.scene3d.visual_policy import ( + policy_for_settings, + select_flow_samples, +) def _depth_frame(assets, *, flow=None, contact_xy=None, fn=0.0): @@ -81,10 +83,20 @@ def test_composer_hides_overlays_when_disabled() -> None: def test_2d_3d_flow_sample_points_match() -> None: + """Both figures show the same arrows for the same frame — hysteresis included. + + The regression: the composer kept a "was this sample on last frame" grid and the 2D + canvas did not, so a sample between ``flow_off_px`` and ``flow_on_px`` was drawn by + one view and not the other. Measured on a real recording before the fix: 622 arrows + in the 3D overlay against 432 in the canvas, differing on 34 of 41 frames. + + Frame A is above ``flow_on_px`` (both views draw it); frame B is inside the band — + above the old hysteresis floor, below the threshold the canvas applies. Both views + must agree on frame B, which is what fails if the composer re-acquires state the + canvas does not have. + """ assets = poc_scene() h, w = assets.shape_hw - flow = np.zeros((h, w, 2), dtype=np.float64) - flow[1, 1] = (0.7, 0.2) mask = np.ones((h, w), dtype=bool) settings = DisplaySettings( flow_on_px=0.4, @@ -92,23 +104,45 @@ def test_2d_3d_flow_sample_points_match() -> None: flow_grid_spacing=1, max_arrows=50, ) - policy_3d = _policy_from_style( - "finger", - flow_on_px=settings.flow_on_px, - flow_off_px=settings.flow_off_px, - flow_grid_spacing=settings.flow_grid_spacing, - max_arrows=settings.max_arrows, + policy = policy_for_settings(settings) + composer = SceneComposer() + + def both_views(flow_value: float) -> tuple[int, int]: + flow = np.zeros((h, w, 2), dtype=np.float64) + flow[1, 1] = (flow_value, 0.0) + composed = composer.compose( + assets, _depth_frame(assets, flow=flow), settings + ) + arrows_3d = ( + 0 if composed.flow_arrows is None else int(composed.flow_arrows.origins.shape[0]) + ) + # Exactly what the 2D canvas draws from: ``draw_arrow_display`` calls + # ``select_flow_samples`` with the policy the window passes it. + samples_2d = select_flow_samples(flow, mask, policy) + return arrows_3d, int(samples_2d.rows.size) + + above, above_2d = both_views(0.30) # |2*flow| = 0.6 px, clearly on + assert above == 1 and above_2d == 1, (above, above_2d) + inside_band, inside_band_2d = both_views(0.175) # |2*flow| = 0.35 px: 0.3 < 0.35 < 0.4 + assert inside_band_2d == 0, inside_band_2d + assert inside_band == inside_band_2d, ( + "the 3D overlay drew an arrow the 2D canvas did not: the two views are not " + f"triggering at the same strength ({inside_band} against {inside_band_2d})" ) - policy_shared = VisualPolicy( - flow_on_px=settings.flow_on_px, - flow_off_px=settings.flow_off_px, - flow_grid_spacing=settings.flow_grid_spacing, - max_arrows=settings.max_arrows, - flow_edge_margin=0, - ).normalised() - samples_2d = select_flow_samples(flow, mask, policy_shared) - samples_3d = select_flow_samples(flow, mask, policy_shared) - assert list(zip(samples_2d.rows, samples_2d.cols)) == list( - zip(samples_3d.rows, samples_3d.cols) + + +def test_both_views_build_their_policy_from_the_same_settings() -> None: + """One mapping, one policy: the thresholds are display settings, not per-view ones.""" + settings = DisplaySettings( + flow_on_px=0.7, + flow_off_px=0.2, + flow_grid_spacing=6, + max_arrows=123, ) - assert policy_3d.flow_on_px == settings.flow_on_px + policy = policy_for_settings(settings) + assert (policy.flow_on_px, policy.flow_off_px) == (0.7, 0.2) + assert policy.flow_grid_spacing == 6 + assert policy.max_arrows == 123 + # Normalised, so a hand-edited preference cannot invert the hysteresis band. + inverted = policy_for_settings(DisplaySettings(flow_on_px=0.1, flow_off_px=0.9)) + assert inverted.flow_on_px >= inverted.flow_off_px diff --git a/apps/studio/tests/test_scene3d_stage1.py b/apps/studio/tests/test_scene3d_stage1.py index 76c267a..c3da292 100644 --- a/apps/studio/tests/test_scene3d_stage1.py +++ b/apps/studio/tests/test_scene3d_stage1.py @@ -27,6 +27,7 @@ from orisys_studio.scene3d.gl_backend import ( LUT_SIZE, STRIDE_FLOATS, VALID_BYTES_PER_TEXEL, + _valid_scalar_range, estimate_gpu_bytes, is_software_renderer, lut_rgba, @@ -289,6 +290,45 @@ def test_the_lut_is_an_ordered_ramp_and_the_diverging_one_is_centred(): assert not np.array_equal(diverging, sequential) +def test_the_automatic_range_is_anchored_at_the_fields_zero(): + """The base colour is a property of the field, not of how hard the frame is pressed. + + A flat field used to be *centred* on its own midpoint (the middle of the ramp), and + the first press moved the range to [P5, P95] — so the resting surface jumped from + the ramp's middle colour to its first one, and stayed there after the release + (measured on a real recording: RGB [248, 0, 240] at rest against [8, 24, 48] once + pressed). Anchoring an unsigned field at its own zero — the ramp's first colour — + is what makes the base colour constant. + """ + rest = np.zeros((16, 16), dtype=np.float32) + valid = np.ones((16, 16), dtype=bool) + pressed = rest.copy() + # A press big enough to move the robust upper bound (P95): a quarter of the field. + pressed[4:12, 4:12] = 1.0 + pressed[8, 8] = 0.6 + + rest_range = _valid_scalar_range(rest, valid, signed=False) + pressed_range = _valid_scalar_range(pressed, valid, signed=False) + assert rest_range is not None and pressed_range is not None + assert rest_range[0] == 0.0 and pressed_range[0] == 0.0, ( + "an unsigned field's lower bound must be its own zero, or the base colour " + "moves with the measurement" + ) + assert pressed_range[1] > rest_range[1], ( + "the upper bound must still follow the frame's response" + ) + # ...and the colour the shader draws for a field value of 0 is therefore identical + # in both frames: the ramp's first colour. + for lower, upper in (rest_range, pressed_range): + assert upper > lower + assert (0.0 - lower) / (upper - lower) == 0.0 + + # A signed quantity keeps the diverging ramp's *middle* at zero instead. + signed = _valid_scalar_range(pressed, valid, signed=True) + assert signed is not None and signed[0] < 0.0 < signed[1] + assert abs(signed[0]) == pytest.approx(signed[1]) + + def test_software_renderers_are_recognised(): assert is_software_renderer("Mesa", "llvmpipe (LLVM 15.0.7, 256 bits)") assert is_software_renderer("Microsoft", "Microsoft Basic Render Driver") diff --git a/apps/studio/tests/test_ui_smoke.py b/apps/studio/tests/test_ui_smoke.py index 3fbdd8f..31b0188 100644 --- a/apps/studio/tests/test_ui_smoke.py +++ b/apps/studio/tests/test_ui_smoke.py @@ -2142,6 +2142,89 @@ def test_the_live_view_keeps_the_tracker_reset(make_window): window.close() +def test_the_tracker_reset_follows_the_session_across_a_view_switch(make_window): + """Zeroing the tracker is a *device* action: which panel is on screen is not a fact + about the session, so switching live → calibration → live must not take the reset + away from a session that is still running. + + The reported defect: with a session running, opening the 标定 view left the button + unavailable, and coming back to 实时 did not restore it — the session was unchanged + throughout. The state asserted at every step is the session's, so the test fails if + the view switch is what drives the control rather than the session. + """ + from orisys_app_shared.i18n import t + from PySide6.QtWidgets import QApplication + + window = _show(make_window(sensor_type="box", source_mode="camera"), (1440, 880)) + stuck = ui_check._StuckWorker() + window._ensure_workers = lambda: None + window._box_worker = stuck + window._finger_worker = stuck + window._start_run() + window._run_timer.stop() + try: + # A first frame is what makes the session a *running* one: the phase, and with it + # the device controls, key off it (see the rail's own rule). + stuck.emit(ui_check._fake_packet(1)) + window._on_run_tick() + assert window._session_phase == "running", window._session_phase + assert window.toolbar.btn_reset.isEnabled() and window.act_reset.isEnabled(), ( + "a running session must offer the tracker reset before any view switch" + ) + assert window.toolbar.btn_reset.text() == t("toolbar.reset") + + window.brand_bar.btn_calibration.click() + QApplication.processEvents() + assert window.layout_ctl.view_preset == "calibration" + assert window.toolbar.btn_reset.isEnabled() and window.act_reset.isEnabled(), ( + "the session still runs in the calibration view: the reset went unavailable" + ) + + window.brand_bar.btn_live.click() + QApplication.processEvents() + assert window.layout_ctl.view_preset == "live" + assert window.toolbar.btn_reset.isEnabled() and window.act_reset.isEnabled(), ( + "the return to the live view did not restore the session's reset" + ) + finally: + window._stop_run(reason="user_stop") + stuck.release() + + +def test_the_two_figures_are_drawn_from_one_arrow_policy(make_window, monkeypatch): + """The 2D canvas and the 3D surface must sample the same policy. + + The pane owns the arrow thresholds (they are display settings and are persisted), + so the 2D renderer is handed a policy built from that same object rather than + inventing its own. A canvas that picks its own threshold is how the two figures + ended up drawing a different number of arrows for one frame (measured: 432 against + 622 on a real recording, differing on 34 of 41 frames). + """ + import orisys_studio.app_window as app_window + from orisys_app_shared.acquisition.image_backend import ImageFrame + from orisys_app_shared.scene3d.visual_policy import policy_for_settings + + seen: dict = {} + + def spy(frame, plan=None, **kwargs): + seen.update(kwargs) + return None # the panel is not the subject here + + monkeypatch.setattr(app_window, "render_image", spy) + window = _show(make_window(sensor_type="finger"), (1440, 880)) + expected = policy_for_settings(window.mesh3d_panel.display_settings()) + assert window._arrow_policy() == expected + + flow = np.zeros((12, 16, 2), dtype=np.float32) + window._show_image_frame( + ImageFrame(np.zeros((12, 16, 3), np.uint8), flow=flow, flow_valid=True) + ) + assert seen.get("arrow_policy") == expected, ( + "the 2D canvas was not drawn with the pane's arrow policy" + ) + window.close() + + def test_a_bare_video_plays_even_though_it_is_not_a_recording(make_window, record_root): """A clip with no forces.csv beside it is still something to watch. diff --git a/shared/src/orisys_app_shared/acquisition/arrows.py b/shared/src/orisys_app_shared/acquisition/arrows.py index 718ca6b..c7542d7 100644 --- a/shared/src/orisys_app_shared/acquisition/arrows.py +++ b/shared/src/orisys_app_shared/acquisition/arrows.py @@ -254,6 +254,7 @@ def draw_arrow_display( flow_off_px: float | None = None, flow_grid_spacing: int | None = None, max_arrows: int | None = None, + policy=None, ) -> np.ndarray: """Draw flow arrows on a solid canvas or over the processed frame. @@ -267,8 +268,11 @@ def draw_arrow_display( :func:`select_flow_samples`: the arrows stay this frame's own measurement, so latching the boundary cannot put an arrow on a sample the device does not vouch for. - Finger style uses the shared :mod:`visual_policy` sampler so 2D and 3D arrows - share the same UV sample points and thresholds. Box style keeps the SDK path. + ``policy`` is the **shared** :class:`~orisys_app_shared.scene3d.visual_policy.VisualPolicy` + the 3D surface also draws its overlay from (``visual_policy.policy_for_settings``). + Passing it is what makes the two figures trigger at the same strength; the + finger-style defaults below are only the fallback for a caller that has no pane + (a tool, a test). Box style keeps the SDK path either way. """ base = _ensure_bgr(processed_frame) if flow is None: @@ -309,18 +313,22 @@ def draw_arrow_display( canvas = base if finger: - policy = _policy_from_style( - draw_style, - flow_on_px=flow_on_px, - flow_off_px=flow_off_px, - flow_grid_spacing=flow_grid_spacing, - max_arrows=max_arrows, + selected = ( + policy + if policy is not None + else _policy_from_style( + draw_style, + flow_on_px=flow_on_px, + flow_off_px=flow_off_px, + flow_grid_spacing=flow_grid_spacing, + max_arrows=max_arrows, + ) ) return _paint_shared_flow_arrows( canvas, np.asarray(flow), valid_mask=valid_mask, - policy=policy, + policy=selected, colormap=colormap, below_threshold_color=tuple(draw_kw["below_threshold_color"]), ) diff --git a/shared/src/orisys_app_shared/acquisition/image_backend.py b/shared/src/orisys_app_shared/acquisition/image_backend.py index 727ddd0..a5f954b 100644 --- a/shared/src/orisys_app_shared/acquisition/image_backend.py +++ b/shared/src/orisys_app_shared/acquisition/image_backend.py @@ -285,6 +285,7 @@ def render_image( depth_range: tuple[float, float] | None = None, canvas_bgr: Sequence[int] | None = None, domain_mask: np.ndarray | None = None, + arrow_policy=None, ) -> np.ndarray | None: """Draw ``frame`` in ``mode``; ``None`` when there is nothing to draw. @@ -298,6 +299,11 @@ def render_image( latch to should do. It never gates the arrows or the depth values: those stay this frame's own data, so a sample the device does not vouch for is still not drawn just because the boundary around it was latched. + + ``arrow_policy`` is the shared flow-arrow policy (``visual_policy.policy_for_settings`` + over the pane's display settings): the same object the 3D surface samples its overlay + with, so both figures trigger at the same strength. ``None`` means the draw style's + own defaults. """ plan = plan or plan_image(frame) wanted = plan.resolve(mode) @@ -328,6 +334,7 @@ def render_image( valid_mask=frame.valid_mask, domain_mask=domain_mask, colormap=flow_colormap(theme), + policy=arrow_policy, ) # ``recorded``: the frame is the picture, with nothing drawn over it. return ensure_bgr(np.asarray(frame.image_bgr)) diff --git a/shared/src/orisys_app_shared/scene3d/composer.py b/shared/src/orisys_app_shared/scene3d/composer.py index 82460ec..cf5d13f 100644 --- a/shared/src/orisys_app_shared/scene3d/composer.py +++ b/shared/src/orisys_app_shared/scene3d/composer.py @@ -57,7 +57,10 @@ from orisys_app_shared.scene3d.sampling import ( check_field_domain, sample_vertex_displacement, ) -from orisys_app_shared.scene3d.visual_policy import VisualPolicy, select_flow_samples +from orisys_app_shared.scene3d.visual_policy import ( + policy_for_settings, + select_flow_samples, +) #: Reported when a display gain is applied to a displacement that is only #: approximate. Informational: the picture is legitimate as an illustration, and @@ -89,8 +92,6 @@ class SceneComposer: #: draw, the picture is the asset's own geometry, and recomputing its normals #: every frame was pure cost (measured as the bulk of a 20k-vertex compose). self._rest_geometry: dict[str, tuple[np.ndarray, np.ndarray]] = {} - #: Last UV grid of "arrow was on" samples for flow hysteresis. - self._flow_previously_on: np.ndarray | None = None #: The resolved ``#RRGGBB,...`` ramp both views build their lookup table from. #: Set by the view when the theme changes; empty means this build's default. self.ramp = str(ramp) @@ -252,12 +253,7 @@ class SceneComposer: ) return flow_arrows, contact_arrow - policy = VisualPolicy( - flow_on_px=float(settings.flow_on_px), - flow_off_px=float(settings.flow_off_px), - flow_grid_spacing=int(settings.flow_grid_spacing), - max_arrows=int(settings.max_arrows), - ).normalised() + policy = policy_for_settings(settings) if ( settings.flow_arrows_enabled and frame.flow_uv is not None @@ -265,20 +261,17 @@ class SceneComposer: ): try: flow = np.asarray(frame.flow_uv) - h, w = int(flow.shape[0]), int(flow.shape[1]) - previously_on = self._flow_previously_on - if previously_on is not None and previously_on.shape != (h, w): - previously_on = None + # Selection is a pure function of (field, mask, policy): the 2D canvas + # runs the same one on the same frame, so the two figures show the same + # arrows. Hysteresis used to live only here, which made this figure + # trigger at ``flow_off_px`` (0.3 px) where the canvas needed + # ``flow_on_px`` (0.4 px) — measured on a real recording as 622 arrows + # here against 432 there, and a difference on 34 of 41 frames. samples = select_flow_samples( flow, frame.flow_valid_mask, policy, - previously_on=previously_on, ) - on_mask = np.zeros((h, w), dtype=bool) - if samples.rows.size: - on_mask[samples.rows, samples.cols] = True - self._flow_previously_on = on_mask flow_arrows, extra = build_flow_overlay( samples, mapping, @@ -294,9 +287,6 @@ class SceneComposer: ) ) flow_arrows = None - self._flow_previously_on = None - else: - self._flow_previously_on = None if settings.contact_arrow_enabled and frame.contact_valid: contact_arrow, extra = build_normal_overlay( diff --git a/shared/src/orisys_app_shared/scene3d/visual_policy.py b/shared/src/orisys_app_shared/scene3d/visual_policy.py index cdf0225..e035d59 100644 --- a/shared/src/orisys_app_shared/scene3d/visual_policy.py +++ b/shared/src/orisys_app_shared/scene3d/visual_policy.py @@ -13,6 +13,7 @@ import numpy as np from orisys_app_shared.scene3d.contracts import ( Diagnostic, + DisplaySettings, Mask, diagnostic, error, @@ -58,6 +59,32 @@ class VisualPolicy: ) +def policy_for_settings(settings: DisplaySettings) -> VisualPolicy: + """The **one** flow-arrow policy both views use, from the display settings. + + The 2D canvas and the 3D surface draw the same measurement, so they have to + trigger at the same strength: "arrows in one figure and not the other" was a + difference in the policy each path built for itself, not a difference in the + data. One mapping, from one settings object, is what removes it — the pane's + ``DisplaySettings`` is where the thresholds and the sample grid are chosen (and + persisted), so neither view is free to invent its own. + + ``flow_off_px`` travels with the policy but neither view applies hysteresis today. + A band only one of the two views honoured was the mismatch itself (measured on a + real recording: 622 arrows in the 3D overlay against 432 in the canvas, differing + on 34 of 41 frames), and the band was never an operator-facing control. It stays in + the value so a *shared* stateful selection can use it later; a state owned by one + view cannot be shared with the other anyway — they run on different threads at + different rates, which is what made the two selections diverge in the first place. + """ + return VisualPolicy( + flow_on_px=float(settings.flow_on_px), + flow_off_px=float(settings.flow_off_px), + flow_grid_spacing=int(settings.flow_grid_spacing), + max_arrows=int(settings.max_arrows), + ).normalised() + + @dataclass(frozen=True) class FlowSamples: """Shared UV sample points for 2D and 3D flow arrows.""" diff --git a/shared/tests/test_image_backend.py b/shared/tests/test_image_backend.py index 1552e1d..053856d 100644 --- a/shared/tests/test_image_backend.py +++ b/shared/tests/test_image_backend.py @@ -110,6 +110,44 @@ def test_a_mode_the_plan_does_not_offer_falls_back_to_the_default(): # --------------------------------------------------------------- one renderer +def test_the_2d_canvas_draws_from_the_policy_it_is_given(): + """The threshold is the caller's — the pane's — not a per-view default. + + ``arrow_policy`` is the policy the 3D surface samples its overlay with + (``visual_policy.policy_for_settings`` over the display settings). A sample inside + the hysteresis band is the case that used to differ between the two figures, so it + is the case pinned here: with a 0.4 px threshold nothing is drawn, with 0.3 px the + same frame carries arrows. + """ + from orisys_app_shared.scene3d.contracts import DisplaySettings + from orisys_app_shared.scene3d.visual_policy import policy_for_settings + + h, w = 12, 16 + flow = np.zeros((h, w, 2), dtype=np.float32) + flow[..., 0] = 0.175 # |2*flow| = 0.35 px: above flow_off_px, below flow_on_px + frame = backend.ImageFrame(_image(h, w), flow=flow, flow_valid=True) + base = np.zeros((h, w, 3), dtype=np.uint8) + + strict = policy_for_settings(DisplaySettings(flow_on_px=0.4, flow_off_px=0.3)) + loose = policy_for_settings(DisplaySettings(flow_on_px=0.3, flow_off_px=0.2)) + strict_image = backend.render_image( + frame, mode="arrows_pure", draw_style="finger", arrow_policy=strict + ) + loose_image = backend.render_image( + frame, mode="arrows_pure", draw_style="finger", arrow_policy=loose + ) + assert len(np.unique(strict_image.reshape(-1, 3), axis=0)) == 1, ( + "a 0.4 px threshold drew something for a 0.35 px sample" + ) + assert len(np.unique(loose_image.reshape(-1, 3), axis=0)) > 1, ( + "a 0.3 px threshold drew nothing for a 0.35 px sample" + ) + # The same policy governs the primitive directly, which is what the window calls + # through the backend. + direct = draw_arrow_display(base, flow, draw_style="finger", policy=strict) + assert len(np.unique(direct.reshape(-1, 3), axis=0)) == 1 + + def test_the_backend_renders_what_the_primitives_rendered_before(): """Live went through these two calls; the backend must not change a pixel.""" image = _image()