[kixelated]

Finishes quest/m0/browser-popups.md, raised on moq-dev/studio#8.

Problem

CEF paints a page's popup widgets, such as a <select>'s list, apart from the page (PET_POPUP). The helper dropped those paints, so a dropdown opened invisibly and could not be picked from. README.md called this out as a limitation.

Approach

• Helper (host.rs): forwards PET_POPUP paints like the view's, on both OnPaint (pixels) and OnAcceleratedPaint (shared texture), tagged with a Layer. OnPopupSize and OnPopupShow(false) become Event::Popup(id, Some(area) | None). They go through the same per-page id and Painted handshake as the view.
• Studio (layers.rs, run.rs): each page's route keeps its newest clean view and, while one is open, the popup's area and picture.
  • With no popup open, the view is shown as before, with no extra work.
  • With one open, the studio copies the view into a new texture and draws the popup over it, premultiplied, in a one-triangle render pass whose viewport is the popup's area.
  • The clean view stays untouched, so when the popup repaints, moves, or closes, the page redraws without a CEF invalidate.
  • Both paint paths end in a wgpu::Texture, so one pass serves both. It reads the popup with textureLoad, so a popup's channel order need not match its page's.
  • Uploaded pages gain COPY_SRC; imported ones already have it.
• Why blend, not copy: a popup's first paints are fully transparent. On nixos, a transparent <select> over a blue source showed the blue scene through the page for 4 frames when its popup was copied. Blended, it never does.
• Input needs no change. CEF's offscreen host routes mouse events over the popup to it. Without a GetScreenInfo handler, CEF uses the view as the screen, so Chromium keeps the popup inside the view.
• Untrusted page: the area comes from the page and may reach past the view or disagree with the painted size. clip draws only the part that lands on the view, from the matching part of the picture, in i64, so no area makes an out-of-bounds draw. Sizes keep their existing bounds (MAX_MESSAGE, the device texture limit).
• window.open and target="_blank" stay cancelled, as the quest says.

Cost: nothing while no popup is open. While one is open, each paint allocates one view-sized texture, makes one view-sized GPU copy, and draws one popup-sized pass. The two pipelines (RGBA and BGRA) are built once per helper start.

Impact

• A browser source's dropdown lists draw over the page and take clicks under Interact. README.md drops the "not drawn yet" note.
• Public API: none. moq_scene::browser exports are unchanged.
• Wire: the private studio-helper socket protocol changes (Event::Texture and Event::Pixels carry a Layer, and there is a new Event::Popup). Both ends come from the same binary, so there is nothing to migrate.
• just render-sw also runs browser::layers::tests::.

Alternatives

• Compose in the helper, as cefclient's software renderer does: this works only for pixels. On the GPU path the helper holds DMA-BUF descriptors and has no device, so it would need a second implementation.
• Copy the popup with copy_texture_to_texture (this PR's first version): simpler, but it punches transparent popup pixels through to the scene behind, as reproduced above.
• Draw the popup into the incoming view texture in place: this saves one view copy per paint while a popup is open. But the clean view would be lost, so a moved or closed popup would need Invalidate(PET_VIEW) and a wait for a repaint. Not worth it for a widget open for seconds.
• Clamp the popup into the view and offset input, as cefclient does: cefclient needs that because it reports the real screen. Here the screen is the view, so clipping is enough and input lands where the popup is drawn.
• Hand the compositor a second layer: this would touch the shared Picture type and the compositor for one source kind.

Validation

• macOS: just ci passed (163 tests) at 035cf86.
• nixos (Linux, x86_64), at 035cf86:
  • cargo clippy --workspace --all-targets -D warnings passed with and without moq-scene/capture,moq-studio/capture,moq-studio/openh264,moq-studio/cef.
  • cargo nextest run -p moq-scene --features cef --lib browser passed (10 tests, including the new clip test).
• nixos just render-sw:
  • compositor and popup tests: 20/20, including the new a_popup_blends_over_the_page_where_placed;
  • engine tests: 3/3;
  • browser tests: 5/6, including the new a_dropdown_draws_and_takes_clicks.
• The new browser test opens a <select> and waits for its lime list to cover the page center. It then clicks an option and waits for the page's onchange to turn the page blue. That checks drawing, input into the popup, and the popup clearing on close.
• The new device test checks:
  • placement of a popup clipped off the view's edge;
  • a transparent pixel showing the page;
  • half green blending over red;
  • a BGRA popup over an RGBA page;
  • the page's own texture staying clean;
  • a closed popup showing the view again.
• Negative control: the new browser test on main fails with "the center stayed [255, 0, 0, 255], not [0, 255, 0, 255]", so the list was never drawn.
• Pre-existing: a_crash_drops_only_its_source fails with "renderer never failed" both on this branch and on main (same command, just render-sw's browser step, nixos).
• Paint paths: the CEF test passed on the CPU fallback path, with lavapipe and with the NVIDIA RTX 3070 Ti under Xvfb (CEF composites in software there). I could not exercise the GPU import path on any host I can reach. With NVIDIA on the real X display, CEF's GPU process fails with "Can't create buffer -- gbm device is missing" (dri_gbm.so: Permission denied), and no page paints at all, on main too. The GPU branch is the same import call the view uses, with the PET_POPUP type passed through, and the draw after import is shared with the CPU path. moq-dev/studio#8 validated the view's GPU path on an AMD GPU.
• When a page starts its helper first, CEF's GPU-process fallback on GL-less hosts sometimes stalled past the 20 s wait in a cold, single-test run. This affects every browser test, not just the new one: other runs of the new test passed in 0.8 to 4 s.

Follow-ups

• Verify the shared-texture path, including popups, on an AMD or Intel host. The NVIDIA host here cannot run CEF's GPU path (GBM).

Review

Codex (quest review), gpt-6.1-sol, medium effort. Session 01a1a787.

• Round 1 on 66974b0, 1 finding: "Alpha-blend the popup over the page" (layers.rs:66). Fixed in eaf0655 and 035cf86 after reproducing it (transparent first paints showed the scene behind). The popup is now drawn premultiplied, with a device test.
• Round 2 on 035cf86: no findings.

(Written by Claude Opus 5.5)

[coderabbitai]

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 31 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: 040dba1b-eb2b-4c73-87a1-1d879dcf0351

Commits

Reviewing files that changed from the base of the PR and between cf23534 and 21266e4.

Files selected for processing: 10

• README.md
• justfile
• quest/m0/README.md
• quest/m0/browser-popups.md
• rs/moq-scene/src/browser/host.rs
• rs/moq-scene/src/browser/layers.rs
• rs/moq-scene/src/browser/mod.rs
• rs/moq-scene/src/browser/protocol.rs
• rs/moq-scene/src/browser/run.rs
• rs/moq-studio/tests/browser.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

• X
• Mastodon
• Reddit
• LinkedIn

[kixelated]

Automated review by review (OpenAI)

Reviewed head 035cf8643f5f10037b4c5de9d390000a4d40cb25 against base 3a4171a1020f4aebcb70002b919a14f9d9c17974. No actionable finding. The popup is clipped to the view, blended over a retained clean picture, and cleared on close; the existing shared-texture copy/acknowledgement handoff is retained for popup paints.

Verification: GitHub-only static review of lifecycle, composition, input paths and tests. I did not run CEF or GPU tests. The author reports the accelerated popup import path remains untested and one existing browser crash test failed on both this branch and main; I did not independently reproduce either.

(Written by review (OpenAI))

[pullfrog]

✅ No new issues found.

Reviewed changes

Reviewed all 10 files (5 commits, 5527dea..035cf86) covering the CEF popup event path, the new GPU overlay composition, and its tests and docs.

• Helper popup events — on_popup_show/on_popup_size forward the popup/close as Event::Popup, and both paint paths now carry a Layer.
• Overlay composition — layers.rs clips the popup to the view (i64 math, no OOB draw) and blends it premultiplied over a copy of the clean page, keeping the clean view for moves and closes.
• Routing and uploads — run.rs routes popup paints through Layers and redraws on area changes; CPU uploads add COPY_SRC so both paint paths compose the same way.
• Tests and docs — clip/device/dropdown tests added, render-sw runs the layer tests, the README limitation is dropped, and the completed quest file/entry are removed.

  ^{｜ View workflow run ｜ Using big-pickle (free) ｜ 𝕏}