[ios, macos] Add support for data driven styling

[boundsj]

iOS portion of #4860
depends on: #7372
related to: #7752

Implement DDS getters and setters

• Setters
  • Data Driven Property Value
    • Constant value
    • Camera Function with
      • Exponential stops
      • Interval stops
    • Source Function with
      • categorical stops
        • string attribute key
        • bool attribute key
        • int attribute key
        • handle default value
      • exponential stops (compiles and runs but nothing appears)
      • interval stops (compiles and runs but nothing appears)
      • identity stops
    • Composite function
      • Stops with categorical functions
      • Stops with interval functions
      • Stops with exponential functions
  • Property Value
    • Non-enum
      • Constant value
      • Camera function
    • Enum
      • Constant value
      • Camera function
• Getters
  • Get Data Driven Property Values
    • Constant
    • Camera Function with
      • Exponential stops
      • Interval stops
    • Source Function with
      • categorical stops
        • string attribute key
        • bool attribute key
        • int attribute key
        • handle default value
      • exponential stops
      • interval stops
      • identity stops
    • Composite function
      • Stops with categorical functions
      • Stops with interval functions
      • Stops with exponential functions
  • Get Property Values
    • Non-enum
      • Constant value
      • Camera function
    • Enum
      • Constant value
      • Camera function

[mention-bot]

@boundsj, thanks for your PR! By analyzing this pull request, we identified @1ec5, @jfirebaugh and @friedbunny to be potential reviewers.

[boundsj]

@jfirebaugh

• The below classes were modified to support round tripping values. Do the changes (example) make sense?

  • data_driven_property_value.hpp
  • camera_function.hpp
  • categorical_stops.hpp
  • composite_function.hpp
  • source_function.hpp

• What is the expected behavior of source functions with exponential (or interval) stops? This statement compiles and runs with no errors but causes the layer it is applied to to disappear completely:

mbgl::style::SourceFunction<MBGLType> sourceFunction = {function.attributeName.UTF8String, exponentialStops}

• Noting here that the work described in the second to last checkbox in Port data-driven styling (property functions) from GL JS #4860 (comment) is probably required to eliminate an issue where DDS property changes don't take effect immediately in some cases. I'm also seeing the property function styling flicker (i.e. a color is applied to a circle / the circle is colored black) from time to time when the map is tilted. I'm not sure if that is related to the yet to be done worker round trip implementation.

[jfirebaugh]

@boundsj I'm not seeing where those additions are used elsewhere in the diff. Can you point me to where they were needed?

The omission of methods such as isConstant/isCameraFunction/asCameraFunction was by design, and I'd like to remove the existing such methods from PropertyValue as well. Using the visitor functionality of variant is a safer way to implement type-specific dispatching.

[jfirebaugh]

Ah, GitHub was hiding parts of the diff so they didn't show up in text search.

Yes, code such as this should instead be written using a visitor (example). One advantage of doing so is that future additions to the variant types will produce a compile-time error until the code is updated to handle the new type.

[1ec5]

This PR has the potential to introduce a lot of classes, more than we’d want the developer to have to juggle. The point of implementing MGLStyleValue as a class cluster in the first place was to reduce the number of classes the developer would have to know about in order to use style values in a basic way.

I’d prefer if we could somehow draw class-level distinctions based on only the most important axis (in terms of mapbox/mapbox-gl-style-spec#613) and represent any other axes with enum-typed properties or optional properties/parameters. By analogy, the NSPredicate class cluster publicly exposes only NSCompoundPredicate and NSComparisonPredicate as concrete subclasses, but NSComparisonPredicate has a predicateOperatorType property (and privately a _predicateOperator ivar) that allows further variance.

A composite (aka "zoom and property") function that can have exponential,interval, categorial zoom portion stops can be one of MGLStyleCompositeExponentialFunction, MGLStyleCompositeIntervalFunction, MGLStyleCompositeCategoricalFunction (the interval and categorical composite functions are not actually implemented at this time).

Can we implement composite functions through composition? That is, could we represent an exponential composite function as an MGLStyleExponentialFunction with functions as stop values? This does mean shifting some validation of the stop values from design time to runtime, but I think this is a reasonable concession.

While we’re going about renaming classes, the class names proposed here are a bit ungrammatical. A subclass usually inserts an adjective right after the class prefix. (I’ve always felt weird about MGLStyleConstantValue; it should be MGLConstantStyleValue.) For example:

• MGLStyleExponentialFunction → MGLExponentialStyleFunction
• MGLStyleIntervalFunction → MGLIntervalStyleFunction
• etc.

[1ec5]

Let’s delete the fill extrusion layer type and its properties from spec right off the bat so we don’t forget to account for this omission when we introduce additional code that enumerates the others (or forget to remove these special cases when we do implement fill extrusion layers).

[1ec5]

Nit: no need for == true, because true is truthy.

[1ec5]

Use NS_DICTIONARY_OF for compatibility with applications linked against the iOS 8.x SDK.

[1ec5]

#7486 renamed base to interpolationBase. That’ll come over in the next merge to master.

[boundsj]

oops. thanks for the reminder

[boundsj]

@jfirebaugh thanks for pointing me towards the variants and visitor pattern. The previous implementation has been replaced with a set of evaluators and visitors. In addition to the benefit you mentioned, I think this approach makes it easier to follow the partitions of the conversion of functions and stops in the implementation.

The omission of methods such as isConstant/isCameraFunction/asCameraFunction was by design, and I'd like to remove the existing such methods from PropertyValue as well.

I've been able to eliminate all references to PropertyValue's is(as)Constant in the getter converters (the refactor I just completed). I should be able to eliminate them in the setter converters next. This should help make it possible to refactor PropertyValue as you said.

[boundsj]

@1ec5 thanks for notes in #7596 (comment). Here is an updated proposal for this API:

The principle axis of function categories are:

• Camera -- zoom-driven, possible stops: exponential, interval stops
• Source -- data-driven: exponential, interval, categorical, identity stops
• Composite -- both zoom and data driven (previously known as "zoom and property”): exponential, interval, categorical primary stops and each stop can hold a camera or source function with exponential, interval, categorical stops

All of these mbgl types could be represented by the single existing class:

• MGLStyleFunction

Internally, MGLStyleFunction could gain a new enumeration ivar that allows it to keep track of where if falls on the primary axis. The ivar would be set based on the MGLStyleFunction factory method used to create the function.

The stops axis could be represented by a publicly exposed enumeration with the values:

• MGLPropertyFunctionTypeExponential
• MGLPropertyFunctionTypeInterval
• MGLPropertyFunctionTypeCategorical
• MGLPropertyFunctionTypeIdentity

With these classes, MGLStyleValue could expose the entire DDS API as follows:

Camera functions

• cameraFunctionValueWithType:Stops:InterpolationBase:

param type: MGLPropertyFunctionTypeExponential or MGLPropertyFunctionTypeInterval
param stops: NSDictionary<NSNumber *, MGLStyleValue<T> *> *
param interpolationBase: CGFloat value used when the type is exponential

• cameraFunctionValueWithType:Stops:

When the type is exponential this is a convenience for cameraFunctionValueWithType:Stops:InterpolationBase: where interpolationBase is always 1.0. When the type is interval this convenience method creates a camera
function with interval stops.

These methods would create an MGLStyleFunction with an internal enum that specifies that it is a camera function that would be used to create a mbgl::style::CameraFunction with mbgl::style::ExponentialStops or mbgl::style::IntervalStops and a base.

Source functions

• sourceFunctionValueWithType:attributeName:stops:interpolationBase

param type: MGLPropertyFunctionTypeExponential, MGLPropertyFunctionTypeInterval, MGLPropertyFunctionTypeCategorical, MGLPropertyFunctionTypeIdentity
param attributeName: String value representing the attribute key to connect the function to
param stops: Optional NSDictionary<id, MGLStyleValue<T> *> * value used when the type is exponential, interval, or categorical
param interpolationBase: CGFloat value used when the type is exponential

• sourceFunctionValueWithType:attributeName:stops

When the type is exponential this is a convenience for sourceFunctionValueWithType:attributeName:stops:interpolationBase where interpolationBase is always 1.0. When the type is interval, categorial, or identity this convenience method creates a source function with the appropriate type of stops.

• sourceFunctionIdentityValueWithAttributeName:

Convenience method that creates an identity function. This could be included since an identity function does not need any input except for the attribute key name.

These methods would create an MGLStyleFunction with an internal enum that specifies that it is a source function. This would be used to create a mbgl::style::SourceFunction with mbgl::style::ExponentialStops, mbgl::style::IntervalStops, mbgl::style::CategoricalStops, or mbgl::style::IdentityStops, an attributeName, and a base.

Composite functions

• compositeFunctionValueWithType:Stops:InterpolationBase:

param type: MGLPropertyFunctionTypeExponential, MGLPropertyFunctionTypeInterval, MGLPropertyFunctionTypeCategorical
param stops: A NSDictionary<id, MGLStyleValue<T> *> * with MGLStyleFunction values that are exponential, interval, or categorical camera or source functions
param interpolationBase: CGFloat value used when the type is exponential

The composite function is similar to cameraFunctionValueWithType:Stops:InterpolationBase: but take dictionaries with MGLStyleFunction values for the stops instead of constants.

This method would create an MGLStyleFunction with an internal enum that specifies that it is a composite function. This would be used to create a mbgl::style::CompositeFunction with mbgl::style::ExponentialStops, mbgl::style::IntervalStops, mbgl::style::CategoricalStops that holds mbgl::style::SourceFunction as the value. or mbgl::style::CameraFunction as values.

Legacy function support

The following legacy methods could continue to be exposed and will create functions that are now known as camera functions on the primary axis.

valueWithStops:
valueWithInterpolationBase:stops:

[boundsj]

@1ec5 Can we implement composite functions through composition? That is, could we represent an exponential composite function as an MGLStyleExponentialFunction with functions as stop values? This does mean shifting some validation of the stop values from design time to runtime, but I think this is a reasonable concession.

#7596 (comment) proposes essentially that. Note that mbgl requires the attribute name to be specified up front (i.e. mbgl::style::CompositeFunction<MBGLType> compositeFunction = {function.attributeName.UTF8String, compositeStops}; so we could extract that string out of the function based stops passed in. Stops with functions with a mixture of attribute names don't make sense so we may want to assert that they are the same.

[boundsj]

@1ec5 when thinking about the proposal in #7596 (comment), one alternative that comes to mind is to create a set of classes to represent the stop types. We could still classify that secondary axis with an internal enum value in MGLStyleFunction but, if the stops were actual classes (like they are in mbgl) I think that would make the public facing API less error prone.

For example, instead of creating a kind of source function that has interval stops like this:

[MGLStyleValue<UIColor *> sourceFunctionValueWithType:MGLPropertyFunctionTypeInterval :attributeName:@"foo" stops:stops]

we could offer something like this:

MGLIntervalStops *intervalStops = [[MGLIntervalStops alloc] initWithStops:stopsDictionary];
[MGLStyleValue<UIColor *> sourceFunctionValueWithAttributeName:@"foo" intervalStops:intervalStops]

I think the advantage of this is that it would help reduce mistakes like specifying that the function is
exponential put passing stops with strings as keys (interval stops).

[jfirebaugh]

When designing this API, you should expect that interpolationBase is not the only property that will exist that's specific to exponential stops. Exponential stops will also need a property to specify the color space (#6721).

[boundsj]

When designing this API, you should expect that interpolationBase is not the only property that will exist that's specific to exponential stops. Exponential stops will also need a property to specify the color space (#6721).

That reminds me, interpolationBase is currently taken in the the initializer for the MGL functions themselves. If we created classes to represent stops (as proposed in #7596 (comment)) then base and color space could be part of the stops object they are intended to be directly associated with. And, both of these values may make more sense as properties rather than extending the initializers.

Parts of #7596 (comment) seem strange since base is a float that only sometimes makes sense depending on the enumerated type of the function. But base and color space properties would always make sense for a proper exponential stops object.

[boundsj]

@jfirebaugh tests are passing, but please hold off on merging this branch to dds-wip until there is final agreement on the platform level API and the API documentation is complete (this week). In that time, I also want to add more Swift tests of runtime and data-driven styling, update our generated examples in Darwin header files, and add DDS examples to iosapp.

Note: to support assertions in the Darwin tests, I backfilled an inequality operator in: https://github.com/mapbox/mapbox-gl-native/blob/boundsj-dds-darwin-wip/include/mbgl/style/data_driven_property_value.hpp#L29-L32

[boundsj]

Here is an updated status report, with examples, on the state of the Darwin platform API:

API

data-driven, camera function with exponential color stop values

let stops = [
  0: MGLStyleValue<UIColor>(rawValue: .red),
  10: MGLStyleValue<UIColor>(rawValue: .red),
  15: MGLStyleValue<UIColor>(rawValue: .green)
]
circleStyleLayer.circleColor = MGLStyleValue<UIColor>.cameraFunctionValue(
  stopType: .exponential, 
  stops: stops, 
  options: [.interpolationBase: 10.0]
)

data-driven, source function with categorical color stop values with integer attribute keys

let stops = [
  0: MGLStyleValue<UIColor>(rawValue: .green),
  100: MGLStyleValue<UIColor>(rawValue: .orange)
]
circleStyleLayer.circleColor = MGLStyleValue<UIColor>.sourceFunctionValue(
  stopType: .categorical,
  stops: stops,
  attributeName: "temp",
  options: [.defaultValue: MGLStyleValue<UIColor>(rawValue: .red)]
)

Naming

there are camera, source, and composite function types, and there are exponential, interval, categorical, and identity property types. But “property type” already exists as a concept: the fillColor property is of type UIColor (or technically MGLStyleValue).

As you can see above, I went with stopType as the parameter name and it gets set to a MGLStyleFunctionStopType. I chose that name because the "type of stop" is exactly what we want the user to tell us here and I could not come up with a better name for the enum.

Composite functions

Can we implement composite functions through composition? That is, could we represent an exponential composite function as an MGLStyleExponentialFunction with functions as stop values? This does mean shifting some validation of the stop values from design time to runtime, but I think this is a reasonable concession.

I ended up not implementing that approach because a) the core implementation truly expects a composite of stops with constant values and b) the API was much harder to use (and made less sense) if the developer had to make full on source functions to then stuff in stops (that we would then just throw away and take the stops from). It also did not make sense for the outer function to specify an attributeName and then have inner functions that should each specify the same attributeName but may not.

The API we have now looks more like a Swift version of the js examples and Android implementation noted in #7752 (comment)

data-driven, composite function with inner exponential color stop values nested in outer camera stops

let stops: [NSNumber: [NSNumber: MGLStyleValue<NSNumber>]] = [
  0: [0: smallRadiusValue],        // zoom level 0, temp value of 0 use small radius
  10: [200: smallRadiusValue],  // zoom level 10, temp value of 200 use small radius
  20: [200: largeRadiusValue]   // zoom level 20, temp value of 200 use small radius
]
circleStyleLayer.circleRadius = MGLStyleValue<NSNumber>.compositeFunctionValue(
  stopType: .exponential,
  stops: stops,
  attributeName: "temp",
  options: nil
)

Getters

It is still the case that getting values back out requires some casting gymnastics:

// get a value back
if let returnedCircleRadius = circleStyleLayer.circleRadius as? MGLCompositeStyleFunction<NSNumber> {
  if let returnedStops = returnedCircleRadius.stops as NSDictionary? as? [NSNumber: [NSNumber:   MGLStyleValue<NSNumber>]] {
  let lhs: MGLStyleValue<NSNumber> = returnedStops[0]!.values.first!
  let rhs: MGLStyleValue<NSNumber> = radiusCompositeExponentialOrIntervalStops[0]!.values.first!
                XCTAssertEqual(lhs, rhs)
  }
}

🏋️

Next steps

All of the examples above (and many more) are available in MGLStyleValueTests.swift. To finish up I need to:

• Address code review feedback already given that I've not taken care of so far
• Add even more coverage for the various source and composite function types in the generated Objective-C tests
• Update iOS app with a few examples and remove some of the temporary changes I've made to it for testing
  - [ ] Initial API documentation

cc @1ec5 @incanus @jmkiley @jfirebaugh

[boundsj]

Noting that node tests started failing here (and on dds-wip), ignoring that for now. cc @jfirebaugh

[1ec5]

The overridden Swift name notwithstanding, it feels awkward to name an initializer such that it bridges into Swift as MGLStyleValue.cameraFunctionValue(…, using the class method syntax, instead of MGLStyleValue.init(…, using the initializer syntax. For one thing, it means repetition at the call site, which Swift strives to avoid.

Naming the factory methods +valueWith… would have the unfortunate effect of obscuring the type (camera, source, etc.), which is important. To disambiguate the various factory methods at the call site, how about naming the selectors along the lines of +valueWithStopType:cameraStops:options:?

[1ec5]

A @compatibility_alias is needed from MGLStyleFunction to MGLCameraStyleFunction to maintain backwards compatibility with iOS SDK v3.4.0. Alternatively, we could keep MGLStyleFunction as the abstract superclass of MGLCameraStyleFunction et al.

[boundsj]

Done

[1ec5]

What term would refer to UIColor in MGLCameraStyleValue<UIColor>? No matter what we call it, I think developers would gravitate toward “stop type” or “type of stop” for that concept as well.

Instead of referring to exponential, interval, etc. as “stop types”, how about referring to them as “interpolation modes”? MGLInterpolationModeExponential wouldn’t be that much typing. 😛

[boundsj]

I like that and thanks for referencing the helpful SpriteKit example. My only concern is that, with DDS, the identify function actually uses the notion of stops only as a placeholder and interpolation is not a factor. I suppose that, no matter what we call this enum, some clarification about what an identity function is will be required.

[jfirebaugh]

I agree with your choices on composite function design. I was even considering changing the core API so that it uses less composition, in order to reflect the style spec reality that an exponential stop interpolation base is provided once, function-wide, not on a per-inner-function basis.

[boundsj]

@jfirebaugh I think this can be merged to dds-wip.

I created #7924 to track documentation work for DDS on this platform and that can be done on master or the eventual release branch.

[nitrag]

Really exciting! Is color conversion supported yet? (spec) My colors are in rgb hex and are not displaying, not sure if I should open a new issue or if I'm missing something.

style.lineColor = MGLSourceStyleFunction<UIColor>(
    interpolationMode: .identity,
    sourceStops: nil,
    attributeName: "color",
    options: [.defaultValue: MGLStyleValue<UIColor>(rawValue: .gray)]
)

...
properties: {
  ...
  color: '#D50000',
  ...
}

It's worth noting the lines are completely invisible, not defaulting to gray.

[1ec5]

RGB color interpolation should be supported. Can you open a new ticket so we can track this issue specifically? (Alternative color spaces for interpolation are tracked in #6442, by the way.)

[boundsj]

@nitrag @1ec5 here is an example of a feature with rgb hex color in attributes that gets converted correctly with the current 3.5.0 beta 1

let feature = MGLPointFeature()
feature.coordinate = centerCoordinate
feature.attributes = ["hexColor": "#00ff00"] // green

let shapeSource = MGLShapeSource(identifier: "shape-source", features: [feature], options: nil)
style.addSource(shapeSource)

let circleStyleLayer = MGLCircleStyleLayer(identifier: "circle-layer", source: shapeSource)

// Use the value in the attributes dictionary with the key `hexColor`, otherwise use red
circleStyleLayer.circleColor = MGLStyleValue(
    interpolationMode: .identity,
    sourceStops: nil,
    attributeName: "hexColor",
    options: [.defaultValue: MGLStyleValue<UIColor>(rawValue: .red)]
)

style.addLayer(circleStyleLayer)

[1ec5]

These examples inspired me to whip up #8025. 😄

[nitrag]

Nevermind, it works when you remember to add the style layer to the mapView. Much noob!