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

Enable Within Expression with paint property + Filter Expression #16157

Merged
merged 7 commits into from
Feb 12, 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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@

The new `Source::setPrefetchZoomDelta(optional<uint8_t>)` method allows overriding default tile prefetch setting that is defined by the Map instance.

- [core] Add support for `within expression`. Implement the use of `within expression` with paint propery and filter expression. ([#16157](https://github.com/mapbox/mapbox-gl-native/pull/16157))

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.

### 🏁 Performance improvements

- [core] Loading images to style optimization ([#16187](https://github.com/mapbox/mapbox-gl-native/pull/16187))
Expand Down
2 changes: 2 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ add_library(
${PROJECT_SOURCE_DIR}/include/mbgl/style/expression/step.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/style/expression/type.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/style/expression/value.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/style/expression/within.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/style/filter.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/style/image.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/style/layer.hpp
Expand Down Expand Up @@ -578,6 +579,7 @@ add_library(
${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/util.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/util.hpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/value.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/within.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/filter.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/image.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/image_impl.cpp
Expand Down
29 changes: 25 additions & 4 deletions expression-test/expression_test_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

#include <args.hxx>

#include <regex>

using namespace mbgl;
using namespace mbgl::style;
using namespace mbgl::style::conversion;
Expand Down Expand Up @@ -254,6 +256,18 @@ bool parseInputs(const JSValue& inputsValue, TestData& data) {
heatmapDensity = evaluationContext["heatmapDensity"].GetDouble();
}

// Parse canonicalID
optional<CanonicalTileID> canonical;
if (evaluationContext.HasMember("canonicalID")) {
const auto& canonicalIDObject = evaluationContext["canonicalID"];
assert(canonicalIDObject.IsObject());
assert(canonicalIDObject.HasMember("z") && canonicalIDObject["z"].IsNumber());
assert(canonicalIDObject.HasMember("x") && canonicalIDObject["x"].IsNumber());
assert(canonicalIDObject.HasMember("y") && canonicalIDObject["y"].IsNumber());
canonical = CanonicalTileID(
canonicalIDObject["z"].GetUint(), canonicalIDObject["x"].GetUint(), canonicalIDObject["y"].GetUint());
}

// Parse availableImages
std::set<std::string> availableImages;
if (evaluationContext.HasMember("availableImages")) {
Expand Down Expand Up @@ -282,8 +296,11 @@ bool parseInputs(const JSValue& inputsValue, TestData& data) {
feature.id = mapbox::geojson::convert<mapbox::feature::identifier>(featureObject["id"]);
}

data.inputs.emplace_back(
std::move(zoom), std::move(heatmapDensity), std::move(availableImages), std::move(feature));
data.inputs.emplace_back(std::move(zoom),
std::move(heatmapDensity),
std::move(canonical),
std::move(availableImages),
std::move(feature));
}
return true;
}
Expand All @@ -294,11 +311,11 @@ std::tuple<filesystem::path, std::vector<filesystem::path>, bool, uint32_t> pars
args::ArgumentParser argumentParser("Mapbox GL Expression Test Runner");

args::HelpFlag helpFlag(argumentParser, "help", "Display this help menu", { 'h', "help" });
args::Flag shuffleFlag(argumentParser, "shuffle", "Toggle shuffling the tests order",
{ 's', "shuffle" });
args::Flag shuffleFlag(argumentParser, "shuffle", "Toggle shuffling the tests order", {'s', "shuffle"});
args::ValueFlag<uint32_t> seedValue(argumentParser, "seed", "Shuffle seed (default: random)",
{ "seed" });
args::PositionalList<std::string> testNameValues(argumentParser, "URL", "Test name(s)");
args::ValueFlag<std::string> testFilterValue(argumentParser, "filter", "Test filter regex", {'f', "filter"});

try {
argumentParser.ParseCLI(argc, argv);
Expand Down Expand Up @@ -336,6 +353,7 @@ std::tuple<filesystem::path, std::vector<filesystem::path>, bool, uint32_t> pars
paths.emplace_back(rootPath);
}

auto testFilter = testFilterValue ? args::get(testFilterValue) : std::string{};
// Recursively traverse through the test paths and collect test directories containing "test.json".
std::vector<filesystem::path> testPaths;
testPaths.reserve(paths.size());
Expand All @@ -346,6 +364,9 @@ std::tuple<filesystem::path, std::vector<filesystem::path>, bool, uint32_t> pars
}

for (auto& testPath : filesystem::recursive_directory_iterator(path)) {
if (!testFilter.empty() && !std::regex_search(testPath.path().string(), std::regex(testFilter))) {
continue;
}
if (testPath.path().filename() == "test.json") {
testPaths.emplace_back(testPath.path());
}
Expand Down
3 changes: 3 additions & 0 deletions expression-test/expression_test_parser.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,17 @@ using namespace mbgl;
struct Input {
Input(optional<float> zoom_,
optional<double> heatmapDensity_,
optional<CanonicalTileID> canonical_,
std::set<std::string> availableImages_,
Feature feature_)
: zoom(std::move(zoom_)),
heatmapDensity(std::move(heatmapDensity_)),
canonical(std::move(canonical_)),
availableImages(std::move(availableImages_)),
feature(std::move(feature_)) {}
optional<float> zoom;
optional<double> heatmapDensity;
optional<CanonicalTileID> canonical;
std::set<std::string> availableImages;
Feature feature;
};
Expand Down
10 changes: 8 additions & 2 deletions expression-test/expression_test_runner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,14 @@ TestRunOutput runExpressionTest(TestData& data, const std::string& rootPath, con
std::vector<Value> outputs;
if (!data.inputs.empty()) {
for (const auto& input : data.inputs) {
auto evaluationResult =
expression->evaluate(input.zoom, input.feature, input.heatmapDensity, input.availableImages);
mbgl::style::expression::EvaluationResult evaluationResult;
if (input.canonical) {
evaluationResult = expression->evaluate(
input.zoom, input.feature, input.heatmapDensity, input.availableImages, *input.canonical);
} else {
evaluationResult =
expression->evaluate(input.zoom, input.feature, input.heatmapDensity, input.availableImages);
}
if (!evaluationResult) {
std::unordered_map<std::string, Value> error{{"error", Value{evaluationResult.error().message}}};
outputs.emplace_back(Value{std::move(error)});
Expand Down
25 changes: 19 additions & 6 deletions include/mbgl/style/expression/expression.hpp
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
#pragma once

#include <mbgl/util/optional.hpp>
#include <mbgl/util/variant.hpp>
#include <mbgl/util/color.hpp>
#include <mbgl/style/expression/parsing_context.hpp>
#include <mbgl/style/expression/type.hpp>
#include <mbgl/style/expression/value.hpp>
#include <mbgl/style/expression/parsing_context.hpp>
#include <mbgl/tile/tile_id.hpp>
#include <mbgl/util/color.hpp>
#include <mbgl/util/optional.hpp>
#include <mbgl/util/variant.hpp>

#include <array>
#include <vector>
Expand Down Expand Up @@ -53,14 +54,20 @@ class EvaluationContext {
return *this;
};

EvaluationContext& withCanonicalTileID(const mbgl::CanonicalTileID* canonical_) noexcept {
canonical = canonical_;
return *this;
};

optional<float> zoom;
optional<mbgl::Value> accumulated;
GeometryTileFeature const * feature = nullptr;
GeometryTileFeature const* feature = nullptr;
optional<double> colorRampParameter;
// Contains formatted section object, std::unordered_map<std::string, Value>.
const Value* formattedSection = nullptr;
const FeatureState* featureState = nullptr;
const std::set<std::string>* availableImages = nullptr;
const mbgl::CanonicalTileID* canonical = nullptr;
};

template <typename T>
Expand Down Expand Up @@ -160,7 +167,8 @@ enum class Kind : int32_t {
FormatExpression,
FormatSectionOverride,
NumberFormat,
ImageExpression
ImageExpression,
Within
};

class Expression {
Expand All @@ -183,6 +191,11 @@ class Expression {
const Feature& feature,
optional<double> colorRampParameter,
const std::set<std::string>& availableImages) const;
EvaluationResult evaluate(optional<float> zoom,
const Feature& feature,
optional<double> colorRampParameter,
const std::set<std::string>& availableImages,
const CanonicalTileID& canonical) const;
EvaluationResult evaluate(optional<mbgl::Value> accumulated, const Feature& feature) const;

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

#include <mbgl/style/conversion.hpp>
#include <mbgl/style/expression/expression.hpp>
#include <mbgl/style/expression/parsing_context.hpp>
#include <mbgl/util/geojson.hpp>

#include <memory>

namespace mbgl {
namespace style {
namespace expression {

class Within final : public Expression {
public:
explicit Within(GeoJSON geojson);

~Within() override;

EvaluationResult evaluate(const EvaluationContext&) const override;

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

void eachChild(const std::function<void(const Expression&)>&) const override {}

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

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

mbgl::Value serialize() const override;
std::string getOperator() const override { return "within"; }

private:
GeoJSON geoJSONSource;
zmiao marked this conversation as resolved.
Show resolved Hide resolved
};

} // namespace expression
} // namespace style
} // namespace mbgl
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"network": [
[
"probeNetwork - default - end",
0,
0
],
[
"probeNetwork - default - start",
0,
0
]
],
"gfx": [
[
"probeGFX - default - end",
4,
4,
10,
1,
[
49152,
49152
],
[
66,
66
],
[
100,
100
]
]
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"network": [
[
"probeNetwork - default - end",
0,
0
],
[
"probeNetwork - default - start",
0,
0
]
],
"gfx": [
[
"probeGFX - default - end",
16,
13,
29,
1,
[
196608,
196608
],
[
246,
246
],
[
528,
528
]
]
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"network": [
[
"probeNetwork - default - end",
2,
211659
],
[
"probeNetwork - default - start",
0,
0
]
],
"gfx": [
[
"probeGFX - default - end",
13,
17,
22,
1,
[
154848,
154848
],
[
174,
174
],
[
528,
528
]
]
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"network": [
[
"probeNetwork - default - end",
1,
84942
],
[
"probeNetwork - default - start",
0,
0
]
],
"gfx": [
[
"probeGFX - default - end",
13,
17,
22,
1,
[
207700,
207700
],
[
246,
246
],
[
1680,
1680
]
]
]
}
Loading