Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Batchmesh: Fix cases where calling optimize can result in inconsistent state #29624

Merged
merged 8 commits into from
Oct 16, 2024
Merged

Batchmesh: Fix cases where calling optimize can result in inconsistent state #29624

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
48049b8
Fix skipping over active rather than inactive geometry
gkjohnson Oct 11, 2024
8462db0
Fix optimize
gkjohnson Oct 11, 2024
a416ae5
Correct the shifting of geometry data
gkjohnson Oct 11, 2024
6a11097
Account for a case where the index bit size can be increased.
gkjohnson Oct 11, 2024
3b724a9
typo fix
gkjohnson Oct 11, 2024
88e5f35
Remove comments
gkjohnson Oct 11, 2024
4e706b1
Add comments, removed unnecessary changes
gkjohnson Oct 11, 2024
d3dfa9d
Dispose of textures on instance resize
gkjohnson Oct 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 67 additions & 53 deletions src/objects/BatchedMesh.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ const _batchIntersects = [];
// @TODO: geometry.drawRange support?
// @TODO: geometry.morphAttributes support?
// @TODO: Support uniform parameter per geometry
// @TODO: Add an "optimize" function to pack geometry and remove data gaps

// copies data from attribute "src" into "target" starting at "targetOffset"
function copyAttributeData( src, target, targetOffset = 0 ) {
Expand Down Expand Up @@ -132,8 +131,23 @@ function copyAttributeData( src, target, targetOffset = 0 ) {
// safely copies array contents to a potentially smaller array
function copyArrayContents( src, target ) {

const len = Math.min( src.length, target.length );
target.set( new src.constructor( src.buffer, 0, len ) );
if ( src.constructor !== target.constructor ) {

// if arrays are of a different type (eg due to index size increasing) then data must be per-element copied
const len = Math.min( src.length, target.length );
for ( let i = 0; i < len; i ++ ) {

target[ i ] = src[ i ];

}

} else {

// if the arrays use the same data layout we can use a fast block copy
const len = Math.min( src.length, target.length );
target.set( new src.constructor( src.buffer, 0, len ) );

}

}

Expand Down Expand Up @@ -180,6 +194,10 @@ class BatchedMesh extends Mesh {
this._multiDrawInstances = null;
this._visibilityChanged = true;

// used to track where the next point is that geometry should be inserted
this._nextIndexStart = 0;
this._nextVertexStart = 0;

// Local matrix per geometry by using data texture
this._matricesTexture = null;
this._indirectTexture = null;
Expand Down Expand Up @@ -425,16 +443,11 @@ class BatchedMesh extends Mesh {
indexCount: - 1,
};

let lastRange = null;
const reservedRanges = this._reservedRanges;
const drawRanges = this._drawRanges;
const bounds = this._bounds;
if ( this._geometryCount !== 0 ) {

lastRange = reservedRanges[ reservedRanges.length - 1 ];

}

reservedRange.vertexStart = this._nextVertexStart;
if ( vertexCount === - 1 ) {

reservedRange.vertexCount = geometry.getAttribute( 'position' ).count;
Expand All @@ -445,20 +458,11 @@ class BatchedMesh extends Mesh {

}

if ( lastRange === null ) {

reservedRange.vertexStart = 0;

} else {

reservedRange.vertexStart = lastRange.vertexStart + lastRange.vertexCount;

}

const index = geometry.getIndex();
const hasIndex = index !== null;
if ( hasIndex ) {

reservedRange.indexStart = this._nextIndexStart;
if ( indexCount === - 1 ) {

reservedRange.indexCount = index.count;
Expand All @@ -469,16 +473,6 @@ class BatchedMesh extends Mesh {

}

if ( lastRange === null ) {

reservedRange.indexStart = 0;

} else {

reservedRange.indexStart = lastRange.indexStart + lastRange.indexCount;

}

}

if (
Expand Down Expand Up @@ -531,6 +525,10 @@ class BatchedMesh extends Mesh {
// update the geometry
this.setGeometryAt( geometryId, geometry );

// increment the next geometry position
this._nextIndexStart = reservedRange.indexStart + reservedRange.indexCount;
this._nextVertexStart = reservedRange.vertexStart + reservedRange.vertexCount;

return geometryId;

}
Expand Down Expand Up @@ -698,41 +696,56 @@ class BatchedMesh extends Mesh {
let nextVertexStart = 0;
let nextIndexStart = 0;

// iterate over all geometry ranges
// Iterate over all geometry ranges in order sorted from earliest in the geometry buffer to latest
// in the geometry buffer. Because draw range objects can be reused there is no guarantee of their order.
const drawRanges = this._drawRanges;
const reservedRanges = this._reservedRanges;
const indices = drawRanges
.map( ( e, i ) => i )
.sort( ( a, b ) => {

return reservedRanges[ a ].vertexStart - reservedRanges[ b ].vertexStart;

} );

const geometry = this.geometry;
for ( let i = 0, l = drawRanges.length; i < l; i ++ ) {

// if a geometry range is inactive then don't copy anything
const drawRange = drawRanges[ i ];
const reservedRange = reservedRanges[ i ];
const index = indices[ i ];
const drawRange = drawRanges[ index ];
const reservedRange = reservedRanges[ index ];
if ( drawRange.active === false ) {

continue;

}

// if a geometry contains an index buffer then shift it, as well
if ( geometry.index !== null && reservedRange.indexStart !== nextIndexStart ) {
if ( geometry.index !== null ) {

const { indexStart, indexCount } = reservedRange;
const index = geometry.index;
const array = index.array;
if ( reservedRange.indexStart !== nextIndexStart ) {

// shift the index pointers based on how the vertex data will shift
// adjusting the index must happen first so the original vertex start value is available
const elementDelta = nextVertexStart - reservedRange.vertexStart;
for ( let j = indexStart; j < indexStart + indexCount; j ++ ) {
const { indexStart, indexCount } = reservedRange;
const index = geometry.index;
const array = index.array;

array[ j ] = array[ j ] + elementDelta;
// shift the index pointers based on how the vertex data will shift
// adjusting the index must happen first so the original vertex start value is available
const elementDelta = nextVertexStart - reservedRange.vertexStart;
for ( let j = indexStart; j < indexStart + indexCount; j ++ ) {

}
array[ j ] = array[ j ] + elementDelta;

}

index.array.copyWithin( nextIndexStart, indexStart, indexStart + indexCount );
index.addUpdateRange( nextIndexStart, indexCount );

index.array.copyWithin( nextIndexStart, indexStart, indexStart + indexCount );
index.addUpdateRange( nextIndexStart, indexCount );
reservedRange.indexStart = nextIndexStart;

}

reservedRange.indexStart = nextIndexStart;
nextIndexStart += reservedRange.indexCount;

}
Expand All @@ -752,12 +765,16 @@ class BatchedMesh extends Mesh {
}

reservedRange.vertexStart = nextVertexStart;
nextVertexStart += reservedRange.vertexCount;

}

nextVertexStart += reservedRange.vertexCount;
drawRange.start = geometry.index ? reservedRange.indexStart : reservedRange.vertexStart;

// step the next geometry points to the shifted position
this._nextIndexStart = geometry.index ? reservedRange.indexStart + reservedRange.indexCount : 0;
this._nextVertexStart = reservedRange.vertexStart + reservedRange.vertexCount;

}

return this;
Expand Down Expand Up @@ -857,9 +874,6 @@ class BatchedMesh extends Mesh {

setMatrixAt( instanceId, matrix ) {

// @TODO: Map geometryId to index of the arrays because
// optimize() can make geometryId mismatch the index

const drawInfo = this._drawInfo;
const matricesTexture = this._matricesTexture;
const matricesArray = this._matricesTexture.image.data;
Expand Down Expand Up @@ -898,9 +912,6 @@ class BatchedMesh extends Mesh {

}

// @TODO: Map id to index of the arrays because
// optimize() can make id mismatch the index

const colorsTexture = this._colorsTexture;
const colorsArray = this._colorsTexture.image.data;
const drawInfo = this._drawInfo;
Expand Down Expand Up @@ -1055,14 +1066,17 @@ class BatchedMesh extends Mesh {
const matricesTexture = this._matricesTexture;
const colorsTexture = this._colorsTexture;

indirectTexture.dispose();
this._initIndirectTexture();
copyArrayContents( indirectTexture.image.data, this._indirectTexture.image.data );

matricesTexture.dispose();
this._initMatricesTexture();
copyArrayContents( matricesTexture.image.data, this._matricesTexture.image.data );

if ( colorsTexture ) {

colorsTexture.dispose();
this._initColorsTexture();
copyArrayContents( colorsTexture.image.data, this._colorsTexture.image.data );

Expand Down Expand Up @@ -1158,7 +1172,7 @@ class BatchedMesh extends Mesh {
const drawRange = drawRanges[ geometryId ];
_mesh.geometry.setDrawRange( drawRange.start, drawRange.count );

// ge the intersects
// get the intersects
this.getMatrixAt( i, _mesh.matrixWorld ).premultiply( matrixWorld );
this.getBoundingBoxAt( geometryId, _mesh.geometry.boundingBox );
this.getBoundingSphereAt( geometryId, _mesh.geometry.boundingSphere );
Expand Down