Buffer.create implementation disagrees with documentation

Bugs
1

Here is the function with the JsDoc :

/**
 * Store data into the buffer. If the buffer was already used it will be either recreated or updated depending on isUpdatable property
 * @param data defines the data to store
 */
public create(data: Nullable<DataArray> = null): void {
    if (!data && this._buffer) {
        return; // nothing to do
    }

    data = data || this._data;

    if (!data) {
        return;
    }

    if (!this._buffer) { // create buffer
        if (this._updatable) {
            this._buffer = this._engine.createDynamicVertexBuffer(data);
            this._data = data;
        } else {
            this._buffer = this._engine.createVertexBuffer(data);
        }
    } else if (this._updatable) { // update buffer
        this._engine.updateDynamicVertexBuffer(this._buffer, data);
        this._data = data;
    }
}

I don’t know which one is wrong, but with the current implementation, calling updateVerticesData on a mesh whose VertexBuffer exists and is not updatable does nothing. I was kind of expecting the buffer to be recreated.

Side question : I can do it manually, but I’m wondering whether I have to call dispose()on the VertexBuffer first ? I would say yes, but current implementation of Buffer._rebuild does not do that… :

public _rebuild(): void {
    this._buffer = null;
    this.create(this._data);
}

Thanks a lot for your help. If there is some change to make in the code, I can do a PR.

Cheers
JB

2

Hi @Deltakosh! it’s been a while :slight_smile:
Any take on that one ? I saw that you made some changes in that file recently.

3

Welcome aboard!

The implementation has never recreated the buffer if it was not updatable, so it’s the doc which is wrong. If you want to make a PR to fix the doc you are welcome!

Yes, you should call dispose on the buffer. _rebuild does not do it because this function is called after a context lost to recreate the buffer, and in that case the existing buffer is invalid as the context has been lost (so disposing it would generate a gl error).

4

@Evgeni_Popov thanks for your reply.

I’ll create a PR to fix the doc.

Regarding the Buffer.create() stuff after dipose, it did not work for some reason, whether I disposed the buffer or not. I had to call setVerticesDataat the mesh level to get things working as expected.

5

What did you do exactly? Calling dispose on the buffer then setVerticesData on the mesh should do it.

6

I tried something like

 const data = mesh.getVerticesData(VertexBuffer.PositionKind)
// ---
// modify data 
// ---
const vertexBuffer = mesh.getVertexBuffer(VertexBuffer.PositionKind)!
vertexBuffer .dispose()
vertexBuffer .create(data)
7

That should work except for two things:

  • the bounding info won’t be updated according to the new positions. To have the bounding info updated, you must use mesh.updateVerticesData instead.
  • if you are using vertex array objects (they are automatically used if the GPU supports them) they are not recreated, so you won’t see the change. You can set engine.disableVertexArrayObjects = true to force disabling the support (but perf will be reduced)
1 Like
8

very likely to be because of the vertex array objects :slight_smile: