Disallow expressions for text-font, or require fontstacks to be listed up front

[jfirebaugh]

One of the components of an offline region download is the complete set of glyph range PBFs for all the font stacks used by that region's style. We currently determine the set of font stacks via this method:

mapbox-gl-native/src/mbgl/style/parser.cpp

Lines 271 to 294 in aa4dff1

	std::vector<FontStack> Parser::fontStacks() const {
	std::set<FontStack> optional;
	for (const auto& layer : layers) {
	if (layer->is<SymbolLayer>()) {
	PropertyValue<FontStack> textFont = layer->as<SymbolLayer>()->getTextFont();
	if (textFont.isUndefined()) {
	optional.insert({"Open Sans Regular", "Arial Unicode MS Regular"});
	} else if (textFont.isConstant()) {
	optional.insert(textFont.asConstant());
	} else if (textFont.isCameraFunction()) {
	textFont.asCameraFunction().stops.match(
	[&] (const auto& stops) {
	for (const auto& stop : stops.stops) {
	optional.insert(stop.second);
	}
	}
	);
	}
	}
	}
	return std::vector<FontStack>(optional.begin(), optional.end());
	}

This will fail for text-font properties that use an expression. We need to either disallows the use of expressions for text-font, or introduce a new top-level style spec property that explicitly declares all the font stacks that the style uses (or at least the font stacks that should be available offline).

cc @anandthakker

[anandthakker]

@jfirebaugh as an in-between compromise, we could allow expressions of some tightly constrained form for text-font, such that we could implement the equivalent of the above code to collect the possible values from literals in the expression.

[nickidlugash]

we could allow expressions of some tightly constrained form for text-font

Could expressions that only include literal values be allowed? I think the most common use cases for expressions for text-font would mainly just involve literal values (e.g. in match expressions and/or zoom curves).

[anandthakker]

Yeah, along the lines of @nickidlugash's point, one way to formulate this restriction that seems pretty reasonable both to implement and communicate:

text-font expressions must have a finite number of output values. I.e., varying expressions like [zoom], [get, property_name], [id] may not be used to construct the output value for a text-font expression.

So [ "concat", "Wingdings", ["match", ["get", "font-weight"], "regular", "", "bold", " Bold"]] would fail validation with the above error message, but [ "match", ["get", "font-weight"], "regular", "Wingdings", "bold", "Wingdings Bold"] would pass.

update: Hmm, actually the stated restriction doesn't quite capture it, since it technically would describe both of the examples above.

[jfirebaugh]

That also sounds more restrictive than what @nickidlugash wants. E.g., it disallows things like ["curve", ["step"], ["zoom"], "Stack A", 4, "Stack B"].

[jfirebaugh]

So long as text-font allows only feature-constant expressions, static analysis can determine the bounded set of possible output values. If we allow text-font to have feature-varying expressions, then some expressions will be ok (e.g. ["match", ["get", "foo"], "a", "Stack A", "Stack B"]), while others will not (["get", "stack"]). Still, static analysis should be able to tell when the set of possible output values is unbounded, and we can disallow those cases.