Repository navigation
feat(scene)!: overlays restart on air - #66
Conversation
Quest-Claim: ec9618a013f99914
An overlay's CSS timeline starts over each time a scene showing it goes on program, or into preview while program does not show it, as keyframes do. A new per-overlay `restart` flag, true by default, turns it off for a clock or ticker that should run on across cuts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…aving scene shows Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 3 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head: b50a9cf3db096e5a16bd134847b54c039cd0fa25.
[P2] Protect overlays used by an active stinger from preview restarts — rs/moq-scene/src/compositor.rs:575–583
on_air includes the outgoing scene and top layer, but not the active stinger's source-filter masks. An overlay is a valid mask source, and Draw::stinger applies those filters to the live program. While such a stinger is playing, selecting a preview scene containing the same restarting overlay (absent from both transition scenes and the top layer) therefore passes this guard and resets its shared texture, visibly replaying the mask on air. shown() already accounts for these stinger masks, so painting and restart protection disagree.
Include active-stinger mask dependencies in the on-air set and add a regression test that changes preview during the stinger, asserting its program mask continues uninterrupted.
Limits: GitHub-only static review of all eight changed files and relevant call paths; no local or GPU tests run. The exact-head GitHub Check runs currently show skipped/cancelled, so this is not a verified CI pass.
The on-air set that guards overlays from preview restarts now comes from the same helper as the paint set, so it includes an active stinger's clip and the sources its filters mask with. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Re the P2 on b50a9cf (stinger masks missing from the on-air set): agreed and fixed in f32fe19. (Written by Claude Opus 5.5) |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head: f32fe198999763a45e3b25b37159109b95f93252.
The prior P2 stinger-mask finding is addressed by inspection: restart() and shown() now share on_air_sources(), including the active stinger's mask dependencies. The new a_stinger_mask_runs_on_under_a_preview regression test checks that selecting a preview scene containing the mask leaves its timeline start unchanged while the stinger remains active.
No new actionable findings in the change since b50a9cf3db096e5a16bd134847b54c039cd0fa25. No further code change requested from this review; wait for the current CI result.
Limits: GitHub-only static review of the fix, regression test, and relevant call paths. No local or GPU tests run; the exact-head GitHub Check is still in progress.
A restart lays the overlay out again through #62's open, so its preloaded images and served fonts survive it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head: ddebcb709b6f6637afc56e08872a53fb3165b22f.
No new actionable findings in the update since f32fe198999763a45e3b25b37159109b95f93252, accounting for the merged base changes. The restart now rebuilds the document through open(&document.assets, ...), preserving the preloaded images and fonts without rereading disk. The expanded assets_load_before_the_first_paint test checks fonts, images, and an animated background across two layouts using the same assets. The prior stinger-mask fix remains intact.
No further code change requested from this review; wait for the current CI result.
Limits: GitHub-only static review of the merge integration, changed regression coverage, and relevant restart/paint paths. No local or GPU tests run. The exact-head GitHub Check is still in progress; Quest passed.

Finishes quest/m1/overlay-restart.md.
Problem
An overlay's CSS timeline started when the overlay opened and never again, so a lower third played its entrance once, at launch or edit, and sat settled on every later cut.
Approach
collection.rs):Source::Overlaygetsrestart: bool, true by default and written only when false, as the quest decided (like OBS's "refresh when the scene becomes active").overlay.rs):Overlay::restart(start)lays the document out afresh from its HTML on the overlay thread, keeping its textures, and repaints it as ofstart.Overlay::startreports where its timeline starts.compositor.rs): each render, before painting, a restarting overlay whose timeline started before the moment showing it went on starts over at that moment: the program scene'sprogram_startif program shows it, else the preview scene'spreview_startunless the top layer or a scene a transition leaves shows it on air, the same instants keyframes run from.shownis split intodrawn(what given scenes draw on any canvas, through clones and masks) andon_air_sources(what program draws: its scene, the scene a transition leaves, the top layer, and a stinger's clip under way with its masks), so the restart and the paint filter agree on what "shows" and "on air" mean.ui.rs): the Overlay add form gets Restart on program, on by default.Impact
restart = falseto keep running across cuts. Existing overlays restart by default, a behavior change: one that should keep running (a clock) needsrestart = false.moq-scenepublic API:Source::Overlaygains arestartfield (breaking for anyone constructing it by struct literal; addrestart: truefor the new default). No wire impact.Choices made without asking
Alternatives
set_on_program, as media files do: that fires only when a source joins the on-program set, so a cut between two scenes both showing the lower third would not replay it, and it knows nothing of preview.Validation
just cipasses (236 tests).compositor::tests::, including the newan_overlay_restarts_going_on(restarts going into preview while program does not show it, restarts on the cut, a preview under program does not restart it, arestart = falseoverlay runs on). It fails with the restart call removed (preview shows the settled overlay).an_overlay_animates_on_the_program_clocknow setsrestart: false, which is what it tests.a_stinger_mask_runs_on_under_a_preview: during a stinger whose clip is masked by a restarting overlay no scene on air shows, putting a scene with that overlay into preview leaves its timeline alone. It fails with the old on-air set (the overlay restarts 100 ms later).just cipasses (236 tests), and all 37 ignoredcompositor::tests::pass on Metal.collection::tests::round_tripscovers both values, the default written by omission.openfrom the same checkedAssets, so its preloaded images and served fonts survive it.overlay::tests::assets_load_before_the_first_paintnow lays the document out a second time, as a restart does, and checks the fonts and both background images again. The README now says a restart reuses the assets read when the overlay opened.quest gate just cipasses (240 tests), and all 37 ignoredcompositor::tests::pass on Metal, including feat(scene): overlays load their own images and fonts #62'san_overlay_draws_its_image_on_its_first_frame,an_overlay_restarts_going_on, anda_stinger_mask_runs_on_under_a_preview.Review
quest review --base origin/main), reviewed head ddebcb7.Follow-ups
drawnmust reach through nested scenes as itsshowndoes, so an overlay in a nested scene restarts when the scene drawing it goes on; resolved at merge either way.🤖 Generated with Claude Code
(Written by Claude Opus 5.5)