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

Instanced rendering #1377

Merged

jacomyal merged 6 commits into v3 from v3-instanced-rendering

Oct 4, 2023

Owner

jacomyal commented Sep 18, 2023 •

edited

Loading

This PR implements instanced rendering for relevant programs. This should highly reduce RAM usage of sigma.js in most cases, and might increase performances as well. It fixes #1322.

Details:

Removes ARRAY_ITEMS_PER_VERTEX, and automatically computes it instead
Removes everything indices related from programs, since instanced rendering makes it useless
Improves common/program.ts to handle instanced rendering:
- Adds new Program#drawWebGL(method: GLenum) method, that handles calling the proper WebGL method, with the proper arguments
- If the program's definition has CONSTANT_DATA and CONSTANT_ATTRIBUTES, it will be rendered using instanced rendering (works with both WebGL and WebGL2)
- Also, programs now have an #unbind method that clears the related WebGL instance state before another program is used
Ports every program that has multiple vertices to instanced rendering

jacomyal added enhancement v3 labels

jacomyal requested a review from Yomguithereal

September 18, 2023 14:47

jacomyal mentioned this pull request

Implement picking rather than custom maths for mouse collision #1382

Closed

6 tasks

jacomyal force-pushed the v3-instanced-rendering branch from 65901f9 to 08a2940 Compare

September 26, 2023 06:58

Collaborator

Yomguithereal commented Oct 2, 2023

Some misc remarks:

the canUse32BitsIndices can be dropped from utils because I think it is not being used anymore

Yomguithereal reviewed

View reviewed changes

src/rendering/webgl/programs/common/program.ts

+                  // For instanced programs:
+                  if ("CONSTANT_ATTRIBUTES" in definition) {
+                    this.isInstanced = true;

Collaborator

Yomguithereal Oct 2, 2023

You cannot be instanced without any constant attributes?

src/rendering/webgl/programs/common/program.ts

                 capacity = 0;
                 verticesCount = 0;
-                abstract getDefinition(): ProgramDefinition<Uniform>;
+                isInstanced: boolean;
+                constantBuffer: WebGLBuffer = {} as WebGLBuffer;

Collaborator

Yomguithereal Oct 2, 2023

Is it normal to set a default buffer as an empty JavaScript object? Just a trick to avoid nastiness of nullable property before constructor is over?

Owner Author

jacomyal Oct 3, 2023

Yes, just that trick...

src/rendering/webgl/programs/common/program.ts Outdated

+                          `Program: error while getting constant data (one vector has ${vector.length} items instead of ${constantAttributesItemsCount})`,
+                        );
+                      constantData.push(...vector);

Collaborator

Yomguithereal Oct 2, 2023

I guess the length of this vector cannot overflow JS stack size regarding maximum function arity (I think so but just making sure)?

Owner Author

jacomyal Oct 4, 2023

Good call, I'll switch to a classic for loop.

src/rendering/webgl/programs/common/program.ts

+                    this.STRIDE = this.ATTRIBUTES_ITEMS_COUNT;
+                    this.CONSTANT_DATA = new Float32Array(constantData);
+                    this.CONSTANT_ATTRIBUTES = definition.CONSTANT_ATTRIBUTES;

Collaborator

Yomguithereal Oct 2, 2023

No risk of mutating this property and affecting a user's definition (probably not, it is returned by a function, just making sure).

Owner Author

jacomyal Oct 4, 2023

No more than with other definition elements (ATTRIBUTES, for instance) before this PR.

src/rendering/webgl/programs/common/program.ts Outdated

+                      gl.vertexAttribDivisor(location, 1);
+                    } else {
+                      const ext = gl.getExtension("ANGLE_instanced_arrays");
+                      if (ext) ext.vertexAttribDivisorANGLE(location, 1);

Collaborator

Yomguithereal Oct 2, 2023

If ext is not available should we yell?

Owner Author

jacomyal Oct 4, 2023

Yes we can indeed, I'll add that.

src/rendering/webgl/programs/common/program.ts

                 [WebGL2RenderingContext.FLOAT]: 4,
               };
+              function getAttributeItemsCount(attr: ProgramAttributeSpecification): number {
+                return attr.normalized ? 1 : attr.size;

Collaborator

Yomguithereal Oct 2, 2023

Can there be cases where the attribute is normalized but the size is larger than one? I don't think we used something of the kind in the past but could this break in the future?

src/rendering/webgl/programs/edge.arrowHead.ts

    
            @@ -102,6 +80,6 @@ export default class EdgeArrowHeadProgram extends EdgeProgram<typeof UNIFORMS[nu
          
                  gl.uniform1f(u_sizeRatio, params.sizeRatio);

                  gl.uniform1f(u_correctionRatio, params.correctionRatio);

                  gl.drawArrays(gl.TRIANGLES, 0, this.verticesCount);

                  this.drawWebGL(gl.TRIANGLES);

Collaborator

Yomguithereal Oct 2, 2023

I think we discussed this but maybe the gl method can be a property of the class because we won't call drawWebGL with alternating ones at runtime?

Owner Author

jacomyal Oct 4, 2023 •

edited

Loading

Branch v3-1370-1371 does that indeed.

src/rendering/webgl/programs/edge.clamped.ts

+                    ],
+                    CONSTANT_DATA: [
+                      [0, 1, 0],
+                      [0, -1, 0],

Collaborator

Yomguithereal Oct 2, 2023

I wish prettier could let us align the data array to be beautiful :)

Owner Author

jacomyal Oct 4, 2023

agreed ^^

src/rendering/webgl/programs/node.circle.ts

    
            @@ -72,6 +58,6 @@ export default class NodeCircleProgram extends NodeProgram<typeof UNIFORMS[numbe
          
                  gl.uniform1f(u_correctionRatio, params.correctionRatio);

                  gl.uniformMatrix3fv(u_matrix, false, params.matrix);

                  gl.drawArrays(gl.TRIANGLES, 0, this.verticesCount);

                  this.drawWebGL(gl.TRIANGLES);

Collaborator

Yomguithereal Oct 2, 2023

Maybe there is something to be done wrt naming to avoid the confusion between draw, drawWebGL and such?

Owner Author

jacomyal Oct 4, 2023

After merging branch v3-1370-1371 the naming will be better.

jacomyal added 5 commits

October 4, 2023 15:19


          Removes ARRAY_ITEMS_PER_VERTEX

a7a23a1

This value is now deduced by the ATTRIBUTES definitions instead.


          Removes everything indices related from programs

debf42d

Since the next feature to come is instanced rendering, the gain from
indices allocation and "elements rendering" will become negligible.
I prefer to completely remove the code for this optimisation to better
handle the debt.


          Allows writing instanced nodes/edges programs

eb675e5

This ticket partially solves #1322. The remaining programs must be
ported to the instanced system before the ticket could be completely
closed.

Details:
- Adds a new `#drawWebGL` method, that handles calling the proper WebGL
  (1 or 2) methods to draw
- Adds a new InstancedProgramDefinition interface, that contains
  everything ProgramDefinition contains, but with constant attributes
  and data as well
- Enhances the Program class so that it uses instanced rendering when it
  receives an InstancedProgramDefinition instead of ProgramDefinition
- Updates all programs so that they properly use the new `#drawWebGL`
  method
- Ports node.circle.ts to be an instanced program


          Ports various programs for instanced rendering

044dc36

WARNING: The EdgeArrowHead is currently broken, and I don't know why
yet.

Details:
- InstancedProgramDefinition['CONSTANT_DATA'] becomes a number[][]
  instead of a number[] (it makes more sense, since it's an array of
  vertices)
- Ports edge.arrowHead, edge.clamped and edge.triangle as instanced
  rendering programs
- Fixes CONSTANT_DATA in node.circle


          Fixes EdgeArrowHeadProgram with instanced rendering

699fc58

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.

jacomyal force-pushed the v3-instanced-rendering branch from 08a2940 to 699fc58 Compare

October 4, 2023 13:21


          Fixes review changes requests for PR #1377

6b27c92

jacomyal merged commit 6b27c92 into v3

2 checks passed

jacomyal deleted the v3-instanced-rendering branch

October 4, 2023 14:11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels