RFC/POC: Extend View system to support multi-canvas rendering#10229
RFC/POC: Extend View system to support multi-canvas rendering#10229ibgreen wants to merge 14 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72fa41c755
ℹ️ 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".
| if (!canvasViewports.length) { | ||
| continue; |
There was a problem hiding this comment.
Clear canvases when no views render to them
When running in multi-canvas mode, canvases with zero mapped viewports are skipped via continue, so they are never rendered or presented again. If a view is removed or reassigned to another canvas at runtime, the old frame remains visible indefinitely on the now-unused canvas, producing stale content during dynamic layout/view updates.
Useful? React with 👍 / 👎.
79b90e5 to
8ba7afe
Compare
Pessimistress
left a comment
There was a problem hiding this comment.
-
Did you test widgets at all? I did not see any changes to the widget manager on placement handling w/ multiple canvases.
-
I strongly recommend that you exclude react from the initial PR. The implementation looks very hacky and not conforming to reactive patterns. And this is adding on top of existing complex React flow with known bugs.
| new MapView({id: 'tokyo', canvasId: 'canvas-tokyo', controller: true}) | ||
| ]; | ||
|
|
||
| <DeckGL canvases={['canvas-london', 'canvas-tokyo']} views={views} layers={layers}> |
There was a problem hiding this comment.
Use a non-react example here? DeckGL react component creates the canvas, so what are the canvas ids in this case?
| }); | ||
| } | ||
|
|
||
| private _getCanvasEventRoot(canvas: HTMLCanvasElement): HTMLElement { |
There was a problem hiding this comment.
Deck currently has an option to customize the event root by class name, this seems to be breaking the use case.
There was a problem hiding this comment.
Deck currently has an option to customize the event root by class name, this seems to be breaking the use case.
I had codex address this comment, could use another look.
85cee04 to
ff8fefb
Compare
e33dd8e to
84979d1
Compare
be0136a to
53bd285
Compare
RFC: Multi-Canvas Presentation for deck.gl
PresentationContext@deck.gl/core@deck.gl/reactSummary
Add a multi-canvas presentation mode to deck.gl built on luma.gl 9.3
PresentationContexts.The proposal adds:
DeckProps.canvases?: (string | HTMLCanvasElement)[]View.canvasId?: stringWhen
canvasesis defined, deck.gl creates or uses an offscreen-backed default device canvas and presents rendered output into one or more DOM canvases. Views are assigned to canvases bycanvasId. Each canvas gets its ownPresentationContext, event handling root, and controller binding.This enables a single
Deckinstance to render multiple independently interactive views into separate canvases on a normal web page, while keeping React basemaps and otherViewchildren bound to the correct view and canvas.A proof-of-concept app demonstrates four independently navigable city maps on one page, each with its own basemap, controller, and filtered data.
Motivation
deck.gl today assumes a single presentation surface. This makes certain layouts awkward or impossible without creating multiple
Deckinstances:Creating multiple
Deckinstances works, but it duplicates GPU state, layer setup, animation loops, and application plumbing. luma.gl 9.3 introduces the missing primitive: one device can present to multiple canvases through multiplePresentationContexts.This RFC proposes a deck.gl integration that:
Deckand single layer graphGoals
Deckinstance.Viewmodel.Viewchildren, including basemaps, colocated with the correct canvas.canvasesprop, including add/remove/reorder.canvasesis not supplied.Non-Goals
@deck.gl/mapbox@deck.gl/google-maps@deck.gl/arcgisBackground
luma.gl 9.3 supports multiple
PresentationContexts per device. deck.gl can use this by separating:This RFC treats canvas assignment as a property of a
View, not of a layer or basemap. That keeps the model consistent:View.idbinds child content to the viewView.canvasIdbinds the view to a presentation canvasProposal
Public API
DeckProps.canvases