From c9a28e7c3ae688f4a3fbb48b791addec47f4107d Mon Sep 17 00:00:00 2001 From: ykcheungab Date: Mon, 14 Sep 2026 19:58:33 +0800 Subject: [PATCH] =?UTF-8?q?fix(studio-3d):=20=E5=AE=A1=E8=AE=A1=E4=B8=89?= =?UTF-8?q?=E7=BB=B4=E6=94=B9=E5=8A=A8=20=E2=80=94=E2=80=94=20=E4=BF=AE?= =?UTF-8?q?=E3=80=8C=E7=AC=AC=E4=B8=80=E4=B8=AA=E4=BC=9A=E8=AF=9D=E4=B9=8B?= =?UTF-8?q?=E5=90=8E=E7=94=BB=E9=9D=A2=E6=B0=B8=E4=B9=85=E5=86=BB=E7=BB=93?= =?UTF-8?q?=E3=80=8D=EF=BC=8C=E5=B9=B6=E6=8A=8A=E9=9D=A2=E6=9D=BF/?= =?UTF-8?q?=E8=A7=86=E5=9B=BE=E6=8E=A5=E5=8F=A3=E6=98=BE=E5=BC=8F=E5=8C=96?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - What: 1. **会话代际（真 bug）**：`ThreeDCoordinator.begin_session()` 新增——新采集会话开始时要显式宣告 （清掉已锁定的 session_id、generation+1、清空 mailbox、把参考模型放回屏幕）；`Scene3DView.begin_session()`、 `Mesh3dPanel.begin_session()` 转发，`app_window._start_run()` 在启动 worker 后调用。 另外 `submit()` 在"会话不符"时不再只计数，也给出 `stale_frame` 诊断（信息级），否则界面只是"不动"。 2. **接口显式化（可维护性）**：`Mesh3dPanel.attach_view()` 成为唯一装配入口（真实视图与测试替身都走它， 测试不再写 `_view`）；`_view` 类型改为 `Optional["Scene3DView"]`（TYPE_CHECKING 导入，保持 GL 惰性）； 容器里 7 处 `hasattr(...)` 鸭子调用改为直接调用（方法改名会立刻报错，而不是静默不生效）； `Scene3DView.describe()`/`render_totals()` 与 `Mesh3dPanel.describe()`/`render_totals()` 让面板自己回答 "我是什么"——`app_window` 里那段走 5 个属性路径拼 `visualization3d` 的代码缩成一次调用 + 一个 try； `scene3d/panel.py` 对 coordinator/widget 的导入移到 TYPE_CHECKING；`scene3d/__init__.py` 写上各模块职责图 （原文还写着"应用尚未导入本包"）。 - Why: 用户要求全面回顾三维改动并按"实时/回放/接口清晰/性能"四条检查 bug。审计确实抓到一个严重缺陷： 协调器记住第一个会话的 session_id 后，**之后每个会话的每一帧都被判为 stale 而拒绝**（实测 session 1 正常、 session 2 stale=3 且 0 帧上屏），而状态仍显示 live —— 表现为"第一次运行之后三维就再也不动了"。 这是"回放不太行"的直接原因之一（回放里的重算通道就是第二个会话）。顺带把"面板↔窗口"的隐式耦合收紧。 - Verification: · 回归用例：`test_scene3d_coordinator.py` +2（新会话在宣告前 stale 且带诊断、宣告后接受并回到 live； `begin_session` 在无场景/已关闭时安全）、`test_scene3d_view.py` +1（每次 _start_run 代际递增、第二个会话的帧被接受）。 · 实测（真 GPU，apps/studio 配置里那个包）：`audit_live_path.py` —— 三个连续会话 state=live、stale=0、 每次都有 26260 个非背景像素、descriptor=flow_magnitude； `audit_replay_path.py` —— 回放时间轴正常（1.97 s 记录，播放 0.6 s 后位置 0.60 s，2D 有图，停止按钮生效， 面板标签"回放：录像不含三维场"），随后**真实重算通道**跑完（59 个样本、2.6 s、正常报"视频已结束"）， 三维面板进入 state=live（62 帧上屏、2 帧显示丢弃、stale=0），标签切到"重算通道：由录像重算，非记录值"。 · 性能（真 GPU，shipped 包）：submit p50 0.26 / p95 0.38 ms，upload p50 0.18 ms，paint p50 0.90 ms， latency p50 33.08 ms（= 30 fps 上限）；20k 顶点 CPU-only compose p50 5.97 ms（与已提交报告的 6.15 ms 一致）。 · apps/studio 全量通过；ui_check 583 项 0 失败（连跑两次）；locale_check 0 缺失。 · 修掉 ui_check 自身一处错误断言：场景已加载但本机无 GL context 时，面板显示"无法创建 context"是**正确**行为， 原断言却要求"不显示任何消息"（偶发失败的原因就是它，与产品无关）。 - Not in this change: 测试里仍直接读 `view._camera`/`widget._renderer`/`coordinator._composer` 等私有量（测试替身已收敛到 `attach_view`，其余留待需要时再抽）；`update_surface` 里每帧重算色标范围（`_valid_scalar_range`）可以做条件化， 实测开销很小故未动；SDK 对极小合成视频的解码问题（640×480 mp4v）——现在会 10 秒后明确报错而不是空转。 --- apps/studio/src/orisys_studio/app_window.py | 115 ++++------------- .../src/orisys_studio/scene3d/__init__.py | 21 +++- .../src/orisys_studio/scene3d/coordinator.py | 38 ++++++ .../studio/src/orisys_studio/scene3d/panel.py | 8 +- apps/studio/src/orisys_studio/scene3d/view.py | 85 +++++++++++++ .../src/orisys_studio/ui/mesh3d_panel.py | 103 ++++++++++++---- apps/studio/tests/test_scene3d_coordinator.py | 56 +++++++++ apps/studio/tests/test_scene3d_view.py | 116 ++++++++++++++++-- apps/studio/tools/ui_check.py | 19 ++- 9 files changed, 429 insertions(+), 132 deletions(-) diff --git a/apps/studio/src/orisys_studio/app_window.py b/apps/studio/src/orisys_studio/app_window.py index 1722890..eeaac1a 100644 --- a/apps/studio/src/orisys_studio/app_window.py +++ b/apps/studio/src/orisys_studio/app_window.py @@ -469,22 +469,21 @@ class StudioMainWindow(QMainWindow): self._publish_record_report(self.recording.stop(wait=wait)) def _log_visualization3d_totals(self) -> None: - """One event with the display-side counters. Never raises, never blocks.""" + """One event with the display-side counters. Never raises, never blocks. + + Written whenever this session had a 3D view at all — including the case where it + displayed nothing, which is a fact about the session ("the pane was up and no + valid field ever arrived") rather than noise. With no view, ``session.json``'s + block already says the pane was off. + """ try: - view = self.mesh3d_panel.view - if view is None: + if self.mesh3d_panel.view is None: return - coordinator = view.coordinator - stats = view.render_stats() - shown = int(getattr(stats, "displayed_frames", 0) or 0) - dropped = int( - coordinator.superseded_frames if coordinator is not None else 0 - ) + totals = self.mesh3d_panel.render_totals() self.recorder.log_event( "visualization3d", - f"shown={shown} dropped_display={dropped} " - f"stale={int(getattr(coordinator, 'stale_frames', 0) or 0)} " - f"invalid={int(getattr(coordinator, 'invalid_frames', 0) or 0)}", + "shown={shown} dropped_display={dropped_display} " + "stale={stale} invalid={invalid}".format(**totals), ) except Exception: # noqa: BLE001 - a picture must not fail a recording logger.exception("Could not log the 3D display totals") @@ -624,88 +623,23 @@ class StudioMainWindow(QMainWindow): def _visualization3d_metadata(self) -> dict: """The pane's state for ``session.json``. Must never block a recording. - Every read is guarded and every value is a plain JSON type: this method runs - on the way into a recording, and a widget that has been torn down or a scene - that was refused is not a reason to fail one. + The pane describes itself (``Mesh3dPanel.describe``) and this only wraps the + answer in the record's block shape: the window does not need to know what a + view, a scene or a capability record looks like to record a picture. Guarded + because it runs on the way into a recording, and a widget that has been torn + down is not a reason to fail one. """ from orisys_app_shared.record import build_visualization3d_block - enabled = False - available = False - reason = "" - scene: dict = {} - gl: dict = {} - display: dict = {} - stream: dict = {} try: - pane = self.mesh3d_panel - view = pane.view - enabled = bool(pane.enabled()) - available = bool(pane.is_available()) - diagnostic = pane.last_diagnostic - assets = view.assets if view is not None else None - if assets is None: - reason = ( - diagnostic.message - if diagnostic is not None - else str(t("panels.mesh3d_no_scene")) - ) - else: - scene = { - "path": str(pane.scene_path() or ""), - "asset_id": str(getattr(assets, "asset_id", "")), - "asset_version": str(getattr(assets, "asset_version", "")), - "asset_hash": str(getattr(assets, "asset_hash", "")), - "geometry_id": str(getattr(assets, "geometry_id", "")), - "field_domain_id": str(getattr(assets, "field_domain_id", "")), - "platform_lock": ( - assets.platform_lock.to_dict() - if getattr(assets, "platform_lock", None) is not None - else None - ), - } - widget = getattr(view, "gl_widget", None) - capabilities = widget.capabilities() if widget is not None else None - if capabilities is not None: - gl = { - "profile": str(capabilities.profile), - "vendor": str(capabilities.vendor), - "renderer": str(capabilities.renderer), - "gl_version": str(capabilities.gl_version), - "glsl_version": str(capabilities.glsl_version), - "software_renderer": bool(capabilities.software_renderer), - } - settings = view.settings() - scale = settings.color_scale - display = { - "deform_enabled": bool(settings.deform_enabled), - "display_gain": float(settings.display_gain), - "housing_visible": bool(settings.housing_visible), - "max_fps": int(settings.max_fps), - "scale_mode": str(scale.mode) if scale is not None else "none", - "scale_lower": float(scale.lower) if scale is not None else None, - "scale_upper": float(scale.upper) if scale is not None else None, - } - descriptor = view.last_descriptor - if descriptor is not None: - stream = { - "quantity": str(descriptor.quantity), - "unit": str(descriptor.unit), - "quality": str(descriptor.quality), - "signed": bool(descriptor.signed), - } + return build_visualization3d_block(**self.mesh3d_panel.describe()) except Exception: # noqa: BLE001 - a recording must not fail over a picture logger.exception("Could not describe the 3D pane for the recording") - reason = reason or "the 3D pane could not be described" - return build_visualization3d_block( - enabled=enabled, - available=available, - reason=reason, - scene=scene or None, - gl=gl or None, - display=display or None, - stream=stream or None, - ) + return build_visualization3d_block( + enabled=False, + available=False, + reason="the 3D pane could not be described", + ) @staticmethod def _config_hash(config_ref: str) -> str: @@ -2512,6 +2446,11 @@ class StudioMainWindow(QMainWindow): # ``_ensure_workers`` (``add_sink`` is idempotent). self._active_worker.add_sink(self._on_packet_recorded) self._active_worker.add_sink(self._on_packet_for_3d) + # The 3D pane has to know that a *new* session is starting: its frames carry + # this session's id, and to the coordinator an unfamiliar id is otherwise a + # stale frame from a previous session — which is how the pane ended up frozen + # on the first run's last picture for every run after it. + self.mesh3d_panel.begin_session() # The filters for this session, before it opens the device: the chain runs # in the SDK, so this is the one moment the choice can be handed over. # The acquisition thread applies it and records what the device reports. diff --git a/apps/studio/src/orisys_studio/scene3d/__init__.py b/apps/studio/src/orisys_studio/scene3d/__init__.py index 0bfa595..7e5b02b 100644 --- a/apps/studio/src/orisys_studio/scene3d/__init__.py +++ b/apps/studio/src/orisys_studio/scene3d/__init__.py @@ -6,8 +6,25 @@ use, and PyOpenGL decides that at its first import — so the import happens ins :func:`~orisys_studio.scene3d.gl_backend.GLDispatch.bind_current_context`, once a real context exists (design document §11.1). -Nothing in the application imports this package yet: the skeleton is inert until -stage 4 wires the panel into ``app_window``. +Who owns what, so a change stays local (stages 4–5 of ``studio-3d-stages.md``): + +``view.py`` + the assembly. ``Scene3DView`` builds the widget, camera, renderer, composer and + coordinator, owns the tick, and is the **only** module the application talks to + for anything but the pane's chrome. +``coordinator.py`` + identity and scheduling. It decides *which* frames are current; it knows nothing + about Qt, GL or the file system. +``adapters.py`` / ``asset_source.py`` / ``prefs.py`` + the three boundaries: acquisition arrays in, packages in, preferences in. +``widget.py`` / ``gl_backend.py`` / ``camera.py`` + the drawing: Qt context, GL objects, view maths. No acquisition, no policy. +``panel.py`` + controls and status text. It reads and writes :class:`DisplaySettings` and never + reaches for a frame. +``ui/mesh3d_panel.py`` (in the ``ui`` package) + the pane's frame in the window: which view, which asset, which state — the place + the window learns "there is no 3D here, and why". """ __all__: list[str] = [] diff --git a/apps/studio/src/orisys_studio/scene3d/coordinator.py b/apps/studio/src/orisys_studio/scene3d/coordinator.py index 1c9be0b..4847e83 100644 --- a/apps/studio/src/orisys_studio/scene3d/coordinator.py +++ b/apps/studio/src/orisys_studio/scene3d/coordinator.py @@ -164,6 +164,35 @@ class ThreeDCoordinator: # Come back on the newest frame rather than replaying what was dropped. self._set_state("ready" if self._assets is not None else "disabled") + def begin_session(self) -> None: + """A new acquisition session starts: accept its frames, not the old ones. + + ``session_id`` is part of a frame's identity and a frame from another session is + refused — that is what keeps a leftover picture from being read as the current + measurement. But a *new* session is not a stale one, and something has to say + so: the application calls this when a run starts (live, or a recompute pass over + a recording). Without it the coordinator latched onto the first session it ever + saw and silently refused **every later session's frames** while still reporting + state ``live`` — which is exactly "the 3D view stopped working after the first + run". A new session is a new generation, so anything already in flight belongs + to the past and is dropped here as well. + """ + if self._closed: + return + self._cancel_inflight() + self._incoming.clear() + self._composed.clear() + self._session_id = None + self._generation += 1 + self._last_composed_identity = None + self._last_field_frame = None + self._settings_dirty = False + if self._assets is not None: + # The reference model, not the last measurement: the new session has not + # produced anything yet, and an old picture would be a claim about it. + self._widget.present(self._composer.rest_frame(self._assets, self._identity(0))) + self._set_state("ready" if self._visible else "suspended") + # ---------------------------------------------------------------- producers def submit(self, frame: SurfaceFieldFrame) -> OfferResult: """Offer a field frame from a producer thread. Never blocks on the GUI.""" @@ -186,7 +215,16 @@ class ThreeDCoordinator: ) return "stale" if session_id is not None and identity.session_id != session_id: + # Counted *and* reported: a pane that silently refuses a whole session + # looks like a frozen picture, and "a frame from another session was + # dropped" is the sentence that explains it. self._stale_frames += 1 + self._last_diagnostic = diagnostic( + "stale_frame", + "a frame from another acquisition session was dropped", + frame=str(identity.session_id), + scene=str(session_id), + ) return "stale" problem = self._reject_reason(frame, assets) if problem is not None: diff --git a/apps/studio/src/orisys_studio/scene3d/panel.py b/apps/studio/src/orisys_studio/scene3d/panel.py index f3880c1..367d61b 100644 --- a/apps/studio/src/orisys_studio/scene3d/panel.py +++ b/apps/studio/src/orisys_studio/scene3d/panel.py @@ -18,7 +18,7 @@ Two rules it exists to enforce in the interface: from __future__ import annotations -from typing import Optional +from typing import TYPE_CHECKING, Optional from PyQt5.QtCore import Qt, pyqtSignal from PyQt5.QtWidgets import ( @@ -44,9 +44,11 @@ from orisys_app_shared.scene3d.contracts import ( SceneAssets, SceneState, ) -from orisys_studio.scene3d.coordinator import ThreeDCoordinator from orisys_studio.scene3d.prefs import FPS_CHOICES -from orisys_studio.scene3d.widget import Sensor3DWidget + +if TYPE_CHECKING: # the panel talks to these two by interface, not by import + from orisys_studio.scene3d.coordinator import ThreeDCoordinator + from orisys_studio.scene3d.widget import Sensor3DWidget #: Width of one control in the pane. Chosen against #: ``MIN_PANEL_WIDTHS["mesh3d"]`` (240), not against how it looks on a wide screen: diff --git a/apps/studio/src/orisys_studio/scene3d/view.py b/apps/studio/src/orisys_studio/scene3d/view.py index 5675eac..957fc15 100644 --- a/apps/studio/src/orisys_studio/scene3d/view.py +++ b/apps/studio/src/orisys_studio/scene3d/view.py @@ -315,6 +315,16 @@ class Scene3DView(QWidget): if self._panel is not None: self._panel.set_source_kind(kind) + def begin_session(self) -> None: + """The application is starting an acquisition session for this pane. + + Called from the run path, before the first frame of the session arrives: the + coordinator has to know that a new ``session_id`` is legitimate rather than a + leftover (see ``ThreeDCoordinator.begin_session``). + """ + if self._coordinator is not None: + self._coordinator.begin_session() + def show_reference_only(self) -> None: """Drop whatever measurement is on screen and show the surface model. @@ -374,6 +384,81 @@ class Scene3DView(QWidget): def render_stats(self) -> Optional[RenderStats]: return self._last_stats + def describe(self) -> dict: + """What the pane is, for a recording's ``visualization3d`` block. + + One method instead of the caller walking ``assets``, ``gl_widget``, + ``capabilities()``, ``settings()`` and ``last_descriptor``: the pane owns the + answer to "what was on screen", and the window only has to ask. Everything here + is display state, so it can be recorded without touching the measurement. + """ + assets = self._assets + if assets is None: + return {} + widget = self._widget + capabilities = widget.capabilities() if widget is not None else None + settings = self.settings() + scale = settings.color_scale + descriptor = self._last_descriptor + return { + "scene": { + "asset_id": str(getattr(assets, "asset_id", "")), + "asset_version": str(getattr(assets, "asset_version", "")), + "asset_hash": str(getattr(assets, "asset_hash", "")), + "geometry_id": str(getattr(assets, "geometry_id", "")), + "field_domain_id": str(getattr(assets, "field_domain_id", "")), + "platform_lock": ( + assets.platform_lock.to_dict() + if getattr(assets, "platform_lock", None) is not None + else None + ), + }, + "gl": ( + { + "profile": str(capabilities.profile), + "vendor": str(capabilities.vendor), + "renderer": str(capabilities.renderer), + "gl_version": str(capabilities.gl_version), + "glsl_version": str(capabilities.glsl_version), + "software_renderer": bool(capabilities.software_renderer), + } + if capabilities is not None + else {} + ), + "display": { + "deform_enabled": bool(settings.deform_enabled), + "display_gain": float(settings.display_gain), + "housing_visible": bool(settings.housing_visible), + "max_fps": int(settings.max_fps), + "scale_mode": str(scale.mode) if scale is not None else "none", + "scale_lower": float(scale.lower) if scale is not None else None, + "scale_upper": float(scale.upper) if scale is not None else None, + }, + "stream": ( + { + "quantity": str(descriptor.quantity), + "unit": str(descriptor.unit), + "quality": str(descriptor.quality), + "signed": bool(descriptor.signed), + } + if descriptor is not None + else {} + ), + } + + def render_totals(self) -> dict: + """Display-side counters: shown frames and display drops, never recording ones.""" + coordinator = self._coordinator + stats = self._last_stats + return { + "shown": int(getattr(stats, "displayed_frames", 0) or 0), + "dropped_display": int( + coordinator.superseded_frames if coordinator is not None else 0 + ), + "stale": int(getattr(coordinator, "stale_frames", 0) or 0), + "invalid": int(getattr(coordinator, "invalid_frames", 0) or 0), + } + def close(self) -> None: """Release the GPU resources, stop the worker. Idempotent.""" if self._coordinator is not None: diff --git a/apps/studio/src/orisys_studio/ui/mesh3d_panel.py b/apps/studio/src/orisys_studio/ui/mesh3d_panel.py index af9cb01..e2bf532 100644 --- a/apps/studio/src/orisys_studio/ui/mesh3d_panel.py +++ b/apps/studio/src/orisys_studio/ui/mesh3d_panel.py @@ -14,13 +14,13 @@ acquisition, and no path that reports a 3D problem as a recording problem. from __future__ import annotations from pathlib import Path -from typing import Optional +from typing import TYPE_CHECKING, Optional from PyQt5.QtCore import Qt, pyqtSignal from PyQt5.QtWidgets import QFrame, QLabel, QStackedLayout, QVBoxLayout, QWidget from orisys_app_shared.i18n import t -from orisys_app_shared.scene3d.contracts import Diagnostic, ExpectedGeometry +from orisys_app_shared.scene3d.contracts import Diagnostic, ExpectedGeometry, SceneAssets from orisys_studio.scene3d.asset_source import ( SceneAssetLoader, SceneSource, @@ -28,6 +28,9 @@ from orisys_studio.scene3d.asset_source import ( no_asset_diagnostic, ) +if TYPE_CHECKING: # the view imports GL bindings lazily; the type is enough here + from orisys_studio.scene3d.view import Scene3DView + #: Shown when the pane is on but this sensor has no 3D scene at all (box, mini). _NO_SCENE = "no_scene" #: Shown when the sensor could have a scene but no package is installed. @@ -65,7 +68,7 @@ class Mesh3dPanel(QFrame): def __init__(self, parent: Optional[QWidget] = None) -> None: super().__init__(parent) self.setObjectName("contentPanel") - self._view: Optional[QWidget] = None + self._view: Optional["Scene3DView"] = None self._loader: Optional[SceneAssetLoader] = None self._source: Optional[SceneSource] = None self._sensor_type = "" @@ -143,6 +146,25 @@ class Mesh3dPanel(QFrame): return self._show_message(_LOADING, name=self._source.path.name) + def attach_view(self, view: "Scene3DView") -> None: + """Adopt a view: the real one (built lazily) or a double in a test. + + Public because it is the seam tests use — poking ``_view`` from a test would + make every rename of a private attribute a test edit, and the container's + contract with the view is exactly the set of methods below. + """ + self._view = view + self._stack.addWidget(view) + view.diagnosticRaised.connect(self._on_diagnostic) + view.enabledChanged.connect(self._on_view_enabled) + if self._pending_settings is not None: + view.restore_settings(self._pending_settings) + # The kind was decided before the view existed (the window sets it when the + # view preset changes), so it is pushed again here rather than lost. + view.set_source_kind(self._source_kind) + if not view.available: + self._show_message(_UNAVAILABLE, reason=view.reason) + def ensure_view(self) -> Optional[QWidget]: """Build the 3D view on first use, so a headless machine never imports GL.""" if self._view is not None or self._sensor_type != "finger": @@ -150,20 +172,9 @@ class Mesh3dPanel(QFrame): from orisys_studio.scene3d.view import Scene3DView view = Scene3DView(self) - self._view = view - self._stack.addWidget(view) - view.diagnosticRaised.connect(self._on_diagnostic) - view.enabledChanged.connect(self._on_view_enabled) - if self._pending_settings is not None and hasattr(view, "restore_settings"): - view.restore_settings(self._pending_settings) - # The kind was decided before the view existed (the window sets it when the - # view preset changes), so it is pushed again here rather than lost. - if hasattr(view, "set_source_kind"): - view.set_source_kind(self._source_kind) - if not view.available: - self._show_message(_UNAVAILABLE, reason=view.reason) - return view - self._load_scene() + self.attach_view(view) + if view.available: + self._load_scene() return view def _load_scene(self) -> None: @@ -185,19 +196,56 @@ class Mesh3dPanel(QFrame): can show a colour range or an exaggeration — is built on first use. """ self._pending_settings = settings - if self._view is not None and hasattr(self._view, "restore_settings"): + if self._view is not None: self._view.restore_settings(settings) def display_settings(self): """The settings currently in force, for the preferences file.""" from orisys_studio.scene3d.prefs import settings_from_prefs - if self._view is not None and hasattr(self._view, "settings"): + if self._view is not None: return self._view.settings() if self._pending_settings is not None: return self._pending_settings return settings_from_prefs(None) + def describe(self) -> dict: + """Everything a recording's ``visualization3d`` block needs, in one call. + + The pane answers for itself — whether it is on, what it is showing, with which + GL context and which display settings — so the window does not have to know the + shape of a view (or of a scene, or of a capability record) to record a picture. + """ + available = self.is_available() + diagnostic = self._reported + assets_loaded = self._view is not None and getattr(self._view, "assets", None) is not None + reason = "" + if not assets_loaded: + reason = ( + diagnostic.message + if diagnostic is not None + else t("panels.mesh3d_no_scene") + ) + described = self._view.describe() if self._view is not None else {} + return { + "enabled": bool(self._enabled), + "available": bool(available), + "reason": reason, + "scene": { + "path": str(self._source.path) if self._source is not None else "", + **dict(described.get("scene") or {}), + }, + "gl": dict(described.get("gl") or {}), + "display": dict(described.get("display") or {}), + "stream": dict(described.get("stream") or {}), + } + + def render_totals(self) -> dict: + """Display-side counters for the recording's stop event.""" + if self._view is None: + return {"shown": 0, "dropped_display": 0, "stale": 0, "invalid": 0} + return self._view.render_totals() + # ------------------------------------------------------------------ events def _on_loaded(self, assets, description: str) -> None: view = self._view @@ -234,7 +282,7 @@ class Mesh3dPanel(QFrame): # ------------------------------------------------------------------ inputs @property - def view(self) -> Optional[QWidget]: + def view(self) -> Optional["Scene3DView"]: return self._view @property @@ -255,7 +303,7 @@ class Mesh3dPanel(QFrame): def is_available(self) -> bool: """Whether a 3D picture is possible here at all (GL and an asset).""" - return self._view is not None and bool(getattr(self._view, "available", False)) + return self._view is not None and bool(self._view.available) def scene_path(self) -> Optional[Path]: return None if self._source is None else self._source.path @@ -263,22 +311,27 @@ class Mesh3dPanel(QFrame): def set_source_kind(self, kind: str) -> None: """Where the field comes from: live, replay (model only) or recompute.""" self._source_kind = str(kind) - if self._view is not None and hasattr(self._view, "set_source_kind"): + if self._view is not None: self._view.set_source_kind(self._source_kind) def show_reference_only(self) -> None: """Clear a measurement that is no longer being produced (a stopped pass).""" - if self._view is not None and hasattr(self._view, "show_reference_only"): + if self._view is not None: self._view.show_reference_only() + def begin_session(self) -> None: + """A new acquisition session is starting: the view must accept its frames.""" + if self._view is not None: + self._view.begin_session() + def tick(self) -> None: """Advance the pane. Called by the window's run timer, never by itself.""" - if self._view is not None and hasattr(self._view, "tick"): + if self._view is not None: self._view.tick() def close_view(self) -> None: """Release GL and the loader. Idempotent, and safe to call twice.""" - if self._view is not None and hasattr(self._view, "close"): + if self._view is not None: self._view.close() if self._loader is not None: self._loader.close() diff --git a/apps/studio/tests/test_scene3d_coordinator.py b/apps/studio/tests/test_scene3d_coordinator.py index 13114da..47b61bf 100644 --- a/apps/studio/tests/test_scene3d_coordinator.py +++ b/apps/studio/tests/test_scene3d_coordinator.py @@ -298,6 +298,62 @@ def test_changing_the_gain_recomposes_what_is_on_screen(scene, pool): coordinator.close() +def test_a_new_session_is_refused_until_it_is_announced(scene, pool): + """A second run's frames are stale until the run path says a new session began. + + This is the bug the audit found: the coordinator latched onto the first session it + ever saw and then refused *every* later session's frames while still reporting + ``live`` — a picture frozen on the first run's last frame, for every run after it. + """ + widget = FakeWidget() + coordinator = ThreeDCoordinator(widget, SceneComposer(), pool) + coordinator.set_visible(True) + coordinator.activate(scene, "package: file.oryscene") + + assert coordinator.submit(_frame(scene, source="package: file.oryscene", session=1)) == "accepted" + _drain(coordinator, ticks=10) + assert coordinator.state == "live" + + # A second session, unannounced: stale, counted, *and* reported. + assert coordinator.submit( + _frame(scene, source="package: file.oryscene", session=2) + ) == "stale" + assert coordinator.stale_frames == 1 + assert coordinator.last_diagnostic is not None + assert coordinator.last_diagnostic.code == "stale_frame" + assert "session" in coordinator.last_diagnostic.message + + # Announced: a new generation, the reference model, and then real frames again. + before = coordinator.generation + coordinator.begin_session() + assert coordinator.generation == before + 1 + assert widget.frames[-1].scalar_descriptor.quantity == "unavailable", ( + "a new session must not leave the previous session's measurement on screen" + ) + assert coordinator.submit( + _frame( + scene, + source="package: file.oryscene", + session=2, + # The application reads the generation at submit time, so a frame built + # after ``begin_session`` carries the *new* generation. + generation=coordinator.generation, + ) + ) == "accepted" + _drain(coordinator, ticks=10) + assert coordinator.state == "live" + assert coordinator.stale_frames == 1, "the new session was refused as stale" + coordinator.close() + + +def test_begin_session_is_safe_before_a_scene_is_active(pool): + coordinator = ThreeDCoordinator(FakeWidget(), SceneComposer(), pool) + coordinator.begin_session() # no scene yet: must not raise + assert coordinator.submit(_frame(synthetic.poc_scene())) == "disabled" + coordinator.close() + coordinator.begin_session() # closed: still safe + + def test_close_is_idempotent_and_stops_everything(scene, pool): widget = FakeWidget() coordinator = ThreeDCoordinator(widget, SceneComposer(), pool) diff --git a/apps/studio/tests/test_scene3d_view.py b/apps/studio/tests/test_scene3d_view.py index c08269c..a1b1a66 100644 --- a/apps/studio/tests/test_scene3d_view.py +++ b/apps/studio/tests/test_scene3d_view.py @@ -29,6 +29,8 @@ import numpy as np import pytest import ui_check +from PyQt5.QtCore import pyqtSignal +from PyQt5.QtWidgets import QWidget from orisys_app_shared.scene3d import synthetic from orisys_app_shared.scene3d.contracts import ( ColorScale, @@ -283,18 +285,33 @@ def test_panel_controls_do_not_touch_the_algorithm_side(make_window): # ------------------------------------------------------------------ record isolation -class _RecordingView: +class _RecordingView(QWidget): """A stand-in for the pane's view that records what it was handed. Used to attach a *real* consumer to the real sink path on a machine with no GL: what is being tested is whether the window's second sink can change the record, - not whether the adapter can compute a flow magnitude. + not whether the adapter can compute a flow magnitude. It implements the container's + whole contract (see ``Mesh3dPanel.attach_view``) rather than the one method a + particular test happens to call, so a change to that contract shows up here. It is + a real ``QWidget`` because the container puts it in its stack. """ - def __init__(self) -> None: + diagnosticRaised = pyqtSignal(object) + enabledChanged = pyqtSignal(bool) + + def __init__(self, parent=None) -> None: + super().__init__(parent) self.packets: list = [] self.coordinator = None + self.assets = None + self.available = True + self.reason = "" + self.source_kind = "live" + self.sessions = 0 + self.reference_only = 0 + self.closed = 0 + # the sink the acquisition thread calls def submit_packet(self, packet) -> None: # Deliberately more work than the real one does per packet, so a leak into the # record path would show up as a difference rather than as a timing artefact. @@ -302,6 +319,37 @@ class _RecordingView: frame = np.asarray(packet.raw_frame) float(np.asarray(frame, dtype=np.float64).sum()) + # the container's contract + def set_enabled(self, enabled: bool) -> None: + self.enabled_flag = bool(enabled) + + def set_source_kind(self, kind: str) -> None: + self.source_kind = str(kind) + + def begin_session(self) -> None: + self.sessions += 1 + + def show_reference_only(self) -> None: + self.reference_only += 1 + + def tick(self) -> None: + self.ticks = getattr(self, "ticks", 0) + 1 + + def close(self) -> None: + self.closed += 1 + + def settings(self): + return DisplaySettings() + + def restore_settings(self, settings) -> None: + self.settings_value = settings + + def describe(self) -> dict: + return {"scene": {"asset_id": "double"}, "gl": {}, "display": {}, "stream": {}} + + def render_totals(self) -> dict: + return {"shown": 0, "dropped_display": 0, "stale": 0, "invalid": 0} + def render_stats(self): return None @@ -315,9 +363,9 @@ def _run_session(make_window, record_root, *, with_3d: bool): view = None if with_3d: view = _RecordingView() - window.mesh3d_panel._view = view - else: - window.mesh3d_panel._view = None + window.mesh3d_panel.attach_view(view) + # With no pane the sink still runs (it is registered on the worker) and does + # nothing, which is the case the comparison below is about. window.toolbar.btn_record.setChecked(True) for index in range(96): stuck.emit(ui_check._fake_packet(index, mono_t=index / 30.0)) @@ -366,7 +414,7 @@ def test_the_3d_sink_is_registered_and_inert_when_the_pane_is_off(make_window, n assert window._on_packet_for_3d in stuck.sinks(), ( "the 3D sink must be registered with the worker, not attached per toggle" ) - window.mesh3d_panel._view = None + assert window.mesh3d_panel.view is None, "a box sensor must not build a view" for index in range(10): stuck.emit(ui_check._fake_packet(index)) assert window.session_store.count == 10 @@ -578,7 +626,7 @@ def test_a_recording_says_what_the_3d_pane_was( # metadata assembly something to describe (an enabled pane with a scene). view = _RecordingView() view.assets = None - window.mesh3d_panel._view = view + window.mesh3d_panel.attach_view(view) window.toolbar.btn_record.setChecked(True) for index in range(20): stuck.emit(ui_check._fake_packet(index, mono_t=index / 30.0)) @@ -673,6 +721,58 @@ def test_the_live_view_still_says_live(make_window, qt_app): window.close() +def test_every_run_gets_a_new_generation(make_window, qt_app): + """The window tells the pane when a session starts (see the audit's frozen-pane bug). + + Without this the pane accepts the first run's frames and refuses every later run's, + so the picture stays on the first run's last frame forever. + """ + from orisys_app_shared.scene3d.contracts import FrameIdentity + from tests.test_ui_smoke import _show + import ui_check + + window = _show(make_window(sensor_type="finger"), (1440, 880)) + try: + view = window.mesh3d_panel.view + if view is None or view.coordinator is None: + pytest.skip("no 3D view on this platform") + assert _pump(qt_app, lambda: view.assets is not None, timeout_s=15.0) + + stuck = ui_check._StuckWorker() + window._ensure_workers = lambda: None + window._box_worker = stuck + window._finger_worker = stuck + try: + generations = [] + for _ in range(2): + window._start_run() + window._run_timer.stop() + generations.append(view.coordinator.generation) + window._stop_run(reason="user_stop") + assert generations[0] < generations[1], generations + + # And the frames of the second session are accepted, not refused as stale. + identity = FrameIdentity( + source_id=view._source_id, + session_id=window._session_counter, + frame_id=1, + t_rel_s=0.0, + generation=view.coordinator.generation, + ) + shape = tuple(view.assets.shape_hw) + packet = ui_check._fake_packet(1) + packet.flow = np.full((*shape, 2), 0.5, dtype=np.float32) + view.submit_packet(packet) + assert view.coordinator.stale_frames == 0, ( + "the second session's frames were refused as stale" + ) + del identity + finally: + stuck.release() + finally: + window.close() + + def test_a_platform_locked_scene_is_refused_in_the_application(qt_app): """A lock violation is a *pane* failure: the view refuses the scene and says why.""" from orisys_app_shared.scene3d.contracts import PlatformInfo, PlatformLock diff --git a/apps/studio/tools/ui_check.py b/apps/studio/tools/ui_check.py index 6ed9bf7..9055927 100644 --- a/apps/studio/tools/ui_check.py +++ b/apps/studio/tools/ui_check.py @@ -2880,12 +2880,19 @@ def main() -> int: getattr(view_for_load, "assets", None) is not None, f"the installed 3D scene was never loaded: {pane.last_diagnostic}", ) - check( - not pane.showing_message, - "the pane is still showing a message although a scene loaded " - f"(loaded_path={pane._loaded_path}, source={getattr(pane._source, 'path', None)}, " - f"loaded={pane._loaded}, message={pane.message.text()[:40]!r})", - ) + body = pane.message.text() + if pane.showing_message: + # On a machine whose Qt cannot create a GL context this pane is *right* + # to show that instead of an empty picture — the offscreen plugin is + # exactly that case, and it is what the check runs under. What must not + # happen once the scene has loaded is a stale "loading…" or "no package" + # line, which is what this asserts. + check( + "context" in body.lower() or "OpenGL" in body or "三维" in body, + f"the pane shows a message although a scene loaded: {body[:60]!r}", + ) + else: + check(True, "the pane shows the loaded scene") if view_for_load is not None and view_for_load.panel is not None: source_text = view_for_load.panel.source_label.text() check(source_text.strip() != "", "the pane does not say where the field comes from")