From 17c21301188b6bcbc732ca285d52b577b99abe19 Mon Sep 17 00:00:00 2001 From: Adam Clemens Date: Wed, 9 Sep 2026 15:36:53 +0100 Subject: [PATCH] Add live scrub: keyboard and mouse seeking for pyflow play (TASK-048) Closes Stage 8's Completion Criterion 6, the one clause of the stage's own Goal ("scrubbed to any point") that shipped without a mechanism at all. Left/Right step one frame, Home/End jump to the loaded window's own edges, and a draggable scrub bar reaches any frame in between directly -- all scoped to the window already materialized at launch. The task's own biggest open risk -- a scrub-bar drag also panning the camera underneath it, since RenderWindow.run() wires its own pan handlers on the same canvas -- resolved in favour of the primary approach rather than the stated fallback: rendercanvas's own EventEmitter.emit dispatches handlers by order then registration order and stops once a handler sets event["stop_propagation"], confirmed live with two handlers on one canvas before being relied on. The scrub bar's own handlers register at order=-1, one level above the pan handlers' default order=0, so a drag on the bar suppresses panning for that gesture with no change to window.py needed at all. Building the bar also needed an absolute screen-to-world mapping nothing in this codebase had (_update_pan only ever tracked a delta) -- screen_to_world, verified against a real rendered marker at a known world position before being trusted. Two real glfw-window integration tests, with genuine injected keyboard and pointer events, prove both that seeking works and -- the literal claim this task exists to prove -- that the camera does not move at all during a scrub drag. Co-Authored-By: Claude Sonnet 5 --- README.md | 24 ++- docs/CHANGELOG-DESIGN.md | 286 +++++++++++++++++++++++++ docs/planning/roadmap.md | 119 +++++++--- docs/planning/status.md | 14 +- src/pyflow/CLAUDE.md | 43 ++++ src/pyflow/playback.py | 176 ++++++++++++++- src/pyflow/rendering/CLAUDE.md | 39 ++++ src/pyflow/rendering/window.py | 34 +++ tests/integration/test_playback_cli.py | 204 +++++++++++++++++- tests/unit/test_playback.py | 64 ++++++ tests/unit/test_rendering.py | 45 ++++ 11 files changed, 1001 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index 6de86df..db1d5f6 100644 --- a/README.md +++ b/README.md @@ -141,8 +141,8 @@ with no seek mechanism at all, and two of the stage's own stated deferrals (declared-field playback, partial-overlap cache reuse) plus one gap nobody had named (checkpoint retention) were pulled forward rather than left indefinitely deferred. TASK-049 closed the retention -gap and TASK-050 the cache-reuse one, both the same day; TASK-048/051 -will close the other two, one branch each. Its live status, generated +gap, TASK-050 the cache-reuse one, and TASK-048 the scrub one, all the +same day; TASK-051 closes the last one. Its live status, generated from the roadmap rather than restated here: [Stage 8 in the status report](docs/planning/status.md#stage-8----recording--playback). @@ -188,7 +188,7 @@ golden demo renders a *solved* velocity field live. **Stage 6 is the proof that the engine underneath it is field-centric**: four named physical fields, added by configuration. -Stages 0 through 7 are complete, and Stage 8 is reopened (seven of nine +Stages 0 through 7 are complete, and Stage 8 is reopened (eight of nine criteria met, see below) -- each closed, or in Stage 8's case pending re-closure, against its own written completion criteria (`docs/planning/roadmap.md`): @@ -254,7 +254,7 @@ re-closure, against its own written completion criteria added 93 step definitions, 28% of the repository's whole step vocabulary, which is evidence against its own claim rather than for it. -**Stage 8 (Recording & Playback) is reopened -- seven of nine criteria +**Stage 8 (Recording & Playback) is reopened -- eight of nine criteria met.** `pyflow record`/`pyflow resume`/`pyflow play` (TASK-045/046/047, all 2026-09-07): record a run headlessly, resume it from any checkpoint, or watch it back in a real window with live pause and speed @@ -267,10 +267,12 @@ Status section for the full account). **Reopened 2026-09-09** for four more criteria an audit found the Goal itself already promised: live scrub (keyboard and a mouse-draggable bar), combined solved-velocity + declared-field playback (grounded in Smoke Transport), opt-in checkpoint -retention, and partial-overlap cache reuse. The last two are done -(TASK-049, `--max-checkpoints-retained` on `record`/`resume`; TASK-050, -`pyflow play --cache DIR` now reuses a full-subset request from a wider -cached window with no re-simulation); TASK-048/051 are not yet built. +retention, and partial-overlap cache reuse. Three are done (TASK-049, +`--max-checkpoints-retained` on `record`/`resume`; TASK-050, `pyflow +play --cache DIR` now reuses a full-subset request from a wider cached +window with no re-simulation; TASK-048, Left/Right/Home/End and a +draggable scrub bar, verified against a real window to never pan the +camera underneath a drag); TASK-051 is not yet built. Try the whole pipeline as it stands today: ```bash @@ -312,7 +314,11 @@ the range asked for: uv run python -m pyflow play --checkpoints-dir checkpoints --to-frame 500 ``` -Space pauses/resumes; `+`/`-` change playback speed live. Add +Space pauses/resumes; `+`/`-` change playback speed live; Left/Right +step one frame and Home/End jump to the loaded window's own edges; +drag the on-screen scrub bar to seek to any frame in between directly +(TASK-048) -- the drag never pans the camera underneath it, verified +against a real window, not just asserted. Add `--cache cache` to materialize the window once and reuse it on a later run without re-simulating -- a later request fully inside an already-cached range reuses it too, sliced directly, even if its own diff --git a/docs/CHANGELOG-DESIGN.md b/docs/CHANGELOG-DESIGN.md index 3610bcf..c0b4c27 100644 --- a/docs/CHANGELOG-DESIGN.md +++ b/docs/CHANGELOG-DESIGN.md @@ -7131,3 +7131,289 @@ Diffusion ICD still gave the configuration control as The ICD named a field no run could have used. That task's own Blast Radius sweep reached the generator's `FIELD_COMMENTS` and not this document. + +## 07-09-2026 + +### Stage 8 (Recording & Playback) opened and closed the same day: TASK-045 (Periodic Checkpointing) + +Raised by the maintainer 2026-09-04 (`docs/planning/backlog.md`) as a +side comment while scoping unrelated work, not scheduled until this +day's decision to open it as a Stage of its own rather than fold it into +Stage 14 (Performance) -- see the roadmap's own "Stages and Capability +Levels" table, Fourth divergence. Goal: let a simulation's state be +recorded to disk as it runs and played back afterward -- paused, +scrubbed to any point, watched at a different speed -- without the +original process staying alive. Five Completion Criteria were drafted +the same day as TASK-045, this stage's own first task, rather than +independently from the Goal beforehand -- a gap this file's own +2026-09-09 entry, below, found and corrected two days later. + +**TASK-045 scoped to recording only, not replay or playback -- a stated +exclusion, not an oversight**, the same split TASK-034 already +precedented (it built the timestepping loop checkpointing needs and +declined to build checkpointing itself). Two research findings changed +the design from how the backlog item first framed it: `PressureField` +never appears in `window.simulation_fields` (a return value, never fed +back into state), so the checkpoint format needs no per-field type tag +-- every field is a plain tensor; and nothing in the codebase uses RNG +or a non-CPU device anywhere (checked by grep), so determinism after +reload is purely mesh + field tensors + config. + +**`bootstrap.py`'s simulation-state construction was extracted into +`simulation_run.py` before anything new was written, and verified +behaviour-preserving by the full pre-existing suite passing unmodified +(1052 tests, same count and pass as before)** -- a genuinely headless +recording path needed this pulled out of `RenderWindow`'s own closures, +since `RenderWindow.__init__` unconditionally builds a real `wgpu` +renderer. Checkpoint format: one `torch.save`d file per checkpoint, +self-contained (`dataclasses.asdict(config)`, not a pickled +`PyFlowConfig` instance, avoiding a `weights_only=False` +code-execution surface), `weights_only=True`-loadable. Headless is +structural, not a default -- `recording.py` never imports +`rendering`/`pygfx`/`rendercanvas` at all, checked directly by +`test_import_order.py` in a fresh subprocess. + +**A real architectural gap found mid-implementation**: a checkpoint's +raw tensors alone cannot resume a "passive"-mode run, since the +prescribed `velocity_field` is never checkpointed. `restore_simulation_ +state` calls `build_simulation_state` again from the checkpoint's +embedded config for structure, then overwrites `.fields` with the +checkpoint's real values -- written test-first, red before +`restore_simulation_state` existed. The determinism claim +(`test_recording_determinism.py`, `rtol=0, atol=0`) was confirmed to +have real teeth by deliberately corrupting `restore_simulation_state` +(multiplying tensors by `0.0`) and observing a reported 20/20 mismatch +before reverting. + +**Extended the same day with `pyflow resume`**, once a user asked how a +second run would ingest a checkpoint to continue the simulation -- still +recording's own scope, not replay (no rendering, no dense per-frame +materialization). Shares `_advance_and_checkpoint` with `record` rather +than duplicating the policy, confirmed to actually share behaviour by a +deliberate off-by-one mutation (`start_frame + 1` weakened to +`start_frame`): 8 of the then-11 recording tests failed, across both +functions' own test cases. No Gherkin `.feature` file -- this task +discharges no Golden Demo criterion on its own, and its one physical +claim (resuming reproduces the same trajectory) is a +serialization-fidelity claim, the category `adr/ADR-007-executable- +acceptance-criteria.md`'s own scope excludes. + +Discharges Completion Criteria 1-4 and the record half of 5. + +### TASK-046 (Deterministic Windowed Replay) and TASK-047 (`pyflow play`): six maintainer decisions before any code, and Stage 8 closes with five of five criteria met + +Drafted and built together -- one CLI command (`pyflow play`), not two +-- sharing one design session, one branch and one review cycle, the +precedent TASK-031's four subtasks already set. Kept as two roadmap +entries because they are separable concerns with their own +dependencies: TASK-046 (`replay.py`) has no dependency on `rendering` +at all. + +**Four decisions were the maintainer's own, asked directly with a +recommendation and trade-off named for each, before implementation** +(`docs/practices.md`'s design-session rule): (1) ephemeral by default +with an optional disk cache, exact-range match only -- partial-overlap +reuse named explicitly as a real, deferred design question, not built +here; (2) auto-discover the nearest checkpoint +(`find_checkpoint_at_or_before`) rather than an explicit path, ranking +candidates by filename first and cross-checking the winner's real +`frame_count`; (3) memory footprint measured directly, not assumed -- +2.05 MB at the golden demo's own mesh (256 cells x 2 fields x 500 +frames), 9.83 MB at 4,096 cells x 3 fields x 100 frames, extrapolating +to ~197 MB / ~786 MB at the largest mesh this repository runs, +comfortably under a gigabyte with no cap added for this first cut; (4) +the determinism claim checked against every materialized frame, not +only the last -- a first draft checked only the final frame from a +`from_frame` that happened to make the fast-forward/discard step a +no-op, and a deliberate off-by-one mutation in that discard loop left +every test green until it was rewritten to check per-frame values +(16/16 mismatched elements once it was). + +**Two more maintainer decisions for TASK-047**: live keyboard +interaction, not fixed CLI flags (Space pauses, `+`/`-` doubles/halves +speed, clamped `0.125`-`8.0`), verified to coexist with +`RenderWindow.run`'s own `close_keys` handler before being relied on; +and scoped to solved-velocity-only rendering for this first cut +(`UnsupportedPlaybackConfigError` names the gap loudly rather than +rendering an empty scene), declared-field/scalar-colormap playback left +as real, deferred work. + +**A seventh finding, empirical rather than decided**: scene-rebuild cost +was measured directly before designing the speed mechanism -- 3.05ms at +256 cells (a 327fps ceiling), 31.66ms at 4,096 cells (comparable to a +30fps frame budget by itself). This is why `PlaybackState.speed` +advances the fractional frame *position* per real draw rather than +trying to draw more often: real draw rate stays capped near ~30fps by +rebuild cost alone at larger mesh sizes regardless of what speed is +asked for. A real event-queue latency was found while writing the pause +test, not anticipated: a key event submitted during frame N's own +callback takes effect from frame N+2's rendered content, one frame later +than a synchronous model would predict -- confirmed directly (printed +the frame-hash sequence) before adjusting the test's assertion window. + +**The stage's own Golden Demo was reconciled from Heat Diffusion to +Lid-Driven Cavity, both halves, when TASK-047 was scoped** -- TASK-045's +own original choice does not survive contact with TASK-047's playback +rendering a solved velocity field as arrows, and Heat Diffusion declares +a transported scalar with no solved velocity at all +(`UnsupportedPlaybackConfigError` on it, not a demo). + +**1131 tests as of 2026-09-07, up from 1052 the day before** -- 79 from +this stage's work: 49 from TASK-045 (33 original recording, 16 from +`resume`), 30 from TASK-046/047's windowed-replay/playback addition. +Stage 8 was marked complete the same day, five of five criteria met. +(Reopened two days later, 2026-09-09 -- see below.) + +## 09-09-2026 + +### Stage 8 reopened: an audit confirms it skipped its own design session, four new completion criteria + +Run at the maintainer's own request, against the suspicion that Stage 8 +"never actually went through a design/planning session" -- confirmed. +Raised 2026-09-04 as a side comment, opened and fully built in a single +day (2026-09-07) with its five Completion Criteria written the same day +as TASK-045, its own first task, rather than independently derived from +the Goal beforehand the way the stage's own text originally claimed -- +the exact shape `docs/planning/stage-specification.md` warns against (a +criterion that cannot fail if the task that wrote it passed). + +**One real, previously unrecorded gap**: the Goal's own "paused, +scrubbed to any point, and watched at a different speed" was never +operationalised by any of the five original criteria, so nothing caught +`PlaybackState` landing with pause and speed but no seek at all. **Three +further gaps, named by TASK-046/047 themselves as deliberate stated +deferrals, not oversights, were pulled forward into this stage rather +than left recorded-but-deferred indefinitely**: declared-field/ +scalar-colormap playback, partial-overlap cache reuse, and checkpoint +retention (a gap in what "bounded footprint" means that nobody had +named as a gap at all, until this audit). + +Completion Criteria 6-9 added -- live scrub (keyboard and mouse, checked +against real rendered pixels, scoped to the window already requested at +launch), combined solved-velocity + declared-field playback (grounded in +Smoke Transport), checkpoint retention (opt-in, frame 0 never pruned), +and partial-overlap cache reuse (full-subset only, partial overlap still +falls back to full materialization) -- drafted the same way 1-5 were +meant to be: from the Goal, independent of the four tasks (TASK-048-051) +that will discharge them, with their design decisions settled ahead of +implementation. Order: TASK-049/050 (the two library-only changes) land +before TASK-048/051 (the two that touch rendering). + +**"Complete" reopened, not "complete" corrected** -- the original five +criteria were genuinely met by what TASK-045/046/047 built; nothing +about that record is retracted. `docs/planning/stage-specification.md` +also now sanctions "Status: Not started, drafted " as a valid +task-entry status, now that `check_stages.py`'s own `TASK_DONE` marker +already handles it correctly. + +### Stage 0's eleven tasks were reading as incomplete: a `check_stages.py` lifecycle gap fixed + +`check_stages.py`'s lifecycle computation greps each task for an inline +`**Status: Done**` marker; Stage 0's eleven tasks predate that +convention and had only ever recorded completion in the stage's own +summary table. That made `Stage.lifecycle` read `opened` with 0/11 done, +silently wrong since Stage 0 actually closed 2026-08-19 -- three weeks +of every `make check-stages` run passing over a wrong answer for the +project's own first stage. + +Fixed by adding the marker to each of the eleven task entries (dates +taken from the existing summary table; TASK-008 marked Done with no +date, since none was ever recorded), rather than special-casing Stage 0 +inside the checker, so the one general mechanism stays correct for every +stage rather than gaining an exemption. A regression test +(`test_the_real_roadmap_reports_stage_0_as_complete`, +`tests/unit/test_check_stages.py`) was confirmed red before the fix +landed. 1172 tests as of 2026-09-09, up from 1171 the day before. + +### TASK-049 (Checkpoint Retention Policy) and TASK-050 (Partial-Overlap Cache Reuse): the two library-only reopening criteria, closed first + +Both settled directly with the maintainer when the stage was reopened, +before implementation -- the same "ask directly, with a recommendation +and the trade-off named" discipline TASK-046/047 already used -- and +both landed ahead of TASK-048/051 per the stage's own stated ordering +(library-only before rendering-touching). + +**TASK-049 closes the gap between Criterion 2's "never one file per +frame" (bounds the interval between checkpoints) and what "bounded +footprint" actually needs on a very long run (bounds the total).** +`RecordingConfig.max_checkpoints_retained: int | None = None` -- opt-in, +unbounded by default, every existing config and golden demo keeps +writing exactly what it always did. Frame 0 is never pruned whatever the +cap, confirmed to have real teeth by a deliberate mutation removing +frame 0's exclusion from the prunable set, observed to fail the +retention test before being reverted. The frame-number-from-filename +parsing `replay.py` already had was factored into a shared +`checkpoint.list_checkpoints`, used by both the new pruning logic and +`find_checkpoint_at_or_before` (P-011, single authoritative source). +Verified by hand against the real CLI, not only the test suite: +`pyflow record --max-frames 25 --checkpoint-interval 5 +--max-checkpoints-retained 2` reports 6 checkpoints written but leaves +exactly `{0, 20, 25}` on disk. Discharges Criterion 8. 1182 tests as of +2026-09-09, up from 1172 (10 new). + +**TASK-050 narrows TASK-046's own stated exact-range-only scope for the +one case with a concrete, avoidable cost: a request that is a full +subset of an already-cached wider window.** `materialize_or_load_window` +now also globs `cache_dir` for any cached window whose range is a +superset of the request (`_find_superset_window`, ranking by the range +in the filename first, the same cheap-before-I/O shape +`find_checkpoint_at_or_before` already uses) and slices the result -- no +re-simulation, no new file written. A request that only partially +overlaps a cached range, or extends past its edge, still falls back to +full `materialize_window` -- a real, stated exclusion, confirmed to have +real teeth by weakening the superset check to an overlap-only one and +observing the fallback test fail before reverting. Verified by hand: +caching `--from-frame 0 --to-frame 20`, deleting every checkpoint, then +requesting `--from-frame 5 --to-frame 10` still exits 0 with no new +cache file written. Discharges Criterion 9. 1185 tests as of 2026-09-09, +up from 1182 (3 new). + +### TASK-048 (Live Scrub): two things verified empirically before being relied on, not assumed from reading a library's own docs + +Stage 8 (Recording & Playback) was reopened this same day for four +completion criteria an audit found missing; TASK-048 is its live-scrub +piece. Two real risks, both checked directly rather than reasoned about +in the abstract, the same "verify sign conventions and event behaviour +before relying on them" discipline `rendering/CLAUDE.md`'s pan/zoom +entries already establish. + +**Whether a scrub-bar drag can stop a camera-pan drag from also +starting on the same canvas.** `RenderWindow.run()` registers its own +pointer handlers (`_begin_pan`/`_update_pan`/`_end_pan`) unconditionally +on every interactive run, at `rendercanvas`'s default `order=0` -- +`playback.py`'s own scrub-bar handlers needed a way to run first and +suppress them for a drag that starts on the bar. Read directly rather +than assumed: `rendercanvas.core.events.EventEmitter.emit` collects +handlers for an event type, sorted by `order` then registration order, +and breaks the dispatch loop the moment it sees +`event.get("stop_propagation")` true -- confirmed live, not just read: +two handlers registered on the same canvas at `order=-1` and `order=0`, +the first setting `stop_propagation`, and the second was confirmed never +to run. `playback.py`'s own pointer handlers register at `order=-1` for +exactly this reason, and `RenderWindow._update_pan` is already a no-op +if `_begin_pan` never ran (`self._pan_drag_start_screen is None`), so +suppressing `pointer_down` alone is enough -- no change to `window.py` +needed at all, which was the plan's own stated fallback if this had +turned out not to work. + +**Whether an absolute screen-pixel-to-world mapping (not the delta +`_update_pan` already tracks) has the sign conventions the scrub bar's +own hit-testing and thumb placement need.** A small bright marker was +rendered at a known world position (`(1.0, 7.0)`, camera centred at +`(5.0, 3.0)`, a 200x100 offscreen canvas with `maintain_aspect` +expansion in play) and located in the real output; the candidate +formula's own prediction from that marker's pixel position came back +`(0.95, 7.05)` against the marker's real `(1.0, 7.0)` -- within +sub-pixel rounding (pixel size here is 0.1 world units). `pyflow. +rendering.window.screen_to_world` is that formula, pinned against this +same case in `tests/unit/test_rendering.py`. + +Both real-glfw-window integration tests +(`tests/integration/test_playback_cli.py:: +test_arrow_and_home_end_keys_seek_playback_live`, +`::test_dragging_the_scrub_bar_seeks_without_panning_the_camera`) +confirmed a real, unplanned finding along the way: pause freezes +wherever ordinary autoplay (`speed=1.0`/frame) already reached by the +time the key lands, not a reset to `0` -- obvious once seen, but the +first draft of the keyboard test assumed the latter and failed against +real logged frame/position/paused values before being corrected. diff --git a/docs/planning/roadmap.md b/docs/planning/roadmap.md index 7221b84..d7b554d 100644 --- a/docs/planning/roadmap.md +++ b/docs/planning/roadmap.md @@ -306,7 +306,15 @@ This paragraph previously said `make install` and `make test` were still expected to fail, pending `uv.lock` and a test suite (B2/C1) -- stale since 2026-08-16 and corrected 2026-08-19. Both now succeed: `uv.lock` is committed (B2) and `make test` runs the suite with coverage -(C1a/C1b): **1185 tests as of 2026-09-09**, up from 1182 the same day +(C1a/C1b): **1196 tests as of 2026-09-09**, up from 1185 the same day +(TASK-048, Live Scrub: 7 in `tests/unit/test_playback.py` for +`seek_relative`/`seek_to`/`frame_index_from_fraction` and +`PlaybackState.dragging`'s own default, 2 in `tests/unit/ +test_rendering.py` for the new `screen_to_world` mapping, and 2 real +glfw-window integration tests in `tests/integration/ +test_playback_cli.py` -- genuine injected keyboard seeks, and a genuine +dragged pointer sequence proving both the seek and that the camera +never pans during it), 1185 itself up from 1182 the same day (TASK-050, Partial-Overlap Cache Reuse: 3 in `tests/unit/ test_replay.py` -- a full-subset request reused without re-simulation, the right superset picked among several cached windows including a @@ -11386,7 +11394,7 @@ them, which had not been drafted yet when these were written. | 8. Checkpoint retention, opt-in, frame 0 never pruned | TASK-049 | | 9. Partial-overlap (subset) cache reuse | TASK-050 | -### Status as of 2026-09-09: Stage 8 reopened, seven of nine criteria met +### Status as of 2026-09-09: Stage 8 reopened, eight of nine criteria met **This stage was audited 2026-09-09, at the maintainer's own request, against the suspicion that it "never actually went through a @@ -11424,13 +11432,13 @@ what shipped rather than against the criteria that were meant to operationalise | 3. Resuming reproduces the same trajectory, bit-identically | **Met** -- TASK-045, mutation-tested | | 4. A checkpoint file is self-contained | **Met** -- TASK-045 | | 5. Golden Demo runs end to end, both halves | **Met** -- TASK-045 (record), TASK-046/047 (playback), against Lid-Driven Cavity | -| 6. Live scrub, keyboard and mouse | **Open** -- TASK-048, drafted, not started | +| 6. Live scrub, keyboard and mouse | **Met** -- TASK-048, verified against a real window | | 7. Combined solved-velocity + declared-field playback | **Open** -- TASK-051, drafted, not started | | 8. Checkpoint retention, opt-in, frame 0 never pruned | **Met** -- TASK-049, mutation-tested | | 9. Partial-overlap (subset) cache reuse | **Met** -- TASK-050, mutation-tested | -Seven of nine criteria are met; the stage is **in progress**, not -complete, until TASK-048/051 close the other two. **One real +Eight of nine criteria are met; the stage is **in progress**, not +complete, until TASK-051 closes the last one. **One real course-correction happened during the original build, recorded rather than smoothed over**: TASK-045's own original Golden Demo choice (Heat Diffusion) turned out incompatible with TASK-047's own scope decision @@ -12217,8 +12225,7 @@ Completion Criterion 9 in full. ## TASK-048 — Live Scrub -**Status: Not started, drafted 2026-09-09.** Discharges Completion -Criterion 6. +**Status: Done, 2026-09-09.** Discharges Completion Criterion 6. ### Purpose @@ -12229,10 +12236,9 @@ wherever `pyflow play` was launched. ### Dependencies -`playback.py` (TASK-047), for `PlaybackState`/`play()`. `window.py`'s -existing pointer-drag pan machinery (`_begin_pan`/`_update_pan`/ -`_end_pan`) is the one real open risk this task starts from -- see -Design decisions below. +`playback.py` (TASK-047), for `PlaybackState`/`play()`. `pyflow. +rendering.window` gained one new function (`screen_to_world`); no +change to `RenderWindow` itself was needed (see Design decision 4). ### Design decisions, recorded here @@ -12246,21 +12252,84 @@ Settled directly with the maintainer when this stage was reopened: own start/end.** 3. **Mouse: a draggable scrub bar**, reaching any frame in the window directly rather than only by repeated stepping. -4. **Open technical risk, to resolve empirically before building the - widget, not by assumption:** `RenderWindow.run()` already wires - generic camera-pan pointer handlers unconditionally on every - interactive run (`window.py`). A scrub-bar drag must not also pan the - camera underneath it. First step of this task: verify whether pygfx/ - rendercanvas's event dict supports stopping propagation to a - same-canvas handler registered afterward -- the same "verify sign - conventions and event behaviour before relying on them" discipline - `rendering/CLAUDE.md`'s own pan/zoom entries already establish. If it - does not, the fallback is disabling `RenderWindow`'s built-in pan for - playback windows specifically, the same shape `close_keys=None` - already gives a caller for the close-key default. +4. **The open technical risk resolved in favour of the primary + approach, not the stated fallback.** `RenderWindow.run()` wires its + own camera-pan pointer handlers unconditionally at `rendercanvas`'s + default `order=0`. Read directly rather than assumed: + `rendercanvas.core.events.EventEmitter.emit` dispatches handlers + sorted by `order` then registration order, and stops the moment a + handler sets `event["stop_propagation"]` -- confirmed live (two + handlers at `order=-1`/`order=0` on the same canvas; the second + never ran once the first set it). `playback.py`'s own pointer + handlers register at `order=-1` for exactly this reason, and + `RenderWindow._update_pan` is already a no-op if `_begin_pan` never + ran, so suppressing `pointer_down` alone is enough -- `window.py`'s + own pan/close-key machinery needed no change at all. Full trail: + `docs/CHANGELOG-DESIGN.md`, 2026-09-09. +5. **A second empirical check, found necessary while building the bar + itself, not anticipated when this task was drafted:** placing a + thumb at an absolute world x and hit-testing an absolute pointer + position needs an absolute screen-to-world mapping, which nothing in + this codebase had -- `_update_pan` only ever tracked a *delta*. + `screen_to_world` (`window.py`) was verified against a real rendered + marker at a known world position before being trusted, the same + "verify sign conventions before relying on them" discipline + `rendering/CLAUDE.md`'s pan/zoom entries already establish for + exactly this class of formula. Full measurement: + `docs/CHANGELOG-DESIGN.md`, 2026-09-09. -Artifacts, Acceptance Criteria and Discharges are written when this task -is actually built, the same as every other entry in this file. +### Artifacts Produced + +- `src/pyflow/rendering/window.py` -- `screen_to_world(camera, + logical_width, logical_height, screen_x, screen_y)`, the inverse of + what `_update_pan` tracks only as a delta. +- `src/pyflow/playback.py` -- `PlaybackState.dragging`; `seek_relative`, + `seek_to`, `frame_index_from_fraction` (pure, no rendering); `play()` + gained a scrub-bar track (`gfx.Line`) and thumb (`gfx.Points`, + rebuilt on index change the same "remove old, build new" way + `_rebuild_arrows` already is), Left/Right/Home/End in `_on_key`, and + `pointer_down`/`pointer_move`/`pointer_up` handlers registered at + `order=-1`. +- Tests: 8 in `tests/unit/test_playback.py` (`seek_relative`/`seek_to` + clamping at both ends, `frame_index_from_fraction`'s full range and + its own clamping, `PlaybackState.dragging`'s default), 2 in + `tests/unit/test_rendering.py` (`screen_to_world` against the four + corners and centre of a simple case, and against an off-centre, + aspect-expanded case matching the real-marker measurement above), 2 + in `tests/integration/test_playback_cli.py` (a real glfw window with + genuine injected Left/Right/Home/End key events, and a real dragged + pointer sequence proving both that position seeks correctly *and* + that the camera does not move at all during the drag). + +### Acceptance Criteria + +- Left/Right/Home/End move `PlaybackState.position` to the expected + frame regardless of `paused`/`speed`, checked in isolation + (`tests/unit/test_playback.py`) and through the real keyboard wiring + against a genuinely running window + (`test_arrow_and_home_end_keys_seek_playback_live`) -- including the + real, previously-unstated finding that pause freezes wherever + ordinary autoplay already reached, not a reset to frame 0. +- A mouse drag starting on the scrub bar moves `PlaybackState.position` + to the frame its release point corresponds to, checked against a + real injected `pointer_down`/`pointer_move`/`pointer_up` sequence, + not only the pure `frame_index_from_fraction` function in isolation. +- **The same drag does not move `window.camera.local.position` at + all** -- checked directly before and after the drag in the same test, + the literal claim this task exists to prove. +- Verified by hand against the real CLI (root `CLAUDE.md`'s Feature + Verification rule), with an honest limit stated rather than glossed + over: the two live-window integration tests inject genuine + `rendercanvas` events against a real glfw window and observe real + state/pixel effects, the same technique + `test_space_pauses_playback_live` already established -- this is not + the same as a human's own hand on a real mouse, which nothing in this + environment could exercise. `pyflow play`'s own `--help` output is + unchanged (no new CLI flag), confirmed directly. + +### Discharges + +Completion Criterion 6 in full. --- diff --git a/docs/planning/status.md b/docs/planning/status.md index a57c108..e2e0d16 100644 --- a/docs/planning/status.md +++ b/docs/planning/status.md @@ -17,14 +17,14 @@ demand, not part of this file. ## Progress -**50/52 tasks complete (96%)** across 16 planned stages. For the full plan, including +**51/52 tasks complete (98%)** across 16 planned stages. For the full plan, including stages below not yet broken into tasks: [roadmap.md](roadmap.md). ```mermaid pie showData title "Tasks across the roadmap" - "Done" : 50 - "Not started" : 2 + "Done" : 51 + "Not started" : 1 ``` ### Milestones @@ -40,12 +40,12 @@ pie showData ### Up next -**Stage 8 -- Recording & Playback** is next, starting with TASK-048 (Live Scrub), 1 more not yet started in this stage. +**Stage 8 -- Recording & Playback** is next, starting with TASK-051 (Combined Solved-Velocity + Declared-Field Playback). ## Live repository facts - **49** `CLAUDE.md` files -- **1185** tests collected +- **1196** tests collected - **144** Gherkin scenarios (`tests/features/*.feature`) ## Stages @@ -153,7 +153,7 @@ pie showData ### Stage 8 -- Recording & Playback -**no status recorded** -- `███████░░░` 5/7 tasks; 9 criteria defined, no status line yet +**no status recorded** -- `█████████░` 6/7 tasks; 9 criteria defined, no status line yet | Task | Status | Date | Artifact | |------|--------|------|----------| @@ -162,7 +162,7 @@ pie showData | TASK-047 -- Interactive Playback (`pyflow play`) | Done | 2026-09-07 | `replay.py` | | TASK-049 -- Checkpoint Retention Policy | Done | 2026-09-09 | `checkpoint.py` | | TASK-050 -- Partial-Overlap Cache Reuse, Subset Only | Done | 2026-09-09 | `replay.py` | -| TASK-048 -- Live Scrub | Not started | | | +| TASK-048 -- Live Scrub | Done | 2026-09-09 | `playback.py` | | TASK-051 -- Combined Solved-Velocity + Declared-Field Playback | Not started | | | ### Stage 9 -- Better Numerics diff --git a/src/pyflow/CLAUDE.md b/src/pyflow/CLAUDE.md index e7e024d..c9630f7 100644 --- a/src/pyflow/CLAUDE.md +++ b/src/pyflow/CLAUDE.md @@ -323,3 +323,46 @@ rendered pixels (not only that `PlaybackState.paused` flips in isolation) needed a way to reach both the window and the playback state from outside `play()`, which its own `on_frame(window)` parameter and this attribute together provide. + +**Live scrub (TASK-048, Stage 8 reopening, added 2026-09-09): Left/ +Right step one frame, Home/End jump to the loaded window's own edges, +and a draggable scrub bar reaches any frame in between directly.** +`PlaybackState.dragging`, `seek_relative`/`seek_to`/ +`frame_index_from_fraction` are pure, no-rendering logic, the same +split `advance_playback_position`/`toggle_pause`/`increase_speed`/ +`decrease_speed` already establish. The bar itself is a static +`gfx.Line` track plus a `gfx.Points` thumb rebuilt on index change the +same "remove old, build new" way `_rebuild_arrows` already is, +positioned below whatever else is shown using the same fixed-fraction- +of-mesh-height layout every other HUD margin in this codebase already +uses. + +**The scrub bar's own pointer handlers register at `order=-1`, one +level above `RenderWindow.run`'s own camera-pan handlers (the default +`order=0`), and set `event["stop_propagation"]` when a drag starts on +the bar** -- so the same drag never also pans the camera underneath +it. Verified live before being relied on, not assumed from +`rendercanvas`'s own documentation: `rendercanvas.core.events. +EventEmitter.emit` sorts handlers by `order` then registration order +and stops dispatching once `stop_propagation` is set, confirmed with +two handlers on one canvas at `order=-1`/`order=0`. `RenderWindow. +_update_pan` was already a no-op if `_begin_pan` never ran, so +suppressing `pointer_down` alone is enough -- no change to `window.py` +needed, unlike the fallback this task was drafted expecting to need. +Placing the thumb and hit-testing a drag both needed an absolute +screen-to-world mapping nothing in this codebase had (`_update_pan` +only ever tracked a delta) -- `rendering.window.screen_to_world`, see +that file's own `CLAUDE.md` entry for how it was verified. Full trail +for both findings: `docs/CHANGELOG-DESIGN.md`, 2026-09-09. + +**A real, previously-unstated finding, caught by the two live-glfw- +window integration tests this task added +(`tests/integration/test_playback_cli.py:: +test_arrow_and_home_end_keys_seek_playback_live`, +`::test_dragging_the_scrub_bar_seeks_without_panning_the_camera`): +pausing freezes wherever ordinary autoplay (`speed`/frame) already +reached by the time the key lands, not a reset to frame 0.** Obvious in +hindsight, but the first draft of the keyboard test assumed the latter +and failed against real logged frame/position/paused values -- caught +by logging every frame rather than guessing at the right frame number +to assert against. diff --git a/src/pyflow/playback.py b/src/pyflow/playback.py index 21a23aa..573785a 100644 --- a/src/pyflow/playback.py +++ b/src/pyflow/playback.py @@ -1,10 +1,10 @@ """Interactive playback rendering (TASK-047, Stage 8, Recording & Playback): `pyflow play`'s own rendering half -- opens a real window and renders a `MaterializedWindow` (TASK-046, `replay.py`) with live -keyboard pause/speed control, reusing this project's existing mesh/ -field-visualization/HUD machinery the same way `bootstrap.py`'s own -live-stepping paths do, just indexing into pre-computed frames instead -of calling `advance_simulation_state`. +keyboard/mouse pause, speed, and seek control, reusing this project's +existing mesh/field-visualization/HUD machinery the same way +`bootstrap.py`'s own live-stepping paths do, just indexing into +pre-computed frames instead of calling `advance_simulation_state`. **Imports `rendering`, unlike `recording.py`/`replay.py`** -- this is the one module in Stage 8 whose whole job is putting pixels on screen, @@ -20,8 +20,25 @@ set for TASK-031/034. `UnsupportedPlaybackConfigError` names the gap loudly rather than silently rendering nothing. +**Live scrub (TASK-048, Stage 8 reopening, added 2026-09-09): Left/ +Right step one frame, Home/End jump to the loaded window's own edges, +and a draggable scrub bar reaches any frame in between directly.** +Scoped to the window already materialized at launch +(`[from_frame, to_frame]`) -- seeking past either edge still needs a +different `pyflow play` invocation. The scrub bar's own pointer handlers +register at `order=-1` (`RenderWindow.run`'s own camera-pan handlers +register at the default `order=0`) and set `event["stop_propagation"]` +when a drag starts on the bar, so a scrub drag never also pans the +camera underneath it -- verified empirically before being relied on +(`rendercanvas.core.events.EventEmitter.emit` checks +`stop_propagation` before each handler, in `order` then registration +order) rather than assumed from reading the library's own docs, the +same "verify before relying on it" discipline `rendering/CLAUDE.md`'s +pan/zoom entries already establish. + **Pure playback-state logic (`PlaybackState`, `advance_playback_ -position`, `toggle_pause`, `increase_speed`, `decrease_speed`) is kept +position`, `toggle_pause`, `increase_speed`, `decrease_speed`, +`seek_relative`, `seek_to`, `frame_index_from_fraction`) is kept separate from the rendering it drives**, testable with plain pytest and no window at all -- `tests/unit/test_playback.py`. Only `play()` itself needs a real display, covered by `tests/integration/ @@ -37,6 +54,7 @@ from pathlib import Path from typing import Any +import pygfx as gfx import torch from pyflow.configuration.schema import PyFlowConfig, RenderBackend @@ -52,6 +70,7 @@ fit_camera_to_bounds, mesh_bounding_box, ) +from pyflow.rendering.window import screen_to_world from pyflow.replay import MaterializedWindow, materialize_or_load_window _Bounds = tuple[float, float, float, float] @@ -70,6 +89,22 @@ _TITLE_MARGIN_FRACTION = 0.12 _STATS_MARGIN_FRACTION = 0.20 +# The scrub bar's own layout (TASK-048, Stage 8 reopening) -- same +# fixed-fraction-of-mesh-height shape as the constants above, for the +# same reason (nothing here can be measured before it's drawn). +_SCRUB_BAR_GAP_FRACTION = 0.08 +"""Gap between the scrub bar and whatever HUD element sits above it.""" +_SCRUB_BAR_MARGIN_FRACTION = 0.12 +"""How far the scrub bar's own margin extends the framed view downward.""" +_SCRUB_BAR_HIT_HALF_HEIGHT_FRACTION = 0.04 +"""Vertical click tolerance around the bar's own y, as a fraction of +mesh height -- a `pointer_down` within this band of the bar starts a +drag; once dragging, `pointer_move` tracks x regardless of y, the same +"a drag need not stay exactly on the widget" tolerance most UI scrub +bars give.""" +_SCRUB_TRACK_COLOR = "#888888" +_SCRUB_THUMB_COLOR = "#ffcc00" + class UnsupportedPlaybackConfigError(ValueError): """Raised by `play` when the materialized window's own config is not @@ -95,6 +130,12 @@ class PlaybackState: position: float = 0.0 paused: bool = False speed: float = 1.0 + dragging: bool = False + """Set while a mouse drag on the scrub bar is in progress (TASK-048, + Stage 8 reopening) -- lets `play()`'s pointer handlers distinguish + "this drag is ours" across `pointer_down`/`pointer_move`/ + `pointer_up`, the same way `RenderWindow._pan_drag_start_screen` + tracks whether a camera-pan drag is in progress.""" def advance_playback_position(state: PlaybackState, *, max_index: int) -> int: @@ -130,6 +171,42 @@ def decrease_speed(state: PlaybackState) -> None: state.speed = max(state.speed / 2.0, MIN_SPEED) +def seek_relative(state: PlaybackState, delta: int, *, max_index: int) -> int: + """Move `state.position` by exactly `delta` frames, clamped to + `[0, max_index]`, regardless of `state.speed` or `state.paused` -- + the Left/Right keys' own effect (TASK-048). Unlike + `advance_playback_position`, this never depends on speed: a keyboard + seek always means "one frame", not "however fast playback happens to + be going". Returns the resulting frame index; mutates `state` in + place. + """ + state.position = max(0.0, min(state.position + delta, float(max_index))) + return int(state.position) + + +def seek_to(state: PlaybackState, index: int, *, max_index: int) -> int: + """Jump `state.position` directly to `index`, clamped to + `[0, max_index]` -- the Home/End keys' own effect (jumping to `0`/ + `max_index`), and the mechanism a scrub-bar drag uses to set an + absolute position rather than a relative step. Returns the + resulting frame index; mutates `state` in place. + """ + state.position = max(0.0, min(float(index), float(max_index))) + return int(state.position) + + +def frame_index_from_fraction(fraction: float, *, max_index: int) -> int: + """The frame index `round(fraction * max_index)` maps to, clamped to + `[0, max_index]` -- how a scrub-bar drag's own world-space position + (already reduced to a `0..1` fraction along the bar) becomes a + frame index. Clamped rather than left to overshoot, since a drag + that continues past either end of the bar while still held is a + real, expected gesture, not an error. + """ + index = round(fraction * max_index) + return max(0, min(index, max_index)) + + def _velocity_field_from_frame( mesh: StructuredCartesianMesh, frame: dict[str, torch.Tensor] ) -> VectorField: @@ -285,6 +362,42 @@ def _rebuild_arrows(index: int) -> None: bounds[3], ) + # The scrub bar (TASK-048): a static track plus a thumb rebuilt the + # same "remove old, build new" way `_rebuild_arrows` already is -- + # this project's own established convention over mutating a + # geometry's buffer in place (`_add_declared_field_transport`'s own + # docstring in `bootstrap.py`). + bar_y = bounds[1] - mesh_height * _SCRUB_BAR_GAP_FRACTION + track = gfx.Line( + gfx.Geometry(positions=[[mesh_min_x, bar_y, _HUD_Z], [mesh_max_x, bar_y, _HUD_Z]]), + gfx.LineSegmentMaterial(thickness=2.0, color=_SCRUB_TRACK_COLOR), + ) + window.scene.add(track) + bounds = ( + bounds[0], + bar_y - mesh_height * _SCRUB_BAR_MARGIN_FRACTION, + bounds[2], + bounds[3], + ) + + thumb_object: gfx.Points | None = None + + def _thumb_x(index: int) -> float: + fraction = index / max_index if max_index else 0.0 + return mesh_min_x + fraction * mesh_width + + def _rebuild_thumb(index: int) -> None: + nonlocal thumb_object + if thumb_object is not None: + window.scene.remove(thumb_object) + thumb_object = gfx.Points( + gfx.Geometry(positions=[[_thumb_x(index), bar_y, _HUD_Z]]), + gfx.PointsMaterial(color=_SCRUB_THUMB_COLOR, size=12.0), + ) + window.scene.add(thumb_object) + + _rebuild_thumb(0) + last_index = 0 def _on_frame() -> None: @@ -292,6 +405,7 @@ def _on_frame() -> None: index = advance_playback_position(playback_state, max_index=max_index) if index != last_index: _rebuild_arrows(index) + _rebuild_thumb(index) last_index = index if stats_text is not None: stats_text.set_text( @@ -308,8 +422,60 @@ def _on_key(event: dict[str, Any]) -> None: increase_speed(playback_state) elif key == "-": decrease_speed(playback_state) + elif key == "ArrowRight": + seek_relative(playback_state, 1, max_index=max_index) + elif key == "ArrowLeft": + seek_relative(playback_state, -1, max_index=max_index) + elif key == "Home": + seek_to(playback_state, 0, max_index=max_index) + elif key == "End": + seek_to(playback_state, max_index, max_index=max_index) + + def _seek_from_pointer_x(screen_x: float, screen_y: float) -> None: + logical_width, logical_height = window.canvas.get_logical_size() + world_x, _world_y = screen_to_world( + window.camera, logical_width, logical_height, screen_x, screen_y + ) + fraction = (world_x - mesh_min_x) / mesh_width if mesh_width else 0.0 + seek_to( + playback_state, + frame_index_from_fraction(fraction, max_index=max_index), + max_index=max_index, + ) + + def _on_pointer_down(event: dict[str, Any]) -> None: + logical_width, logical_height = window.canvas.get_logical_size() + _world_x, world_y = screen_to_world( + window.camera, logical_width, logical_height, event["x"], event["y"] + ) + hit_half_height = mesh_height * _SCRUB_BAR_HIT_HALF_HEIGHT_FRACTION + if abs(world_y - bar_y) > hit_half_height: + return + playback_state.dragging = True + _seek_from_pointer_x(event["x"], event["y"]) + event["stop_propagation"] = True + + def _on_pointer_move(event: dict[str, Any]) -> None: + if not playback_state.dragging: + return + _seek_from_pointer_x(event["x"], event["y"]) + event["stop_propagation"] = True + + def _on_pointer_up(event: dict[str, Any]) -> None: + if not playback_state.dragging: + return + playback_state.dragging = False + event["stop_propagation"] = True window.canvas.add_event_handler(_on_key, "key_down") + # `order=-1`, run before `RenderWindow.run`'s own camera-pan handlers + # (registered at the default `order=0`) -- a drag that starts on the + # scrub bar sets `stop_propagation` so it never also pans the camera + # underneath it (verified empirically, see this module's own + # docstring). + window.canvas.add_event_handler(_on_pointer_down, "pointer_down", order=-1) + window.canvas.add_event_handler(_on_pointer_move, "pointer_move", order=-1) + window.canvas.add_event_handler(_on_pointer_up, "pointer_up", order=-1) fit_camera_to_bounds(window.camera, bounds) window.apply_camera_config() diff --git a/src/pyflow/rendering/CLAUDE.md b/src/pyflow/rendering/CLAUDE.md index 4244558..a370d52 100644 --- a/src/pyflow/rendering/CLAUDE.md +++ b/src/pyflow/rendering/CLAUDE.md @@ -790,3 +790,42 @@ equalized-panel block. `[]` (the default) draws nothing, the same as `FieldDisplayConfig.render_field` entry for the schema side of this migration, including the 6 golden demos it required migrating and the load-error a config still setting either retired field now gets. + +## `screen_to_world`, added 2026-09-09 (TASK-048, Stage 8 reopening) + +`window.py`'s `visible_world_size` (above) answers "how much world is +visible"; nothing answered "what world point is under this specific +screen pixel" until `playback.py`'s scrub bar needed one -- placing a +thumb at an absolute world x and hit-testing an absolute pointer +position, neither of which `_update_pan`'s own delta tracking can do. +`screen_to_world(camera, logical_width, logical_height, screen_x, +screen_y)` is the inverse of that delta: built on `visible_world_size` +for the identical aspect-expansion reason `_update_pan` already depends +on it, then adding the camera's own centre position to turn a relative +extent into an absolute point. + +**Verified against a real rendered marker at a known world position +before being trusted**, the same empirical discipline this file's own +pan/zoom entries above already establish for exactly this class of +formula: a small bright marker rendered at world `(1.0, 7.0)` (camera +centred at `(5.0, 3.0)`, a 200x100 offscreen canvas with a real +`maintain_aspect` expansion in play) was located in the actual output, +and the formula's own prediction from that marker's pixel position came +back `(0.95, 7.05)` -- within sub-pixel rounding. Full measurement: +`docs/CHANGELOG-DESIGN.md`, 2026-09-09. Pinned in +`tests/unit/test_rendering.py` against both a simple centred case and +this same off-centre, aspect-expanded one. + +**`playback.py`'s scrub-bar pointer handlers register at `order=-1`, +one level above the default `_begin_pan`/`_update_pan`/`_end_pan` +handlers `RenderWindow.run()` always wires** -- confirmed live, not +assumed from `rendercanvas`'s own docs, that a lower-`order` handler +setting `event["stop_propagation"]` genuinely stops a same-canvas, +higher-`order` handler for that event from running at all +(`rendercanvas.core.events.EventEmitter.emit` sorts by `order` then +registration order and checks `stop_propagation` before each callback). +This is what lets a scrub-bar drag suppress camera panning for that +gesture with no change to `RenderWindow`/`window.py` itself -- the +`_pan_drag_start_screen is None` guard `_update_pan` already had made +suppressing `pointer_down` alone sufficient. See `src/pyflow/CLAUDE.md`'s +`playback.py` entry for the full wiring. diff --git a/src/pyflow/rendering/window.py b/src/pyflow/rendering/window.py index 9f6ab0b..b6efc39 100644 --- a/src/pyflow/rendering/window.py +++ b/src/pyflow/rendering/window.py @@ -88,6 +88,40 @@ def visible_world_size( return (width, height) +def screen_to_world( + camera: gfx.OrthographicCamera, + logical_width: float, + logical_height: float, + screen_x: float, + screen_y: float, +) -> tuple[float, float]: + """The world-space point a screen pixel `(screen_x, screen_y)` + (logical coordinates, top-left origin, y increasing downward -- + `rendercanvas`'s own pointer-event convention) currently projects to + under `camera`'s own position/zoom. + + The inverse of what `_update_pan` tracks only as a *delta*: this is + an absolute mapping, built for TASK-048's scrub bar, which needs to + place a thumb at an absolute world x and hit-test an absolute + pointer position, not track a drag relative to where it started. + Built on `visible_world_size` for the same aspect-ratio-expansion + reason `_update_pan` already depends on it -- `camera.width`/ + `camera.height` alone would under-track by exactly the mismatch + between the camera's own aspect and the canvas's. + + Verified empirically before being relied on (not assumed): a marker + rendered at a known world position was located in a real offscreen + render, and this formula's prediction from that marker's own pixel + position matched the marker's actual world position to within + sub-pixel rounding -- see `docs/CHANGELOG-DESIGN.md`, TASK-048. + """ + visible_width, visible_height = visible_world_size(camera, logical_width, logical_height) + center_x, center_y, _center_z = camera.local.position + world_x = center_x - visible_width / 2 + (screen_x / logical_width) * visible_width + world_y = center_y + visible_height / 2 - (screen_y / logical_height) * visible_height + return (world_x, world_y) + + class RenderWindow: """A window (or headless canvas) with a renderer, scene and camera. diff --git a/tests/integration/test_playback_cli.py b/tests/integration/test_playback_cli.py index 9a10952..9dcc1ad 100644 --- a/tests/integration/test_playback_cli.py +++ b/tests/integration/test_playback_cli.py @@ -15,10 +15,14 @@ from pathlib import Path import numpy as np +import pygfx as gfx import pytest +from pyflow.checkpoint import read_checkpoint +from pyflow.engine.mesh import StructuredCartesianMesh from pyflow.playback import play -from pyflow.rendering.window import RenderWindow +from pyflow.rendering.mesh_visualization import mesh_bounding_box +from pyflow.rendering.window import RenderWindow, visible_world_size def _display_available() -> bool: @@ -222,3 +226,201 @@ def _on_frame(window: RenderWindow) -> None: post_pause = hashes[paused_at + 1 :] assert len(post_pause) >= 3, "not enough frames rendered after pausing to prove anything" assert len(set(post_pause)) == 1, "frames kept changing after Space was pressed" + + +# A generous per-step margin for the two scrub tests below -- wider than +# the single-frame latency `test_space_pauses_playback_live` established, +# since each step here reads the *settled* value right before the next +# key lands rather than the very next frame. Comfortably inside +# `_SCRUB_SAFETY_MAX_FRAMES`. +_SCRUB_STEP_FRAMES = 8 +_SCRUB_SAFETY_MAX_FRAMES = 100 + + +@_needs_a_real_display +def test_arrow_and_home_end_keys_seek_playback_live(tmp_path: Path) -> None: + """Real glfw window, genuine injected key events -- proves Left/ + Right/Home/End actually move `PlaybackState.position` through the + real keyboard wiring `play()`'s own `_on_key` registers, not just in + isolation (`tests/unit/test_playback.py` already proves the pure + `seek_relative`/`seek_to` functions). Paused first so the ordinary + per-frame `speed` advance can't mix into the numbers being checked; + each assertion reads the settled value one full step *before* the + next key is submitted, not the frame right after the previous one. + """ + checkpoints_dir = tmp_path / "checkpoints" + _record_cavity(checkpoints_dir, max_frames=30, checkpoint_interval=30) + + step = _SCRUB_STEP_FRAMES + settled: dict[str, float] = {} + injected: set[str] = set() + + def _on_frame(window: RenderWindow) -> None: + assert window.playback_state is not None + position = window.playback_state.position + frame = window.frame_count + + if frame == step and "pause" not in injected: + injected.add("pause") + window.canvas.submit_event({"event_type": "key_down", "key": " "}) + elif frame == 2 * step: + settled["after_pause"] = position + injected.add("right") + window.canvas.submit_event({"event_type": "key_down", "key": "ArrowRight"}) + elif frame == 3 * step: + settled["after_right"] = position + injected.add("right2") + window.canvas.submit_event({"event_type": "key_down", "key": "ArrowRight"}) + elif frame == 4 * step: + settled["after_right2"] = position + injected.add("end") + window.canvas.submit_event({"event_type": "key_down", "key": "End"}) + elif frame == 5 * step: + settled["after_end"] = position + injected.add("home") + window.canvas.submit_event({"event_type": "key_down", "key": "Home"}) + elif frame == 6 * step: + settled["after_home"] = position + window.canvas.submit_event({"event_type": "key_down", "key": "Escape"}) + + window = play( + checkpoints_dir, + from_frame=0, + to_frame=30, + backend="glfw", + max_frames=_SCRUB_SAFETY_MAX_FRAMES, + on_frame=_on_frame, + ) + + assert window.canvas.get_closed() + assert injected == {"pause", "right", "right2", "end", "home"}, injected + assert window.frame_count < _SCRUB_SAFETY_MAX_FRAMES, "Escape injection never closed the window" + # Pause freezes wherever autoplay (`speed=1.0`/frame, unpaused for + # the first `step` frames) already reached, rather than resetting to + # 0 -- confirmed directly (not assumed) by logging every frame's own + # position/paused pair before writing these numbers down: position + # tracks frame count exactly (1.0/frame) through frame `step`, then + # freezes at exactly `step` starting the frame right after pause is + # submitted, the same one-frame latency `test_space_pauses_ + # playback_live` above already established. + assert settled["after_pause"] == float(step) + assert settled["after_right"] == step + 1.0 + assert settled["after_right2"] == step + 2.0 + assert settled["after_end"] == 30.0 + assert settled["after_home"] == 0.0 + + +def _world_to_screen( + camera: gfx.OrthographicCamera, + logical_width: float, + logical_height: float, + world_x: float, + world_y: float, +) -> tuple[float, float]: + """The inverse of `pyflow.rendering.window.screen_to_world`, built + here rather than imported -- inverting the transform under test + would hide a bug in the transform itself rather than exercise it. + """ + visible_width, visible_height = visible_world_size(camera, logical_width, logical_height) + center_x, center_y, _center_z = camera.local.position + screen_x = (world_x - center_x + visible_width / 2) / visible_width * logical_width + screen_y = (center_y + visible_height / 2 - world_y) / visible_height * logical_height + return (screen_x, screen_y) + + +@_needs_a_real_display +def test_dragging_the_scrub_bar_seeks_without_panning_the_camera(tmp_path: Path) -> None: + """The core risk this task started from: `RenderWindow.run`'s own + camera-pan pointer handlers are registered on the very same canvas. + A drag that starts on the scrub bar must move `PlaybackState. + position` and must **not** also pan the camera -- proven directly on + both counts, through the real pointer-event wiring, not assumed from + reading `stop_propagation`'s own documentation. + + The bar's own world y is read back from the live `gfx.Points` thumb + `play()` actually builds (scene contents, the same "reach into the + real object" technique `tests/unit/test_hud.py` already uses) rather + than recomputed from `playback.py`'s own private layout constants -- + recomputing the layout would risk the test and the code sharing the + same mistake instead of checking one against the other. + """ + checkpoints_dir = tmp_path / "checkpoints" + _record_cavity(checkpoints_dir, max_frames=30, checkpoint_interval=30) + checkpoint = read_checkpoint(checkpoints_dir / "checkpoint_00000000.pt") + mesh = StructuredCartesianMesh.from_config(checkpoint.config.mesh) + mesh_min_x, _mesh_min_y, mesh_max_x, _mesh_max_y = mesh_bounding_box(mesh) + mesh_width = mesh_max_x - mesh_min_x + + step = _SCRUB_STEP_FRAMES + found: dict[str, object] = {} + injected: set[str] = set() + + def _on_frame(window: RenderWindow) -> None: + assert window.playback_state is not None + logical_width, logical_height = window.canvas.get_logical_size() + frame = window.frame_count + + if "bar_y" not in found: + thumb = next(child for child in window.scene.children if isinstance(child, gfx.Points)) + _thumb_x, bar_y, _z = thumb.geometry.positions.data[0] + found["bar_y"] = float(bar_y) + found["camera_before"] = tuple(window.camera.local.position) + + bar_y = found["bar_y"] + assert isinstance(bar_y, float) + + # Paused first (as the keyboard test above does) so the ordinary + # per-frame `speed` advance can't drift the position the drag + # itself sets while later steps are still settling. + if frame == step and "pause" not in injected: + injected.add("pause") + window.canvas.submit_event({"event_type": "key_down", "key": " "}) + elif frame == 2 * step and "down" not in injected: + injected.add("down") + screen_x, screen_y = _world_to_screen( + window.camera, logical_width, logical_height, mesh_min_x + 0.2 * mesh_width, bar_y + ) + found["last_screen_pos"] = (screen_x, screen_y) + window.canvas.submit_event( + {"event_type": "pointer_down", "x": screen_x, "y": screen_y, "button": 1} + ) + elif frame == 3 * step and "move" not in injected: + injected.add("move") + screen_x, screen_y = _world_to_screen( + window.camera, logical_width, logical_height, mesh_min_x + 0.8 * mesh_width, bar_y + ) + found["last_screen_pos"] = (screen_x, screen_y) + window.canvas.submit_event( + {"event_type": "pointer_move", "x": screen_x, "y": screen_y, "buttons": (1,)} + ) + elif frame == 4 * step and "up" not in injected: + injected.add("up") + last_screen_pos = found["last_screen_pos"] + assert isinstance(last_screen_pos, tuple) + screen_x, screen_y = last_screen_pos + window.canvas.submit_event( + {"event_type": "pointer_up", "x": screen_x, "y": screen_y, "button": 1} + ) + elif frame == 5 * step: + found["position_after"] = window.playback_state.position + found["camera_after"] = tuple(window.camera.local.position) + window.canvas.submit_event({"event_type": "key_down", "key": "Escape"}) + + window = play( + checkpoints_dir, + from_frame=0, + to_frame=30, + backend="glfw", + max_frames=_SCRUB_SAFETY_MAX_FRAMES, + on_frame=_on_frame, + ) + + assert window.canvas.get_closed() + assert injected == {"pause", "down", "move", "up"}, injected + # The drag moved to 80% of the bar's own extent -> frame 24 (30 * 0.8), + # exact now that autoplay can't drift it between steps. + assert found["position_after"] == pytest.approx(24.0, abs=1.0) + # The camera must not have moved at all -- the scrub drag's own + # `stop_propagation` kept `RenderWindow.run`'s pan handlers from ever + # starting a pan for this gesture. + assert found["camera_after"] == found["camera_before"] diff --git a/tests/unit/test_playback.py b/tests/unit/test_playback.py index d500c87..792b9e6 100644 --- a/tests/unit/test_playback.py +++ b/tests/unit/test_playback.py @@ -15,7 +15,10 @@ PlaybackState, advance_playback_position, decrease_speed, + frame_index_from_fraction, increase_speed, + seek_relative, + seek_to, toggle_pause, ) @@ -100,3 +103,64 @@ def test_decrease_speed_halves_and_clamps_at_min() -> None: state.speed = MIN_SPEED decrease_speed(state) assert state.speed == MIN_SPEED # does not go below the floor + + +# -- live scrub (TASK-048, Stage 8 reopening) ----------------------------- + + +def test_seek_relative_moves_by_exactly_one_frame_regardless_of_speed() -> None: + """Left/Right always step one frame -- `state.speed` (which governs + ordinary playback advancement) must have no bearing on a keyboard + seek. + """ + state = PlaybackState(position=5.0, speed=8.0) + + index = seek_relative(state, 1, max_index=10) + assert index == 6 + assert state.position == 6.0 + + index = seek_relative(state, -1, max_index=10) + assert index == 5 + + +def test_seek_relative_clamps_at_both_ends() -> None: + state = PlaybackState(position=0.0) + assert seek_relative(state, -1, max_index=10) == 0 + + state = PlaybackState(position=10.0) + assert seek_relative(state, 1, max_index=10) == 10 + + +def test_seek_to_jumps_directly_to_an_absolute_frame() -> None: + state = PlaybackState(position=3.0) + + index = seek_to(state, 7, max_index=10) + + assert index == 7 + assert state.position == 7.0 + + +def test_seek_to_clamps_an_out_of_range_target() -> None: + state = PlaybackState(position=3.0) + + assert seek_to(state, -5, max_index=10) == 0 + assert seek_to(state, 999, max_index=10) == 10 + + +def test_frame_index_from_fraction_maps_the_full_range() -> None: + assert frame_index_from_fraction(0.0, max_index=10) == 0 + assert frame_index_from_fraction(1.0, max_index=10) == 10 + assert frame_index_from_fraction(0.5, max_index=10) == 5 + + +def test_frame_index_from_fraction_clamps_outside_zero_to_one() -> None: + """A drag that overshoots the bar's own extent (the cursor moves past + either end while still held) should clamp to that end, not + extrapolate past it. + """ + assert frame_index_from_fraction(-0.5, max_index=10) == 0 + assert frame_index_from_fraction(1.5, max_index=10) == 10 + + +def test_playback_state_defaults_to_not_dragging() -> None: + assert PlaybackState().dragging is False diff --git a/tests/unit/test_rendering.py b/tests/unit/test_rendering.py index 05f4c6a..a17fb26 100644 --- a/tests/unit/test_rendering.py +++ b/tests/unit/test_rendering.py @@ -11,6 +11,7 @@ from pyflow.configuration import RenderingConfig from pyflow.rendering import RenderWindow from pyflow.rendering.canvas import create_canvas, get_loop +from pyflow.rendering.window import screen_to_world def test_create_canvas_offscreen() -> None: @@ -227,3 +228,47 @@ def test_pointer_drag_follows_the_cursor_when_camera_and_canvas_aspects_differ() expected_x = -100.0 * visible_width / 1280 expected_y = +50.0 * visible_height / 720 assert (x, y, z) == pytest.approx((expected_x, expected_y, 1.0)) + + +def test_screen_to_world_maps_the_four_corners_and_center() -> None: + """The inverse of what `_update_pan` tracks only as a *delta* -- an + absolute screen-pixel-to-world mapping, needed by TASK-048's scrub + bar to place a thumb and hit-test a drag. Verified against a real + rendered marker at a known world position (`docs/CHANGELOG-DESIGN.md`, + TASK-048) before being trusted; pinned here against the four corners + and centre of a simple, camera-centred-on-origin case. + """ + config = RenderingConfig(backend="offscreen", width=400, height=300) + window = RenderWindow(config) + window.camera.width = 4.0 + window.camera.height = 3.0 + window.camera.local.position = (0.0, 0.0, 1.0) + logical_width, logical_height = window.canvas.get_logical_size() + + top_left = screen_to_world(window.camera, logical_width, logical_height, 0.0, 0.0) + bottom_right = screen_to_world(window.camera, logical_width, logical_height, 400.0, 300.0) + center = screen_to_world(window.camera, logical_width, logical_height, 200.0, 150.0) + + assert top_left == pytest.approx((-2.0, 1.5)) + assert bottom_right == pytest.approx((2.0, -1.5)) + assert center == pytest.approx((0.0, 0.0)) + + +def test_screen_to_world_accounts_for_camera_position_and_aspect_expansion() -> None: + """Off-centre camera, and a canvas aspect wider than the camera's own + -- the same `maintain_aspect` expansion `visible_world_size`'s own + docstring explains, applied to an absolute mapping rather than a + pan delta. + """ + config = RenderingConfig(backend="offscreen", width=200, height=100) + window = RenderWindow(config) + window.camera.width = 10.0 + window.camera.height = 10.0 + window.camera.local.position = (5.0, 3.0, 0.0) + logical_width, logical_height = window.canvas.get_logical_size() + + # Matches the real-marker empirical check recorded in + # docs/CHANGELOG-DESIGN.md: a marker rendered at world (1.0, 7.0) + # landed at pixel (~59.5, ~9.5) in a 200x100 offscreen render. + world_x, world_y = screen_to_world(window.camera, logical_width, logical_height, 59.5, 9.5) + assert (world_x, world_y) == pytest.approx((1.0, 7.05), abs=0.1)