From eb1dfbce8ae113b2e74a2d926472657b96f15204 Mon Sep 17 00:00:00 2001 From: ykcheungab Date: Mon, 14 Sep 2026 17:40:49 +0800 Subject: [PATCH] =?UTF-8?q?fix(studio):=20=E5=9B=9E=E6=94=BE=E8=A7=86?= =?UTF-8?q?=E5=9B=BE=E5=8A=A0=E4=B8=80=E4=B8=AA=E6=97=A0=E6=9D=A1=E4=BB=B6?= =?UTF-8?q?=E3=80=8C=E5=81=9C=E6=AD=A2=E3=80=8D=EF=BC=88Esc=20=E5=90=8C?= =?UTF-8?q?=E6=95=88=EF=BC=89=EF=BC=8C=E5=B9=B6=E8=AE=A9=E3=80=8C=E6=96=87?= =?UTF-8?q?=E4=BB=B6=E4=B8=80=E5=B8=A7=E9=83=BD=E4=B8=8D=E5=87=BA=E3=80=8D?= =?UTF-8?q?=E7=9B=B4=E6=8E=A5=E6=8A=A5=E9=94=99=E8=80=8C=E4=B8=8D=E6=98=AF?= =?UTF-8?q?=E4=B8=80=E7=9B=B4=E7=AD=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - What: `ToolbarWidget` 新增 `set_reset_role("reset"/"stop")`，回放视图把轨道第二个按钮变成「停止」； `StudioMainWindow._stop_replay_activity()` 一次停掉回放（play_timer）与「从视频重算」通道（worker）， 并清掉三维面板上的重算画面，状态栏说明"已停止了什么"；`_on_reset`/`_on_stop_shortcut`(Esc) 按角色路由； `_start_playback`/`_pause_playback` 结束时调用 `_update_run_enabled()`，让停止按钮在有东西可停时才可用； 两个采集 worker 新增 `FILE_FIRST_FRAME_TIMEOUT_S=10s`：文件源一帧都没出来就报错结束会话 （原先 `saw_frame` 为假时 EOF 分支永不执行，循环空转到天荒地老、窗口一直停在"连接中"）。 - Why: 用户报告"视频播放完之后无法再次按回放，卡住了"，并建议加手动停止。复现得到两种状态： ① 普通回放播到末尾后再按回放**是可以**重新从头播的（探针实测 0.967→0→0.371，计时器在跑）， 不是这条路径的问题；② 「从视频重算」通道跑文件时，工作线程可能既不出帧也不结束 （实测：1.5 s 的合成视频跑 25 s，packets=0 且 is_running=True），于是界面永远停在运行/连接状态， 操作者看到"卡住"。所以既给一个无论什么状态都能按的停止，也让"读不出帧"变成一条明确的错误。 - Verification: 新增/更新用例：`test_worker_frame_rate.py` 的"文件一帧都不出→error+消息"（缩短时限跑， 真实 worker）；`test_ui_smoke.py` 三项：回放视图的停止按钮（播放中可用、按下停回放、重算中停下重算、 无事可停时说明）、Esc 同样能停、实时视图仍是「重置」；apps/studio 全量通过；shared 全量通过； ui_check 583 项 0 失败（新增回放停止的 5 项检查，并给"场景已加载却仍在显示消息"那条加了诊断细节， 它此前偶发失败过一次、这次跑通）。 - Not in this change: 普通回放"播到末尾→再按回放"的路径（实测正常，未改）；让回放直接播放"非记录"的视频 （仍走「从视频重算」）；`_recompute_running` 与 worker 真实状态的进一步合并（现在停止按钮同时看两者）。 --- apps/studio/config/locales/en_US.json | 6 ++ apps/studio/config/locales/zh_CN.json | 6 ++ apps/studio/src/orisys_studio/app_window.py | 56 ++++++++++- apps/studio/src/orisys_studio/ui/toolbar.py | 31 ++++++- apps/studio/tests/test_ui_smoke.py | 92 +++++++++++++++++++ apps/studio/tools/ui_check.py | 43 ++++++++- .../acquisition/finger_worker.py | 15 +++ .../acquisition/sdk_worker.py | 20 ++++ shared/tests/test_worker_frame_rate.py | 37 ++++++++ 9 files changed, 302 insertions(+), 4 deletions(-) diff --git a/apps/studio/config/locales/en_US.json b/apps/studio/config/locales/en_US.json index 300bbab..fe622ff 100644 --- a/apps/studio/config/locales/en_US.json +++ b/apps/studio/config/locales/en_US.json @@ -104,6 +104,12 @@ "playback_disabled": "Open a recorded video first (“Choose a recorded video…” at the top left)", "playback_needs_recording": "This video has no recording beside it (no forces.csv): replay needs a recording; to just watch the video use View ▸ Recompute forces from the video", "reset": "Reset", + "stop_playback": "Stop", + "stop_playback_tip": "Stop the replay: plain playback and a “recompute from the video” pass both stop here (Esc does the same).", + "stopped": "Stopped: {what}", + "stopped_playback": "playback", + "stopped_pass": "the recompute pass", + "nothing_to_stop": "Nothing is playing or recomputing", "units": "Units", "units_newton": "Newton", "units_raw": "Raw", diff --git a/apps/studio/config/locales/zh_CN.json b/apps/studio/config/locales/zh_CN.json index 0ba546e..ef23331 100644 --- a/apps/studio/config/locales/zh_CN.json +++ b/apps/studio/config/locales/zh_CN.json @@ -104,6 +104,12 @@ "playback_disabled": "先打开一段录制视频（左上「选择录制视频…」）", "playback_needs_recording": "这段视频旁边没有记录（缺 forces.csv）：回放需要一条记录；只想看视频请用 视图 ▸ 从视频重算力值", "reset": "重置", + "stop_playback": "停止", + "stop_playback_tip": "停止回放：无论是普通播放还是「从视频重算」通道，都在这里停下（Esc 同效）。", + "stopped": "已停止：{what}", + "stopped_playback": "回放", + "stopped_pass": "重算通道", + "nothing_to_stop": "当前没有正在进行的回放或重算", "units": "单位", "units_newton": "牛顿", "units_raw": "原始", diff --git a/apps/studio/src/orisys_studio/app_window.py b/apps/studio/src/orisys_studio/app_window.py index e51b9b2..1722890 100644 --- a/apps/studio/src/orisys_studio/app_window.py +++ b/apps/studio/src/orisys_studio/app_window.py @@ -1033,6 +1033,9 @@ class StudioMainWindow(QMainWindow): self._play_timer.start() self.toolbar.set_run_checked(True) self.toolbar.refresh_labels() + # The stop button appears with the playback: one place decides what the rail + # offers, so it is told here rather than kept in step in two places. + self._update_run_enabled() def _pause_playback(self) -> None: if self._play_timer.isActive(): @@ -1040,6 +1043,7 @@ class StudioMainWindow(QMainWindow): if self.toolbar.btn_run.isChecked(): self.toolbar.set_run_checked(False) self.toolbar.refresh_labels() + self._update_run_enabled() def _on_play_tick(self) -> None: """Advance the playhead from the wall clock, not from the tick count.""" @@ -1486,8 +1490,10 @@ class StudioMainWindow(QMainWindow): self._pause_playback() self.toolbar.set_source_view("replay" if preset == "replay" else "live") # The rail's primary button is this view's main action: acquire while - # measuring, play back while replaying. + # measuring, play back while replaying; and the second button is that view's + # own reset — or, in the replay view, an unconditional stop. self.toolbar.set_run_role("playback" if preset == "replay" else "acquire") + self.toolbar.set_reset_role("stop" if preset == "replay" else "reset") self._apply_panel_visibility() if apply_sizes and preset in VIEW_SPLITTER_SIZES: self.content_splitter.setSizes(self._splitter_sizes_for(preset)) @@ -1529,6 +1535,14 @@ class StudioMainWindow(QMainWindow): reason = t("toolbar.playback_disabled") self.toolbar.set_run_tooltip_reason(reason) self.toolbar.set_run_enabled(loaded or self._play_timer.isActive()) + # The stop button is offered whenever there is something to stop; the + # operator must never have to guess whether that is the case. + if self.toolbar.reset_role() == "stop": + self.toolbar.set_reset_enabled( + self._play_timer.isActive() + or self._recompute_running + or self._session_active + ) self.toolbar.set_record_enabled( self._running or self.recorder.is_recording ) @@ -2272,7 +2286,16 @@ class StudioMainWindow(QMainWindow): self.toolbar.btn_run.setChecked(not self.toolbar.btn_run.isChecked()) def _on_stop_shortcut(self) -> None: - """Esc must always be able to cancel a session, even a stuck connect.""" + """Esc must always be able to cancel a session, even a stuck connect. + + In the replay view it is the same escape hatch as the rail's 停止 button: a + replay or a recompute pass that will not stop by itself has to be stoppable + from the keyboard. + """ + if self.toolbar.run_role() == "playback": + if self._play_timer.isActive() or self._session_active: + self._stop_replay_activity() + return if not self._session_active: return self._stop_run(reason="user_stop") @@ -2576,7 +2599,36 @@ class StudioMainWindow(QMainWindow): else: self.statusBar().showMessage(t("status.run_stopped")) + def _stop_replay_activity(self) -> None: + """Stop whatever the replay view is doing. The operator's escape hatch. + + Two different things can be running there — plain playback of the recorded + timeline, and a recompute pass over the video — and the person in front of the + screen should not have to work out which one is stuck before they can stop it. + So this stops both, clears a recomputed 3D picture, and says what it stopped. + Safe to press when nothing is running (it says so rather than doing nothing + silently, which is what made a stuck state feel stuck). + """ + stopped: list[str] = [] + if self._recompute_running or self._session_active: + self._stop_run(reason="user_stop") + stopped.append(t("toolbar.stopped_pass")) + if self._play_timer.isActive(): + self._pause_playback() + stopped.append(t("toolbar.stopped_playback")) + self.mesh3d_panel.show_reference_only() + self._update_run_enabled() + if stopped: + self._notify(t("toolbar.stopped", what=", ".join(stopped))) + else: + self._notify(t("toolbar.nothing_to_stop")) + def _on_reset(self) -> None: + if self.toolbar.reset_role() == "stop": + # In the replay view that button is the operator's way out of a replay or + # a recompute pass, whatever state the transport is in. + self._stop_replay_activity() + return if self._session_phase != "running" or self._active_worker is None: return try: diff --git a/apps/studio/src/orisys_studio/ui/toolbar.py b/apps/studio/src/orisys_studio/ui/toolbar.py index 81ed81f..97c7339 100644 --- a/apps/studio/src/orisys_studio/ui/toolbar.py +++ b/apps/studio/src/orisys_studio/ui/toolbar.py @@ -107,6 +107,7 @@ class ToolbarWidget(QWidget): self._video_path = "" #: ``acquire`` while measuring, ``playback`` while replaying a recording. self._run_role = "acquire" + self._reset_role = "reset" outer = QVBoxLayout(self) outer.setContentsMargins(2, 2, 2, 2) @@ -387,6 +388,34 @@ class ToolbarWidget(QWidget): def run_role(self) -> str: return self._run_role + def set_reset_role(self, role: str) -> None: + """What the second rail button does: zero the tracker, or stop the replay. + + The replay view needs a *stop* the operator can always reach. The transport + button is a play/pause toggle, which is ambiguous precisely when something is + wrong (is it playing? is a recompute pass running?), so the second button — + disabled in that view otherwise — becomes an unconditional stop instead of a + second thing to reason about. + """ + wanted = "stop" if str(role) == "stop" else "reset" + if wanted == self._reset_role: + return + self._reset_role = wanted + if wanted == "reset": + self.btn_reset.setEnabled(False) + self._refresh_reset_label() + + def reset_role(self) -> str: + return self._reset_role + + def _refresh_reset_label(self) -> None: + if self._reset_role == "stop": + self.btn_reset.setText(t("toolbar.stop_playback")) + self.btn_reset.setToolTip(t("toolbar.stop_playback_tip")) + return + self.btn_reset.setText(t("toolbar.reset")) + self.btn_reset.setToolTip("") + def source_view(self) -> str: return self._source_view @@ -686,7 +715,7 @@ class ToolbarWidget(QWidget): self.btn_use_camera.setText(t("toolbar.use_camera")) self.btn_replay_video.setText(t("toolbar.choose_video")) self.refresh_labels() - self.btn_reset.setText(t("toolbar.reset")) + self._refresh_reset_label() self.units_label.setText(t("toolbar.units")) self.units_combo.setItemText(0, t("toolbar.units_newton")) self.units_combo.setItemText(1, t("toolbar.units_raw")) diff --git a/apps/studio/tests/test_ui_smoke.py b/apps/studio/tests/test_ui_smoke.py index 58186a1..cc288a7 100644 --- a/apps/studio/tests/test_ui_smoke.py +++ b/apps/studio/tests/test_ui_smoke.py @@ -1994,6 +1994,98 @@ def test_the_recompute_mode_reruns_the_pipeline_over_the_video( assert not window.toolbar.btn_run.isChecked(), "the transport stayed checked" +def test_the_replay_view_offers_a_stop_that_always_works(make_window, replay_recording): + """The operator must not have to work out *what* is stuck before stopping it. + + The transport is a play/pause toggle, which is ambiguous exactly when something is + wrong. So the rail's second button becomes an unconditional stop in this view, and + it covers both things that can run there: plain playback and a recompute pass. + """ + from PyQt5.QtWidgets import QApplication + + from orisys_app_shared.i18n import t + + window = _show(make_window(), (1440, 880)) + window.brand_bar.btn_playback.click() + QApplication.processEvents() + assert window._load_recording(replay_recording) + QApplication.processEvents() + + assert window.toolbar.reset_role() == "stop", ( + "the replay view kept the tracker reset button instead of a stop" + ) + assert window.toolbar.btn_reset.text() == t("toolbar.stop_playback") + assert not window.toolbar.btn_reset.isEnabled(), ( + "a stop is offered while nothing is running" + ) + + # Plain playback: the stop button appears while it plays and pressing it stops. + window.toolbar.btn_run.setChecked(True) + QApplication.processEvents() + assert window._play_timer.isActive() + assert window.toolbar.btn_reset.isEnabled(), "no stop offered while playing" + window.toolbar.btn_reset.click() + QApplication.processEvents() + assert not window._play_timer.isActive(), "the stop did not stop the playback" + assert not window.toolbar.btn_run.isChecked(), "the transport still shows playing" + assert t("toolbar.stopped_playback") in window.statusBar().currentMessage(), ( + window.statusBar().currentMessage() + ) + + # A recompute pass: the same button stops it, without stopping the recording. + stopped: list = [] + window._stop_run = lambda **kwargs: stopped.append(kwargs.get("reason")) + window._recompute_running = True + window._update_run_enabled() + assert window.toolbar.btn_reset.isEnabled(), "no stop offered while a pass runs" + window.toolbar.btn_reset.click() + QApplication.processEvents() + assert stopped == ["user_stop"], stopped + assert t("toolbar.stopped_pass") in window.statusBar().currentMessage(), ( + window.statusBar().currentMessage() + ) + + # Nothing running: it says so instead of doing nothing at all. + # (The stubbed ``_stop_run`` above records the reason but does not clear the flag + # the real one clears, so the state is completed by hand here.) + window._recompute_running = False + window.toolbar.btn_reset.click() + QApplication.processEvents() + assert t("toolbar.nothing_to_stop") in window.statusBar().currentMessage(), ( + window.statusBar().currentMessage() + ) + window.close() + + +def test_escape_stops_the_replay_too(make_window, replay_recording): + """Esc is the documented way out; in the replay view it stops the replay.""" + from PyQt5.QtWidgets import QApplication + + window = _show(make_window(), (1440, 880)) + window.brand_bar.btn_playback.click() + QApplication.processEvents() + assert window._load_recording(replay_recording) + QApplication.processEvents() + window.toolbar.btn_run.setChecked(True) + QApplication.processEvents() + assert window._play_timer.isActive() + window.act_stop.trigger() + QApplication.processEvents() + assert not window._play_timer.isActive(), "Esc did not stop the playback" + assert not window.toolbar.btn_run.isChecked() + window.close() + + +def test_the_live_view_keeps_the_tracker_reset(make_window): + from PyQt5.QtWidgets import QApplication + + window = _show(make_window(), (1440, 880)) + window.brand_bar.btn_live.click() + QApplication.processEvents() + assert window.toolbar.reset_role() == "reset" + window.close() + + def test_a_video_that_is_not_a_recording_is_refused_with_a_reason( make_window, record_root ): diff --git a/apps/studio/tools/ui_check.py b/apps/studio/tools/ui_check.py index 5c47896..6ed9bf7 100644 --- a/apps/studio/tools/ui_check.py +++ b/apps/studio/tools/ui_check.py @@ -2882,7 +2882,9 @@ def main() -> int: ) check( not pane.showing_message, - "the pane is still showing a message although a scene loaded", + "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})", ) if view_for_load is not None and view_for_load.panel is not None: source_text = view_for_load.panel.source_label.text() @@ -3076,6 +3078,45 @@ def main() -> int: "the rail's primary button is not the replay's transport", ) check(replay_window.toolbar.btn_run.isEnabled(), "the replay has no transport") + + # The replay view offers an unconditional stop (the rail's second button), because + # the transport is a play/pause toggle and "which one is stuck?" is not a question + # to ask an operator in front of a frozen picture. + check( + replay_window.toolbar.reset_role() == "stop", + "the replay view has no stop button", + ) + check( + replay_window.toolbar.btn_reset.text() == t("toolbar.stop_playback"), + f"the stop button is not labelled as one: " + f"{replay_window.toolbar.btn_reset.text()!r}", + ) + # Start playing so there is something to stop, then stop it with that button. + replay_window.toolbar.btn_run.setChecked(True) + app.processEvents() + check( + replay_window._play_timer.isActive(), + "the playback did not start, so the stop check would prove nothing", + ) + check( + replay_window.toolbar.btn_reset.isEnabled(), + "the stop is not offered while the transport is playing", + ) + replay_window.toolbar.btn_reset.click() + app.processEvents() + check( + not replay_window.toolbar.btn_run.isChecked(), + "the stop did not stop the transport", + ) + check( + not replay_window._play_timer.isActive(), + "the stop left the playback timer running", + ) + check( + t("toolbar.stopped_playback") in replay_window.statusBar().currentMessage(), + f"the stop did not say what it stopped: " + f"{replay_window.statusBar().currentMessage()!r}", + ) check( not replay_window.toolbar.btn_run.isChecked(), "the replay starts while playing", diff --git a/shared/src/orisys_app_shared/acquisition/finger_worker.py b/shared/src/orisys_app_shared/acquisition/finger_worker.py index 9eb2062..2333eda 100644 --- a/shared/src/orisys_app_shared/acquisition/finger_worker.py +++ b/shared/src/orisys_app_shared/acquisition/finger_worker.py @@ -23,6 +23,7 @@ from orisys_app_shared.acquisition.models import ( ) from orisys_app_shared.acquisition.sdk_worker import ( CAMERA_DISCONNECT_TIMEOUT_S, + FILE_FIRST_FRAME_TIMEOUT_S, FPS_WINDOW_S, HANDOVER_TIMEOUT_S, STOP_JOIN_TIMEOUT_S, @@ -417,6 +418,8 @@ class FingerAcquisitionWorker(RecordSinkMixin): # Per-session monotonic frame index: the join key shared by the video, # the CSV and the calibration records. frame_id = 0 + #: When this session's loop started, for the file-source deadline below. + session_started = time.perf_counter() while not self._stop.is_set() and session == self._session_id and runtime is not None: try: @@ -432,6 +435,18 @@ class FingerAcquisitionWorker(RecordSinkMixin): if (now - last_frame_mono) >= CAMERA_DISCONNECT_TIMEOUT_S: self._set_end_reason("disconnected") break + elif is_file and (now - session_started) >= FILE_FIRST_FRAME_TIMEOUT_S: + # The same deadline as the box worker: a file that never yields a + # frame is an error to report, not a "connecting" state to sit in. + message = ( + f"this video produced no frames in " + f"{FILE_FIRST_FRAME_TIMEOUT_S:.0f} s: {source} may be " + "unreadable, truncated, or in a codec this build cannot decode" + ) + if self._on_error is not None and session == self._session_id: + self._on_error(message) + self._set_end_reason("error") + break t0 = time.perf_counter() ok, source_frame = runtime.read_source() diff --git a/shared/src/orisys_app_shared/acquisition/sdk_worker.py b/shared/src/orisys_app_shared/acquisition/sdk_worker.py index cf757ac..88b5e56 100644 --- a/shared/src/orisys_app_shared/acquisition/sdk_worker.py +++ b/shared/src/orisys_app_shared/acquisition/sdk_worker.py @@ -31,6 +31,13 @@ from orisys_app_shared.acquisition.models import ( from orisys_app_shared.acquisition.sinks import RecordSinkMixin VIDEO_EOF_TIMEOUT_S = 0.5 +#: How long a file source may deliver *nothing at all* before the session gives up. +#: The end-of-file branch needs at least one frame to have arrived (``saw_frame``), so +#: a video this build cannot decode — a wrong codec, a truncated file, a text file +#: renamed to .mp4 — used to loop forever: no frames, no error, and the window sat in +#: "connecting" until somebody pressed Stop. A failed open is a fact to report, not a +#: state to wait in. +FILE_FIRST_FRAME_TIMEOUT_S = 10.0 CAMERA_DISCONNECT_TIMEOUT_S = 1.5 #: How long a *blocking* ``stop(wait=True)`` may hold the caller. STOP_JOIN_TIMEOUT_S = 5.0 @@ -851,6 +858,9 @@ class SdkAcquisitionWorker(RecordSinkMixin): # the CSV and the calibration records. frame_id = 0 + #: When this session's loop started, for the file-source deadline below. + session_started = time.perf_counter() + while not self._stop.is_set() and session == self._session_id: try: now = time.perf_counter() @@ -865,6 +875,16 @@ class SdkAcquisitionWorker(RecordSinkMixin): if (now - last_frame_mono) >= CAMERA_DISCONNECT_TIMEOUT_S: self._set_end_reason("disconnected") break + elif is_file and (now - session_started) >= FILE_FIRST_FRAME_TIMEOUT_S: + message = ( + f"this video produced no frames in " + f"{FILE_FIRST_FRAME_TIMEOUT_S:.0f} s: {source} may be " + "unreadable, truncated, or in a codec this build cannot decode" + ) + if self._on_error is not None and session == self._session_id: + self._on_error(message) + self._set_end_reason("error") + break t0 = time.perf_counter() # ``t0`` is when this frame was *collected*: the device's own rate diff --git a/shared/tests/test_worker_frame_rate.py b/shared/tests/test_worker_frame_rate.py index 6dc53c0..b3747e1 100644 --- a/shared/tests/test_worker_frame_rate.py +++ b/shared/tests/test_worker_frame_rate.py @@ -373,6 +373,43 @@ def test_a_file_source_runs_on_the_video_s_own_timeline(): ) +def test_a_file_that_yields_no_frames_ends_the_session_with_an_error(monkeypatch): + """A video this build cannot decode must report it, not wait forever. + + The end-of-file branch needs at least one frame to have arrived (``saw_frame``), + so a file that never produces one used to leave the loop spinning: no frames, no + error, and the window in "connecting" until somebody pressed Stop. The shipping + deadline is ten seconds; it is shortened here, because the point is the behaviour + rather than the wait. + """ + import orisys_app_shared.acquisition.sdk_worker as worker_module + + monkeypatch.setattr(worker_module, "FILE_FIRST_FRAME_TIMEOUT_S", 0.3) + # A sensor that never produces a frame: what an unreadable codec looks like. + monkeypatch.setattr(_Sensor, "get_img", lambda self: None) + + CREATED.clear() + saved = _install() + errors: list = [] + packets: list = [] + worker = worker_module.SdkAcquisitionWorker(on_error=errors.append) + try: + worker.add_sink(packets.append) + worker.start("unreadable.mp4", config_ref=BOX_CONFIG) + deadline = time.perf_counter() + 10.0 + while time.perf_counter() < deadline and worker.is_running: + time.sleep(0.05) + assert not worker.is_running, ( + "the worker is still running on a file that produced no frames" + ) + assert worker.consume_end_reason() == "error", worker.consume_end_reason() + assert packets == [] + assert errors and "no frames" in errors[0], errors + finally: + worker.stop(wait=True) + _restore(saved) + + if __name__ == "__main__": # pragma: no cover - manual runner failures = 0 for name, function in sorted(globals().items()):