Three.js: geometry._maxInstanceCount is not cleared on dispose

[sam6321]

Description of the problem

The BufferGeometry._maxInstanceCount property is set by internal renderer code in WebGLBindingStates.js, but this property is not cleared when calling dispose() on the geometry.

As this property seems to be used internally the renderer to cache this value, I feel as though it should also be cleared on dispose().

The change that added this property broke a buffer resizing technique that I had been using to implement resizeable buffers for ThreeJS without needing to know or allocate a max buffer size beforehand.

The main part of my implementation that broke was something like the following:

const geometry = new THREE.InstancedBufferGeometry();

const data = new Float32Array(1, 2, 3, 4, 5, 6);
const attribute = new THREE.InstancedBufferAttribute(data, 3);
geometry.setAttribute("attributeName", attribute);

// Other attributes, like position and index are added
// Geometry is added to a mesh and to the scene etc.

// Render the geometry somewhere in the rendering loop at least once
renderer.render(scene, camera);

// geometry._maxInstanceCount now has a value.

// To resize the geometry, I dispose of it first to remove it fully from the renderer's memory
// and re-use it as though it's a fresh geometry. This means I don't need to fully re-create
// the object, but the renderer will treat it as though it's a completely new geometry.
geometry.dispose();
// Problem: geometry._maxInstanceCount is still set.

attribute.array = new Float32Array(1, 2, 3, 4, 5, 6, 7); // Array size increased by 1
attribute.count = attribute.array.length / attribute.itemSize;

// Next render loop, the renderer should be treating the geometry as though it's never seen it before
// as I've called dispose on it. The problem is, geometry._maxInstanceCount still has a 
// value that will be used in WebGLRenderer.js, and the incorrect number of instances will be rendered.

I'm not sure if ThreeJS is not designed to support this usage pattern, but I feel as though it's acceptable given the renderer should not retain any information about the geometry after dispose() is called. As far as the renderer is concerned, it's being passed an entirely new geometry with entirely new buffers.

Unfortunately, the geometry._maxInstanceCount property isn't cleared and this use case breaks.

I'm happy to write a more complete fiddle illustrating this problem when I get the chance :)

Three.js version

• [x] Dev
• [x] r117
• [ ] ...

Browser

• [x] All of them
• [ ] Chrome
• [ ] Firefox
• [ ] Internet Explorer

OS

• [x] All of them
• [ ] Windows
• [ ] macOS
• [ ] Linux
• [ ] Android
• [ ] iOS

[WestLangley]

There may be a valid issue here, but did you see the discussion in #19595?

[sam6321]

Yup, I did read that discussion while I was debugging this issue.

I can see the issue there concerning the resizing of buffers on a geometry instance (as the How to update things guide literally states that this is unsupported), but I think my own case is a bit different given the additional usage of dispose().

[Mugen87]

In any event, buffer attributes have to be fixed sized. Meaning this line:

attribute.array = new Float32Array(1, 2, 3, 4, 5, 6, 7);

is definitely invalid. I don't think clearing _maxInstanceCount is still necessary against this backdrop, right?

[sam6321]

Is it still invalid if I dispose the buffer geometry that was using the attribute and allow the renderer to fully re-create the associated webgl buffers using the new array?

It's a lot more convenient in this case to not create an entirely new buffer attribute and instead re-use the existing attribute.

[Mugen87]

Sorry, but you have to create a new instance of BufferAttribute.

[sam6321]

Even so, there's still potentially a problem with not clearing _maxInstanceCount.

Think about the following steps:

• Create the geometry with initial set of attributes
• Render the geometry at least once, allowing the renderer to set _maxInstanceCount.
• Dispose the geometry
• Remove all instanced attributes from the geometry.
• Create new instanced attributes and assign them to the geometry.
• Render the geometry again.

In this case, on the second render, _maxInstanceCount will still have its old value from the first render, which is now incorrect as the geometry has been disposed and assigned new attributes.

[sam6321]

Example fiddle showing the issue: https://jsfiddle.net/3txn8b4m/

[Mugen87]

The library assumes that once an entity is disposed, you are not using it anymore. You have to create a new geometry to solve the issue: https://jsfiddle.net/u980Legp/

[sam6321]

In the documentation (particularly the _What happens when I call dispose() and then use the respective object at a later point?_ section), it does seem to imply that disposing of an object and re-using it later is possible, with the only caveat being a negative performance hit for the current frame.

Should this be changed to say that disposed objects should never be re-used instead?

[Mugen87]

Damn, I forgot this section...Well, even if I don't like this approach (I personally think reusing geometries is a misusage) it should be possible to delete _maxInstanceCount in onGeometryDispose().

[WestLangley]

// To resize the geometry,

@sam6321 Out of curiosity, how often are you "resizing the geometry"?

And what is the reason you are not allocating a sufficiently large buffer and just updating the data?

[sam6321]

@WestLangley

I'm using this resizing technique as part of a 3D globe application that displays devices on a globe and can render trails behind the devices showing where they've moved.

These trails need to support a potentially large number of points (1mil+ in some cases), but some devices may only need a small number of points on their trails (100s / 1000s).

Trails are initially allocated with a small number of points, and the allocation size doubles whenever a new point is added that pushes the line over its current allocation size. I'm using the dispose and re-create method I described above to resize the geometry.

Basically, I want to avoid allocating the full upper limit of trail points ahead of time to save on GPU memory for trails that don't require even close to that number of points.

[WestLangley]

Trails are initially allocated with a small number of points, and the allocation size doubles whenever a new point is added that pushes the line over its current allocation size.

OK, good!