diff --git a/packages/dev/core/src/Layers/selectionOutlineLayer.ts b/packages/dev/core/src/Layers/selectionOutlineLayer.ts index 871a19f21518..b31622f6673f 100644 --- a/packages/dev/core/src/Layers/selectionOutlineLayer.ts +++ b/packages/dev/core/src/Layers/selectionOutlineLayer.ts @@ -320,6 +320,15 @@ export class SelectionOutlineLayer extends EffectLayer { this._thinEffectLayer.addSelection(meshOrGroup); } + /** + * Removes mesh or group of meshes from the current selection + * @param meshOrGroup Meshes to remove from the selection + */ + public removeSelection(meshOrGroup: AbstractMesh | AbstractMesh[]): void { + this._thinEffectLayer.removeSelection(meshOrGroup); + this._mainTexture.renderList = this._thinEffectLayer._selection; // update render list + } + /** * Free any resources and references associated to a mesh. * Internal use diff --git a/packages/dev/core/src/Layers/thinSelectionOutlineLayer.ts b/packages/dev/core/src/Layers/thinSelectionOutlineLayer.ts index 0b99025d73ed..51f5ad85c76c 100644 --- a/packages/dev/core/src/Layers/thinSelectionOutlineLayer.ts +++ b/packages/dev/core/src/Layers/thinSelectionOutlineLayer.ts @@ -94,6 +94,8 @@ export class ThinSelectionOutlineLayer extends ThinEffectLayer { /** @internal */ public _selection: Nullable = []; private _nextSelectionId = 1; + private _freeSelectionIds: number[] = []; + private _selectionIdRefCount: number[] = []; /** * Instantiates a new selection outline Layer and references it to the scene.. @@ -708,15 +710,111 @@ export class ThinSelectionOutlineLayer extends ThinEffectLayer { if (mesh.instancedBuffers?.[ThinSelectionOutlineLayer.InstanceSelectionIdAttributeName] !== undefined) { delete mesh.instancedBuffers[ThinSelectionOutlineLayer.InstanceSelectionIdAttributeName]; } + if (mesh.hasThinInstances) { + mesh.thinInstanceSetBuffer(ThinSelectionOutlineLayer.InstanceSelectionIdAttributeName, null); + } } this._selection.length = 0; this._meshUniqueIdToSelectionId.length = 0; + this._selectionIdRefCount.length = 0; + this._freeSelectionIds.length = 0; this._nextSelectionId = 1; this._shouldRender = false; } + /** + * Removes mesh or group of meshes from the current selection + * @param meshOrGroup Meshes to remove from the selection + */ + public removeSelection(meshOrGroup: AbstractMesh | AbstractMesh[]): void { + if (!this._selection) { + return; + } + + const group = Array.isArray(meshOrGroup) ? meshOrGroup : [meshOrGroup]; + + for (let meshIndex = 0; meshIndex < group.length; ++meshIndex) { + const mesh = group[meshIndex]; + const index = this._selection.indexOf(mesh); + if (index === -1) { + continue; + } + + // Look up the selection ID before cleanup removes it + let selectionId: number | undefined; + if (mesh.hasInstances || mesh.isAnInstance) { + selectionId = mesh.instancedBuffers?.[ThinSelectionOutlineLayer.InstanceSelectionIdAttributeName]; + } else if (mesh.hasThinInstances) { + selectionId = (mesh as Mesh)._userThinInstanceBuffersStorage?.data[ThinSelectionOutlineLayer.InstanceSelectionIdAttributeName]?.[0]; + } else { + selectionId = this._meshUniqueIdToSelectionId[mesh.uniqueId]; + delete this._meshUniqueIdToSelectionId[mesh.uniqueId]; + } + + this._selection.splice(index, 1); + + /* + Clean up GPU resources for this mesh. + + `_userInstancedBuffersStorage` lives on the source `Mesh` and is shared + by all its `InstancedMesh`es. For an `InstancedMesh` passed here, this + property is undefined so the block below is safely skipped. + + However, if the source mesh itself is passed to `removeSelection()` + while some of its instances are still in the selection, this will + dispose the shared GPU vertex buffers (and VAOs / render-pass VBOs) + that those instances still reference. `clearSelection()` does not + have this problem because it removes everything at once. + + A future fix could skip the `_userInstancedBuffersStorage` teardown + when the source mesh still has selected instances, or defer cleanup + until the last instance is removed. + */ + const kind = ThinSelectionOutlineLayer.InstanceSelectionIdAttributeName; + const m = mesh as Mesh; + if (m._userInstancedBuffersStorage) { + if (m._userInstancedBuffersStorage.renderPasses) { + for (const passId of this._objectRenderer.renderPassIds) { + const passVBOs = m._userInstancedBuffersStorage.renderPasses[passId]; + if (passVBOs?.[kind]) { + passVBOs[kind]!.dispose(); + delete passVBOs[kind]; + } + } + } + m._userInstancedBuffersStorage.vertexBuffers[kind]?.dispose(); + const vao = m._userInstancedBuffersStorage.vertexArrayObjects?.[kind]; + if (vao) { + (this._engine as ThinEngine).releaseVertexArrayObject(vao); + delete m._userInstancedBuffersStorage.vertexArrayObjects![kind]; + } + delete m._userInstancedBuffersStorage.data[kind]; + delete m._userInstancedBuffersStorage.vertexBuffers[kind]; + delete m._userInstancedBuffersStorage.strides[kind]; + delete m._userInstancedBuffersStorage.sizes[kind]; + if (Object.keys(m._userInstancedBuffersStorage.vertexBuffers).length === 0) { + m._userInstancedBuffersStorage = undefined!; + } + } + if (m.instancedBuffers?.[kind] !== undefined) { + delete m.instancedBuffers[kind]; + } + if (m.hasThinInstances) { + m.thinInstanceSetBuffer(kind, null); + } + + // Reclaim the selection ID when no more meshes reference it + if (selectionId !== undefined && --this._selectionIdRefCount[selectionId] <= 0) { + delete this._selectionIdRefCount[selectionId]; + this._freeSelectionIds.push(selectionId); + } + } + + this._shouldRender = this._selection.length > 0; + } + /** * Adds mesh or group of mesh to the current selection * @@ -728,13 +826,15 @@ export class ThinSelectionOutlineLayer extends ThinEffectLayer { return; } - const nextId = this._nextSelectionId; + const nextId = this._freeSelectionIds.length > 0 ? this._freeSelectionIds.pop()! : this._nextSelectionId++; const group = Array.isArray(meshOrGroup) ? meshOrGroup : [meshOrGroup]; if (group.length === 0) { return; } + this._selectionIdRefCount[nextId] = group.length; + for (let meshIndex = 0; meshIndex < group.length; ++meshIndex) { const mesh = group[meshIndex]; @@ -759,7 +859,6 @@ export class ThinSelectionOutlineLayer extends ThinEffectLayer { this._meshUniqueIdToSelectionId[mesh.uniqueId] = nextId; } } - this._nextSelectionId += 1; this._shouldRender = true; }