Skip to content

viewer: expose snapshot render channels - #644

Closed
thomas-yanxin wants to merge 1 commit into
pascalorg:mainfrom
thomas-yanxin:codex/snapshot-capture-channels
Closed

viewer: expose snapshot render channels#644
thomas-yanxin wants to merge 1 commit into
pascalorg:mainfrom
thomas-yanxin:codex/snapshot-capture-channels

Conversation

@thomas-yanxin

@thomas-yanxin thomas-yanxin commented Aug 13, 2026

Copy link
Copy Markdown

What does this PR do?

Adds a backward-compatible captureChannels() API to the viewer snapshot pipeline. It returns same-camera color, albedo, view-normal, and linear camera-range depth captures for visual evidence and diagnostic consumers.

  • Reuses the existing scene MRT; the scene geometry is still rendered once.
  • Creates albedo/depth conversion pipelines and targets lazily, so existing capture() calls pay no extra resource cost.
  • Keeps display color as WebP and encodes the three data views losslessly as PNG.
  • Leaves the existing capture() API and output behavior unchanged.

How to test

  1. Run bun check.
  2. Run bun run check-types.
  3. Run bun run test.
  4. Start bun dev and confirm the editor and IFC routes load.

Additional browser smoke coverage rendered a lit box and floor through the WebGL fallback, then verified all four captures share dimensions, have the declared MIME types, and contain non-empty, visually distinct color/albedo/normal/depth data.

Screenshots / screen recording

No UI changes. The browser smoke test visually verified aligned color, unlit albedo, packed view-normal, and linear depth outputs.

Checklist

  • I've tested this locally with bun dev
  • My code follows the existing code style (run bun check to verify)
  • I've documented the public channel semantics inline
  • This PR targets the main branch

Note

Medium Risk
Touches GPU render targets, MRT readback, and WebGPU/WebGL pixel paths, but existing capture() callers are unchanged and new resources are lazy.

Overview
Adds captureChannels() on the snapshot pipeline so consumers can get color, albedo, view-normal, and linear depth from the same camera in one call, alongside unchanged capture() (still WebP display color only).

Channel metadata is exported (SNAPSHOT_CHANNELS, per-channel MIME: WebP for color, PNG for data buffers). Pixel readback, crop/resize, and WebGPU vs WebGL2 row handling are centralized in readCapture, reused by both APIs. Albedo and depth use lazy extra render pipelines/targets; capture() alone does not allocate them until captureChannels() runs.

Reviewed by Cursor Bugbot for commit b72d31c. Bugbot is set up for automated code reviews on this repo. Configure here.

@thomas-yanxin
thomas-yanxin marked this pull request as ready for review August 13, 2026 22:36
@Aymericr

Copy link
Copy Markdown
Contributor

Thanks for this, and welcome — the MRT reuse here is the right instinct. Sampling texture(albedoTexture) rather than the pass node so the scene geometry still renders once is exactly the thing a first attempt usually gets wrong, and you got it right.

I'm going to close this one, for reasons that are about the shape of the change rather than your execution.

The main one: nothing in the codebase consumes these channels. The only callers of the snapshot pipeline are bake-thumbnail.tsx and thumbnail-generator.tsx, and both want one WebP. We keep a hard rule against landing public API ahead of its consumer (AGENTS.md: no speculative abstractions), because an unused export in @pascal-app/viewer is an npm compatibility promise we then have to keep. SNAPSHOT_CHANNELS, SNAPSHOT_CHANNEL_MIME_TYPES, SnapshotChannel, SnapshotChannelCaptureResult and captureChannels are five new promises with no caller.

The cost side is what makes it a close rather than a hold: the diff moves 124 lines of the existing capture path into readCapture. That path was hardened twice recently (#622 for the WebP encode, #639 for the 2048px long-edge clamp) and every regression this can introduce is in code that works today. I'd want a real consumer on the table before spending that.

Three specific things, in case this comes back:

  • The test asserts SNAPSHOT_CHANNELS equals ['color', 'albedo', 'normal', 'depth'] and the MIME map equals itself. That's a restatement of the source, not coverage — none of the new GPU code is exercised. The browser smoke run you describe in the body is the valuable part, and it isn't committed.
  • The depth math hand-rolls perspectiveDepthToViewZviewZToOrthographicDepth with its own near/far uniforms. That's PassNode.getLinearDepthNode(), which reads the pass's own _cameraNear/_cameraFar. Two sources of truth for the same two numbers will drift.
  • projectionCamera.isOrthographicCamera is read once at pipeline construction, but applyEnvironment({ camera }) takes a camera per capture. Today both callers pass the same PerspectiveCamera, so the ortho branch never runs; once the 2D stage cameras from feat(editor): add synchronized 2D viewer modes #672 reach a capture, it produces wrong depth with no error.

Also worth saying: this and #646 are two independent takes on overlapping scope, from the same base, with different API shapes and opposite depth encodings (linear near-black here, inverted near-white there). Only one could ever land. I'll reply on #646 with where I'd go if we do this.

Genuinely glad you're poking at the render pipeline — it's the least-explored corner of the viewer. If you want a channel export that lands, the path is a consumer first.

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.

2 participants