-
Notifications
You must be signed in to change notification settings - Fork 2.2k
refactor(core): honor canvas contexts throughout render passes #10474
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,8 +3,10 @@ | |
| // Copyright (c) vis.gl contributors | ||
|
|
||
| import type { | ||
| CanvasContext, | ||
| Device, | ||
| Parameters, | ||
| PresentationContext, | ||
| RenderPassParameters, | ||
| RenderPipelineParameters | ||
| } from '@luma.gl/core'; | ||
|
|
@@ -36,6 +38,8 @@ const WEBGPU_DEFAULT_DRAW_PARAMETERS: RenderPipelineParameters = { | |
| export type LayersPassRenderOptions = { | ||
| /** @deprecated TODO v9 recommend we rename this to framebuffer to minimize confusion */ | ||
| target?: Framebuffer | null; | ||
| /** Canvas context that provides framebuffer dimensions and pixel conversion. */ | ||
| canvasContext?: CanvasContext | PresentationContext; | ||
| isPicking?: boolean; | ||
| pass: string; | ||
| layers: Layer[]; | ||
|
|
@@ -87,7 +91,7 @@ export default class LayersPass extends Pass { | |
| } | ||
|
|
||
| protected _render(options: LayersPassRenderOptions): RenderStats[] { | ||
| const canvasContext = this.device.canvasContext!; | ||
| const {canvasContext = this.device.canvasContext!} = options; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in 0309acd. Regression coverage: |
||
| const framebuffer = options.target ?? canvasContext.getCurrentFramebuffer(); | ||
| const [width, height] = canvasContext.getDrawingBufferSize(); | ||
|
|
||
|
|
@@ -115,7 +119,7 @@ export default class LayersPass extends Pass { | |
| }); | ||
|
|
||
| try { | ||
| return this._drawLayers(renderPass, options); | ||
| return this._drawLayers(renderPass, {...options, target: framebuffer}); | ||
| } finally { | ||
| renderPass.end(); | ||
| // TODO(ibgreen): WebGPU - submit may not be needed here but initial port had issues with out of render loop rendering | ||
|
|
@@ -126,6 +130,7 @@ export default class LayersPass extends Pass { | |
| /** Draw a list of layers in a list of viewports */ | ||
| private _drawLayers(renderPass: RenderPass, options: LayersPassRenderOptions) { | ||
| const { | ||
| canvasContext = this.device.canvasContext!, | ||
| target, | ||
| shaderModuleProps, | ||
| viewports, | ||
|
|
@@ -156,6 +161,7 @@ export default class LayersPass extends Pass { | |
| renderPass, | ||
| { | ||
| target, | ||
| canvasContext, | ||
| shaderModuleProps, | ||
| viewport: subViewport, | ||
| view, | ||
|
|
@@ -184,6 +190,7 @@ export default class LayersPass extends Pass { | |
| cullRect, | ||
| views, | ||
| effects, | ||
| canvasContext = this.device.canvasContext!, | ||
| shaderModuleProps | ||
| }: LayersPassRenderOptions, | ||
| /** Internal flag, true if only used to determine whether each layer should be drawn */ | ||
|
|
@@ -223,6 +230,7 @@ export default class LayersPass extends Pass { | |
| layer, | ||
| effects, | ||
| pass, | ||
| canvasContext, | ||
| shaderModuleProps | ||
| ); | ||
| const defaultParams = | ||
|
|
@@ -251,6 +259,7 @@ export default class LayersPass extends Pass { | |
| shaderModuleProps: globalModuleParameters, | ||
| pass, | ||
| target, | ||
| canvasContext, | ||
| viewport, | ||
| view, | ||
| isPicking | ||
|
|
@@ -259,13 +268,15 @@ export default class LayersPass extends Pass { | |
| shaderModuleProps: Record<string, any>; | ||
| pass: string; | ||
| target?: Framebuffer | null; | ||
| canvasContext: CanvasContext | PresentationContext; | ||
| viewport: Viewport; | ||
| view?: View; | ||
| isPicking?: boolean; | ||
| }, | ||
| drawLayerParams: DrawLayerParameters[] | ||
| ): RenderStats { | ||
| const glViewport = getGLViewport(this.device, { | ||
| canvasContext, | ||
| shaderModuleProps: globalModuleParameters, | ||
| target, | ||
| viewport | ||
|
|
@@ -431,10 +442,10 @@ export default class LayersPass extends Pass { | |
| layer: Layer, | ||
| effects: Effect[] | undefined, | ||
| pass: string, | ||
| canvasContext: CanvasContext | PresentationContext, | ||
| overrides: any | ||
| ): any { | ||
| // @ts-expect-error TODO - assuming WebGL context | ||
| const devicePixelRatio = this.device.canvasContext.cssToDeviceRatio(); | ||
| const devicePixelRatio = canvasContext.cssToDeviceRatio(); | ||
| const layerProps = layer.internalState?.propsInTransition || layer.props; | ||
|
|
||
| const shaderModuleProps = { | ||
|
|
@@ -530,23 +541,22 @@ export function layerIndexResolver( | |
| function getGLViewport( | ||
| device: Device, | ||
| { | ||
| canvasContext = device.canvasContext!, | ||
| shaderModuleProps, | ||
| target, | ||
| viewport | ||
| }: { | ||
| canvasContext?: CanvasContext | PresentationContext; | ||
| shaderModuleProps: any; | ||
| target?: Framebuffer | null; | ||
| viewport: Viewport; | ||
| } | ||
| ): [number, number, number, number] { | ||
| const pixelRatio = | ||
| shaderModuleProps?.project?.devicePixelRatio ?? | ||
| // @ts-expect-error TODO - assuming WebGL context | ||
| device.canvasContext.cssToDeviceRatio(); | ||
| shaderModuleProps?.project?.devicePixelRatio ?? canvasContext.cssToDeviceRatio(); | ||
|
|
||
| // Default framebuffer is used when writing to canvas | ||
| // @ts-expect-error TODO - assuming WebGL context | ||
| const [, drawingBufferHeight] = device.canvasContext.getDrawingBufferSize(); | ||
| const [, drawingBufferHeight] = canvasContext.getDrawingBufferSize(); | ||
| const height = target ? target.height : drawingBufferHeight; | ||
|
|
||
| // Convert viewport top-left CSS coordinates to bottom up WebGL coordinates | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a non-default
canvasContextis rendered with a built-inPostProcessEffectand the canvas target isnull, this only sizes the intermediate buffers from the selected context._postRenderstill gives the final effectopts.target, soPostProcessEffect/ScreenPassstarts its final pass with a null framebuffer, which selects the device's default canvas rather thancanvasContext.getCurrentFramebuffer(); the selected canvas receives no composited output while the default canvas is overwritten.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 0309acd.
DeckRenderer._postRendernow resolves the final output asopts.target ?? opts.canvasContext?.getCurrentFramebuffer() ?? opts.target, so the last post-processing effect renders to the selected canvas while preserving existing default-canvas behavior when no alternate context is supplied.Regression coverage:
DeckRenderer#post processing renders to the supplied canvas framebufferexercises a two-effect chain and verifies the intermediate effect receives no target while the final effect receives the selected framebuffer. Revalidated with 14 affected rendering/picking tests passing.