Repository navigation
Document the shipped motion menu system and record the remaining gaps - #128
Merged
Merged
Conversation
ScottMorris
added
documentation
Contributor
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0953abf7a7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
Author
|
@codex review |
ScottMorris
force-pushed
the
menu/09-motion-docs
branch
from
August 19, 2026 14:42
0953abf to
e8ba88a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8ba88a7c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This was referenced Aug 19, 2026
ScottMorris
force-pushed
the
menu/09-motion-docs
branch
from
August 19, 2026 16:29
e8ba88a to
631cc3b
Compare
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 631cc3b82d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ScottMorris
force-pushed
the
menu/09-motion-docs
branch
from
August 19, 2026 17:18
631cc3b to
afb2f89
Compare
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afb2f89824
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
Author
|
@codex review |
ScottMorris
force-pushed
the
menu/09-motion-docs
branch
from
August 19, 2026 19:29
afb2f89 to
3e9b258
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e9b2583db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ScottMorris
force-pushed
the
menu/09-motion-docs
branch
from
August 19, 2026 20:29
3e9b258 to
007d64c
Compare
ScottMorris
force-pushed
the
menu/09-motion-docs
branch
from
August 20, 2026 00:13
c271ab6 to
fbeac50
Compare
…line The document still described the retired flat-field design (`Menu.backgroundMode`, `motionDurationSecs`, `motionAudioAssetId`, per-button `highlightKeyframes`) and a hypothetical `transcodeMotionMenu` job. Rewrite it to describe what actually shipped: - **Model**: `MenuDocument.timing` (`MenuTiming`), `MenuDocument.animation` (`AnimationTrack`/`Keyframe`/`Easing` with the `hold` DCSQ sampling mode), the `lift_highlight_keyframes` load migration, and the parity-pinned Rust/TS evaluator pair. - **Pipeline**: motion rides `BuildJob::RenderMenu` via `introCommand`/`durationSecs`; one ffmpeg compose per segment with input-side trim, the three-way audio-bed fallback, always-AC-3 cells, `-flags +cgop` plus the `-sc_threshold` workaround, and BT.601 tagging via `dvd_colour_flags()`; spumux on the loop cell only; dvdauthor `<post>` looping on the `g1` counter with the K-without-timeout infinite-loop degrade. - **DCSQ lowering**: union-of-keyframes overlay schedule, multi-`<spu>` XML with identical `<button>` children, the byte-identical no-tracks pin, the still-menu first-keyframe degrade, and the one-CLUT last-track-wins limitation. - **Validation**: replace the speculative rule list with the actual current codes from `validation/menu.rs` and `validation/scene.rs`. - **Gaps recorded explicitly**: `MenuCompiler` trait deferred to the `build/menu_motion.rs` module seam, BD backend unbuilt, video buttons model-only (Slice E), colour flags motion-only, timeline frame-step fixed at 30 fps.
Add a status line under each Section 4 slice instead of rewriting the plan: - **Slice A shipped** earlier as the `FormatProfile`/role/terminology work. - **Slice B partial** — the legacy `Menu.buttons` mirror retirement landed; render parity (#53) and shared visual properties (#106) still pending. - **Slice C implemented (#107)** — motion backend shipped; the `MenuCompiler` trait was deliberately deferred to the `build/menu_motion.rs` module seam. - **Slice D implemented (#108)** — animation tracks, timeline strip, and the per-keyframe multi-`<spu>` DCSQ lowering; the raw-DCSQ-writer fallback was not needed. - **Slices E (#109) / F (#110) pending**, unchanged. Also flag the shipped slices in the document's header status line.
…t scoping - **§8.6 / §14.1 / §25.1**: record motion menu compilation (video backgrounds, audio beds, intro+loop cells, loop count/timeout), keyframe-track animated highlights, and the timeline editor as shipped menu capabilities. - **§14.4**: motion menus are no longer "a future expansion area" — describe current support and narrow the future list to per-button video regions and format-dependent animated state bitmaps. - **§24.1**: document the runtime asset-scope model — the webview is granted access to exactly the imported assets' source paths at project open/import/relink, and grants reset on restart. - **§26.3.5**: drop the stale "still menus only" v1 simplification. - **§30**: replace "motion menus" in Future Expansion with the actually-remaining motion button thumbnails.
…e summary The capability list still described menus as "motion timing support" only; motion menus now build end-to-end and animated highlights ship with a timeline editor.
Correct `docs/motion-menus.md` and `SPEC.md` to match the code as it stands after the PR8/PR9 review-fix commits: - `AnimatableProperty` gained `activate-colour`/`activate-opacity` (spumux's "select" state), both in the model doc and the lowering, migration, and preview sections that describe it. - The validation table gained `menu.animation-node-not-compiled` (Warning) and `menu.animation-keyframe-invalid` (Error). - The timeline frame-step/snapping is `disc.standard`-aware (`fpsForStandard`); the "fixed at 30 fps" known gap is gone. - The preview-animates bullet now describes the still-menu first-keyframe degrade, honest-preview's union-of-keyframes quantization, and activate-track sampling for the activated state.
The security paragraph claimed asset-protocol grants are "runtime-only, resetting when the app restarts." Neither is true: `allow_asset_scope` only ever calls `allow_file` (nothing revokes a path on project close, relink, or a different project opening in the same process), and `tauri_plugin_persisted_scope` is registered in `apps/spindle/src-tauri/src/lib.rs`, so grants also survive a restart. Rewrote the paragraph to describe the actual cumulative, persisted scope and reference issue #129, which tracks revoking stale grants and dropping persistence.
The Animated Highlights bullet claimed keyframe tracks are "evaluated identically in the editor and the build" — true for Hold easing (and Honest Preview), but not for `linear`/`ease-in`/`ease-out`/`ease-in-out`: the disc can only change a subpicture bitmap at a keyframe timestamp and holds each value until the next one, while the normal editor preview interpolates continuously via `evaluateTrack`. Rewrote the bullet to describe the stepped DVD lowering and mention the `menu.animation-easing-quantised` warning (menu/07) that flags a non-hold-eased track on a DVD-lowered menu.
…ven raster/safe-area Slice A's own acceptance criteria include the UI reflecting the disc type's raster and safe area from `FormatProfile`, but `FormatProfile` has neither field, and `SceneCanvas.tsx` still hardcodes `CANVAS_DESIGN_WIDTH = 720` plus fixed `ACTION_SAFE_PCT`/`TITLE_SAFE_PCT` percentages. "Shipped" hid this remaining format-readiness work; the format-profile/menu-role plumbing that did land stays as described.
…ivate track authoring The shipped model, evaluators, timeline strip, and DCSQ lowering handle all four `AnimatableProperty` variants once a track exists, but nothing in the UI can CREATE a `highlight-opacity`, `activate-colour`, or `activate-opacity` track from scratch: `ButtonInspector`'s "Add keyframe at playhead" is hard-coded to `highlight-colour`, and the timeline strip only renders a lane for a track that already exists (e.g. lifted from legacy `highlightKeyframes` data). "Implemented" overstated authoring coverage; noted the remaining gap instead.
…mplemented clamp The doc claimed the LAST frame's `end` is "defensively clamped to `loopDuration − 1 frame`" — backwards from what `build_overlay_keyframe_schedule` (menu/07) actually does: each keyframe timestamp is clamped to `loopDuration − 1 frame` when it becomes a schedule instant (a START), and the last frame's `end` is always the full, unclamped `loopDuration`. Rewrote both bullets to describe the clamp on the instant/start side and why that alone is what keeps the last frame from ever coming out zero-length.
…curately Same inaccuracy as SPEC.md's security paragraph, duplicated here: grants are cumulative (nothing revokes an earlier path) and persisted across restarts via `tauri_plugin_persisted_scope`, not "runtime-only (reset on restart)". Links the same follow-up, issue #129.
Three staleness fixes, all downstream of the menu/08 preview-sampling and menu/07 validation fixes: - **Preview description.** The doc still described the pre-fix behaviour — each button previewing its own track, and honest preview unioning the highlight and activate groups separately. Rewrote it to match `sampleHonestFold`: honest preview folds every button's relevant track into the SAME menu-wide value (document-order last-track-wins, the disc's one CLUT) and quantises against the union of BOTH groups together; also names `activate-opacity`, now sampled and baked into the outline's alpha in both preview modes. - **`menu.animation-on-still-menu` severity.** Listed as Error in both the bullet and the validation table; menu/07 downgraded it to Warning so the first-keyframe degrade stays buildable. - **`menu.animation-keyframe-density` scope.** Didn't mention that the density count unions all four properties (highlight-colour/-opacity, activate-colour/-opacity) — a menu that only animates the activate pair can still trip it, same union `build_overlay_keyframe_schedule` and the validator both use.
`synthesized`/`deserialize` are the American spellings; AGENTS.md requires Canadian spelling in prose (`synthesised`, `deserialise`) except where a term names an exact API identifier — the camelCase field names in the same sentences (`Menu.backgroundMode`, etc.) stay as-is since those are literal code references, not English prose.
…utton fix Three doc corrections following menu/08's honest-preview fixes: - **Preview animates**: now says opacity is baked into the outline's alpha for BOTH the focused and activated states (previously only claimed the activated state), and spells out that the honest-preview schedule's clamp is the same standard-aware last-presentable frame (`loopDuration − 1 frame` via `fpsForStandard`) the planner uses, and that only top-level (compiled) button tracks are ever relevant — matching `menu_ref.buttons()`'s scope exactly, now that the code filters the same way. - **Validation table**: broadens `menu.animation-node-not-compiled`'s description to cover both a group-nested button AND a non-button node entirely — `src/validation/scene.rs` already validates both cases, the table only ever described the group-nested one. - Adds the missing `menu.animation-easing-quantised` and `menu.animation-palette-exhausted` rows, matching `src/validation/scene.rs`.
…ideo preview WebKitGTK cannot stream `<video>` over the custom `asset://` scheme, so the canvas preview section now documents the blob-URL fallback (`SceneCanvas`'s `BackgroundVideo`), its 512 MiB cap, and that the disc build is unaffected.
… D status Codex round-G docs findings on the motion docs: - `menu.animation-easing-quantised`: the row now states the validator's actual predicate — only a non-Hold easing on a keyframe that begins a segment warns; a track's final keyframe alone never does (`split_last` in `validate_animation_tracks`). - `menu.animation-palette-exhausted`: replaced the "conservative estimate" claim — the count only sees colour values stored literally in keyframes, so eased tracks evaluated at other tracks' schedule boundaries can bake interpolated colours it misses (false negatives), while never-coexisting colours inflate it (false positives). - `menu.animation-keyframe-density`: noted that the density and palette estimates count every non-empty highlight/activate track, including ones the planner drops as uncompiled (`menu.animation-node-not-compiled`), so they can overcount relative to the disc. - `rich-menu-editor-plan.md`: the header claimed "C and D have shipped" while §4 marks Slice D partial — the header now says C shipped and D is partial.
Codex round-H docs findings: - The evaluator section claimed the DCSQ lowering samples "at keyframe timestamps" where "every easing yields the keyframe's own value exactly". It actually steps over the global union of every relevant track's timestamps — an eased track evaluated at a timestamp another track contributed bakes its interpolated mid-segment value into that overlay frame. The bullet now describes stepped sampling on the global union and scopes exactness to all-`hold` tracks. - The "bed windows are continuous across intro+loop" guarantee only holds for an authored bed asset; fallback (2) trims the background video's own audio to each segment's source-relative window, so first-transition continuity requires `introStartSecs + introDurationSecs == loopStartSecs`. The bullet now states the contiguity requirement.
…mismatch, list dangling-asset codes Codex round-I docs findings: - `menu.animation-easing-quantised` is scoped to DVD motion menus and the four highlight/activate properties — still menus and `opacity`/`position` tracks never raise it; the row now says so. - The density estimate's overcount note now includes the endpoint-clamp mismatch: validation clamps timestamps to `loopDuration` while the planner clamps to `loopDuration − 1 frame`, so last-frame + exact-loop-end keyframes count as two here but collapse to one on the disc. - Added `menu.scene-dangling-background` and `menu.scene-dangling-video` rows (both Error), matching `validation/menu.rs` and `validation/scene.rs` — the table already listed the button-video counterpart.
…opped or not Codex round-J finding: `validate_animation_tracks`' easing predicate does not require membership in the compiled top-level button set, so a non-Hold track targeting a grouped button or non-button node still warns (alongside `menu.animation-node-not-compiled`) even though the planner drops it. The SPEC line claimed the warning applied to "a track that gets DVD-lowered" — it now states the actual scope.
…ew gap Codex round-K docs findings: - The overlay-render bullet claimed anti-aliasing stays off for every frame; the `quantize_overlay_palette` developer option deliberately renders WITH AA before reducing to a 4-entry palette (`render_menu_overlay_image_skia_quantized`), so the claim is now scoped to the normal path with the diagnostic exception documented. - Slice A's status listed only the raster/safe-area hardcodes as outstanding; the compile preview is also not profile-driven yet (`CompileOverlay` unconditionally renders the DVD banner/prose/CLUT treatment, reading only `formatProfile.maxButtonsPerMenu`), so the status now names it.
…tracks as model-only Codex round-L findings: `serializable` → `serialisable` per the repo's Canadian-spelling convention, and the `opacity`/`position` gap now states these tracks are model-supported only — the editor has no control to create them, so they enter a project via hand-edited or imported data.
… scope Codex round-M finding: the "exactly ... nothing broader" wording ignored the static asset-protocol scope (`$APPCACHE/**`/`$APPDATA/**` in `tauri.conf.json`). The bullet now describes imported-path grants as runtime additions on top of that static scope, matching `docs/motion-menus.md`'s wording.
ScottMorris
force-pushed
the
menu/09-motion-docs
branch
from
August 20, 2026 14:04
fbeac50 to
06aea8f
Compare
2 tasks done
ScottMorris
added a commit
that referenced
this pull request
Aug 21, 2026
## Summary - Prepare the v0.4.0 release: bump all release-facing manifest versions (root and desktop `package.json`, `tauri.conf.json`, both Cargo manifests, `Cargo.lock`) from 0.3.0 to 0.4.0. - Since 0.3.0, this brings in the full motion-menu authoring stack (per-format constraint profiles, semantic menu roles, colour-management rules, motion menus with animated button highlights, a timeline editor, and supporting docs — PRs #116–#128) and the audio/DVD-compatibility fixes (bitrate ceilings, DTS refusal, authored audio track order, titleset audio consistency validation — PRs #137–#138). ## Test plan - [x] `pnpm release:version:check` — all manifests synchronised at 0.4.0 - [x] `pnpm validate` (format:check, vitest, tsc build, cargo fmt/clippy -D warnings, cargo nextest) — all green
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Documentation
docs/motion-menus.mdrewritten to describe the shipped system instead of the pre-implementation design: theMenuDocument.timing/animationmodel, the per-segment ffmpeg compose (intro + loop cells, 3-way audio fallback, always-AC-3, closed GOPs, BT.601 flags), spumux on the loop cell only, the dvdauthorg1loop counter with its<pre>reset, the DCSQ overlay schedule and multi-<spu>lowering, the timeline editor, runtime asset-scope grants, a table of all 20 current motion/animation validation codes, and an explicit "Known gaps" section.docs/rich-menu-editor-plan.md: per-slice status lines — A shipped, B partial (mirror retirement done; Fix silent canvas rendering/preview failures #53/Slice B: visual fidelity foundation — render-any-state Skia, parity tests, node visual properties, legacy mirror retirement #106 pending), C implemented (Slice C: motion menu build backend + MenuCompiler seam #107,MenuCompilerdeferral noted), D implemented (Slice D: timeline editor & animation property tracks (DCSQ lowering) #108, multi-<spu>route), E/F pending.SPEC.md: menu feature coverage updated (motion compilation, animated highlights, timeline, asset scoping);README.md: one stale feature bullet replaced.Test plan
validation/menu.rsandvalidation/scene.rsprettier --checkclean on all four changed filesStack
Part of the rich menu editor stack — merge bottom → top: