Idea: restore `drawMode` in some form or other, to avoid user bugs

Discussion
,
1

.drawMode (f.e. gl.TRIANGLE_STRIP) were removed in r112.

However, the solutions that have since been put in place are not ideal:

BufferGeometryUtils.toTrianglesDrawMode allows emulating TRIANGLE_STRIP mode but (besides the fact that it mutates the incoming geometry which is questionable) it produces a new geometry with significantly different meaning such that the BufferGeometry APIs must be used entirely differently.

For example, before converting to triangle strip mode, this code is correct:

geometry.setDrawRange(numberOfPositionsToDraw)

That code sets the number of positions (for the position attribute) that we wish to draw.

However, if we change to triangle strip mode,

const newGeometry = BufferGeometryUtils.toTrianglesDrawMode(geometry, THREE.TriangleStripDrawMode)

not only is render behavior of the original geometry side-effected (added indices), but this code will now be totally incorrect:

newGeometry.setDrawRange(numberOfPositionsToDraw)

There is no mention about this in the docs anywhere. The problem is that once we’ve switched to a indexed geometry (that’s how the TRIANGLE_STRIP emulation works), the draw range must be the number of indices to draw, no longer the number of positions to draw, which is definitely not obvious.

To correct the problem, the user will have to look at source code, and finally figure out they need to write this instead:

newGeometry.setDrawRange((numberOfPositionsToDraw - 1) * 3)

or if they’re making a series of quads, not triangles, then

newGeometry.setDrawRange((numberOfPositionsToDraw - 2) * 3)

This is totally non-obvious.

Essentially, what setDrawRange now requires is the number of triangles to draw (between positions) multiplied by 3 for the fact each triangle has three numbers for indices.

I really think that modes should be restored, in a way similar to what was mentioned in the drawMode removal PR:

const geometry = new TriangleStripGeometry()
geometry.setAttribute('positions', array)

// this is intuitive, indices are abstracted away from the user
geometry.setDrawRange(numberOfPositionsToDraw)

where TriangleStripGeometry.setDrawRange does the automatic mapping from point count to triangle index count.

Perhaps BufferGeometry.setDrawRange would remain as is, so people who know what it is doing will be able to do custom indexing, but otherwise people that just need a strip are left with a lot of complication.

With this approach, we can remove BufferGeometryUtils.toTrianglesDrawMode. The concept is really meant to be a class, IMO (aligning with all else in Three.js), and we’ll also then get rid of unexpected side-effects.

2

The cost of supporting other draw modes was very high — everything that interacts with triangle geometry needs to know how to deal with these draw modes, from raycasting to merging geometries to indexing geometry. Some of those features just never worked, causing more serious bugs than the confusion of indexed vs. non-indexed geometry you’re mentioning here.

.setDrawRange has always had a different meaning for indexed and non-indexed geometry. I’d certainly be open to clarifying in docs that toTrianglesDrawMode indexes geometry. But I don’t see us bringing back the draw modes – that would create more side effects than it solves.

Also note from the thread — “modern hardware is more efficient at rendering triangle lists” than triangle strips.

2 Likes
3

@donmccurdy I think you misunderstood what I suggested.

Rather than bringing back the original drawModes (and complicating the code), the idea above is only mapping the to the old modes using the same type of emulation as toTrianglesDrawMode does with indexed triangles, but in a more intuitive way.

The example above is similar to what Ricardo proposed in this comment at the end of that GitHub thread.

I’m just saying that we really need it (something like what he proposed) because currently it is easy to make mistakes with plain BufferGeometry (f.e. setDrawRange). I gave an example of a geometry-based class approach (as opposed to having mesh-based classes like Ricardo’s example).

Also note from the thread — “modern hardware is more efficient at rendering triangle lists” than triangle strips.

The above idea is to uses the same indexed triangles emulation, not actually bringing back the original low level modes.

New APIs like TriangleStripGeometry.setDrawRange would simply map from position range to index range to make things super easy for people. No change to raycasting needed, etc.

4

^ @gkjohnson @Mugen87 I think you both misunderstood what I intended to say also (you hearted Don’s reply).

This idea keeps gl.TRIANGLES only. Ricardo already proposed it. I just think it would be great to actually have something like it to avoid user error.

5

Hm, a method for setting the draw range as if the contents of the geometry were an un-indexed triangle strip is a pretty narrowly-useful thing. It doesn’t feel like it justifies entirely new BufferGeometry class(es). What about InstancedTriangleStripGeometry? If anything I wish we had fewer classes related to geometry, with all the BufferAttribute and Float32BufferAttribute and Interleaved and Instanced variations we have now. Subclasses lead to problems when you need different combinations of the features they offer.

Probably a utility function would be enough here?

6

Wanting to draw a certain “number of points” or “number of triangles” in a list is common. I don’t see how that can be narrowly useful.

Sure, we don’t want to limit any abilities, but we also want to make things easy.

F.e., what sort of utility function are you thinking?

Functions like

setPointsDrawRange
setTrianglesDrawRange
setLineSegmentsDrawRange

etc?

7

I think the current setDrawRange(...) does what it should for both indexed and non-indexed geometry. I’m not really seeing another use case here that needs to be solved differently. This thread goes straight to a proposed solution, but I don’t think you’ve really established the problem you’re solving; I disagree that this need is common.