Skip to content

Commit

Permalink
Fixes EdgeArrowHeadProgram with instanced rendering
Browse files Browse the repository at this point in the history
We never cleaned the GL state, while this state is shared between
programs that are used by the same GL context. In this case, some vertex
attributes were using a divisor for non-instanced rendering programs,
which were causing them to fail silently.

The fix is to make program unbind everything they can once they draw, so
that they no more share common state.
  • Loading branch information
jacomyal committed Sep 18, 2023
1 parent faef287 commit 65901f9
Showing 1 changed file with 31 additions and 4 deletions.
35 changes: 31 additions & 4 deletions src/rendering/webgl/programs/common/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ export interface ProgramDefinition<Uniform extends string = string> {
ATTRIBUTES: Array<ProgramAttributeSpecification>;
}

export interface InstancedProgramDefinition<Uniform extends string = string>
extends Omit<ProgramDefinition<Uniform>, "CONSTANT_DATA" | "CONSTANT_ATTRIBUTES"> {
export interface InstancedProgramDefinition<Uniform extends string = string> extends ProgramDefinition<Uniform> {
CONSTANT_ATTRIBUTES: Array<ProgramAttributeSpecification>;
CONSTANT_DATA: number[][];
}
Expand Down Expand Up @@ -185,7 +184,7 @@ export abstract class Program<Uniform extends string = string> implements Abstra

offset = 0;
this.CONSTANT_ATTRIBUTES.forEach((attr) => (offset += this.bindAttribute(attr, offset, false)));
gl.bufferData(gl.ARRAY_BUFFER, this.CONSTANT_DATA, gl.DYNAMIC_DRAW);
gl.bufferData(gl.ARRAY_BUFFER, this.CONSTANT_DATA, gl.STATIC_DRAW);

// Handle "instance specific" data (things that vary for each item):
gl.bindBuffer(gl.ARRAY_BUFFER, this.buffer);
Expand All @@ -194,6 +193,17 @@ export abstract class Program<Uniform extends string = string> implements Abstra
this.ATTRIBUTES.forEach((attr) => (offset += this.bindAttribute(attr, offset, true)));
gl.bufferData(gl.ARRAY_BUFFER, this.array, gl.DYNAMIC_DRAW);
}

gl.bindBuffer(gl.ARRAY_BUFFER, null);
}

protected unbind(): void {
if (!this.isInstanced) {
this.ATTRIBUTES.forEach((attr) => this.unbindAttribute(attr));
} else {
this.CONSTANT_ATTRIBUTES.forEach((attr) => this.unbindAttribute(attr, false));
this.ATTRIBUTES.forEach((attr) => this.unbindAttribute(attr, true));
}
}

private bindAttribute(attr: ProgramAttributeSpecification, offset: number, setDivisor?: boolean): number {
Expand All @@ -204,7 +214,7 @@ export abstract class Program<Uniform extends string = string> implements Abstra

const stride = !this.isInstanced
? this.ATTRIBUTES_ITEMS_COUNT * Float32Array.BYTES_PER_ELEMENT
: getAttributesItemsCount(setDivisor ? this.ATTRIBUTES : this.CONSTANT_ATTRIBUTES) *
: (setDivisor ? this.ATTRIBUTES_ITEMS_COUNT : getAttributesItemsCount(this.CONSTANT_ATTRIBUTES)) *
Float32Array.BYTES_PER_ELEMENT;

gl.vertexAttribPointer(location, attr.size, attr.type, attr.normalized || false, stride, offset);
Expand All @@ -224,6 +234,22 @@ export abstract class Program<Uniform extends string = string> implements Abstra
return attr.size * sizeFactor;
}

private unbindAttribute(attr: ProgramAttributeSpecification, unsetDivisor?: boolean) {
const gl = this.gl;
const location = this.attributeLocations[attr.name];

gl.disableVertexAttribArray(location);

if (this.isInstanced && unsetDivisor) {
if (gl instanceof WebGL2RenderingContext) {
gl.vertexAttribDivisor(location, 0);
} else {
const ext = gl.getExtension("ANGLE_instanced_arrays");
if (ext) ext.vertexAttribDivisorANGLE(location, 0);
}
}
}

reallocate(capacity: number): void {
// If desired capacity has not changed we do nothing
// NOTE: it's possible here to implement more subtle reallocation schemes
Expand Down Expand Up @@ -251,6 +277,7 @@ export abstract class Program<Uniform extends string = string> implements Abstra
this.bind();
this.gl.useProgram(this.program);
this.draw(params);
this.unbind();
}

drawWebGL(method: GLenum): void {
Expand Down

0 comments on commit 65901f9

Please sign in to comment.