diff --git a/modules/aggregation-layers/src/grid-layer/grid-cell-layer.ts b/modules/aggregation-layers/src/grid-layer/grid-cell-layer.ts index 2b4c02a5b2d..0816ed4d2d0 100644 --- a/modules/aggregation-layers/src/grid-layer/grid-cell-layer.ts +++ b/modules/aggregation-layers/src/grid-layer/grid-cell-layer.ts @@ -99,7 +99,6 @@ export class GridCellLayer extends ColumnLayer< protected _updateGeometry() { const geometry = new CubeGeometry(); this._setFillGeometry(geometry); - this._setWireframeGeometry(geometry); } draw({uniforms}) { diff --git a/modules/aggregation-layers/src/hexagon-layer/hexagon-cell-layer.ts b/modules/aggregation-layers/src/hexagon-layer/hexagon-cell-layer.ts index 61b7a2b88a1..88bf8ada2f4 100644 --- a/modules/aggregation-layers/src/hexagon-layer/hexagon-cell-layer.ts +++ b/modules/aggregation-layers/src/hexagon-layer/hexagon-cell-layer.ts @@ -109,8 +109,6 @@ export default class HexagonCellLayer extends Colum const elevationCutoff = this.props.elevationCutoff || [-Infinity, Infinity]; const fillModel = this.state.fillModel!; - fillModel.setVertexCount(this.state.fillVertexCount); - const hexagonProps: Omit = { colorDomain: [ Math.max(colorDomain[0], colorCutoff[0]), // instanceColorValue that maps to colorRange[0] diff --git a/modules/layers/src/column-layer/column-layer.ts b/modules/layers/src/column-layer/column-layer.ts index a3e6cc40d50..20be785ef58 100644 --- a/modules/layers/src/column-layer/column-layer.ts +++ b/modules/layers/src/column-layer/column-layer.ts @@ -234,9 +234,9 @@ export default class ColumnLayer exten state!: { fillModel?: Model; + strokeModel?: Model; wireframeModel?: Model; models?: Model[]; - fillVertexCount: number; edgeDistance: number; }; @@ -320,13 +320,15 @@ export default class ColumnLayer exten const instanceCount = this.getNumInstances(); this.state.fillModel!.setInstanceCount(instanceCount); + this.state.strokeModel!.setInstanceCount(instanceCount); this.state.wireframeModel!.setInstanceCount(instanceCount); if ( regenerateModels || props.diskResolution !== oldProps.diskResolution || props.vertices !== oldProps.vertices || - (props.extruded || props.stroked) !== (oldProps.extruded || oldProps.stroked) + props.extruded !== oldProps.extruded || + props.stroked !== oldProps.stroked ) { this._updateGeometry(props); } @@ -370,6 +372,12 @@ export default class ColumnLayer exten bufferLayout, isInstanced: true }); + const strokeModel = new Model(this.context.device, { + ...shaders, + id: `${this.props.id}-stroke`, + bufferLayout, + isInstanced: true + }); const wireframeModel = new Model(this.context.device, { ...shaders, id: `${this.props.id}-wireframe`, @@ -379,8 +387,9 @@ export default class ColumnLayer exten return { fillModel, + strokeModel, wireframeModel, - models: [wireframeModel, fillModel] + models: [wireframeModel, fillModel, strokeModel] }; } @@ -389,10 +398,6 @@ export default class ColumnLayer exten const positionAttribute = geometry.attributes.POSITION; const normalAttribute = geometry.attributes.NORMAL; - this.setState({ - fillVertexCount: positionAttribute.value.length / 3 - }); - // The fill model renders a triangle-strip with degenerate triangles and does not // use indices. Give it a separate Geometry without `indices` so that later buffer // layout rebuilds (e.g. binary-data transitions, HMR) cannot re-attach the @@ -404,7 +409,21 @@ export default class ColumnLayer exten }) ); - this._setWireframeGeometry(geometry); + if (!extruded && stroked) { + const fillVertexCount = positionAttribute.value.length / 3; + this._setStrokeGeometry( + new Geometry({ + topology: 'triangle-strip', + // remove the cap + vertexCount: fillVertexCount - diskResolution - 1, + attributes: {POSITION: positionAttribute, NORMAL: normalAttribute} + }) + ); + } + + if (extruded) { + this._setWireframeGeometry(geometry); + } } protected _setFillGeometry(geometry: Geometry): void { @@ -415,6 +434,14 @@ export default class ColumnLayer exten fillModel.setGeometry(fillGeometry); } + protected _setStrokeGeometry(geometry: Geometry): void { + const strokeGeometry = makeInterleavedGeometry(geometry, { + attributes: ['POSITION', 'NORMAL'] + }); + const strokeModel = this.state.strokeModel!; + strokeModel.setGeometry(strokeGeometry); + } + protected _setWireframeGeometry(geometry: Geometry): void { const wireframeGeometry = makeInterleavedGeometry(geometry, { attributes: ['POSITION', 'NORMAL'] @@ -442,8 +469,9 @@ export default class ColumnLayer exten angle } = this.props; const fillModel = this.state.fillModel!; + const strokeModel = this.state.strokeModel!; const wireframeModel = this.state.wireframeModel!; - const {fillVertexCount, edgeDistance} = this.state; + const {edgeDistance} = this.state; const columnProps: Omit = { radius, @@ -473,8 +501,6 @@ export default class ColumnLayer exten } if (filled) { - // model.setProps({isIndexed: false}); - fillModel.setVertexCount(fillVertexCount); fillModel.shaderInputs.setProps({ column: { ...columnProps, @@ -485,17 +511,13 @@ export default class ColumnLayer exten } // When drawing 2d: draw fill before stroke so that the outline is always on top if (!extruded && stroked) { - // model.setProps({isIndexed: false}); - // The width of the stroke is achieved by flattening the side of the cylinder. - // Skip the last 1/3 of the vertices which is the top. - fillModel.setVertexCount((fillVertexCount * 2) / 3); - fillModel.shaderInputs.setProps({ + strokeModel.shaderInputs.setProps({ column: { ...columnProps, isStroke: true } }); - fillModel.draw(this.context.renderPass); + strokeModel.draw(this.context.renderPass); } } } diff --git a/modules/layers/src/column-layer/grid-cell-layer.ts b/modules/layers/src/column-layer/grid-cell-layer.ts index fab90ad7e20..ccbae0b717f 100644 --- a/modules/layers/src/column-layer/grid-cell-layer.ts +++ b/modules/layers/src/column-layer/grid-cell-layer.ts @@ -34,7 +34,6 @@ export default class GridCellLayer ext protected _updateGeometry() { const geometry = new CubeGeometry(); this._setFillGeometry(geometry); - this._setWireframeGeometry(geometry); } draw({uniforms}) { diff --git a/test/modules/aggregation-layers/grid-layer.spec.ts b/test/modules/aggregation-layers/grid-layer.spec.ts index 240b44182ef..9d52d615b5b 100644 --- a/test/modules/aggregation-layers/grid-layer.spec.ts +++ b/test/modules/aggregation-layers/grid-layer.spec.ts @@ -52,16 +52,15 @@ test('GridLayer', () => { onBeforeUpdate: ({testCase}) => console.log(testCase.title), onAfterUpdate({layer, subLayer}) { expect(layer.state.aggregator, 'should have aggregator').toBeTruthy(); - for (const model of subLayer?.getModels() || []) { - const bufferNames = model.bufferLayout.map(layout => layout.name); - expect( - bufferNames.filter(name => name === 'geometry'), - `${model.id} has one geometry buffer layout` - ).toHaveLength(1); - expect(new Set(bufferNames).size, `${model.id} has unique buffer layouts`).toBe( - bufferNames.length - ); - } + const model = subLayer?.state.fillModel!; + const bufferNames = model.bufferLayout.map(layout => layout.name); + expect( + bufferNames.filter(name => name === 'geometry'), + `${model.id} has one geometry buffer layout` + ).toHaveLength(1); + expect(new Set(bufferNames).size, `${model.id} has unique buffer layouts`).toBe( + bufferNames.length + ); } }); diff --git a/test/modules/layers/column-layer.spec.ts b/test/modules/layers/column-layer.spec.ts index 1086be43d72..ac7ea577a54 100644 --- a/test/modules/layers/column-layer.spec.ts +++ b/test/modules/layers/column-layer.spec.ts @@ -8,42 +8,37 @@ import {ColumnLayer, GridCellLayer} from '@deck.gl/layers'; import {testLayer} from '@deck.gl/test-utils/vitest'; function expectUniqueGeometryLayout(layer: ColumnLayer): void { - for (const model of layer.getModels()) { - const bufferNames = model.bufferLayout.map(layout => layout.name); - expect(new Set(bufferNames).size, `${model.id} has unique buffer layouts`).toBe( - bufferNames.length - ); - expect( - bufferNames.filter(name => name === 'geometry'), - `${model.id} has one geometry buffer layout` - ).toHaveLength(1); + const model = layer.state.fillModel!; + const bufferNames = model.bufferLayout.map(layout => layout.name); + expect(new Set(bufferNames).size, `${model.id} has unique buffer layouts`).toBe( + bufferNames.length + ); + expect( + bufferNames.filter(name => name === 'geometry'), + `${model.id} has one geometry buffer layout` + ).toHaveLength(1); - const geometryLayout = model.bufferLayout.find(layout => layout.name === 'geometry'); - const attachedGeometryLayout = model._gpuGeometry?.bufferLayout.find( - layout => layout.name === 'geometry' - ); - expect( - geometryLayout?.attributes?.map(attribute => attribute.attribute), - `${model.id} geometry attributes` - ).toEqual(['positions', 'normals']); - expect( - attachedGeometryLayout, - `${model.id} has an attached geometry buffer layout` - ).toBeTruthy(); - expect(geometryLayout, `${model.id} layout matches its attached geometry`).toEqual( - attachedGeometryLayout - ); + const geometryLayout = model.bufferLayout.find(layout => layout.name === 'geometry'); + const attachedGeometryLayout = model._gpuGeometry?.bufferLayout.find( + layout => layout.name === 'geometry' + ); + expect( + geometryLayout?.attributes?.map(attribute => attribute.attribute), + `${model.id} geometry attributes` + ).toEqual(['positions', 'normals']); + expect(attachedGeometryLayout, `${model.id} has an attached geometry buffer layout`).toBeTruthy(); + expect(geometryLayout, `${model.id} layout matches its attached geometry`).toEqual( + attachedGeometryLayout + ); - const pipelineBufferNames = model.pipeline.bufferLayout.map(layout => layout.name); - expect( - new Set(pipelineBufferNames).size, - `${model.id} pipeline has unique buffer layouts` - ).toBe(pipelineBufferNames.length); - expect( - pipelineBufferNames.filter(name => name === 'geometry'), - `${model.id} pipeline has one geometry buffer layout` - ).toHaveLength(1); - } + const pipelineBufferNames = model.pipeline.bufferLayout.map(layout => layout.name); + expect(new Set(pipelineBufferNames).size, `${model.id} pipeline has unique buffer layouts`).toBe( + pipelineBufferNames.length + ); + expect( + pipelineBufferNames.filter(name => name === 'geometry'), + `${model.id} pipeline has one geometry buffer layout` + ).toHaveLength(1); } // Regression test for #9463 / #10021: with binary data the fill model must