Repository navigation
remotion: Add effectsOutputSize to all effect-capable components - #12178
alec-watts wants to merge 10 commits into
Conversation
Avoid redraws from inline size objects and make internal absence explicit with nullable parameters. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep the nullable default on the before composition so the comparison remains accurate. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…view # Conflicts: # packages/media/src/audio/audio-for-preview.tsx # packages/media/src/media-player.ts # packages/media/src/test/media-player.test.ts # packages/media/src/test/premount-arm-kicks-scheduler.test.ts # packages/media/src/test/trim-change-seek.test.ts # packages/media/src/test/video-iterator-manager.test.ts # packages/media/src/test/video-with-no-frames-in-beginning.test.ts # packages/media/src/video-iterator-manager.ts # packages/media/src/video/props.ts # packages/media/src/video/video-for-preview.tsx # packages/media/src/video/video.tsx
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Shapes and <HtmlInCanvas> do not repaint when only effectsOutputSize changes, so the Studio preview keeps the previous effect resolution for those components until an unrelated paint fires. Details below; one minor dependency nit is inline.
Reviewed changes
- Shared size type & pipeline — Adds
EffectsOutputSize(exported fromremotion), threads it throughuseEffectChainState, and makesrunEffectChainresize the source withcreateImageBitmapbefore running the existing chain, owning the bitmap until the run settles. - Component wiring —
effectsOutputSizeadded toVideo,Img,CanvasImage,AnimatedImage,Gif,HtmlInCanvas,Solid,RemotionRiveCanvas, and every shape viaRenderSvg. - Preview redraws —
VideoForPreviewandAnimatedImagere-run their drawing effects on numeric size change;Solid,CanvasImage,Gif, and Rive already depend on the chain-state identity. - Docs & tests — New effects-page section with a
<Video>example; new cancellation/ownership test; removed the filter-mirroring test.
⚠️ Shapes and <HtmlInCanvas> don't repaint on an effectsOutputSize-only change
effectsOutputSize reaches HtmlInCanvas through useEffectChainState (packages/core/src/HtmlInCanvas.tsx:592), whose identity changes on a numeric size change and is only consumed by onPaintCb. Re-registering that paint listener does not call requestPaint(), and the two effects that do request a paint depend on [onPaint, memoizedEffects] (HtmlInCanvas.tsx:897-909) and [width, height, …, renderFrame, …] (911-944) — neither includes effectsOutputSize/chainState. In preview (renderFrame={null}), changing only effectsOutputSize therefore leaves the canvas showing the old resolution. Every other component in this PR wires the size into its drawing effect's deps, so <HtmlInCanvas> and every shape (via RenderSvg) are the outlier.
Technical details
# Repaint on effectsOutputSize-only change
## Affected sites
- packages/core/src/HtmlInCanvas.tsx:592 — `chainState` identity changes with `effectsOutputSize`.
- packages/core/src/HtmlInCanvas.tsx:897-909 — the "repaint when the paint output changes" effect depends on `[onPaint, memoizedEffects]`, not `chainState`.
- packages/core/src/HtmlInCanvas.tsx:911-944 — the mount/paint-barrier effect depends on `width`/`height`/`canvasSizeKey`/`renderFrame`, not `effectsOutputSize` (in preview `renderFrame` is `null`).
- packages/shapes/src/components/render-svg.tsx:277 — forwards `effectsOutputSize` to `HtmlInCanvas`; inherits the same gap.
## Required outcome
- Updating `effectsOutputSize` while `width`, `height`, `effects`, `onPaint`, children, and frame stay identical must still repaint the `HtmlInCanvas` (and thus shapes) in Studio preview so the new effect resolution takes effect.
## Suggested approach
- Add `chainState` (or `effectsOutputSize?.width`/`.height`) to the deps of the effect at `HtmlInCanvas.tsx:897` so it calls `requestPaint()`, matching how `VideoForPreview` and `AnimatedImage` handle the same case.deepseek-v4.1-flash ｜ 𝕏
There was a problem hiding this comment.
ℹ️ The new commits introduce no new issues. This review is non-approving only because the earlier finding from the initial review is still open.
Reviewed changes (delta since 81b31b6)
- Animated-image test fixtures —
d818c8athreads the now-requiredeffectsOutputSize={null}into the two<Canvas>fixtures inpackages/core/src/test/animated-image.test.tsx. No production code changed; the only other delta is the line shift ineffect-internals.test.ts.
deepseek-v4.1-flash ｜ 𝕏
|
Fixed the animated-image dependency finding in 93e5eb7. A native paused-Player probe now changes processing size from 32×24 to 48×36 while retaining one ImageDecoder; the previous code created two. I also checked the reported HtmlInCanvas/shape repaint issue with paused Players, stable effects, stable children, and an unchanged frame. Both HtmlInCanvas and Rect redraw at 48×36; replacing the size object with identical values causes no extra repaint. The chain-state change recreates onPaintCb, and its existing initialization effect re-registers the paint listener and resets the canvas target. The nine-component export probes and WebGL orientation probe still pass. |
This branch was successfully deployed
3 participants
Supersedes #11436 with the cross-component implementation requested by the maintainer. GitHub rejected reopening that closed PR.
Effect sizing previously applied only to
<Video>. This revision exposeseffectsOutputSize={{width, height}}on every effect-capable component, as requested in review, and runs it through the existing shared effect pipeline.The source is resized into the selected effect coordinate space, effects run with matching scratch-canvas dimensions, and the result is composited into the component's existing output canvas. Layout, cropping, and final composition dimensions remain unchanged. Omission preserves the existing path; empty and disabled chains avoid resizing. Numeric size changes rebuild the effect state, while equivalent inline objects reuse it. Video previews redraw a paused frame without restarting their decoder.
Supports Video, Img, CanvasImage, AnimatedImage, Gif, HtmlInCanvas, Solid, RemotionRiveCanvas, and all shapes using the existing RenderSvg wrapper. Documentation and a caller example live on the shared effects API page. Current main is merged; the old Video-only resolver, player callbacks, test fixtures, and screenshots are replaced by the shared implementation. Original contributor commits remain in the branch history.
Verification
Actual upstream-source rendering in hidden Electron with public Mediabunny 1.61.3:
git diff --checkpass. React Doctor reports no diagnostics for the changed sources, although its aggregate score is inconsistent with that empty result.These are focused runtime and source checks. Full upstream CI remains a separate check; no CI, workspace dependency, or temporary harness configuration changes ship.
Why each file changes
packages/core/src/HtmlInCanvas.tsxpackages/core/src/Img.tsxpackages/core/src/animated-image/AnimatedImage.tsxpackages/core/src/animated-image/canvas.tsxpackages/core/src/animated-image/props.tspackages/core/src/canvas-image/CanvasImage.tsxpackages/core/src/canvas-image/props.tspackages/core/src/effects/Solid.tsxpackages/core/src/effects/effect-types.tspackages/core/src/effects/index.tspackages/core/src/effects/run-effect-chain.tspackages/core/src/effects/use-effect-chain-state.tspackages/core/src/index.tspackages/core/src/test/animated-image.test.tsxpackages/core/src/test/effect-internals.test.tspackages/docs/docs/effects/index.mdxpackages/gif/src/Gif.tsxpackages/gif/src/GifForDevelopment.tsxpackages/gif/src/GifForRendering.tsxpackages/gif/src/canvas.tsxpackages/gif/src/props.tspackages/media/src/video/props.tspackages/media/src/video/video-for-preview.tsxpackages/media/src/video/video-for-rendering.tsxpackages/media/src/video/video.tsxpackages/rive/src/RemotionRiveCanvas.tsxpackages/shapes/src/components/render-svg.tsxDiff cleanup found no parallel effect runner, player, decoder, canvas pool, or cache. The private resize boundary owns its bitmap while calling the existing chain; other production behavior remains in that incumbent.
Shared CI test repair
Includes the one-line caption-inspector track selector fix from #12179. Current main fails this test before it can exercise caption persistence. Both caption-inspector E2E cases pass in the actual Studio with this repair; restoring the old selector reproduces the failure.
packages/example/e2e/captions-inspector.test.mtsPaused-preview review verification: HTML-in-canvas and Rect both repaint when only the numeric processing size changes, with stable effects/children/frame; a new object with the same size produces no extra repaint. Fixed an animated-image dependency issue found in review: size changes now redraw without recreating its native ImageDecoder (1 decoder before and after, versus 2 before the correction). The nine component export probes, WebGL orientation probe, video decoder-retention probe, nine core tests, typechecks, and lint pass after this correction.
Reverse-audio CI test synchronization
The media regression test cleared its Web Audio spy before React applied the new global playback rate in a passive effect, allowing forward-playback scheduling during that transition to count against the reverse-silence assertion. It now waits until the Player is actually reversing, clears the spy, and observes another reverse interval before checking silence. Production audio code is unchanged.
Validation on current main with CI’s pinned Playwright Chromium: all 152 media tests across 47 files pass. Removing the production reverse-audio guard still fails this test with a negative start offset, so the original regression remains covered.
packages/media/src/test/player-muted-video.test.tsxFinal-head preflight (
b3c9045ac): a clean checkout withbun install --frozen-lockfile, actual package builds, and CI’s pinned Playwright Chromium passes all 721 core tests (77 files) and all 152 media tests (47 files). The checkout is clean.