[ios, macos] Add to-rgba expression operator. #11725

fabian-guerra · 2018-04-18T17:14:44Z

to-color
to-rgba

1ec5 · 2018-04-19T06:05:52Z

platform/darwin/docs/guides/For Style Authors.md.ejs

@@ -372,7 +372,7 @@ In style specification | Method, function, or predicate type | Format string syn
 `rgb`                  | `+[UIColor colorWithRed:green:blue:alpha:]` |
 `rgba`                 | `+[UIColor colorWithRed:green:blue:alpha:]` |
 <% } -%>
-`to-rgba`              | |
+`to-rgba`              | | `CAST(var, 'UIColor')` or `CAST(var, 'NSColor')`


Within this particular .ejs file, you can use the cocoaPrefix variable to switch between “UI” and “NS” as appropriate:

mapbox-gl-native/platform/darwin/docs/guides/For Style Authors.md.ejs

Line 5 in e122fa4

const cocoaPrefix = iOS ? 'UI' : 'NS';

We don’t have the “Predicates and Expressions” guide set up this way yet, since the platform-specific content is so minimal in that guide.

1ec5 · 2018-04-19T06:12:45Z

platform/darwin/src/NSExpression+MGLAdditions.mm

@@ -1153,6 +1161,15 @@ - (id)mgl_jsonExpressionObject {
                } else if ([type isEqualToString:@"NSNumber"]) {
                    return @[@"to-number", object];
                }
+#if TARGET_OS_IPHONE
+                else if ([type isEqualToString:@"UIColor"]) {
+                    return @[@"to-rgba", object];


I had things backwards in #11011. to-string turns its argument into a string, but oddly enough, to-rgba turns a color into an array, not the other way around. So CAST(…, 'UIColor') would more or less correspond to the rgba operator, and the to-rgba operator would correspond to CAST(…, 'NSArray'). Both coercions would be useful.

1ec5

In addition to the comments below, remember to add a blurb to the iOS and macOS changelogs about the newly supported casting types.

1ec5 · 2018-04-25T18:08:23Z

platform/darwin/docs/guides/For Style Authors.md.ejs

@@ -335,7 +335,11 @@ In style specification | Method, function, or predicate type | Format string syn
 `number`               | |
 `string`               | |
 `to-boolean`           | `boolValue` |
-`to-color`             | |
+<% if (macOS) { -%>
+`to-color`             | | `CAST(var, 'NScolor')`


Typo: NSColor, not NScolor. Also consider using the cocoaPrefix variable so you don’t have to duplicate the row with an if statement.

1ec5 · 2018-04-25T22:25:24Z

platform/darwin/src/NSExpression+MGLAdditions.mm

@@ -802,6 +802,17 @@ + (instancetype)expressionWithMGLJSONObject:(id)object {
        } else if ([op isEqualToString:@"to-string"] || [op isEqualToString:@"string"]) {
            NSExpression *operand = [NSExpression expressionWithMGLJSONObject:argumentObjects.firstObject];
            return [NSExpression expressionWithFormat:@"CAST(%@, 'NSString')", operand];
+        } else if ([op isEqualToString:@"to-color"]) {
+            NSExpression *operand = [NSExpression expressionWithMGLJSONObject:argumentObjects.firstObject];


to-color can take multiple arguments, similar to to-number. I’m not a huge fan of mgl_numberWithFallbackValues:, so I won’t request an mgl_colorWithFallbackValues:. However, if there is more than one argument to this expression, we should at least fall back to MGL_FUNCTION() instead of losing all the fallback values in translation.

1ec5 · 2018-04-25T22:28:47Z

platform/darwin/src/NSExpression+MGLAdditions.mm

+                }
+#endif
+                else if ([type isEqualToString:@"NSArray"]) {
+                    return @[@"to-rgba", object];


to-rgba is only appropriate if the object is a color. If it’s a constant UIColor or NSColor, we can use to-rgba for sure. If it’s a different kind of constant value, such as an NSString, then we should raise an exception since that kind of conversion is currently unsupported. For non-constant expressions, we don’t know the resolved type for sure, so we can use to-rgba in that case.

1ec5 · 2018-04-26T23:12:33Z

platform/darwin/src/NSExpression+MGLAdditions.mm

+                }
+#endif
+                else if ([type isEqualToString:@"NSArray"]) {
+                    return @[@"to-rgba", object];


NSExpression is a bit aggressive in evaluating a constant in a cast even during parsing, so just initializing an NSExpression with a format string of CAST(%@, 'NSArray') would crash. However, a format string of CAST(noindex(%@), 'NSArray') should convert to the expected JSON without crashing.

1ec5 · 2018-05-04T16:55:01Z

platform/darwin/test/MGLExpressionTests.mm

+        XCTAssertEqualObjects(expression.mgl_jsonExpressionObject, jsonExpression);
+    }
+    {
+        NSExpression *expression = [NSExpression expressionWithFormat:@"CAST(noindex(x), 'NSArray')"];


Developers won’t think to use noindex() for this purpose – I didn’t think of it until after much head-scratching. So let’s add a note to the “Predicates and Expressions” guide that points out the need to use noindex() when casting a constant expression to NSArray or NSColor/UIColor.

noindex() is only required when using the operand to-rgba. I am adding the note for how to cast each:

* To cast a value to a color, use `CAST(key, 'UIColor')`. * To cast a value to an rgba color, use `CAST(noindex(var), 'NSArray')`.

1ec5 · 2018-05-04T16:57:46Z

platform/ios/src/UIColor+MGLAdditions.mm

@@ -66,7 +66,7 @@ + (UIColor *)mgl_colorWithRGBComponents:(NSArray<NSExpression *> *)components {
    return [UIColor colorWithRed:[components[0].constantValue doubleValue] / 255.0
                           green:[components[1].constantValue doubleValue] / 255.0
                            blue:[components[2].constantValue doubleValue] / 255.0
-                           alpha:components.count == 3 ? [components[3].constantValue doubleValue] : 1.0];
+                           alpha:components.count == 3 ? 1.0 : [components[3].constantValue doubleValue]];


Good catch! Let’s add a mention to the changelogs that this PR fixes an issue where certain colors were being misrepresented in NSExpressions obtained from MGLStyleLayer getters.

…ions.

…changes.

1ec5 · 2018-05-05T01:38:43Z

platform/darwin/docs/guides/Predicates and Expressions.md

@@ -70,6 +70,8 @@ path or variable into a matching type:

 * To cast a value to a number, use `CAST(key, 'NSNumber')`.
 * To cast a value to a string, use `CAST(key, 'NSString')`.
+* To cast a value to a color, use `CAST(key, 'UIColor')`.
+* To cast a value to an rgba color, use `CAST(noindex(var), 'NSArray')`.


This doesn’t cast a value to a color, it casts a color to an array.

To cast an NSColor or UIColor object to an array, use CAST(noindex(color), 'NSArray').

1ec5 · 2018-05-05T01:39:21Z

platform/ios/CHANGELOG.md

@@ -22,6 +22,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
 * Reduce per-frame render CPU time ([#11811](https://github.com/mapbox/mapbox-gl-native/issues/11811))
 * Fixed a crash when removing an `MGLOfflinePack`. ([#6092](https://github.com/mapbox/mapbox-gl-native/issues/6092))
 * Fixed an issue where an `MGLOverlay` object straddling the antimeridian had an empty `MGLOverlay.overlayBounds` value. ([#11783](https://github.com/mapbox/mapbox-gl-native/pull/11783))
+* Fixed an issue where certain colors were being misrepresented in `NSExpressions` obtained from `MGLStyleLayer` getters. ([#11725](https://github.com/mapbox/mapbox-gl-native/pull/11725))


Nit: NSExpressions. (×2)

1ec5 · 2018-05-05T01:41:59Z

platform/darwin/docs/guides/Predicates and Expressions.md

@@ -70,6 +70,8 @@ path or variable into a matching type:

 * To cast a value to a number, use `CAST(key, 'NSNumber')`.
 * To cast a value to a string, use `CAST(key, 'NSString')`.
+* To cast a value to a color, use `CAST(key, 'UIColor')`.


To cast a value to a color, use CAST(key, 'UIColor') on iOS and CAST(key, 'NSColor') on macOS.

fabian-guerra added GL JS parity For feature parity with Mapbox GL JS iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS labels Apr 18, 2018

fabian-guerra added this to the ios-v4.0.0 milestone Apr 18, 2018

fabian-guerra self-assigned this Apr 18, 2018

fabian-guerra requested a review from 1ec5 April 18, 2018 17:14

1ec5 modified the milestones: ios-v4.0.0, ios-v4.0.1 Apr 18, 2018

1ec5 reviewed Apr 19, 2018

View reviewed changes

fabian-guerra force-pushed the fabian-to-color-11011 branch 3 times, most recently from 6aabfc0 to 38623c8 Compare April 24, 2018 18:40

1ec5 reviewed Apr 25, 2018

View reviewed changes

1ec5 reviewed Apr 26, 2018

View reviewed changes

fabian-guerra force-pushed the fabian-to-color-11011 branch from 80e63e8 to 40564fd Compare April 30, 2018 21:11

1ec5 reviewed May 4, 2018

View reviewed changes

fabian-guerra added 7 commits May 4, 2018 13:48

[ios, macos] Add expression support to to-rgba operator.

2e19e9d

[ios, macos] Update style docs.

8c864ee

[ios, macos] Refactored to-rgba to to-color.

c47f17d

[ios, macos] Add support for to-rgba expression operator.

a980f9c

[ios, macos] Add multiple parameters support to to-color operand.

73fe7ac

[ios, macos] Enable to-rgba operator for MGLColor or key path express…

40164ed

…ions.

[ios, macos] Update predicates and expressions guide to reflect cast …

f8ad4fe

…changes.

fabian-guerra force-pushed the fabian-to-color-11011 branch from 40564fd to 78cd0b2 Compare May 4, 2018 17:52

1ec5 approved these changes May 5, 2018

View reviewed changes

1ec5 reviewed May 5, 2018

View reviewed changes

fabian-guerra added 2 commits May 7, 2018 11:20

[ios, macos] Update changelogs.

5a55a29

[ios, macos] Clarify color casting usage.

1f8d266

fabian-guerra force-pushed the fabian-to-color-11011 branch from 78cd0b2 to 1f8d266 Compare May 7, 2018 15:29

fabian-guerra merged commit a425995 into release-boba May 7, 2018

fabian-guerra deleted the fabian-to-color-11011 branch May 7, 2018 16:01

fabian-guerra mentioned this pull request May 7, 2018

Add NSExpression syntax for coercing array to color #11011

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ios, macos] Add to-rgba expression operator. #11725

[ios, macos] Add to-rgba expression operator. #11725

fabian-guerra commented Apr 18, 2018 •

edited

Loading

1ec5 Apr 19, 2018

1ec5 Apr 19, 2018

1ec5 left a comment

1ec5 Apr 25, 2018

1ec5 Apr 25, 2018

1ec5 Apr 25, 2018

1ec5 Apr 26, 2018 •

edited

Loading

1ec5 May 4, 2018

fabian-guerra May 4, 2018

1ec5 May 4, 2018

1ec5 May 5, 2018 •

edited

Loading

1ec5 May 5, 2018

1ec5 May 5, 2018

[ios, macos] Add to-rgba expression operator. #11725

[ios, macos] Add to-rgba expression operator. #11725

Conversation

fabian-guerra commented Apr 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 Apr 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 May 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabian-guerra commented Apr 18, 2018 •

edited

Loading

1ec5 Apr 26, 2018 •

edited

Loading

1ec5 May 5, 2018 •

edited

Loading