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

[core] Implement 'in' expression. #16162

Merged
merged 8 commits into from
Feb 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,13 @@

The `within expression` enables checking whether a feature is inside a pre-defined geometry set/boundary or not. This `within expression` returns a boolean value, `true` indicates that the feature being evaluated is inside the geometry set. The returned value can be then consumed as input by another expression or used directly by a paint/layer property.

Support for using `within expression` with layout propery will be implemented separately.
Support for using `within expression` with layout property will be implemented separately.

- [core] Add support for using `within expression` with layout propery. ([#16194](https://github.com/mapbox/mapbox-gl-native/pull/16194))
- [core] Add support for using `within expression` with layout property. ([#16194](https://github.com/mapbox/mapbox-gl-native/pull/16194))
Chaoba marked this conversation as resolved.
Show resolved Hide resolved

- [core] Add support for `in expression`. ([#16162](https://github.com/mapbox/mapbox-gl-native/pull/16162))

The `in expression` enables checking whether a Number/String/Boolean type item is in a String/Array and returns a boolean value.

### 🐞 Bug fixes

Expand Down
2 changes: 2 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ add_library(
${PROJECT_SOURCE_DIR}/include/mbgl/style/expression/get_covering_stops.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/style/expression/image.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/style/expression/image_expression.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/style/expression/in.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/style/expression/interpolate.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/style/expression/interpolator.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/style/expression/is_constant.hpp
Expand Down Expand Up @@ -586,6 +587,7 @@ add_library(
${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/get_covering_stops.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/image.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/image_expression.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/in.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/interpolate.cpp
Chaoba marked this conversation as resolved.
Show resolved Hide resolved
${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/is_constant.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/is_expression.cpp
Expand Down
1 change: 1 addition & 0 deletions include/mbgl/style/expression/expression.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ enum class Kind : int32_t {
FormatSectionOverride,
NumberFormat,
ImageExpression,
In,
Within
};

Expand Down
33 changes: 33 additions & 0 deletions include/mbgl/style/expression/in.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#pragma once

#include <mbgl/style/conversion.hpp>
#include <mbgl/style/expression/expression.hpp>
#include <memory>

namespace mbgl {
namespace style {
namespace expression {

class In final : public Expression {
public:
In(std::unique_ptr<Expression> needle_, std::unique_ptr<Expression> haystack_);

static ParseResult parse(const mbgl::style::conversion::Convertible& value, ParsingContext& ctx);

EvaluationResult evaluate(const EvaluationContext& params) const override;
void eachChild(const std::function<void(const Expression&)>&) const override;

bool operator==(const Expression& e) const override;

std::vector<optional<Value>> possibleOutputs() const override;

std::string getOperator() const override;

private:
std::unique_ptr<Expression> needle;
std::unique_ptr<Expression> haystack;
};

} // namespace expression
} // namespace style
} // namespace mbgl
2 changes: 1 addition & 1 deletion mapbox-gl-js
Submodule mapbox-gl-js updated 41 files
+26 −4 .circleci/config.yml
+3 −1 package.json
+102 −0 src/geo/edge_insets.js
+67 −9 src/geo/transform.js
+50 −0 src/render/draw_debug.js
+6 −1 src/render/painter.js
+1 −1 src/style-spec/expression/definitions/in.js
+1 −1 src/style-spec/package.json
+74 −25 src/ui/camera.js
+12 −1 src/ui/control/geolocate_control.js
+1 −1 src/ui/control/scale_control.js
+19 −1 src/ui/map.js
+1 −1 src/ui/marker.js
+1 −1 src/util/mapbox.js
+4 −0 test/README.md
+18 −0 test/browser/README.md
+29 −0 test/browser/drag.test.js
+58 −0 test/browser/fixtures/land.html
+1 −0 test/browser/fixtures/land.json
+118 −0 test/browser/util/browser.js
+30 −0 test/browser/util/doubleclick.js
+45 −0 test/browser/util/mousewheel.js
+21 −0 test/browser/zoom.test.js
+1 −1 test/integration/expression-tests/in/basic-string/test.json
+ test/integration/render-tests/debug/padding/ease-to-btm-distort/expected.png
+112 −0 test/integration/render-tests/debug/padding/ease-to-btm-distort/style.json
+ test/integration/render-tests/debug/padding/ease-to-left-distort/expected.png
+112 −0 test/integration/render-tests/debug/padding/ease-to-left-distort/style.json
+ test/integration/render-tests/debug/padding/ease-to-no-distort/expected.png
+127 −0 test/integration/render-tests/debug/padding/ease-to-no-distort/style.json
+ test/integration/render-tests/debug/padding/ease-to-right-distort/expected.png
+112 −0 test/integration/render-tests/debug/padding/ease-to-right-distort/style.json
+ test/integration/render-tests/debug/padding/ease-to-top-distort/expected.png
+112 −0 test/integration/render-tests/debug/padding/ease-to-top-distort/style.json
+ test/integration/render-tests/debug/padding/set-padding/expected.png
+82 −0 test/integration/render-tests/debug/padding/set-padding/style.json
+1 −0 test/suite_implementation.js
+99 −0 test/unit/geo/edge_insets.test.js
+39 −0 test/unit/ui/camera.test.js
+11 −0 test/unit/ui/control/geolocate.test.js
+60 −3 yarn.lock
6 changes: 3 additions & 3 deletions metrics/binary-size/macos-xcode11/metrics.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,17 @@
[
"mbgl-glfw",
"/tmp/attach/install/macos-xcode11-release/bin/mbgl-glfw",
5616056
5677792
],
[
"mbgl-offline",
"/tmp/attach/install/macos-xcode11-release/bin/mbgl-offline",
5484604
5542252
],
[
"mbgl-render",
"/tmp/attach/install/macos-xcode11-release/bin/mbgl-render",
5530720
5588368
]
]
}
12 changes: 6 additions & 6 deletions metrics/ignores/platform-all.json
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
{
"expression-tests/collator/accent-equals-de": "Locale-specific behavior changes based on platform.",
"expression-tests/in/assert-array": "https://github.com/mapbox/mapbox-gl-native/issues/15893",
"expression-tests/in/assert-string": "https://github.com/mapbox/mapbox-gl-native/issues/15893",
"expression-tests/in/basic-array": "https://github.com/mapbox/mapbox-gl-native/issues/15893",
"expression-tests/in/basic-string": "https://github.com/mapbox/mapbox-gl-native/issues/15893",
"expression-tests/in/invalid-haystack": "https://github.com/mapbox/mapbox-gl-native/issues/15893",
"expression-tests/in/invalid-needle": "https://github.com/mapbox/mapbox-gl-native/issues/15893",
"expression-tests/interpolate-hcl/linear": "https://github.com/mapbox/mapbox-gl-native/issues/8720",
"expression-tests/interpolate-lab/linear": "https://github.com/mapbox/mapbox-gl-native/issues/8720",
"expression-tests/is-supported-script/default": "This tests RTL text plugin behavior specific to GL JS",
Expand Down Expand Up @@ -68,6 +62,12 @@
"render-tests/custom-layer-js/tent-3d": "skip - js specific",
"render-tests/debug/collision": "https://github.com/mapbox/mapbox-gl-native/issues/3841",
"render-tests/debug/overdraw": "https://github.com/mapbox/mapbox-gl-native/issues/15638",
"render-tests/debug/padding/ease-to-btm-distort": "https://github.com/mapbox/mapbox-gl-native/issues/16212",
"render-tests/debug/padding/ease-to-left-distort": "https://github.com/mapbox/mapbox-gl-native/issues/16212",
"render-tests/debug/padding/ease-to-no-distort": "https://github.com/mapbox/mapbox-gl-native/issues/16212",
"render-tests/debug/padding/ease-to-right-distort": "https://github.com/mapbox/mapbox-gl-native/issues/16212",
"render-tests/debug/padding/ease-to-top-distort": "https://github.com/mapbox/mapbox-gl-native/issues/16212",
"render-tests/debug/padding/set-padding": "https://github.com/mapbox/mapbox-gl-native/issues/16212",
"render-tests/debug/raster": "https://github.com/mapbox/mapbox-gl-native/issues/15510",
"render-tests/debug/tile": "https://github.com/mapbox/mapbox-gl-native/issues/3841",
"render-tests/debug/tile-overscaled": "https://github.com/mapbox/mapbox-gl-native/issues/3841",
Expand Down
130 changes: 130 additions & 0 deletions src/mbgl/style/expression/in.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
#include <string.h>
#include <mbgl/style/conversion_impl.hpp>
#include <mbgl/style/expression/in.hpp>
#include <mbgl/style/expression/type.hpp>
#include <mbgl/util/string.hpp>

namespace mbgl {
namespace style {
namespace expression {

namespace {
bool isComparableType(type::Type type) {
return type == type::Boolean || type == type::String || type == type::Number || type == type::Null ||
type == type::Value;
}

bool isComparableRuntimeType(type::Type type) {
return type == type::Boolean || type == type::String || type == type::Number || type == type::Null;
}

bool isSearchableType(type::Type type) {
return type == type::String || type.is<type::Array>() || type == type::Null || type == type::Value;
}

bool isSearchableRuntimeType(type::Type type) {
return type == type::String || type.is<type::Array>() || type == type::Null;
}
} // namespace

In::In(std::unique_ptr<Expression> needle_, std::unique_ptr<Expression> haystack_)
: Expression(Kind::In, type::Boolean), needle(std::move(needle_)), haystack(std::move(haystack_)) {
assert(isComparableType(needle->getType()));
assert(isSearchableType(haystack->getType()));
}

EvaluationResult In::evaluate(const EvaluationContext& params) const {
const EvaluationResult evaluatedHaystack = haystack->evaluate(params);
Chaoba marked this conversation as resolved.
Show resolved Hide resolved
if (!evaluatedHaystack) {
return evaluatedHaystack.error();
}

const EvaluationResult evaluatedNeedle = needle->evaluate(params);
if (!evaluatedNeedle) {
return evaluatedNeedle.error();
}

type::Type evaluatedNeedleType = typeOf(*evaluatedNeedle);
if (!isComparableRuntimeType(evaluatedNeedleType)) {
return EvaluationError{"Expected first argument to be of type boolean, string or number, but found " +
toString(evaluatedNeedleType) + " instead."};
}

type::Type evaluatedHaystackType = typeOf(*evaluatedHaystack);
if (!isSearchableRuntimeType(evaluatedHaystackType)) {
return EvaluationError{"Expected second argument to be of type array or string, but found " +
toString(evaluatedHaystackType) + " instead."};
}

if (evaluatedNeedleType == type::Null || evaluatedHaystackType == type::Null) {
return EvaluationResult(false);
}

if (evaluatedHaystackType == type::String) {
const auto haystackString = evaluatedHaystack->get<std::string>();
const auto needleString = toString(*evaluatedNeedle);
return EvaluationResult(haystackString.find(needleString) != std::string::npos);
} else {
const auto haystackArray = evaluatedHaystack->get<std::vector<Value>>();
return EvaluationResult(std::find(haystackArray.begin(), haystackArray.end(), *evaluatedNeedle) !=
haystackArray.end());
}
}

void In::eachChild(const std::function<void(const Expression&)>& visit) const {
visit(*needle);
visit(*haystack);
}

using namespace mbgl::style::conversion;
ParseResult In::parse(const Convertible& value, ParsingContext& ctx) {
assert(isArray(value));

std::size_t length = arrayLength(value);
if (length != 3) {
ctx.error("Expected 2 arguments, but found " + util::toString(length - 1) + " instead.");
return ParseResult();
}

ParseResult needle = ctx.parse(arrayMember(value, 1), 1, {type::Value});
Chaoba marked this conversation as resolved.
Show resolved Hide resolved
if (!needle) return ParseResult();

ParseResult haystack = ctx.parse(arrayMember(value, 2), 2, {type::Value});
if (!haystack) return ParseResult();

type::Type needleType = (*needle)->getType();
type::Type haystackType = (*haystack)->getType();
Chaoba marked this conversation as resolved.
Show resolved Hide resolved

if (!isComparableType(needleType)) {
ctx.error("Expected first argument to be of type boolean, string or number, but found " + toString(needleType) +
" instead.");
return ParseResult();
}

if (!isSearchableType(haystackType)) {
ctx.error("Expected second argument to be of type array or string, but found " + toString(haystackType) +
" instead.");
return ParseResult();
}
return ParseResult(std::make_unique<In>(std::move(*needle), std::move(*haystack)));
}

bool In::operator==(const Expression& e) const {
if (e.getKind() == Kind::In) {
auto rhs = static_cast<const In*>(&e);
return *needle == *(rhs->needle) && *haystack == *(rhs->haystack);
}
return false;
}

std::vector<optional<Value>> In::possibleOutputs() const {
return {{true}, {false}};
}

std::string In::getOperator() const {
return "in";
}

} // namespace expression
} // namespace style
} // namespace mbgl
2 changes: 2 additions & 0 deletions src/mbgl/style/expression/parsing_context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <mbgl/style/expression/expression.hpp>
#include <mbgl/style/expression/format_expression.hpp>
#include <mbgl/style/expression/image_expression.hpp>
#include <mbgl/style/expression/in.hpp>
#include <mbgl/style/expression/interpolate.hpp>
#include <mbgl/style/expression/length.hpp>
#include <mbgl/style/expression/let.hpp>
Expand Down Expand Up @@ -114,6 +115,7 @@ MAPBOX_ETERNAL_CONSTEXPR const auto expressionRegistry =
{"any", Any::parse},
{"array", Assertion::parse},
{"at", At::parse},
{"in", In::parse},
{"boolean", Assertion::parse},
{"case", Case::parse},
{"coalesce", Coalesce::parse},
Expand Down
20 changes: 20 additions & 0 deletions test/fixtures/expression_equality/in.a.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
[
"number",
[
"in",
[
"number",
[
"get",
"i"
]
],
[
"array",
[
"get",
"arr"
]
]
]
]
20 changes: 20 additions & 0 deletions test/fixtures/expression_equality/in.b.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
[
"number",
[
"in",
[
"number",
[
"get",
"i"
]
],
[
"array",
[
"get",
"arr_other"
]
]
]
]
3 changes: 1 addition & 2 deletions test/style/expression/expression.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ TEST(Expression, IsExpression) {

// TODO: "interpolate-hcl": https://github.com/mapbox/mapbox-gl-native/issues/8720
// TODO: "interpolate-lab": https://github.com/mapbox/mapbox-gl-native/issues/8720
// TODO: "in": https://github.com/mapbox/mapbox-gl-native/issues/15893
if (name == "interpolate-hcl" || name == "interpolate-lab" || name == "in") {
if (name == "interpolate-hcl" || name == "interpolate-lab") {
if (expression::isExpression(conversion::Convertible(expression))) {
ASSERT_TRUE(false) << "Expression name" << name << "is implemented - please update Expression.IsExpression test.";
}
Expand Down