feat(layers): Simplify ArcLayer

[Pessimistress]

Taking advantage of our move to es300.

Change List

• Removes positions attribute, use gl_VertexID instead
• No longer rebuilds model on numSegments change

[coveralls]

coverage: 89.826% (-0.004%) from 89.83%
when pulling c836b34 on x/arc-layer-vertex
into a9acdf8 on master.

[donmccurdy]

It feels a little strange to me that the Model class can be allocated without a vertex count or buffers – is that intended? Just wondering if passing in vertexCount here, or perhaps stricter Model/Geometry APIs in Luma, might make sense at some point. No action needed.

[ibgreen]

It feels a little strange to me that the Model class can be allocated without a vertex count or buffers

Yes, models without geometry are a thing. Since we are now using GLSL 3.0, for small geometries we can define the arrays directly in the shader. We have a geometry-less triangle in GLSL and WGSL in https://github.com/visgl/luma.gl/blob/master/examples/tutorials/hello-triangle/app.ts#L22

I agree it surprised me when I first realized that it was possible. We could disallow it to avoid surprising folks.

[donmccurdy]

Maybe two parts here:

1. model without geometry
2. model without vertex count

For me (2) was the more surprising part, since that would seem to be an invalid initialization state. If a Model without Geometry or Attributes is valid, is a Geometry without Attributes also valid? Not suggesting we add more flexibility, just maybe clarity to make valid/invalid states obvious.

[ibgreen]

Yes... I am not sure how to encode these things in APIs and types.

"Either-or type checks" require a fair bit of typescript machinery (discriminated unions) that makes code less readable.

[felixpalmer]

The geometry & vertexCount props are quite closely linked. Model._setGeometryAttributes() sets the vertexCount based on the number of vertices obtained from the gpuGeometry, which is then optionally overwritten with props.vertexCount. I wonder if it would be better to support a NullGeometry that contained nothing but a vertexCount and then we remove the vertexCount prop altogether and make props.geometry required.

[ibgreen]

I like the general idea... Though we do have a similar situation with instanceCount so it would be nice to handle both in a similar way.

Something I really liked was the design we did in deck.gl-native, where we took the "table view" and avoided the term geometry and instead said that a model can have two tables, basically something like:

model.setAttributes(table: arrow.Table) 
model.setInstancedAttributes(table: arrow.Table)

This makes it clear that these are tables of different lengths, and the table lengths determine the vertexCount and instanceCount (though these can be overridden during draw of course, to just draw a range).

Of course we can still have a convenience function setGeometries that contain table, indices and topology and that calls this underlying function.

We dohave the complication of indexed geometries and vertex-sharing topologies to deal with (though modern GPU programming practices often seem to recommend avoiding indices and vertex-sharing topologies, so we could make the decision to start phasing that out).

[donmccurdy]

My feeling is that indexed geometries are still very much recommended, and are key to certain optimizations. I agree that alternative geometries like strips and lists could be phased out.

While working on porting code to v9, I personally found the number of different ways to get attributes into a Model confusing, I wasn't always sure if I was doing it the correct way, or mixing two correct ways incorrectly. That can be a hidden cost of convenience methods.

I quite like the "two tables" idea!

(and sorry for starting a long thread here, @Pessimistress please feel free to merge!)