Enable property functions for "text-size" and "icon-size"

[Scarysize]

Great! 🎉

[anandthakker]

I was a bit concerned, seeing this line, that property functions for text and icon size would require introducing a new vertex attribute -- which, on the -native side, would take us back over the low-end-device limit of 8.

Some notes from a deep dive trying to understand what's going on with adjustedSize:

1. To deal with going “around corners” for symbol-placement: line, we may produce multiple “quad” layouts for each label.
2. In such cases, each symbol ‘quad’ that’s created includes a minScale and maxScale, which determine the range of scales, relative to the tile’s zoom level, that this instance of the symbol should be drawn. We store these as a “minzoom” and “maxzoom” attribute for each symbol (actually for each vertex).
3. However, since these per-symbol minzoom/maxzoom are determined at layout time, they must be calculated using a fixed font size — causes a problem when font size is zoom-dependent.
4. So, when laying out a tile at zoom z, we calculate minzoom and maxzoom based on the value of text-size at zoom: z+1 (this is what we’re calling adjustedTextSize). Then, at render time, we lie to the shader by giving it a u_zoom value that incorporates the ratio between the actual text-size value and the one we used during placement.

See also https://github.com/mapbox/mapbox-gl-js/commit/cac61d0a1568d7fd51ea32e95ad188a09bc709d2 (h/t @jfirebaugh for finding this)

Based on the fact that minzoom and maxzoom already exist as attributes (via a_data), I think there's hope that we can rework this to allow property functions without adding a new attribute (by moving some calculations into the shader).

[anandthakker]

Alright - looking into things a bit more, I think the main sticking point here is that we need to put more data into the layout attributes--namely:

• a size_min and size_max, to allow zoom-interpolation for size to happen in the shader.
• adjustedSize: the size at which the quad was computed during layout -- needed because the minzoom and maxzoom quad values that determine which copy of a glyph gets rendered are based on adjustedSize. Used in this calculation, which needs to go into the shader.

Right now, the symbol layout attributes look like:

attribute vec4 a_pos_offset;
attribute vec2 a_texture_pos; // backed by uint16
attribute vec4 a_data; // backed by uint8

So, we do have enough bits in here to pack in the size data we need, but to get decent precision I think we're gonna have to get a little hacky. Something like:

• Change a_texture_pos to vec4 a_data2, and use the last two slots for size_min and size_max.
• Back a_data with uint16, and encode the current 4 values into only two components, using bit shifting (like we did for color here).

cc @jfirebaugh @lucaswoj @mollymerp -- anyone see a less ugly way forward here?

[anandthakker]

Hmm, actually, there's a problem I didn't account for in my previous comment: the text and icons in a single symbol layer are rendered in different draw calls, but they share the same layout attributes. To use the strategy described above, we'd need to include both text-size and icon-size data in the layout attribute buffers. With min and max values for each of these and the adjustedSize value, that's 5 values, and only 64 bits of extra space from the packing.

🤔

[jfirebaugh]

they share the same layout attributes

They share the same attribute layout, but don't share attribute buffers. The layout can contain a size attribute that has different values for each buffer.

[jfirebaugh]

👍 on the packing strategy. Looks like we have just enough bits for everything.

[anandthakker]

They share the same attribute layout, but don't share attribute buffers. The layout can contain a size attribute that has different values for each buffer.

Oh! You're totally right--I had psyched myself out. Yay 🎉

[anandthakker]

An issue I'm now realizing as I work on implementing this: both in native and JS, all the machinery around how we bind possibly data-driven properties to attributes(/uniforms) is set up to handle paint properties. Specifically, this machinery makes sure that we only use as big a buffer as necessary, depending on whether a property value is constant, or a {source,camera,composite} function.

Some options:

• Just always include a size min & max value with the layout attributes. Probably the least invasive, code-wise, but means we'd incur a likely-unacceptable performance penalty, because we'd be populating & uploading an O(num_features) buffer for each symbol layer instead of a single uniform or constant attribute.
• Reimplement logic analogous to what exists for paint properties, specialized for this particular pair of layout properties. A lot of duped logic, not great for maintainability. (My gut sense is that refactoring the existing logic to make it work here is unlikely to be fruitful due to the specialized nature of the attribute packing we're doing here.)
• Look into packing {text,icon}-size in with an existing paint property. Sorta hacky and unclear if it's even feasible, but it might end up actually being a more contained hack than the previous option.

[lucaswoj]

all the machinery around how we bind possibly data-driven properties to attributes(/uniforms) is set up to handle paint properties

Up until now, we haven't needed any special "machinery" to handle data-driven layout properties. Unlike paint properties, layout properties are usually already specified as per-feature attributes. Take a look at this PR which adds support for a data-driven layout property, symbol-rotation, https://github.com/mapbox/mapbox-gl-js/pull/2738. (Not sure if this is directly applicable to the case of text-size).

If you do need to build some new "machinery" you may find use from an earlier implementation of Bucket which implemented a more powerful interface for setting up buffer layouts. This was reverted in https://github.com/mapbox/mapbox-gl-js/pull/3527.