BufferAttribute.dynamic: revisited

[WestLangley]

This discussion also applies to InterleavedBuffer.dynamic.

It seems that the dynamic property is being used for multiple purposes:

1. as a hint as to how frequently the data will be updated, [docs], and

2. as a flag, that when true, allows the updating of only a sub-rage, as specified in the updateRange property. [code].

The examples are not consistent here. Even in examples in which the entire range is being updated each frame, sometimes the dynamic property is set to true, and sometimes it is not.

This has been discussed before, and the purpose of the property was modified at that time.

What should we advise users to do in setting this property?

/ping @benaadams
/ping @mattdesl
/ping @mrdoob

• [ x ] Dev
• [ x ] r95

[aardgoose]

The change for case 2. was really to handle changes in size of the buffer via setArray(), rather than choosing between glBufferData and glBufferSubData. I'm not sure why a simple sizeChanged flag which would be set in the setArray() method couldn't be used, and would restore the expected properties of the dynamic flag.

Indeed setting a BufferAttribute as dynamic and the attempting to increase the size with setArray() leads to an overflow error as you would expect, which I think is really a bug. Initially a sizeChanged flag was rejected as fragile, but I think the current behaviour is already fragile.

[WestLangley]

@aardgoose I have no idea what to do with this. Can you propose an API change that makes sense to you -- with an example, perhaps?

@mrdoob Do you understand this? I don't. At all.

[aardgoose]

I think the dynamic flag should be solely for the documented purpose, it is too overloaded to make sense as it is.

attribute.array size changes should be flagged in the setArray() and copy() methods which are the only supported paths to change the array length, so I don't see that as being fragile.

For users who really want to choose between glBufferData and glBufferSubData a flag defaulting to the later (Khronos recommend) could be added.

The would remove the failure path when changing the size of a dynamic buffer noted above and make updateRange work as expected and not be dependent on the dynamic flag. (this limitation isn't documented and in the default case of a dynamic = false attribute, the updateRange is ignored)

[WestLangley]

I think the dynamic flag should be solely for the documented purpose, it is too overloaded to make sense as it is.

Agreed.

How exactly does a user "resize" a "BufferAttribute". In the docs, we say buffers can't be resized. Are you assuming the user is just allocating and assigning a new array to the buffer attribute? Or replacing the attribute entirely?

And what is the use case for supporting that?

[aardgoose]

.setArray() is used to resize. No, I don't have a use case, but some people apparently do. That is what set the original discussion off.

They were just replacing the array property directly which I argued at the time, should be considered a 'private' property as far as possible.

[WestLangley]

Thanks, @aardgoose, for your reply.

[Mugen87]

How about removing support for setArray() and treat BufferAttributes as immutable (in the sense of you can't change the basic structure of a buffer attribute after creation)? This could also simplify the implementation of VAOs.

/cc @takahirox

[WestLangley]

@Mugen87 #9631 (comment) does describe one use case for being able to resize buffers, though...

That being said, I was surprised to see it.

[aardgoose]

Slightly related issue.

If you replace a geometry's attribute with a new attribute of a different size, there is no mechanism to delete the related gl buffers of the removed attribute, usually handled through geometry.dispose().

There was an issue raised about this a while ago where someone was hitting GPU memory exhaustion with this pattern, though obviously rare.

[Mugen87]

Related: #15261

[mrdoob]

I agree that #9631 complicated things quite a bit... In hindsight didn't seem like the best solution.

How do you guys feel about setting attribute.version to 0 when the passed array has a different length and then use that value to inform WebGLAttributes whether to use glBufferData (0) or glBufferSubData (> 0)?

[aardgoose]

That fails if people set needsUpdate afterwards I think.

[aardgoose]

An alternative solution.

Implement BufferAttribute.dispose() for #15261

Instead of using a flag (_resized) or overload version count, call dispose() when attributes are resized.

Then WebGLAttributes will fall back to createBuffer -> gl.BufferData for resized attributes and special casing can be removed from WebGLAttributes.updateBuffer().

The major downside of this is it adds cost to the resize operation (deletion and addition of WeakMap entries, and gl.DeleteBuffer(), but removes special handling in WebGLAttributes.

Maybe too costly?

[Mugen87]

There is already a PR in order to introduce BufferAttribute.dispose(). #15308

From my point of view, the library should not promote logic in order support resizing of attributes. BufferAttributes should be considered to be static in its basic structure (I mean stuff like item count and item size). If you need to "resize" a buffer attribute, create a new one and use dispose() in order to free resources of the old one.

[aardgoose]

I hadn't seen #15308.

I agree on the static argument, but we already have users resizing them, and the hacks to carry on supporting them have obvious issues. The .dispose() approach at least puts all the cost on those users only and removes the need for any workarounds with flags etc in the renderer, and sorts out the overloading of the .dynamic flag, .updateRange etc.

Changing an attributes size could become a deprecated behaviour and flag the availability of dispose(), which as you note, would allow the equivalent behaviour to be coded safely.

[Mugen87]

@aardgoose Would you like to refactor updateBuffer() in WebGLAttributes if #15308 goes in?

[WestLangley]

So the dynamic flag would indicate a hint?

The docs:

Whether the buffer is dynamic or not. Default is false.
If false, the GPU is informed that contents of the buffer are likely to be used often and not change often. This corresponds to the gl.STATIC_DRAW flag.
if true, the GPU is informed that contents of the buffer are likely to be used often and change often. This corresponds to the gl.DYNAMIC_DRAW flag.

And by "change", I assume that means the values change -- not the buffer itself.

And what does "often" mean in this context?

Does this hint even matter? Do we need it?

[aardgoose]

@Mugen87 - no problem, changes to updateBuffer() would be minimal.

@WestLangley - yes, dynamic = values change only, it's a 'performance hint' according to the OpenGL spec, but how much value it has will be GPU/driver specific. Frequency of update, who knows! The spec doesn't mandate any specific measures. Good questions,

The perverse thing now is the you need dynamic = false if you want to change the buffer size, which is undocumented and counter intuitive.

[blairmacintyre]

FWIW, I just ran up against this. My use case is pretty simple: I'm implementing a simple example for a WebXR implementation (on iOS, in WebXR Viewer) that renders the world meshes being detected by the system (i.e., ARKit planes).

The meshes change continuously, both in their values, but also the number of vertices. I am using simple BufferedGeometry objects, with BufferAttributes for the indices, normals, positions and uv. They are marked as dynamic.

Previously, my implementation created a new BufferedGeometry and the data changed. I was trying to update things to be less wasteful, and ran across this issue (I assumed I could update the array, with new sizes, and it would sort out the WebGL buffers as needed (which seems like an obvious thing to do). I am (obviously) updating all the Attributes when the size changes.

I will switch back to recreating the whole geometry when the number of vertices changes, but it would be nice if I didn't have to.

[WestLangley]

@blairmacintyre Why not allocate sufficiently-large buffers and call geometry.setDrawRange() when the size changes?

If you must recreate a new geometry, be sure to dispose() of the old one.

[blairmacintyre]

@WestLangley given this use case (live data coming in from the underlying AR/VR system), there is no way to know what "sufficiently large" is.

I'll be sure to dispose, thanks for the reminder. 👍

[fernandojsg]

@blairmacintyre in a-painter we had a BufferGeometryManager that basically had a pool of buffer geometries, you could keep asking for vertices position and it will just keep updating the drawRange and creating new buffers if needed, so you didn't need to care about sizes or allocating them. I believe a similar solution that should works and performs quite well for your use case too.

[blairmacintyre]

Perhaps. Wouldn't they just keep growing? In apainter, each stroke would be static when it's done. Here, meshes are never done, they keep growing, so who knows how it would behave.

I agree smarter management is necessary; just seems insane for a developer using three to be forced to worry about this.

[fernandojsg]

In a-painter you can create and undo strokes so the buffer is "alive", I believe it's pretty much a similar manager what you need you just request addNewVertex and the BufferGeometry keep increasing the drawrange and if it detect the end, it will just create a new BufferGeometry with a new buffer and inject the new vertice there, and then it will just paint all the buffers together, you could still modify them, remove positions and so on.

#funfact I was trying to do that in Unity and it's even worse as they have a vertex limit per mesh of 65536, so you have to create new meshes as you need more vertices :D and no, the API is not that handy anyway :D

[blairmacintyre]

Perhaps it's something we can add to this little demo at some point (would be good to have code like this refactored and "in" more things) ...

[fernandojsg]

Yep I agree, I'll add that to my to-do list, to adapt the BufferGeometryManager/Helper so we could submit an example with it here too

[WestLangley]

After all of this, I still do not feel I understand the issues well enough to take any action. :(

I would be happy if dynamic could be removed. When used as a so-called hint, we don't even know if it matters. The API and the implementation are a mess.

@Mugen87 Do you feel like you understand the issues well enough to file a PR?

[Mugen87]

My preferred solution has two parts:

• Introducing add dispose method to BufferAttribute #15308 and consider BufferAttribute as fixed-sized
• Rename BufferAttribute.dynamic to BufferAttribute.usage and introduce all respective WebGL constants

THREE.StaticDrawUsage; // default
THREE.DynamicDrawUsage;
THREE.StreamDrawUsage;

// WebGL 2

THREE.StaticReadUsage;
THREE.DynamicReadUsage;
THREE.StreamReadUsage;
THREE.StaticCopyUsage;
THREE.DynamicCopyUsage;
THREE.StreamCopyUsage;

[WestLangley]

@Mugen87 Yay! Have a go. :-)

Rename BufferAttribute.dynamic to BufferAttribute.usage and introduce all respective WebGL constants

Actually, I would prefer to say .dynamic has been removed, and a new property .usage has been added. To say it was "renamed" would imply the behavior has not changed.

[WestLangley]

@donmccurdy I see that you may have an opinion on this topic...

I do not understand the use cases of the sophisticated users well enough. I am hoping that those who do understand can come to some agreement, so we can make some progress on this.

[donmccurdy]

I don't have the experience with this feature to offer any new insight here, but @Mugen87's suggested API sounds like a clarity improvement to me. If we are going to the trouble of creating a .usage property, rather than just removing .dynamic, it would be great if we could run some benchmarks in the process.

[WestLangley]

@aardgoose has some interesting suggestions in #17242 (comment).

[WestLangley]

Many thanks to everyone. :-)