Styles can define custom vertex attributes

[bcamper]

This PR adds support for custom vertex attributes/varyings in user-defined styles. This simplifies existing uses of per-feature property data in shaders, which are usually "smuggled" into the shader by encoding non-color values in the color parameter, by moving these values to more semantic-appropriated attributes names/formats, and also preserving the color attribute for its intended use. It also enables more advanced forms of custom shader visualizations, by allowing for multiple attributes to be defined, and with the potential for more datatypes (only float is covered in the initial PR).

Syntax

• A new attributes object is available under a style's shaders block.
  • Each key under attributes specifies an attribute name, and the value specifies the attribute type and other options.
  • The attribute must specify its type. Currently, only type: float (encoded as a 32-bit GL float) is supported.
  • If the attribute value is only needed in the vertex shader, the user can disable the creation of the varying with varying: false, potentially improving performance.
  • For each attribute, a vertex shader attribute and fragment shader varying is made available in shader blocks using the same name.
  • Internally, prefixes a_, and v_ are used, e.g. for an attribute named custom, the vertex attribute a_custom and varying v_custom are defined. Those prefixed values should be considered a private API. For the public interface, they are aliased to the "plain" name custom for easier consumption by users (see PR discussion below).
• Values are assigned to these attributes on a per-feature basis, inside a new corresponding attributes object inside a draw group. Values can be assigned through common formats:
  • JS function, for example to assign a numeric feature property custom to an attribute of the same name (likely most common case for this functionality): custom: function(){ return feature.custom; }
  • Single value: custom: 5
  • Zoom interpolated values: custom: [[14, 50], [15, 150], [16, 300]]

Example

For example, to create a style that colors buildings by height with a custom attribute named height, set here using default draw parameters:

buildings:
  base: polygons
  shaders:
    attributes:
      height:
        type: float
    blocks:
      color: |
        // Use the custom attribute's varying to shade by height
        color.rgb = vec3(min(height / 100., 1.), 0., 0.);
  draw:
    # use default draw parameters to set the height attribute value from the height feature property
    attributes:
      height: function() { return feature.height; }

(N.B.: for illustration of concepts only, in practice it would be more efficient to implement this kind of style with existing color parameter and JS function, unless some other shader-specific logic needed to be applied.)

This style yields the following:

Discussion items

• There is opportunity to support other data types, though the initial PR has been kept to float for simplicity. Possible candidates:
  • vec4, with byte storage (?) and optional (?) normalization (e.g. mapped to 0..1 or -1..1 ranges); this would also support special cases like wanting to map 64-bit int OSM ids into two vec4s
  • explicit color type, with standard color parsing applied such that CSS syntax like red, hsl(), etc. can be used (maps to an unsigned vec4 with normalization)
• Is the varying: false option to create vertex-shader-only attributes warranted? There are use cases for this (if only vertex-shader blocks like position need access to the shader, or even the normal block with lighting: vertex), but the CPU memory cost is incurred in either case. This only helps if for example you are bumping up against the max number of varyings for current hardware.
• The naming scheme, with a_ and v_ namespace prefixes, diverges from how uniforms are defined (as-is, with the provided name, the u_ convention is encouraged but not required). On the other hand, it's consistent with how other internal attributes and varyings are named.
• Custom attributes can be a good supporting mechanism for more shader interactivity / selection patterns (similar to more complete experiments done in this area previously, but in this case stripped down to a simpler primitive). It is fairly straightforward to extend the feature selection system to store any custom attribute values per features, and set a corresponding uniform with the currently selected (hover, click, etc.) value inside a shader. I've prototyped this, but wanted to keep out of the initial PR for scope.

[bcamper]

cc @meetar @matteblair @tallytalwar @burritojustice @sensescape

[bcamper]

@matteblair happy holidays 🎄Would love some thoughts on this when you can -- I'd like to get the first-pass syntax nailed down soon if possible.

[matteblair]

Hey thanks for the ping, I'd almost forgotten about this.

Generally this feature seems pretty solid to me, I just have thoughts on details:

1. The PR doesn't specify what happens when there isn't a suitable value for the attribute. e.g. If a draw rule for a style with custom attributes doesn't specify values for those attributes, or if the attribute value is a JS function with a syntax error, or if the JS function throws an exception, or if the YAML value is the wrong type, etc. The simplest thing would probably be to just default the attribute value to zero in all of those cases. Is this the current behavior?

2. I think the varying: false option should stay. We've encountered Android shader compilers that will fail with a linking error if you declare a varying that isn't used in the main function -____- also skipping interpolation is just a nice optimization to have available.

3. The naming convention does seem like it could be error-prone - if you forget the a_ or v_ in your GLSL you could be very confused about why your attributes don't seem to be injected. It doesn't help that it differs from the behavior of uniforms. We could just use the user-given name for the attribute (with no prefix) but then we have to differentiate the varying name if one is present. No matter how we name the variables themselves, we'll have the ergonomic difficulty that with attributes (unlike with uniforms) the author needs to know which shader stage each block is in to know which variable to use.

   A potential improvement could be to add a #define directive in each shader stage that allows the author to use the attribute's plain name to refer to the appropriate variable. e.g. in the sample from the PR description we would declare a attribute float a_height and a varying float v_height, then in the vertex shader add a #define height a_height and in the fragment shader add a #define height v_height. Thoughts?

[bcamper]

Thanks @matteblair! These are helpful suggestions.

1. The PR doesn't specify what happens when there isn't a suitable value for the attribute. e.g. If a draw rule for a style with custom attributes doesn't specify values for those attributes, or if the attribute value is a JS function with a syntax error, or if the JS function throws an exception, or if the YAML value is the wrong type, etc. The simplest thing would probably be to just default the attribute value to zero in all of those cases. Is this the current behavior?

You're right this should have more explicit specification. For 0 or null values generated on the JS side, they will be 0 in the shader, yes. For NaN values, they will be the float bit pattern for NaN (which I believe also has multiple definitions which can differ by implementation?), which is true in several places in Tangram JS (possibly unfortunately). Some non-number types may get converted to NaN as well.

We could reasonably standardize this for attributes to be an explicit 0 for any value that is either a non-numeric type or NaN.

2. I think the varying: false option should stay. We've encountered Android shader compilers that will fail with a linking error if you declare a varying that isn't used in the main function -____- also skipping interpolation is just a nice optimization to have available.

👍

3. The naming convention does seem like it could be error-prone - if you forget the a_ or v_ in your GLSL you could be very confused about why your attributes don't seem to be injected. It doesn't help that it differs from the behavior of uniforms. We could just use the user-given name for the attribute (with no prefix) but then we have to differentiate the varying name if one is present. No matter how we name the variables themselves, we'll have the ergonomic difficulty that with attributes (unlike with uniforms) the author needs to know which shader stage each block is in to know which variable to use. A potential improvement could be to add a #define directive in each shader stage that allows the author to use the attribute's plain name to refer to the appropriate variable. e.g. in the sample from the PR description we would declare a attribute float a_height and a varying float v_height, then in the vertex shader add a #define height a_height and in the fragment shader add a #define height v_height. Thoughts?

I like your suggestion for aliasing, it solves the issue nicely, including sidestepping the issue of knowing which shader stage you're in, and allows us to only expose the single "plain" (un-prefixed) name as the public interface for users (I'm assuming this was your thought as well?). I'm updating the branch to include these.

[matteblair]

We could reasonably standardize this for attributes to be an explicit 0 for any value that is either a non-numeric type or NaN.

That would be my inclination, particularly in the case where the attribute definition is absent from a draw rule (I guess that's an Undefined?). NaN however is a float value so I am not opposed to passing it in unmodified.

expose the single "plain" (un-prefixed) name as the public interface for users (I'm assuming this was your thought as well?)

Exactly - this way we're free to change how the attributes are named or stored later on.

My only concern with the "aliasing" is collision with preexisting variables like position or normal - if the compiler errors are too arcane we can always add a blacklist of names.

[bcamper]

I updated the behavior to keep NaN values as-is (this does better match behavior for other draw parameters). Yes, it will match undefined in addition to null or any other value that is not of type number.

[tallytalwar]

Great description @bcamper and good points discussed. I just have one tiny question on this, whats the behavior with style mixing, when same attributes is defined in 2 mixed styles but used at different places? I would expect spurious results, but the author should be aware of the usages of the 2 attributes appropriately in my opinion. It's hard to guard against these user- mischiefs/mistakes.