Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Reimplement style values atop NSExpression #10726

Merged
merged 28 commits into from
Jan 24, 2018
Merged

Reimplement style values atop NSExpression #10726

merged 28 commits into from
Jan 24, 2018

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Dec 18, 2017

This PR migrates the entire iOS/macOS style layer API – all the layout and paint properties – from the MGLStyleValue class cluster to the NSExpression class provided by Foundation, backed by @anandthakker’s work on mbgl::style::expression::Expression in #9439. This is a major, backwards-incompatible change to the iOS and macOS map SDKs’ public APIs. In the case of the iOS map SDK, it’ll require a bump to version 4️⃣. 0️⃣. 0️⃣. 🙊 Since this will be a major version bump, I don’t expect us to leave the older MGLStyleValue-based API around in any form, except perhaps a migration guide.

In exchange for all the upheaval, developers will be able to set individual attributes to more complex values incorporating conditionals and arithmetic while reducing the amount of code and eliminating the longwinded class names. See the detailed rationale for this overhaul in #8074.

For example, this screenshot shows local-language labels with glosses in the system language – but only where needed. Notice how Helsinki is labeled simply “Helsinki”.

labels

Contents

This PR consists of documentation improvements, rote changes to parameter and return types all over the place, and three kinds of conversions:

  1. A conversion from NSExpression to JSON-compatible Foundation types like NSString and NSNumber (NSExpression.mgl_jsonExpressionObject)
  2. A conversion from those JSON-compatible types back to NSExpression (+[NSExpression mgl_expressionWithJSONObject:])
  3. A conversion from mbgl::style::expression::Expression to the JSON-compatible types (MGLJSONObjectFromMBGLExpression())

The three conversions are quite loosely coupled, which raises the risk of undefined behavior and poor round-tripping. The existing tests do a decent job of testing round-tripping for correct input, but not for degenerate cases. I’m eagerly awaiting #10714, which will allow us to ditch the third conversion in favor of a formal method that converts mbgl::style::expression::Expression to a string or JSON-like structure. In the meantime, I’ve added a few things to core to make the expression internals more accessible to SDK code. In driving a beeline path to a running SDK, I probably glossed over some memory management details.

Design considerations

NSExpression is a mixed bag: on the one hand, it’s familiar to Objective-C and Swift developers, being used extensively in Core Data and NSArray filtering. Developers unfamiliar with this class can find quite a bit of information on it online, most of it applicable to style attributes. On the other hand, the style specification’s expression model goes beyond NSExpression in many ways. NSExpression’s format string parser is rather inflexible, perhaps for interoperability reasons.

At various points, I considered rolling a custom Yacc or Bison parser for expression format strings. However, I think NSExpression just barely meets our needs. We’ll need to work closely with the GL JS project to ensure no fancy syntaxes emerge that become impossible to represent in NSExpression. Until we release v4.0.0, cutting over to a Bison-generated NSExpression workalike remains a possibility.

NSExpression comes in two flavors: if you use specialized initializers like +[NSExpression expressionForFunction:selectorName:arguments:], you get a modicum of type checking at the cost of brevity. By contrast, if you use the format string initializer, you shift nearly all type checking to runtime. The format string syntax is intuitive for the most common operations (in terms of general coding usage) but becomes bewildering for less common operations. Overall, once the convenience initializers are implemented, I think we’ll end up striking a sensible balance between type safety and readability, at least from the perspective of an Objective-C programmer.

The screenshot above is the result of applying this expression to the text property of various MGLSymbolStyleLayers:

layer.text = [NSExpression expressionWithFormat:@"TERNARY(name = name_en, name, FUNCTION(name, 'stringByAppendingString:', '\\n(', name_en, ')'))"];

Alternatively:

    NSExpression *name = [NSExpression expressionForKeyPath:@"name"];
    NSExpression *englishName = [NSExpression expressionForKeyPath:@"name_en"];
    NSPredicate *conditional = [NSComparisonPredicate predicateWithLeftExpression:name
                                                                  rightExpression:englishName
                                                                         modifier:NSDirectPredicateModifier
                                                                             type:NSEqualToPredicateOperatorType
                                                                          options:0];
    NSExpression *nameWithGloss = [NSExpression expressionForFunction:name
                                                         selectorName:@"stringByAppendingString:"
                                                            arguments:@[@"\\n(", englishName, @")"]];
    layer.text = [NSExpression expressionForConditional:conditional
                                         trueExpression:name
                                        falseExpression:nameWithGloss];

Progress

Some expression operators defined in the style specification are still unimplemented. I prioritized operators used in current, function-based styles, followed by operators with NSExpression equivalents. So a style that has already been migrated to expressions could break some of the getters. Conversely, some NSExpression functions, such as mode: and stddev:, can’t be implemented generically without built-in support in mbgl and the style specification. In lieu of an inline checklist, please see the enclosed “Predicates and Expressions” guide, as well as the massive table under “Expression operators” in the “Information for Style Authors” guide. The “Using Style Functions at Runtime” guide, soon to be renamed, illustrates what each kind of legacy style function looks like after a migration to expressions.

This has been a long road, and there’s quite a bit more to come:

  • Design NSExpression representation of style specification expressions
  • Convert NSExpression to JSON objects
  • Convert JSON objects to NSExpression
    • Convert every operator that has an analogue in NSExpression
    • Convert enough operators to represent all the layer properties in Mapbox Streets v10
    • Convert enough operators to replace examples in the style function guide
    • Convert every operator in the style specification – tail work
    • Expose this conversion publicly for React Native SDK
  • Convert mbgl::style::expression::Expression to JSON objects
    • Implement accessors on mbgl expression classes
    • Convert every operator that has an analogue in NSExpression
    • Convert enough operators to represent all the layer properties in Mapbox Streets v10
    • Convert enough operators to replace examples in the style function guide
    • Convert every operator in the style specification – tail work
    • Raise exceptions for edge cases instead of failing gracefully
    • Push this conversion down into mbgl for Stringify expressions #10714 – tail work
  • Expand expression coverage to the whole runtime styling API
  • Migrate example applications to expressions
    • Migrate macosapp style options
    • Migrate iosapp style options
  • Improve test coverage
    • Test conversion from NSExpression to JSON
    • Test conversion from JSON to NSExpression
    • Test native evaluation of custom functions using NSExpression
    • Test conversion from mbgl::style::expression::Expression to JSON
  • Write documentation
    • Function/operator support matrix
    • Factor out predicate overview guide (fixes Split out guide about predicates #9948)
    • Expressions overview guide
    • Method-level documentation
    • Example code in headers
    • Line-level documentation – tail work
    • Update changelogs
    • Style value migration guide – tail work
    • Rework style function overview guide – tail work
    • Update expressions in style function overview guide to account for [core] Improve typing for !=, == expressions #10861
  • Get code reviews
    • From a lucky member of the iOS team 🍀
    • From a lucky member of the core team 🎰
    • From a lucky member of the support team 🎲
  • Clean up code
    • Remove MGLStyleValue and associated code
    • Derive expression documentation from style specification JSON – tail work
    • Reorder core commits first
    • Remove heatmap-specific content
    • Remove to-do warnings and file issues for tail work

Fixes #8074 and fixes #9948.

Bloopers brackets

/cc @mapbox/ios @anandthakker @jfirebaugh @ivovandongen @lucaswoj @nitaliano

@1ec5 1ec5 added beta blocker Blocks the next beta release Core The cross-platform C++ core, aka mbgl GL JS parity For feature parity with Mapbox GL JS iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS refactor runtime styling SEMVER-MAJOR Requires a major release according to Semantic Versioning rules ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold labels Dec 18, 2017
@1ec5 1ec5 added this to the ios-v4.0.0 milestone Dec 18, 2017
@1ec5 1ec5 self-assigned this Dec 18, 2017
@@ -74,6 +74,8 @@ class CameraFunction {

bool useIntegerZoom = false;

const expression::Expression* getExpression() const { return expression.get(); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const expression::Expression& getExpression() const { return *expression; }

(x3)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Done.

@1ec5
Copy link
Contributor Author

1ec5 commented Dec 22, 2017

One of the things I’d like to improve ahead of the final release is simplying the syntax for common operations like string concatenation and interpolation.

One possibility would be to categorize the private _NSPredicateUtilities class that determines the set of built-in functions. Since _NSPredicateUtilities is declared in a private header, it isn’t possible to categorize it in the usual way, but we could do it at runtime using NSClassFromString() and class_addMethod(). The result would be fairly elegant:

NSExpression(format: "mgl_join({'foo', 'bar', 'baz'})")

Another possibility would be overload existing operators in ways that can be parsed but not evaluated by NSExpression. For example, this would be an elegant, spreadsheet-like way of concatenating two strings:

NSExpression(format: "'foo' & 'bar' & 'baz'")

This format gets parsed into nested bitwiseAnd:with: functions each with two strings as arguments. Naturally, calling -expressionValueWithObject:context: raises an exception, because strings are invalid bitwise operator arguments. Getting -expressionValueWithObject:context: to work is a low priority, because not all of the custom functions can be natively evaluated either.

The most straightforward option would be to categorize NSExpression itself with some convenience methods like -expressionForMGLFunction:selectorName:arguments: and -mgl_expressionByAppendingStrings:. These methods would sugarcoat the ugly FUNCTION() syntax. Using them in conjunction with a format string would still be inconvenient:

NSExpression(format: "TERNARY(ele > 0, %@, name)",
             NSExpression(forMGLFunction: NSExpression(forKeyPath: "name"),
                          selectorName: "stringByAppendingStrings:",
                          arguments: [" (",
                                      NSExpression(forKeyPath: "ele"),
                                      ")"]))

or:

NSExpression(format: "TERNARY(ele > 0, %@, name)",
             NSExpression(forKeyPath: "name").mgl_appendingStrings(" (",
                                                                   NSExpression(forKeyPath: "ele"),
                                                                   ")")

@1ec5
Copy link
Contributor Author

1ec5 commented Jan 4, 2018

Continuing the discussion about operator conveniences in #8074 (comment).

layer.circleColor = NSExpression(format:
"TERNARY(FUNCTION(type, 'stringValue') = 'earthquake', %@, " +
"TERNARY(FUNCTION(type, 'stringValue') = 'explosion', %@, " +
"TERNARY(FUNCTION(type, 'stringValue') = 'quarry blast', %@, " +
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The calls to stringValue will go away with #10861.

/cc @jmkiley

Implemented a conversion from mbgl::style::PropertyValues to Objective-C JSON objects, which are then converted to NSExpressions.
Extracted documentation about predicates from a documentation comment in MGLVectorStyleLayer.h to a new jazzy guide. Added details about NSExpression support as well. Began updating the “For Style Authors” guide to reflect the transition from style values to expressions.
Updated the Information for Style Authors guide to discuss expressions instead of style functions. Included a table mapping style specification expression operators to NSExpression syntaxes.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
beta blocker Blocks the next beta release Core The cross-platform C++ core, aka mbgl GL JS parity For feature parity with Mapbox GL JS iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS refactor runtime styling SEMVER-MAJOR Requires a major release according to Semantic Versioning rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Split out guide about predicates Reimplement MGLStyleValue atop NSExpression
3 participants