WebGLRenderer: Enable rendering without vertex data.

[filonik]

Fixed #19430

Description

Attempt at cleaning up pull request #19451 to make the logic/control flow easier to follow. The goal is to enable rendering procedural geometries without vertex data (using raw shader materials).

Implements the following logic:

1. Initialize drawStart and drawEnd to 0 and Infinity.
2. If a drawRange is specified, clamp drawStart and drawEnd to drawRange.start and drawRange.end.
3. If a group is specified, clamp drawStart and drawEnd to group.start and group.end.
4. If the geometry is indexed, clamp drawStart and drawEnd to 0 and index.count.
5. If the geometry is non-indexed with a position attribute, clamp drawStart and drawEnd to 0 and position.count .

[filonik]

I created an example with a simple usage example of a geometry without attributes. Is there any documentation on how to add a description/screenshot? (CI fails because of it...)

[LeviPesin]

Is there any documentation on how to add a description/screenshot?

Via the npm run make-screenshot examplename.

[filonik]

Via the npm run make-screenshot examplename.

I tried this, and I am getting:

ERROR! Diff wrong in 0.095 of pixels in file: webgl2_buffergeometry_no_attributes

Maybe this is platform dependent? I use some pseudo random noise in the example. It should be deterministic, but maybe the results differ between platforms.

I can move the example to a separate pull request if that would be preferable.

[filonik]

Via the npm run make-screenshot examplename.

I tried this, and I am getting:

ERROR! Diff wrong in 0.095 of pixels in file: webgl2_buffergeometry_no_attributes

Maybe this is platform dependent? I use some pseudo random noise in the example. It should be deterministic, but maybe the results differ between platforms.

I can move the example to a separate pull request if that would be preferable.

Ok, I swapped out the noise function in the example for a stable (yet slightly more complicated) one and CI finally seems happy.

[Mugen87]

This is a real nice refactoring! 👍

[Mugen87]

Okay, let's try this!

[Mugen87]

I have noticed one thing after the approval. Is the use case mentioned in #19430 actually fixed by this PR?

It works now if you define a draw range on BufferGeometry level but not when using InstancedBufferGeometry. In this context, you only define instanceCount and not draw range. I guess we have to remove the drawCount === Infinity check otherwise the following piece of code is never reached.

three.js/src/renderers/WebGLRenderer.js

Lines 831 to 833 in 76c7c1c

	const instanceCount = Math.min( geometry.instanceCount, geometry._maxInstanceCount );
	renderer.renderInstances( drawStart, drawCount, instanceCount );

[filonik]

Hm, I would have thought that drawCount essentially works the same way for instanced geometries. I am still not sure what the expected result of passing Infinity actually is. I would have thought that both drawCount and instanceCount need to be some positive integer in order for something to be rendered. Let me investigate further...

[filonik]

Yeah, I think the problem in #19430 still exists, but for a reason unrelated to drawCount. It comes down to _maxInstanceCount being undefined/NaN, which results in instanceCount being NaN on this line:

three.js/src/renderers/WebGLRenderer.js

Line 831 in 76c7c1c

const instanceCount = Math.min( geometry.instanceCount, geometry._maxInstanceCount );

Otherwise the following code from #19430 should now work fine:

https://jsfiddle.net/sjpt/Lrhjkstz/latest

My proposed solution would be to initialize _maxInstanceCount to Infinity in InstancedBufferGeometry, or if that is not an option add a check for undefined.

Edit: Yeah, I think a check for undefined is perhaps more appropriate given that _maxInstanceCount is usually computed here:

three.js/src/renderers/webgl/WebGLBindingStates.js

Lines 392 to 396 in 7530207

	if ( object.isInstancedMesh !== true && geometry._maxInstanceCount === undefined ) {
	geometry._maxInstanceCount = data.meshPerAttribute * data.count;
	}

[filonik]

I have pushed a minimal fix which should now also fully address #19430.

[filonik]

Here is my updated working version of the instanced example, if desired I can also add it to the pull request:

https://gist.github.com/filonik/9ff5d59945a16785bbd524534112e3d3

Needs more investigation.

[Mugen87]

I don't think we should touch this code block in this PR.

Your example assumes you use setDrawRange() with InstancedBufferGeometry which should not be necessary. I think we should keep the assumption that Infinity means "render everything". Hence, the drawCount === Infinity definitely needs to be removed.

[filonik]

I can remove the check for Infinity, but it won't fix the problem in #19430, since _maxInstanceCount will still be undefined. The problem does not come from the drawCount === Infinity check, but from this:

isNaN( Math.min( geometry.instanceCount, undefined ) )

[filonik]

I am not sure I understand the use case for an InstancedBufferGeometry where the final drawCount ends up being Infinity. (Note that I am specifically talking about the drawCount that will be passed to drawArraysInstanced.)

https://developer.mozilla.org/en-US/docs/Web/API/WebGL2RenderingContext/drawArraysInstanced

Even when you are rendering multiple instances, you need to somehow specify what range of vertices you want to render multiple instances of. The correct way of handling an InstancedBufferGeometry without attributes like in #19430 is to specify the number of vertices (drawRange.count) and the number of instances (instanceCount) as illustrated in the example. Only specifying either one of both is not enough.

Maybe there's some other use case that I am not seeing...

[Mugen87]

I had to debug this issue more closely to understand what's going on 😅 .

_maxInstanceCount is undefined in your example since setupVertexAttributes() in WebGLBindingStates never sets it. That makes sense since the geometry has no attributes. So you are right, the additional check is actually required.

I've also verified that Infinity is not a valid input for all WebGL drawing methods. So it's true that we can't use it as an input.

I'm feeling more confident with the PR now. The only thing that can be changed is the beginning of the drawStart/drawEnd code block:

const drawRange = geometry.drawRange;
const position = geometry.attributes.position;

let drawStart = drawRange.start * rangeFactor ;
let drawEnd = ( drawRange.start + drawRange.count ) * rangeFactor;

We can use this simplification since drawRange is always defined when using BufferGeometry. It is not a nullable property. So the respective if clause can be removed.

[filonik]

Yes, this reflects my understanding of things. Initializing drawStart/drawEnd as you suggest makes sense. I suppose in this instance I was being overly defensive, trying to account for the possibility of drawRange being set to null. If this can never happen your approach makes more sense, I will adopt it.

Otherwise, I have high confidence that the current code is correct, and that it is a simplification/improvement over the existing code. The only possible alternative that I see for checking _maxInstanceCount against undefined would be to always initialize it to Infinity in setupVertexAttributes(). However, I am not sure if that would really be an improvement...