Skip to content

fix(scene): sample transparent pictures premultiplied - #47

Merged
kixelated merged 9 commits into
mainfrom
quest/m0/premultiply
Nov 5, 2026
Merged

kixelated merged 9 commits into
mainfrom
quest/m0/premultiply

Conversation

@kixelated

@kixelated kixelated commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Finishes quest/m0/premultiply.md: a scaled source with transparency, such as a PNG logo or an overlay, draws without a dark or colored fringe at its edges.

Change

Bilinear sampling mixed straight-alpha texels, so a transparent texel's color (often black, sometimes junk) bled into its neighbours when a picture was scaled. Every picture is now stored premultiplied, so every draw and mask mixes premultiplied on the sampler's hardware path:

  • Stills and colors (Picture::Rgba) are premultiplied on the CPU in compositor.rs before their one upload.
  • Frames are already premultiplied: moq-video's renderer writes alpha 1.0 for every frame (moq/rs/moq-video/src/render/shader.wgsl), and opaque straight equals premultiplied. Canvases used by transitions are opaque too. A page arrives premultiplied, as before.
  • Overlays: Vello unpremultiplies its output, so it now paints into a texture of its own, and one render pass after each paint (overlay.rs, PREMULTIPLY) copies it premultiplied into the texture the compositor samples. The pass runs only when the overlay repaints, never per frame.
  • Masks read the same premultiplied pictures, so an overlay used as a luminance mask no longer leaks a transparent texel's color either.

Since every input is now premultiplied, the effect pass drops its premultiplied flag (Picture, Input, the uniform's mode.y, and the mask op's params.y), and effect.wgsl drops the per-tap filtered() path; straight() always divides.

Decisions

  • Scaled overlays cost +67% frame time with per-tap premultiply in the shader. How should overlays be premultiplied? (maintainer, 2026-11-05)
    • Keep premultiplying each bilinear tap in effect.wgsl
    • ✅ A premultiply pass once per overlay repaint, into a premultiplied texture, so every draw uses the hardware sampler. One GPU copy per repaint, not per frame, accepted.

Frame time

Measured on this Mac (Apple Silicon, Metal) with a temporary bench, not committed: a 1080p canvas rendering preview and program each frame, waiting on the GPU, the clock advancing 33 ms a frame. Medians of 300 frames after 30 to warm up; 15 rounds interleaving the three builds, rotating their order each round; the table gives the median of the 15 medians (the minimum was within 0.1 ms of it in every cell). main is origin/main, per-tap is this PR's previous head merged with main, pass is this head.

Scene main per-tap pass
1280x720 still scaled to fill + 8 transparent 700x700 PNG logos drawn at 1200x1000 1.21 ms 1.25 ms 1.16 ms
1280x720 still + one 1920x1080 overlay lower third, drawn at its own size 0.70 ms 0.75 ms 0.64 ms
1280x720 still + 8 transparent 700x700 overlays drawn at 1200x1000 1.22 ms 2.05 ms 1.16 ms

Scaled overlays are back to main's cost (-43% against per-tap). The bench was rebuilt from the previous description, so its scenes approximate the earlier ones; its per-tap column reproduces the earlier numbers within 0.1 ms.

Repaint cost

The same scenes with the overlay's CSS animating, so it repaints every frame:

Scene main per-tap pass
1920x1080 overlay at its own size, repainting 1.26 ms 1.33 ms 1.34 ms
700x700 overlay drawn 8 times at 1200x1000, repainting 1.79 ms 2.61 ms 1.77 ms

A repaint (Vello and, on pass, the premultiply copy) adds 0.56 ms on main and 0.70 ms on pass for the 1920x1080 overlay, so the copy costs about 0.14 ms per repaint there, and about 0.04 ms for the 700x700 one (0.57 against 0.61 ms).

GPU copies: one per overlay repaint, none per frame. Encoder load: unchanged (the program texture is the same size and format).

Choices made without asking

  • A hybrid instead of one method. Stills are premultiplied on the CPU before their upload, so they cost nothing per frame and need no extra texture. Premultiplying in 8 bits rounds a low-alpha texel's color, as browsers' premultiplied images do; the composited result is within the 8-bit output's own rounding. The overlay pass rounds the same way.
  • Frames trust moq-video's opaque output. If moq-video ever passes alpha through, frames would need a premultiply too. The comment in Compositor::update names that coupling.
  • The overlay pass lives on the overlay thread, right after Vello's paint, so the compositor treats an overlay like any other premultiplied picture. It is a render pass rather than a compute one, so it needs no storage-texture support beyond what Vello already uses.

Validation

  • just ci: passed on af0fe72.
  • Compositor GPU tests on Metal (cargo nextest run -p moq-scene --lib --release --run-ignored only -E 'test(/^compositor::tests::/)'): 31 passed on af0fe72.
  • compositor::tests::scaling_bleeds_no_transparent_color scales a still with a transparent green texel, an overlay with a transparent texel, and a luminance mask with a transparent white texel. It fails on main (logo at 2: [195, 28, 0, 255]), and with the premultiply pass's shader made to copy straight, its overlay row fails.

Review

Codex (gpt-6.1-sol, medium effort), quest review --new --base origin/main round 1 on head ceb6137d3f58ec38232879484edd1241b6999166, after the design change: no findings. Head af0fe72 adds only conflict-free merges of origin/main (#46, #49, #50), which touch none of this PR's code.

Follow-ups

None.

🤖 Generated with Claude Code

kixelated and others added 5 commits November 4, 2026 22:42
Quest-Claim: 2a06fa636f663bc1
Bilinear sampling mixed straight-alpha texels, so a scaled picture took on
the color of its transparent neighbours at its edges. Stills now upload
premultiplied and canvases and rendered frames, being opaque, are marked so,
keeping them on the sampler's fast path. An overlay's straight paint, and a
straight mask, premultiply each of their four taps in the shader before
mixing them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A picture drawn at its own size samples on texel centers, where one
texel is the whole mix, so it skips the four loads.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title Premultiplied source sampling fix(scene): sample transparent pictures premultiplied Nov 5, 2026
@kixelated
kixelated marked this pull request as ready for review November 5, 2026 08:29
@coderabbitai

coderabbitai Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You'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 25 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 771b9a63-1d93-4c50-af79-95761d9f1a83
Commits

Reviewing files that changed from the base of the PR and between 98d52d2 and af0fe72.

Files selected for processing: 6

  • quest/m0/README.md
  • quest/m0/premultiply.md
  • rs/moq-scene/src/compositor.rs
  • rs/moq-scene/src/effect.rs
  • rs/moq-scene/src/effect.wgsl
  • rs/moq-scene/src/overlay.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

@kixelated kixelated left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: 9984e60

No actionable findings. Tracking opaque frames fits the existing upload and composition paths, and the pinned video renderer produces opaque alpha. The author-reported scaled-overlay stress case has a 67% frame-time increase, so that trade-off merits monitoring.

Verification: GitHub-only static review; I did not run GPU tests or benchmarks. The new GPU regression is ignored by ordinary CI, and the hosted Check was still running when inspected.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — this run reviewed the full 5-file diff at 9984e60 and checked that every picture producer's alpha convention matches the new premultiplied mixing path.

  • CPU premultiply for stills and colors — Compositor::update premultiplies Picture::Rgba once before its single upload; Mailbox::take moves the buffer out, so there is no double premultiply.
  • Opaque frame/canvas contract — moq-video writes alpha 1.0 for every frame and transition canvases are opaque, so marking them premultiplied needs no conversion pass; Effects::place now honors the picture's flag.
  • Per-tap premultiply in effect.wgsl — filtered() premultiplies each textureLoad bilinear tap and keeps a one-sample fast path at texel centers; masks and coverage() use the same path without a second premultiply.
  • Vello straight-alpha premise — confirmed against vello v0.11.0 (research/vello_shaders/shader/fine.wgsl), which unpremultiplies at its final store, so overlays correctly stay straight.
  • Quest and test updates — deletes the completed quest plus its README row and adds a GPU regression that fails on main; the #[ignore] reason matches the nightly render-sw lane.

The author-reported +67% frame time for scaled overlays is a measured, documented trade-off with no added GPU copies, not a defect.

Pullfrog  | View workflow run | Using big-pickle (free) | 𝕏

kixelated and others added 3 commits November 5, 2026 07:12
Vello paints straight alpha into a texture of its own, and one pass after
each paint copies it premultiplied into the texture the compositor samples.
Every picture is now premultiplied, so draws and masks take the sampler's
hardware path, and the effect pass drops its premultiplied flag and the
per-tap bilinear path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Contributor Author

Design change: per the maintainer's 2026-11-05 decision, overlays are no longer premultiplied per bilinear tap in effect.wgsl. Vello now paints into a texture of its own, and one render pass after each paint copies it premultiplied into the texture the compositor samples. Every picture is premultiplied, so the effect pass drops its premultiplied flag and the per-tap filtered() path, and draws and masks use the hardware sampler. Scaled overlays are back to main's frame time (1.16 ms against 2.05 ms per-tap); the copy costs about 0.14 ms per repaint of a 1920x1080 overlay. The review restarts with quest review --new.

(Written by Claude Opus 5.5)

@kixelated kixelated left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit af0fe72. No actionable findings in the substantive update since 9984e60, accounting for merged base changes. Premultiplying overlays once per repaint rather than per tap is a reasonable simplification of the rendering path.

Verification: GitHub-only static review of texture lifetimes, ordering, caching, masks, and shared-canvas interactions. I did not run tests or benchmarks.

@kixelated
kixelated merged commit fce9c4c into main Nov 5, 2026
3 checks passed
@kixelated
kixelated deleted the quest/m0/premultiply branch November 5, 2026 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant