Skip to content

refactor(core): honor canvas contexts throughout render passes#10474

Open
ibgreen-openai wants to merge 2 commits into
masterfrom
codex/multi-canvas-context-plumbing
Open

refactor(core): honor canvas contexts throughout render passes#10474
ibgreen-openai wants to merge 2 commits into
masterfrom
codex/multi-canvas-context-plumbing

Conversation

@ibgreen-openai

@ibgreen-openai ibgreen-openai commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Goal

Make deck.gl's existing render and picking passes consistently honor the active luma canvas context without introducing a multi-canvas API or changing the single-canvas default.

Changes

  • Accept either a CanvasContext or PresentationContext in internal rendering and picking options.
  • Propagate that context through synchronous/asynchronous point and rectangle picking, including the picking render pass.
  • Derive framebuffer size, viewport coordinates, pixel ratio, and post-processing buffer sizes from the active context.
  • Add focused coverage for context propagation and drawing-buffer/pixel-ratio behavior.

Review scope

Seven files; no new Deck props, canvas management, view-layout changes, widgets, or application examples. Omitting the context preserves the existing default behavior.

Validation

  • yarn build
  • Repository lint and formatting checks through the pre-commit hook
  • yarn vitest run --project headless test/modules/core/lib/deck-picker.spec.ts test/modules/core/passes/layers-pass.spec.ts test/modules/core/passes/pick-layers-pass.spec.ts
  • Node smoke tests through the pre-commit hook
  • yarn test-headless --retry 1 --silent — 793 passed, 8 skipped; unrelated existing React-mount and LoadingWidget tests required their configured retry.

Stack

  1. This PR: context-aware render/picking plumbing.
  2. refactor(core): size and filter viewports by canvas #10475: a separate, two-file viewport-layout preparation PR.
  3. Existing multi-canvas feature PR RFC/POC: Extend View system to support multi-canvas rendering #10229.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 99c2cbfdc2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


protected _render(options: LayersPassRenderOptions): RenderStats[] {
const canvasContext = this.device.canvasContext!;
const {canvasContext = this.device.canvasContext!} = options;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Clear views on the selected canvas framebuffer

When canvasContext is a non-default presentation context and a View has clear enabled, _render resolves that context's framebuffer here, but _drawLayers still forwards only the original target (typically null or undefined) and _drawLayersInViewport opens its clear pass with framebuffer: target. The clear consequently runs against the device's default canvas rather than the framebuffer used by the outer render pass, leaving stale color/depth in the selected canvas and potentially clearing another canvas; pass the resolved framebuffer into the per-view clear.

Useful? React with 👍 / 👎.


if (this.lastPostProcessEffect) {
this._resizeRenderBuffers();
this._resizeRenderBuffers(opts.canvasContext);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Resolve post-process output against the selected canvas

When a non-default canvasContext is rendered with a built-in PostProcessEffect and the canvas target is null, this only sizes the intermediate buffers from the selected context. _postRender still gives the final effect opts.target, so PostProcessEffect/ScreenPass starts its final pass with a null framebuffer, which selects the device's default canvas rather than canvasContext.getCurrentFramebuffer(); the selected canvas receives no composited output while the default canvas is overwritten.

Useful? React with 👍 / 👎.

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 83.374% (+0.002%) from 83.372% — codex/multi-canvas-context-plumbing into master

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.

3 participants