From 3af6827849149b5a26f638039b649c55ee30ba83 Mon Sep 17 00:00:00 2001 From: Ib Green Date: Fri, 11 Aug 2023 21:42:30 -0400 Subject: [PATCH] fix(engine) Workaround for deck.gl attribute access (#1778) --- .../src/adapter/resources/render-pipeline.ts | 3 + modules/engine/src/model/model.ts | 87 +++++-- .../objects/webgl-vertex-array-object.ts | 216 +++++++++++++++--- .../resources/webgl-render-pipeline.ts | 25 +- .../resources/webgpu-render-pipeline.ts | 9 +- 5 files changed, 297 insertions(+), 43 deletions(-) diff --git a/modules/api/src/adapter/resources/render-pipeline.ts b/modules/api/src/adapter/resources/render-pipeline.ts index efa88528eb..6a89312347 100644 --- a/modules/api/src/adapter/resources/render-pipeline.ts +++ b/modules/api/src/adapter/resources/render-pipeline.ts @@ -1,5 +1,6 @@ // luma.gl, MIT license import type {Device} from '../device'; +import type {TypedArray} from '../../types'; import type {PrimitiveTopology, RenderPipelineParameters} from '../types/parameters'; import type {ShaderLayout, BufferMapping, Binding} from '../types/shader-layout'; // import {normalizeAttributeMap} from '../helpers/attribute-bindings'; @@ -104,6 +105,8 @@ export abstract class RenderPipeline extends Resource { abstract setIndexBuffer(indices: Buffer): void; /** Set attributes (stored on pipeline and set before each call) */ abstract setAttributes(attributes: Record): void; + /** Set constant attributes (WebGL only) */ + abstract setConstantAttributes(attributes: Record): void; /** Set bindings (stored on pipeline and set before each call) */ abstract setBindings(bindings: Record): void; /** Uniforms (only supported on WebGL devices. Reset before each call to enable pipeline sharing) */ diff --git a/modules/engine/src/model/model.ts b/modules/engine/src/model/model.ts index e498743172..b9e896fbe7 100644 --- a/modules/engine/src/model/model.ts +++ b/modules/engine/src/model/model.ts @@ -1,11 +1,19 @@ // luma.gl, MIT license -import type {Device, Buffer, RenderPipelineProps, RenderPass, Binding, PrimitiveTopology} from '@luma.gl/api'; +import { + Device, + Buffer, + RenderPipelineProps, + RenderPass, + Binding, + PrimitiveTopology +} from '@luma.gl/api'; import {RenderPipeline} from '@luma.gl/api'; -import type { ShaderModule } from '@luma.gl/shadertools'; +import type {ShaderModule} from '@luma.gl/shadertools'; import type {Geometry} from '../geometry/geometry'; import {getAttributeBuffersFromGeometry, getIndexBufferFromGeometry} from './model-utils'; import {PipelineFactory} from '../lib/pipeline-factory'; +import {TypedArray} from '@math.gl/core'; export type ModelProps = Omit & { // Model also accepts a string @@ -57,7 +65,7 @@ export class Model { this.device = device; Object.assign(this.userData, this.props.userData); - + // Create the pipeline if (!props.vs) { throw new Error('no vertex shader'); @@ -75,7 +83,8 @@ export class Model { this.topology = this.props.geometry.topology || 'triangle-list'; } - this.pipelineFactory = this.props.pipelineFactory || PipelineFactory.getDefaultPipelineFactory(this.device); + this.pipelineFactory = + this.props.pipelineFactory || PipelineFactory.getDefaultPipelineFactory(this.device); const {pipeline, getUniforms} = this.pipelineFactory.createRenderPipeline({ ...this.props, vs: this.vs, @@ -92,7 +101,7 @@ export class Model { if (this.props.geometry) { this._setGeometry(this.props.geometry); } - this.setUniforms(this._getModuleUniforms()) // Get all default module uniforms + this.setUniforms(this._getModuleUniforms()); // Get all default module uniforms this.setProps(this.props); } @@ -140,17 +149,67 @@ export class Model { return this; } - setAttributes(attributes: Record): this { + // Temporary hack to support deck.gl's dependency on luma.gl v8 Model attribute API. + _splitAttributes( + attributes: Record, + filterBuffers?: boolean + ): { + bufferAttributes: Record; + constantAttributes: Record; + indices?: Buffer; + } { + const bufferAttributes: Record = {}; + const constantAttributes: Record = {}; + const indices: Buffer | undefined = attributes.indices as Buffer; + + delete attributes.indices; + + for (const name in attributes) { + let attribute = attributes[name]; + + if (attribute instanceof Buffer) { + bufferAttributes[name] = attribute; + continue; + } + + // The `getValue` call provides support for deck.gl `Attribute` class + // TODO - remove once deck refactoring completes + // @ts-ignore + if (attribute.getValue) { + // @ts-ignore + attribute = attribute.getValue(); + console.warn(`attribute ${name}: getValue() will be removed`); + } + + if (ArrayBuffer.isView(attribute) && !attribute) { + constantAttributes[name] = attribute as unknown as TypedArray; + continue; + } + + // @ts-ignore + if (filterBuffers && attribute[name]._buffer) { + // @ts-ignore + buffer[name] = attribute[name]._buffer; + } + } + + return {bufferAttributes, constantAttributes, indices}; + } + + setAttributes(attributes: Record, filterBuffers?: boolean): void { + const {bufferAttributes, constantAttributes, indices} = this._splitAttributes(attributes, filterBuffers); + // Temporary HACK since deck.gl v9 sets indices as part of attributes - if (attributes.indices) { - this.setIndexBuffer(attributes.indices); - attributes = {...attributes}; - delete attributes.indices; - console.warn('luma.gl: indices should not be part of attributes') + if (indices) { + this.setIndexBuffer(indices); + console.warn('luma.gl: indices should not be part of attributes'); } - this.pipeline.setAttributes(attributes); - Object.assign(this.props.attributes, attributes); - return this; + + this.pipeline.setAttributes(bufferAttributes); + // TODO - WebGL only. We may have to allocate buffers on WebGPU + this.pipeline.setConstantAttributes(constantAttributes); + + Object.assign(this.props.attributes, bufferAttributes, constantAttributes); } /** Set the bindings */ diff --git a/modules/webgl/src/adapter/objects/webgl-vertex-array-object.ts b/modules/webgl/src/adapter/objects/webgl-vertex-array-object.ts index 7b7bf1b5fd..330f80e664 100644 --- a/modules/webgl/src/adapter/objects/webgl-vertex-array-object.ts +++ b/modules/webgl/src/adapter/objects/webgl-vertex-array-object.ts @@ -1,18 +1,23 @@ -import {assert, ResourceProps} from '@luma.gl/api'; +import type {Device, Buffer, ResourceProps, TypedArray, NumericArray} from '@luma.gl/api'; +import {Resource, assert, getScratchArray, fillArray} from '@luma.gl/api'; import {GL} from '@luma.gl/constants'; import {getBrowser} from '@probe.gl/env'; import {WebGLDevice} from '../webgl-device'; import {WebGLResource} from './webgl-resource'; - import {WEBGLBuffer} from '../resources/webgl-buffer'; +import {BufferWithAccessor} from '../../classic/buffer-with-accessor'; + const ERR_ELEMENTS = 'elements must be GL.ELEMENT_ARRAY_BUFFER'; /** * VertexArrayObject properties + * @param constantAttributeZero Attribute 0 can not be disable on most desktop OpenGL based browsers + * and on iOS Safari browser. */ export type VertexArrayObjectProps = ResourceProps & { + constantAttributeZero?: boolean; }; /** VertexArrayObject wrapper */ @@ -21,11 +26,26 @@ export class WEBGLVertexArrayObject extends WebGLResource + enable + ? this.gl.enableVertexAttribArray(location) + : this.gl.disableVertexAttribArray(location) + ); + } } + // Set (bind) an elements buffer, for indexed rendering. // Must be a Buffer bound to GL.ELEMENT_ARRAY_BUFFER. Constants not supported setElementBuffer(elementBuffer: WEBGLBuffer | null = null, opts = {}) { @@ -50,15 +90,14 @@ export class WEBGLVertexArrayObject extends WebGLResource { this.gl.bindBuffer(GL.ELEMENT_ARRAY_BUFFER, elementBuffer ? elementBuffer.handle : null); }); - - return this; } /** Set a location in vertex attributes array to a buffer, enables the location, sets divisor */ - setBuffer(location: number, buffer: WEBGLBuffer, accessor: any): this { + setBuffer(location: number, buffer: WEBGLBuffer, accessor: any): void { // Check target if (buffer.target === GL.ELEMENT_ARRAY_BUFFER) { - return this.setElementBuffer(buffer, accessor); + this.setElementBuffer(buffer, accessor); + return; } const {size, type, stride, offset, normalized, integer, divisor} = accessor; @@ -83,29 +122,154 @@ export class WEBGLVertexArrayObject extends WebGLResource - enable - ? this.gl.enableVertexAttribArray(location) - : this.gl.disableVertexAttribArray(location) - ); + const constantValue = normalizeConstantArrayValue(value); + + const byteLength = constantValue.byteLength * elementCount; + const length = constantValue.length * elementCount; + + let updateNeeded = !this.buffer; + + this.buffer = this.buffer || this.device.createBuffer({byteLength}) as BufferWithAccessor; + updateNeeded = updateNeeded || this.buffer.reallocate(byteLength); + + // Reallocate and update contents if needed + updateNeeded = + updateNeeded || !compareConstantArrayValues(constantValue, this.bufferValue); + + if (updateNeeded) { + // Create a typed array that is big enough, and fill it with the required data + const typedArray = getScratchArray(value.constructor, length); + fillArray({target: typedArray, source: constantValue, start: 0, count: length}); + this.buffer.subData(typedArray); + this.bufferValue = value; + } + + return this.buffer; + } +} + +function setConstantFloatArray(device: WebGLDevice, location: number, array: Float32Array): void { + switch (array.length) { + case 1: + device.gl.vertexAttrib1fv(location, array); + break; + case 2: + device.gl.vertexAttrib2fv(location, array); + break; + case 3: + device.gl.vertexAttrib3fv(location, array); + break; + case 4: + device.gl.vertexAttrib4fv(location, array); + break; + default: + assert(false); + } +} + +function setConstantIntArray(device: WebGLDevice, location: number, array: Int32Array): void { + device.assertWebGL2(); + device.gl2?.vertexAttribI4iv(location, array); + // switch (array.length) { + // case 1: + // gl.vertexAttribI1iv(location, array); + // break; + // case 2: + // gl.vertexAttribI2iv(location, array); + // break; + // case 3: + // gl.vertexAttribI3iv(location, array); + // break; + // case 4: + // break; + // default: + // assert(false); + // } +} + +function setConstantUintArray(device: WebGLDevice, location: number, array: Uint32Array) { + device.assertWebGL2(); + device.gl2?.vertexAttribI4uiv(location, array); + // switch (array.length) { + // case 1: + // gl.vertexAttribI1uiv(location, array); + // break; + // case 2: + // gl.vertexAttribI2uiv(location, array); + // break; + // case 3: + // gl.vertexAttribI3uiv(location, array); + // break; + // case 4: + // gl.vertexAttribI4uiv(location, array); + // break; + // default: + // assert(false); + // } +} + +// HELPERS + +/** + * TODO - convert Arrays based on known type? (read type from accessor, don't assume Float32Array) + * TODO - handle single values for size 1 attributes? + */ +function normalizeConstantArrayValue(arrayValue: NumericArray) { + if (Array.isArray(arrayValue)) { + return new Float32Array(arrayValue); + } + return arrayValue; +} + +/** + * + */ +function compareConstantArrayValues(v1: NumericArray, v2: NumericArray): boolean { + if (!v1 || !v2 || v1.length !== v2.length || v1.constructor !== v2.constructor) { + return false; + } + for (let i = 0; i < v1.length; ++i) { + if (v1[i] !== v2[i]) { + return false; } - return this; } + return true; } diff --git a/modules/webgl/src/adapter/resources/webgl-render-pipeline.ts b/modules/webgl/src/adapter/resources/webgl-render-pipeline.ts index 34845a02a0..900f5678f8 100644 --- a/modules/webgl/src/adapter/resources/webgl-render-pipeline.ts +++ b/modules/webgl/src/adapter/resources/webgl-render-pipeline.ts @@ -6,7 +6,8 @@ import type { ShaderLayout, PrimitiveTopology, // BindingLayout, - AttributeLayout + AttributeLayout, + TypedArray } from '@luma.gl/api'; import {RenderPipeline, cast, log, decodeVertexFormat} from '@luma.gl/api'; import {GL} from '@luma.gl/constants'; @@ -112,7 +113,27 @@ export class WEBGLRenderPipeline extends RenderPipeline { } } - /** @todo needed for portable model */ + /** + * Constant attributes are only supported in WebGL, not in WebGPU + * Any attribute that is disabled in the current vertex array object + * is read from the context's global constant value for that attribute location. + * @param attributes + */ + setConstantAttributes(attributes: Record): void { + for (const [name, value] of Object.entries(attributes)) { + const attribute = getAttributeLayout(this.layout, name); + if (!attribute) { + log.warn(`Ignoring constant value supplied for unknown attribute "${name}" in pipeline "${this.id}"`)(); + continue; // eslint-disable-line no-continue + } + this.vertexArrayObject.setConstant(attribute.location, value); + } + } + + /** + * Bindings include: textures, samplers and uniform buffers + * @todo needed for portable model + */ setBindings(bindings: Record): void { // if (log.priority >= 2) { // checkUniformValues(uniforms, this.id, this._uniformSetters); diff --git a/modules/webgpu/src/adapter/resources/webgpu-render-pipeline.ts b/modules/webgpu/src/adapter/resources/webgpu-render-pipeline.ts index eca1db794d..98aea06289 100644 --- a/modules/webgpu/src/adapter/resources/webgpu-render-pipeline.ts +++ b/modules/webgpu/src/adapter/resources/webgpu-render-pipeline.ts @@ -1,4 +1,6 @@ -import type {Binding, RenderPass} from '@luma.gl/api'; +// luma.gl MIT license + +import type {Binding, RenderPass, TypedArray} from '@luma.gl/api'; import {Buffer, RenderPipeline, RenderPipelineProps, cast, log, isObjectEmpty} from '@luma.gl/api'; import {applyParametersToRenderPipelineDescriptor} from '../helpers/webgpu-parameters'; import {getWebGPUTextureFormat} from '../helpers/convert-texture-format'; @@ -83,6 +85,11 @@ export class WebGPURenderPipeline extends RenderPipeline { // } } + /** Constant attributes are not available in WebGPU */ + setConstantAttributes(attributes: Record): void { + console.error('not implemented'); + } + /** Set the bindings */ setBindings(bindings: Record): void { if (!isObjectEmpty(this.props.bindings)) {