MGLStyleLayer predicates (filters) #6049

frederoni · 2016-08-17T11:46:28Z

1ec5 · 2016-08-17T19:49:59Z

platform/darwin/src/MGLBaseStyleLayer.h

+/**
+ Updates the layer’s layout and paint properties.
+ */
+- (void)update;


We made this method private in #6018. Any reason you’re reintroducing this method in the public interface?

No reason. It was in my git stash from last week. I'll remove it.

1ec5 · 2016-08-17T19:54:08Z

To convert from NSPredicate to the various mbgl::style::Filter classes, we should use a Visitor pattern like the evaluators in MGLFeature.mm. Whereas PropertyValueEvaluator converts a recursive (arbitrarily nested) C++ structure into an Objective-C structure, we want to convert a recursive Objective-C structure into a C++ structure. We can nonetheless rely on operator overloading in C++ to keep things clean without a series of if/else-if statements.

1ec5 · 2016-08-23T17:16:01Z

platform/darwin/src/NSPredicate+MGLAdditions.mm

+class FilterEvaluator {
+public:
+
+    mbgl::Value getValue(id obj) {


Instead of a catch-all getValue() method, use the Visitor pattern that we’re already inside. Define operators for NSString and NSNumber in a FilterExpressionEvaluator, then apply the FilterExpressionEvaluator to each constantValue you encounter.

1ec5 · 2016-08-23T17:30:21Z

Also make sure to test some edge cases where you may need to escape the format string or use format string specifiers:

Keys with hyphens are converted correctly
$type and $id keys ([core] Add support for $id key to filters #5708) are converted correctly

1ec5 · 2016-08-23T17:48:42Z

platform/darwin/src/NSPredicate+MGLAdditions.mm

+
+    NSComparisonPredicate *predicate = (NSComparisonPredicate *)self;
+    FilterEvaluator evaluator;
+    auto filter = evaluator(predicate);


A C++ evaluator may not be a great tool for converting from NSPredicate to mbgl::style::Filter after all. We generally use this pattern with mapbox::util::variant, not an Objective-C umbrella class. Instead, let’s make this method do nothing but assert, then implement -mgl_filter overrides in categories on NSComparisonPredicate and NSCompoundPredicate.

The logic for converting a key or value to an mbgl::Value should live in a category on NSExpression.

A C++ evaluator remains a good idea for efficiently and compactly converting an mbgl::style::Filter into an NSPredicate. We could have the evaluator call into category initializers on NSComparisonPredicate, NSCompoundPredicate, and NSExpression, if that would make the code easier to understand.

1ec5 · 2016-08-24T15:52:19Z

platform/darwin/src/NSPredicate+MGLAdditions.h

+public:
+
+mbgl::style::Filter operator()(NSComparisonPredicate *predicate) {
+    switch (predicate.predicateOperatorType) {


The entire FilterEvaluator definition shouldn't be inlined in the header. Declare the operators here but define them in the implementation file.

1ec5 · 2016-08-29T17:33:47Z

platform/darwin/test/MGLFilterTests.m

+{
+    NSPredicate *equalPredicate = [NSPredicate predicateWithFormat:@"type == 'neighbourhood'"];
+    NSPredicate *notEqualPredicate = [NSPredicate predicateWithFormat:@"type != 'park'"];
+    NSPredicate *greaterThanPredicate = [NSPredicate predicateWithFormat:@"%K > %@", @"stroke-width", @2.1];


Nit: it should also be possible to inline the 2.1 into the format string.

1ec5 · 2016-08-29T17:39:07Z

platform/darwin/src/MGLCircleStyleLayer.h

+
+ The predicate's left expression must be a string that identifies a feature
+ property, or one of the following special keys:
+    "$type" - identifies the feature by type.


I don’t think this will look like a list in jazzy. We should either insert our own Doxygen or HTML formatting or override this documentation.

boundsj · 2016-08-31T19:32:29Z

platform/darwin/src/NSExpression+MGLAdditions.mm

+            return { number.intValue };
+        } else if ((strcmp([number objCType], @encode(double))) == 0) {
+            return { number.doubleValue };
+        } else if ((strcmp([number objCType], @encode(bool))) == 0) {


@frederoni I expect to be able to write something like [NSPredicate predicateWithFormat:@"%K != %@", @"cluster", [NSNumber numberWithBool:YES]]] however this is not caught by this implementation (ref: http://stackoverflow.com/questions/23425187/nsnumber-with-bool-is-no-bool-on-64bit). Using else if ((strcmp([number objCType], @encode(char))) == 0) worked for me.

Did it raise an exception or returned mbgl::Value with incorrect type? Feel free to push the fix.

It triggered the exception since it was not caught in the previous conditions. Fixed here eefe5cb

boundsj · 2016-09-02T19:17:06Z

👍

1ec5 · 2016-09-02T19:47:18Z

platform/darwin/src/MGLFillStyleLayer.h

+ A predicate that corresponds to the layer's <a href='https://www.mapbox.com/mapbox-gl-style-spec/#types-filter'>filter</a>.
+
+ The predicate's left expression must be a string that identifies a feature
+ property, or one of the special keys.


Possibly tail work, but we need to document inline which NSExpression operator types we support, as well as the valid values of $type and $id. Linking to the style specification will suffice for now, but it’s suboptimal because the JavaScript syntax and terminology is very foreign to the NSPredicate-based interface we ended up exposing.

Oh, I’m seeing now that this documentation was originally inline but was removed in response to #6049 (comment). I meant that we should describe the $type and $id keys but do so in an HTML definition list.

[ios, macos] cleaned up filters [ios] added a filter example [ios] utest filters [ios, macos] nested predicates [ios] refactored [ios] filter -> NSPredicate [ios] fixed mbgl::Any/AllFilter -> NSPredicate [ios] translate nested mbgl::NotIn filters [ios] cleanup and added more utests [ios] fixed a bug in the None filter conversion and improved utests [ios, macos] doc [macos] added missing category [ios, macos] additional utests [ios, macos] updated documentation

We convert NSNumbers (and NSStrings) to the appropriate mbgl value so that we can use NSPredicates to describe mbgl filters we want to apply to style layers at runtime. This change fixes an issue where the conversion from an NSNumber that represented a bool was not recognized as such. encode(bool) returns a 'c' or 'b' on 32 bit and 64 bit systems respectively and objCType of an NSNumber representing a bool always returns 'c'. Now the implementation checks for 'c' always and NSNumbers representing bool don't fall through and trigger the exception.

1ec5 · 2016-09-02T21:11:06Z

platform/darwin/src/NSPredicate+MGLAdditions.mm

+    }
+
+    NSPredicate* operator()(mbgl::style::HasFilter filter) {
+        [NSException raise:@"Unsupported filter type"


This should be translated as key != nil.

1ec5 · 2016-09-02T21:28:25Z

platform/darwin/test/MGLFilterTests.mm

+    NSPredicate *typePredicate = [NSPredicate predicateWithFormat:@"%K == %@", @"$type", @"Feature"];
+    NSPredicate *idPredicate = [NSPredicate predicateWithFormat:@"%K == %@", @"$id", @"1234123"];
+    NSPredicate *specialCharsPredicate = [NSPredicate predicateWithFormat:@"%K == %@", @"ty-’pè", @"sŒm-ethįng"];
+    NSPredicate *booleanPredicate = [NSPredicate predicateWithFormat:@"%K != %@", @"cluster", [NSNumber numberWithBool:YES]];


This can be inlined as %K != YES.

The PR to add filter predicates (#6049) added logic to apply some runtime styling to the iosapp as soon as the map finished loading. This removes that logic.

frederoni added iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold beta blocker Blocks the next beta release runtime styling labels Aug 17, 2016

frederoni assigned incanus and frederoni Aug 17, 2016

frederoni force-pushed the 5975-wip-predicate-filter branch 3 times, most recently from d903e62 to 5b00e90 Compare August 17, 2016 13:58

1ec5 reviewed Aug 17, 2016
View reviewed changes

frederoni force-pushed the 5975-wip-predicate-filter branch 2 times, most recently from 9dfaab0 to ee2f594 Compare August 18, 2016 13:25

1ec5 added this to the ios-v3.4.0 milestone Aug 18, 2016

frederoni force-pushed the 5975-wip-predicate-filter branch 3 times, most recently from a51d3cd to 7416aa0 Compare August 23, 2016 15:07

1ec5 reviewed Aug 23, 2016
View reviewed changes

frederoni changed the title ~~[ios] outlined filter conversion~~ [ios, macos] outlined filter conversion Aug 24, 2016

1ec5 reviewed Aug 24, 2016
View reviewed changes

1ec5 self-assigned this Aug 25, 2016

frederoni force-pushed the 5975-wip-predicate-filter branch 2 times, most recently from 35bfa86 to 4e96dc7 Compare August 29, 2016 16:45

1ec5 changed the title ~~[ios, macos] outlined filter conversion~~ MGLStyleLayer predicates Aug 29, 2016

1ec5 changed the title ~~MGLStyleLayer predicates~~ MGLStyleLayer predicates (filters) Aug 29, 2016

1ec5 reviewed Aug 29, 2016
View reviewed changes

frederoni force-pushed the 5975-wip-predicate-filter branch from 4e96dc7 to 3c346b1 Compare August 29, 2016 17:37

1ec5 reviewed Aug 29, 2016
View reviewed changes

frederoni force-pushed the 5975-wip-predicate-filter branch 3 times, most recently from 6e0ecba to e07c2c4 Compare August 31, 2016 15:57

frederoni added ✓ ready for review and removed ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold labels Aug 31, 2016

boundsj reviewed Aug 31, 2016
View reviewed changes

1ec5 mentioned this pull request Sep 2, 2016

[ios, macos] fixes #5962 added geojson options to support clustering #6217

Merged

2 tasks

1ec5 reviewed Sep 2, 2016
View reviewed changes

frederoni and others added 2 commits September 2, 2016 22:42

frederoni force-pushed the 5975-wip-predicate-filter branch from eefe5cb to 7e208c4 Compare September 2, 2016 20:46

1ec5 reviewed Sep 2, 2016
View reviewed changes

1ec5 mentioned this pull request Sep 2, 2016

Translate more NSComparisonPredicate operators to filters #6244

Closed

6 tasks

1ec5 reviewed Sep 2, 2016
View reviewed changes

boundsj merged commit 7e208c4 into master Sep 2, 2016

boundsj mentioned this pull request Sep 2, 2016

Add predicate property to MGLStyleLayer #5973

Closed

1ec5 removed the ✓ ready for review label Sep 2, 2016

boundsj added a commit that referenced this pull request Sep 2, 2016

[ios] Remove iosapp test stub

52f2361

The PR to add filter predicates (#6049) added logic to apply some runtime styling to the iosapp as soon as the map finished loading. This removes that logic.

jfirebaugh deleted the 5975-wip-predicate-filter branch September 16, 2016 18:01

1ec5 mentioned this pull request Feb 25, 2022

Only unbox NSNumbers 0, 1 to Bool for objCType 'c' mapbox/turf-swift#181

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MGLStyleLayer predicates (filters) #6049

MGLStyleLayer predicates (filters) #6049

frederoni commented Aug 17, 2016 •

edited

Loading

1ec5 Aug 17, 2016

frederoni Aug 17, 2016

1ec5 commented Aug 17, 2016 •

edited

Loading

1ec5 Aug 23, 2016

1ec5 commented Aug 23, 2016 •

edited by frederoni

Loading

1ec5 Aug 23, 2016

1ec5 Aug 24, 2016

1ec5 Aug 29, 2016

1ec5 Aug 29, 2016

boundsj Aug 31, 2016

frederoni Sep 1, 2016

boundsj Sep 1, 2016

boundsj commented Sep 2, 2016

1ec5 Sep 2, 2016

1ec5 Sep 2, 2016

1ec5 Sep 2, 2016

1ec5 Sep 2, 2016

MGLStyleLayer predicates (filters) #6049

MGLStyleLayer predicates (filters) #6049

Conversation

frederoni commented Aug 17, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 commented Aug 17, 2016 • edited Loading

Choose a reason for hiding this comment

1ec5 commented Aug 23, 2016 • edited by frederoni 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boundsj commented Sep 2, 2016

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

frederoni commented Aug 17, 2016 •

edited

Loading

1ec5 commented Aug 17, 2016 •

edited

Loading

1ec5 commented Aug 23, 2016 •

edited by frederoni

Loading