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

Add abs, round, floor, ceil operators #11653

Merged
merged 8 commits into from
Apr 11, 2018
Merged

Add abs, round, floor, ceil operators #11653

merged 8 commits into from
Apr 11, 2018

Conversation

anandthakker
Copy link
Contributor

@anandthakker anandthakker requested review from 1ec5 and jfirebaugh April 11, 2018 01:06
jfirebaugh
jfirebaugh approved these changes Apr 11, 2018
View reviewed changes
src/mbgl/style/expression/compound_expression.cpp Outdated
@@ -362,6 +363,11 @@ std::unordered_map<std::string, CompoundExpressionRegistry::Definition> initiali
return result;
});

define("round", [](double x) -> Result<double> { return { std::round(x) }; });
Copy link
Contributor

Choose a reason for hiding this comment

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

The extra brackets in the return expression should be unnecessary.

@jfirebaugh
Copy link
Contributor

Are we going to include SDK bindings for these in boba?

@1ec5
Copy link
Contributor

1ec5 commented Apr 11, 2018 •
edited
Loading

Are we going to include SDK bindings for these in boba?

abs, floor, and ceil all correspond to built-in NSExpression functions: abs:, floor:, and ceiling:, respectively. The iOS/macOS SDK already synthesizes these functions based on existing operators. We can easily remove the circumlocution here:

mapbox-gl-native/platform/darwin/src/NSExpression+MGLAdditions.mm

Lines 829 to 840 in 33d3c27

} else if ([function isEqualToString:@"ceiling:"]) {
return [NSExpression expressionWithFormat:@"trunc:(%@) + TERNARY(modulus:by:(%@, 1) > 0, 1, 0)",
self.arguments.firstObject, self.arguments.firstObject].mgl_jsonExpressionObject;
} else if ([function isEqualToString:@"trunc:"]) {
return [NSExpression expressionWithFormat:@"%@ - modulus:by:(%@, 1)",
self.arguments.firstObject, self.arguments.firstObject].mgl_jsonExpressionObject;
} else if ([function isEqualToString:@"abs:"]) {
return [NSExpression expressionWithFormat:@"%@ * TERNARY(%@ > 0, 1, -1)",
self.arguments.firstObject, self.arguments.firstObject].mgl_jsonExpressionObject;
} else if ([function isEqualToString:@"floor:"]) {
return [NSExpression expressionWithFormat:@"trunc:(%@) - TERNARY(modulus:by:(%@, 1) < 0, 1, 0)",
self.arguments.firstObject, self.arguments.firstObject].mgl_jsonExpressionObject;

round has no native equivalent in NSExpression, but as of #11472, all JSON expression operators are automatically representable in NSExpression via the MGL_FUNCTION() function. It’d be pretty straightforward to add a dedicated mgl_round: function as syntactic sugar.

@1ec5 1ec5 force-pushed the rounding-expressions branch from be193fa to 8aa1c1e Compare April 11, 2018 05:38
@1ec5
Copy link
Contributor

1ec5 commented Apr 11, 2018

d30fbb4 and 8aa1c1e take care of all the changes needed in the iOS and macOS map SDKs.

@tobrun
Copy link
Member

tobrun commented Apr 11, 2018

Are we going to include SDK bindings for these in boba?

I will run point on getting android integrated. We are planning one more beta release this week before doing a final next. @anandthakker thanks for adding these useful math expressions!

@tobrun tobrun mentioned this pull request Apr 11, 2018
Merged
@tobrun
Copy link
Member

tobrun commented Apr 11, 2018

Android integration in #11655 was approved and has been merged into this PR.

@anandthakker
Copy link
Contributor Author

Thanks @tobrun @1ec5 !

anandthakker
anandthakker commented Apr 11, 2018
View reviewed changes
...roid/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/layers/PropertyFactory.java
@@ -2046,7 +2046,7 @@
}

/**
* Value to use for a text label. Within literal values and zoom functions, property names enclosed in curly brackets (e.g. `{token}`) are replaced with the value of the named property. Expressions and property functions do not support this syntax; for equivalent functionality in expressions, use the [`concat`](#expressions-concat) and [`get`](#expressions-get) operators.
* Value to use for a text label.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reflects these changes: mapbox/mapbox-gl-js#6216

@anandthakker anandthakker merged commit f0f67b4 into release-boba Apr 11, 2018
@anandthakker anandthakker deleted the rounding-expressions branch April 11, 2018 16:04
@tobrun tobrun mentioned this pull request Apr 11, 2018
Closed
10 tasks
@1ec5 1ec5 mentioned this pull request Apr 11, 2018
Merged
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@1ec5 1ec5 1ec5 approved these changes

@jfirebaugh jfirebaugh jfirebaugh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@anandthakker @jfirebaugh @1ec5 @tobrun