Simplify data-driven paint attributes implementation

[mourner]

A work in progress, closes #3525. cc @jfirebaugh @lucaswoj

The tests are failing because of bucket.test.js, which is mocked in a very complex way so I can't figure out how to fix a few of its failing assertions yet.

[jfirebaugh]

Could we do this in the vertex shader instead?

[mourner]

Yes, thought about that too. I just placed it here temporarily.

[mourner]

Looking at the vertex shader, it seems that fill-extrusion-color alpha value is not used there at all. So we could simply remove this code. @lbud does this sound right?

[lbud]

The reason for this is that if you provide an rgba color in the style, we premultiply it in StyleDeclaration, but we don't render using the alpha channel, so providing a non-opaque rgba value leads to unexpected color results. We decided the best of several not-great approaches here would be to deliberately ignore the alpha channel before premultiplying.

[jfirebaugh]

After #3523 I believe we have a regular naming convention for the attribute/uniform name -- it's the name of the paint property with the layer type prefix removed.

I haven't looked at the bucket.test.js failures, but basically the one thing that it mocks is the feature objects. The other collaborator objects in the test -- style layer and properties, bucket subclass, and program interface -- are the real thing.

[mourner]

Yes, noticed about attribute/uniform name convention too — I figured this was the reason you asked for #3523. :)

I'm still unsure about deriving type/multipliers — we can do it for colors, but for things like circle-blur and fill-extrusion-height, converting them to float doubles the memory requirement (16 -> 32 bits), and picking the right maximum in the spec with the intent to make it automagically derived as Uint16 feels a bit too magic and fragile to me. Thoughts?

[mourner]

@lucaswoj @jfirebaugh @lbud @ansis this PR is ready for review. It didn't accomplish everything that was planned in #3525 fully, but instead led me to significantly improve the clarity of data-driven-properties code (and -107 lines net!).

Performance also slightly improved — from 6.1ms to 5.5ms on the Streets z13 benchmark. Buffer bench results are unchanged. All commits are self-contained and pass tests.

I suggest merging this now and following up with the points below in other PRs:

• Move extrude color alpha ignore logic to the shaders.
• Derive type and multiplier from spec min/max.

[lucaswoj]

In the codebase we use the terminology property functions, zoom functions, and zoom and property functions."data-driven" is a broader designation that may include unrelated features in the future.

[lucaswoj]

We could call these methods addZoomAttribute, addPropertyAttribute, and addZoomAndPropertyAttribute

[lucaswoj]

This looks great! I have couple thoughts on terminology in ProgramConfiguration. I feel 👍 on merging this once those and #3527 (review) are cleared up.

[jfirebaugh]

It's definitely easier to understand ProgramConfiguration now. 👍

In gl-native, the implementation is going to center entirely around paint properties. We use code generation to create a unique type for each paint property, which can provide to C++ the data that's in the style specification. Having unique types allows the use of C++ structural metaprogramming techniques to define generic operations over tuples of paint properties. There will be an equivalent of ProgramConfiguration, but it's likely to use a more functional programming style, where things like the list of uniforms and pragmas are generated as needed from a tuple type representing the DDS-enabled paint properties.

In the initial implementation, I'm not going to use multipliers. fill-extrusion-height will use a float attribute. *-color and *-opacity will use normalized uint8s.

I'm also going to use glVertexAttrib* rather than dynamically generated shaders as a first cut.