get rid of text-max-size, icon-max-size

[ansis]

and make text-size, icon-size into layout properties

mapbox/mapbox-gl-js#1419
#255

[jfirebaugh]

I was actually leaning towards keeping *-size as paint properties, because it doesn't introduce a special case in how layout properties are evaluated. Making it a layout property would introduce an exception to the rule that layout properties are evaluated only at integer zoom levels, which per #306 (comment) would need an explicit indication in the style spec.

[ansis]

There are going to be exceptions and special cases either way.

If they are paint properties then we'll have an exception where buffers need to be recreated when a paint property changes (either through the api or a class change).

On the implementation side it's easier and more consistent if they are layout props. I hadn't thought about the chart ui usecase though.

I think we may need to change the paint and layout concepts (especially with the upcoming dds) but that seems like too much for v8. It might be ok to hardcode an exception in the chart usecase for now

[jfirebaugh]

On the implementation side it's easier and more consistent

I'm going to keep pushing back here. 😄 How much harder / more complex would it be to keep them as paint properties?

Another potential issue with making them layout properties is that you lose the ability to change text size via paint classes. (The v7 satellite style has a few of these, though I think its particular use of text-size will still work when migrated to a layout property.)

[ansis]

I still think that it is easier to make them layout properties than paint properties.

How much harder / more complex would it be to keep them as paint properties?

Since the -size properties affect the contents of buffers, a layer that refs another layer can't have a different text-size. If they were paint properties then the spec would allow this. What would happen if a layer with ref provides a new text-size? Would the implementation ignore it and just overwrite it with the paint text-size property from the ref'd layer? Or would the implementation break the ref and create a separate set of buffers?

setPaintProperty would need an exception for these properties and would trigger tile reparsing. I'm not familiar enough with the batch style changes, but there might need to be an exception there.

you lose the ability to change text size via paint classes.

Changing text size with classes would require recreating the buffers. This is ok, but very different from how paint property changes work. We should implement class support for all properties that affect buffers (all layout properties), not just these two. Since /app doesn't support classes I don't think this needs to be in v8.

I think the code mostly thinks of layout properties as properties that affect buffers. Them being evaluated only at round zooms is a secondary effect of that.

history:
The layout/paint/ref concepts add complexity to the spec. The idea was that the cost of surfacing these concepts was worth it because having a deeper understanding of the implementation would let designers make faster maps. Using refs wherever possible (or using identical properties so refs can be generated) can significantly reduce the time it takes to load new tiles.

/app doesn't expose any of these concepts. Layout and paint properties look the same in /app. Refs are completely invisible. If /app doesn't expose the concepts, should we remove them from the spec?

[jfirebaugh]

Ok, I'm convinced; going to merge now.

I think we will probably make refs an internal optimization -- mapbox/mapbox-gl-js#1393.