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

fix(webgl): Pair calls to bind/unbind VAO #2032

Merged
merged 2 commits into from Mar 13, 2024
Merged

fix(webgl): Pair calls to bind/unbind VAO #2032

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dc275cc
fix(webgl): Bind index buffer before render.
donmccurdy Mar 13, 2024
f3aba2c
fix(webgl): Defensively unbind VAO to prevent unintended modification…
donmccurdy Mar 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 9 additions & 21 deletions modules/webgl/src/adapter/resources/webgl-vertex-array.ts
View file
Open in desktop
Expand Up @@ -57,8 +57,8 @@ export class WEBGLVertexArray extends VertexArray {
/**
// Set (bind/unbind) an elements buffer, for indexed rendering.
// Must be a Buffer bound to GL.ELEMENT_ARRAY_BUFFER or null. Constants not supported
*
* @param elementBuffer
*
* @param elementBuffer
*/
setIndexBuffer(indexBuffer: Buffer | null): void {
const buffer = indexBuffer as WEBGLBuffer;
Expand All @@ -68,12 +68,12 @@ export class WEBGLVertexArray extends VertexArray {
}
// In WebGL The GL.ELEMENT_ARRAY_BUFFER_BINDING is stored on the VertexArrayObject
this.device.gl.bindVertexArray(this.handle);
// TODO - this initial binding does not seem to take effect? see bindBeforeRender()
this.device.gl.bindBuffer(GL.ELEMENT_ARRAY_BUFFER, buffer ? buffer.handle : null);
// log.log(1, 'VertexArray.setIndexBuffer', indexBuffer)();
// log.log(1, `Binding vertex array ${this.id}`, buffer?.id)();

this.indexBuffer = buffer;

// Unbind to prevent unintended changes to the VAO.
this.device.gl.bindVertexArray(null);
}

/** Set a location in vertex attributes array to a buffer, enables the location, sets divisor */
Expand All @@ -97,16 +97,16 @@ export class WEBGLVertexArray extends VertexArray {
// Attaches ARRAY_BUFFER with specified buffer format to location
this.device.gl.vertexAttribPointer(location, size, type, normalized, stride, offset);
}
// Clear binding - keeping it may cause [.WebGL-0x12804417100]
// GL_INVALID_OPERATION: A transform feedback buffer that would be written to is also bound to a non-transform-feedback target
this.device.gl.bindBuffer(GL.ARRAY_BUFFER, null);
donmccurdy marked this conversation as resolved.
Show resolved Hide resolved

// Mark as non-constant
this.device.gl.enableVertexAttribArray(location);
// Set the step mode 0=vertex, 1=instance
this.device.gl.vertexAttribDivisor(location, divisor || 0);

this.attributes[location] = buffer;

// Unbind to prevent unintended changes to the VAO.
this.device.gl.bindVertexArray(null);
}

/** Set a location in vertex attributes array to a constant value, disables the location */
Expand All @@ -115,26 +115,14 @@ export class WEBGLVertexArray extends VertexArray {
this.attributes[location] = value;
}

init = false;

override bindBeforeRender(): void {
this.device.gl.bindVertexArray(this.handle);
// TODO - the initial bind does not seem to take effect.
if (!this.init) {
// log.log(1, `Binding vertex array ${this.id}`, this.indexBuffer?.id)();
const webglBuffer = this.indexBuffer as WEBGLBuffer;
this.device.gl.bindBuffer(GL.ELEMENT_ARRAY_BUFFER, webglBuffer?.handle || null);
this.init = true;
}
this._applyConstantAttributes();
}

override unbindAfterRender(): void {
// log.log(1, `Unbinding vertex array ${this.id}`)();
// TODO technically this is not necessary, but we might be interfacing
// with code that does not use vertex array objects
// Unbind to prevent unintended changes to the VAO.
this.device.gl.bindVertexArray(null);
// this.device.gl.bindBuffer(GL.ELEMENT_ARRAY_BUFFER, null);
}

// Internal methods
Expand Down