From ce429dcc1fce2a827e5c9462e3226c3bb04c6a39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Thu, 25 Jan 2018 15:24:29 -0800 Subject: [core] don't force downloading of Open Sans fonts When a SymbolLayer doesn't have a text-font defined, we automatically add Open Sans/Arial Unicode MS. However, when the SymbolLayer is only used for rendering icons, it doesn't have text-field defined either. In those cases, we still force downloading Open Sans/Arial Unicode MS during offline pack creation. If the user doesn't use this font, this change should save ~15MB and a few seconds in download time. --- src/mbgl/style/parser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/mbgl/style/parser.cpp b/src/mbgl/style/parser.cpp index b177f2159c..8d14d7972c 100644 --- a/src/mbgl/style/parser.cpp +++ b/src/mbgl/style/parser.cpp @@ -277,7 +277,7 @@ std::vector Parser::fontStacks() const { std::set result; for (const auto& layer : layers) { - if (layer->is()) { + if (layer->is() && !layer->as()->getTextField().isUndefined()) { layer->as()->getTextFont().match( [&] (Undefined) { result.insert({"Open Sans Regular", "Arial Unicode MS Regular"}); -- cgit v1.2.1 From 954c20b8c1832e0d94c4495a152b795df56ae967 Mon Sep 17 00:00:00 2001 From: Tobrun Date: Thu, 25 Jan 2018 14:04:13 +0100 Subject: [android] - blacklist adreno 2xx for VAO support --- src/mbgl/gl/context.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/mbgl/gl/context.cpp b/src/mbgl/gl/context.cpp index 7444ac112c..b6244d58c7 100644 --- a/src/mbgl/gl/context.cpp +++ b/src/mbgl/gl/context.cpp @@ -258,9 +258,10 @@ UniqueTexture Context::createTexture() { bool Context::supportsVertexArrays() const { static bool blacklisted = []() { - // Blacklist Adreno 3xx as it crashes on glBuffer(Sub)Data + // Blacklist Adreno 2xx, 3xx as it crashes on glBuffer(Sub)Data const std::string renderer = reinterpret_cast(glGetString(GL_RENDERER)); - return renderer.find("Adreno (TM) 3") != std::string::npos; + return renderer.find("Adreno (TM) 2") != std::string::npos + || renderer.find("Adreno (TM) 3") != std::string::npos; }(); return !blacklisted && -- cgit v1.2.1 From 62ae0738e8a3d1b3e4b661fa7e8d8bb1d6339c51 Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Wed, 31 Jan 2018 13:09:00 +0000 Subject: [core] Don't crash on line labels with 0 glyphs. Fixes issue #10956. --- src/mbgl/layout/symbol_projection.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/mbgl/layout/symbol_projection.cpp b/src/mbgl/layout/symbol_projection.cpp index ee6385c93c..9e077e2532 100644 --- a/src/mbgl/layout/symbol_projection.cpp +++ b/src/mbgl/layout/symbol_projection.cpp @@ -318,7 +318,7 @@ namespace mbgl { placedGlyphs.push_back(*placedGlyph); } placedGlyphs.push_back(firstAndLastGlyph->second); - } else { + } else if (symbol.glyphOffsets.size() == 1) { // Only a single glyph to place // So, determine whether to flip based on projected angle of the line segment it's on if (keepUpright && !flip) { @@ -337,7 +337,6 @@ namespace mbgl { return *orientationChange; } } - assert(symbol.glyphOffsets.size() == 1); // We are relying on SymbolInstance.hasText filtering out symbols without any glyphs at all const float glyphOffsetX = symbol.glyphOffsets.front(); optional singleGlyph = placeGlyphAlongLine(fontScale * glyphOffsetX, lineOffsetX, lineOffsetY, flip, projectedAnchorPoint, symbol.anchorPoint, symbol.segment, symbol.line, symbol.tileDistances, labelPlaneMatrix, false); @@ -347,6 +346,8 @@ namespace mbgl { placedGlyphs.push_back(*singleGlyph); } + // The number of placedGlyphs must equal the number of glyphOffsets, which must correspond to the number of glyph vertices + // There may be 0 glyphs here, if a label consists entirely of glyphs that have 0x0 dimensions for (auto& placedGlyph : placedGlyphs) { addDynamicAttributes(placedGlyph.point, placedGlyph.angle, dynamicVertexArray); } -- cgit v1.2.1 From 054d4d5ad676ce1ca9a740bc99ccd7ff2f9fecec Mon Sep 17 00:00:00 2001 From: Asheem Mamoowala Date: Fri, 16 Feb 2018 12:38:23 -0800 Subject: [core] Support a range of zooms in TileRange. Accounts for TilePyramid requesting parent tiles of ideal zoom tiles. --- src/mbgl/renderer/tile_pyramid.cpp | 7 +++-- src/mbgl/util/tile_range.hpp | 60 +++++++++++++++++++++++++------------- 2 files changed, 45 insertions(+), 22 deletions(-) (limited to 'src') diff --git a/src/mbgl/renderer/tile_pyramid.cpp b/src/mbgl/renderer/tile_pyramid.cpp index c4372e7112..8f83a0f982 100644 --- a/src/mbgl/renderer/tile_pyramid.cpp +++ b/src/mbgl/renderer/tile_pyramid.cpp @@ -143,10 +143,13 @@ void TilePyramid::update(const std::vector>& layer auto it = tiles.find(tileID); return it == tiles.end() ? nullptr : it->second.get(); }; - + + // The min and max zoom for TileRange are based on the updateRenderables algorithm. + // Tiles are created at the ideal tile zoom or at lower zoom levels. Child + // tiles are used from the cache, but not created. optional tileRange = {}; if (bounds) { - tileRange = util::TileRange::fromLatLngBounds(*bounds, std::min(tileZoom, (int32_t)zoomRange.max)); + tileRange = util::TileRange::fromLatLngBounds(*bounds, zoomRange.min, std::min(tileZoom, (int32_t)zoomRange.max)); } auto createTileFn = [&](const OverscaledTileID& tileID) -> Tile* { if (tileRange && !tileRange->contains(tileID.canonical)) { diff --git a/src/mbgl/util/tile_range.hpp b/src/mbgl/util/tile_range.hpp index f630a49078..8554cfb65e 100644 --- a/src/mbgl/util/tile_range.hpp +++ b/src/mbgl/util/tile_range.hpp @@ -6,41 +6,61 @@ #include namespace mbgl { - namespace util { class TileRange { public: - Range> range; - uint8_t z; + Range> range; + Range zoomRange; + + // Compute the range of tiles covered by the bounds at maxZoom. + static TileRange fromLatLngBounds(const LatLngBounds& bounds, uint8_t minZoom, uint8_t maxZoom) { + if (minZoom > maxZoom) { + std::swap(minZoom, maxZoom); + } + + auto swProj = Projection::project(bounds.southwest().wrapped(), maxZoom); + auto ne = bounds.northeast(); + auto neProj = Projection::project(ne.longitude() > util::LONGITUDE_MAX ? ne.wrapped() : ne , maxZoom); + + const auto maxTile = std::pow(2.0, maxZoom); + const auto minX = static_cast(std::floor(swProj.x)); + const auto maxX = static_cast(std::floor(neProj.x)); + const auto minY = static_cast(util::clamp(std::floor(neProj.y), 0.0 , maxTile)); + const auto maxY = static_cast(util::clamp(std::floor(swProj.y), 0.0, maxTile)); + + return TileRange({ {minX, minY}, {maxX, maxY} }, {minZoom, maxZoom}); + } // Compute the range of tiles covered by the bounds. static TileRange fromLatLngBounds(const LatLngBounds& bounds, uint8_t z) { - auto swProj = Projection::project(bounds.southwest().wrapped(), z); - auto ne = bounds.northeast(); - auto neProj = Projection::project(ne.longitude() > util::LONGITUDE_MAX ? ne.wrapped() : ne , z); - const auto minX = std::floor(swProj.x); - const auto maxX = std::ceil(neProj.x); - const auto minY = std::floor(neProj.y); - const auto maxY = std::ceil(swProj.y); - return TileRange({ {minX, minY}, {maxX, maxY} }, z); + return fromLatLngBounds(bounds, z, z); } bool contains(const CanonicalTileID& tileID) { - return z == tileID.z && - (range.min.x >= range.max.x ? //For wrapped bounds - tileID.x >= range.min.x || tileID.x < range.max.x : - tileID.x < range.max.x && tileID.x >= range.min.x) && - tileID.y < range.max.y && - tileID.y >= range.min.y; + if (tileID.z <= zoomRange.max && tileID.z >= zoomRange.min) { + if (tileID.z == 0) { + return true; + } + uint8_t dz = (zoomRange.max - tileID.z); + auto x0 = range.min.x >> dz; + auto x1 = range.max.x >> dz; + auto y0 = range.min.y >> dz; + auto y1 = range.max.y >> dz; + return (range.min.x > range.max.x ? //For wrapped bounds + tileID.x >= x0 || tileID.x <= x1 : + tileID.x <= x1 && tileID.x >= x0) && + tileID.y <= y1 && + tileID.y >= y0; + } + return false; } private: - TileRange(Range> range_, uint8_t z_) + TileRange(Range> range_, Range z_) : range(range_), - z(z_) { + zoomRange(z_) { } - }; } // namespace util -- cgit v1.2.1 From b79a1cc82e21bb8249d871691d6a4d250cd6a97c Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Wed, 21 Feb 2018 15:48:51 -0800 Subject: [core] Don't reuse heatmap render texture on viewport resize. Fixes issue #11228. --- src/mbgl/renderer/layers/render_heatmap_layer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/mbgl/renderer/layers/render_heatmap_layer.cpp b/src/mbgl/renderer/layers/render_heatmap_layer.cpp index 0f9e3239ef..4f2e899220 100644 --- a/src/mbgl/renderer/layers/render_heatmap_layer.cpp +++ b/src/mbgl/renderer/layers/render_heatmap_layer.cpp @@ -67,7 +67,7 @@ void RenderHeatmapLayer::render(PaintParameters& parameters, RenderSource*) { } } - if (!renderTexture) { + if (!parameters.context.supportsHalfFloatTextures || !renderTexture) { renderTexture = OffscreenTexture(parameters.context, size, gl::TextureType::UnsignedByte); renderTexture->bind(); } -- cgit v1.2.1 From dfeb5b25d3b78b238253ff2866aaab207fd5daa4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Wed, 21 Feb 2018 12:35:44 +0100 Subject: [core] continue loading style even if we mutate it When we load a stale style from cache, and the user immediately starts mutating it, we should continue loading the style so that we'll get a fresh copy of the data into our cache and avoid perpetually showing the stale style. --- src/mbgl/style/style_impl.cpp | 5 ----- 1 file changed, 5 deletions(-) (limited to 'src') diff --git a/src/mbgl/style/style_impl.cpp b/src/mbgl/style/style_impl.cpp index d330b3120a..0c7f924917 100644 --- a/src/mbgl/style/style_impl.cpp +++ b/src/mbgl/style/style_impl.cpp @@ -55,11 +55,6 @@ void Style::Impl::loadURL(const std::string& url_) { url = url_; styleRequest = fileSource.request(Resource::style(url), [this](Response res) { - // Once we get a fresh style, or the style is mutated, stop revalidating. - if (res.isFresh() || mutated) { - styleRequest.reset(); - } - // Don't allow a loaded, mutated style to be overwritten with a new version. if (mutated && loaded) { return; -- cgit v1.2.1 From 75feda12f17dececadf72986c45af78fe18afc56 Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Thu, 8 Feb 2018 15:23:29 -0800 Subject: [core] Implement Expression::serialize() Issue #10714 - Each expression stores its operator as a string, and default serialization is [operator, serialize(child1), ...] - Custom implementations of `serialize` for Expression types that don't follow the pattern - expression::Value -> mbgl::Value converter - node_expression bindings to expose `serialize` --- src/mbgl/style/expression/array_assertion.cpp | 19 ++++++++++ src/mbgl/style/expression/assertion.cpp | 4 +++ src/mbgl/style/expression/coercion.cpp | 7 ++++ src/mbgl/style/expression/compound_expression.cpp | 42 +++++++++++------------ src/mbgl/style/expression/interpolate.cpp | 24 +++++++++++++ src/mbgl/style/expression/let.cpp | 15 ++++++++ src/mbgl/style/expression/literal.cpp | 8 +++++ src/mbgl/style/expression/match.cpp | 36 +++++++++++++++++++ src/mbgl/style/expression/step.cpp | 12 +++++++ src/mbgl/style/expression/value.cpp | 33 +++++++++++++++++- 10 files changed, 177 insertions(+), 23 deletions(-) (limited to 'src') diff --git a/src/mbgl/style/expression/array_assertion.cpp b/src/mbgl/style/expression/array_assertion.cpp index 29f6a47b10..4049301b0b 100644 --- a/src/mbgl/style/expression/array_assertion.cpp +++ b/src/mbgl/style/expression/array_assertion.cpp @@ -81,6 +81,25 @@ ParseResult ArrayAssertion::parse(const Convertible& value, ParsingContext& ctx) )); } +mbgl::Value ArrayAssertion::serialize() const { + std::vector serialized; + serialized.emplace_back(getOperator()); + + + const auto array = getType().get(); + if (array.itemType.is() + || array.itemType.is() + || array.itemType.is()) { + serialized.emplace_back(type::toString(array.itemType)); + if (array.N) { + serialized.emplace_back(uint64_t(*array.N)); + } + } + + serialized.emplace_back(input->serialize()); + return serialized; +} + } // namespace expression } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/expression/assertion.cpp b/src/mbgl/style/expression/assertion.cpp index 0187921af9..d6f3f1b584 100644 --- a/src/mbgl/style/expression/assertion.cpp +++ b/src/mbgl/style/expression/assertion.cpp @@ -35,6 +35,10 @@ ParseResult Assertion::parse(const Convertible& value, ParsingContext& ctx) { return ParseResult(std::make_unique(it->second, std::move(parsed))); } +std::string Assertion::getOperator() const { + return type::toString(getType()); +} + EvaluationResult Assertion::evaluate(const EvaluationContext& params) const { for (std::size_t i = 0; i < inputs.size(); i++) { EvaluationResult value = inputs[i]->evaluate(params); diff --git a/src/mbgl/style/expression/coercion.cpp b/src/mbgl/style/expression/coercion.cpp index 56ab33fcfd..d9cd3ffdc9 100644 --- a/src/mbgl/style/expression/coercion.cpp +++ b/src/mbgl/style/expression/coercion.cpp @@ -81,6 +81,13 @@ Coercion::Coercion(type::Type type_, std::vector> in } } +std::string Coercion::getOperator() const { + return getType().match( + [](const type::NumberType&) { return "to-number"; }, + [](const type::ColorType&) { return "to-color"; }, + [](const auto&) { assert(false); return ""; }); +} + using namespace mbgl::style::conversion; ParseResult Coercion::parse(const Convertible& value, ParsingContext& ctx) { static std::unordered_map types { diff --git a/src/mbgl/style/expression/compound_expression.cpp b/src/mbgl/style/expression/compound_expression.cpp index 42cb655024..86d968c521 100644 --- a/src/mbgl/style/expression/compound_expression.cpp +++ b/src/mbgl/style/expression/compound_expression.cpp @@ -42,20 +42,19 @@ template struct Signature : SignatureBase { using Args = std::array, sizeof...(Params)>; - Signature(R (*evaluate_)(Params...)) : + Signature(R (*evaluate_)(Params...), std::string name_) : SignatureBase( valueTypeToExpressionType>(), - std::vector {valueTypeToExpressionType>()...} + std::vector {valueTypeToExpressionType>()...}, + std::move(name_) ), - evaluate(evaluate_) - {} + evaluate(evaluate_) {} EvaluationResult apply(const EvaluationContext& evaluationParameters, const Args& args) const { return applyImpl(evaluationParameters, args, std::index_sequence_for{}); } - std::unique_ptr makeExpression(const std::string& name, - std::vector> args) const override { + std::unique_ptr makeExpression(std::vector> args) const override { typename Signature::Args argsArray; std::copy_n(std::make_move_iterator(args.begin()), sizeof...(Params), argsArray.begin()); return std::make_unique>(name, *this, std::move(argsArray)); @@ -80,16 +79,16 @@ template struct Signature&)> : SignatureBase { using Args = std::vector>; - Signature(R (*evaluate_)(const Varargs&)) : + Signature(R (*evaluate_)(const Varargs&), std::string name_) : SignatureBase( valueTypeToExpressionType>(), - VarargsType { valueTypeToExpressionType() } + VarargsType { valueTypeToExpressionType() }, + std::move(name_) ), evaluate(evaluate_) {} - std::unique_ptr makeExpression(const std::string& name, - std::vector> args) const override { + std::unique_ptr makeExpression(std::vector> args) const override { return std::make_unique>(name, *this, std::move(args)); }; @@ -115,16 +114,16 @@ template struct Signature : SignatureBase { using Args = std::array, sizeof...(Params)>; - Signature(R (*evaluate_)(const EvaluationContext&, Params...)) : + Signature(R (*evaluate_)(const EvaluationContext&, Params...), std::string name_) : SignatureBase( valueTypeToExpressionType>(), - std::vector {valueTypeToExpressionType>()...} + std::vector {valueTypeToExpressionType>()...}, + std::move(name_) ), evaluate(evaluate_) {} - std::unique_ptr makeExpression(const std::string& name, - std::vector> args) const override { + std::unique_ptr makeExpression(std::vector> args) const override { typename Signature::Args argsArray; std::copy_n(std::make_move_iterator(args.begin()), sizeof...(Params), argsArray.begin()); return std::make_unique>(name, *this, std::move(argsArray)); @@ -176,14 +175,14 @@ struct Signature::value>> using Definition = CompoundExpressionRegistry::Definition; template -static std::unique_ptr makeSignature(Fn evaluateFunction) { - return std::make_unique>(evaluateFunction); +static std::unique_ptr makeSignature(Fn evaluateFunction, std::string name) { + return std::make_unique>(evaluateFunction, std::move(name)); } std::unordered_map initializeDefinitions() { std::unordered_map definitions; auto define = [&](std::string name, auto fn) { - definitions[name].push_back(makeSignature(fn)); + definitions[name].push_back(makeSignature(fn, name)); }; define("e", []() -> Result { return 2.718281828459045; }); @@ -461,7 +460,7 @@ ParseResult parseCompoundExpression(const std::string name, const Convertible& v } args.push_back(std::move(*parsed)); } - return createCompoundExpression(name, definition, std::move(args), ctx); + return createCompoundExpression(definition, std::move(args), ctx); } @@ -469,12 +468,11 @@ ParseResult createCompoundExpression(const std::string& name, std::vector> args, ParsingContext& ctx) { - return createCompoundExpression(name, CompoundExpressionRegistry::definitions.at(name), std::move(args), ctx); + return createCompoundExpression(CompoundExpressionRegistry::definitions.at(name), std::move(args), ctx); } -ParseResult createCompoundExpression(const std::string& name, - const Definition& definition, +ParseResult createCompoundExpression(const Definition& definition, std::vector> args, ParsingContext& ctx) { @@ -512,7 +510,7 @@ ParseResult createCompoundExpression(const std::string& name, } if (signatureContext.getErrors().size() == 0) { - return ParseResult(signature->makeExpression(name, std::move(args))); + return ParseResult(signature->makeExpression(std::move(args))); } } diff --git a/src/mbgl/style/expression/interpolate.cpp b/src/mbgl/style/expression/interpolate.cpp index 4cb22a3e4f..30b2cba81b 100644 --- a/src/mbgl/style/expression/interpolate.cpp +++ b/src/mbgl/style/expression/interpolate.cpp @@ -216,6 +216,30 @@ std::vector> InterpolateBase::possibleOutputs() const { return result; } +template +mbgl::Value Interpolate::serialize() const { + std::vector serialized; + serialized.emplace_back(getOperator()); + + interpolator.match( + [&](const ExponentialInterpolator& exponential) { + serialized.emplace_back(std::vector{{ std::string("exponential"), exponential.base }}); + }, + [&](const CubicBezierInterpolator& cubicBezier) { + static const std::string cubicBezierTag("cubic-bezier"); + auto p1 = cubicBezier.ub.getP1(); + auto p2 = cubicBezier.ub.getP2(); + serialized.emplace_back(std::vector{{ cubicBezierTag, p1.first, p1.second, p2.first, p2.second }}); + } + ); + serialized.emplace_back(input->serialize()); + for (auto& entry : stops) { + serialized.emplace_back(entry.first); + serialized.emplace_back(entry.second->serialize()); + }; + return serialized; +} + } // namespace expression } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/expression/let.cpp b/src/mbgl/style/expression/let.cpp index fe48138ac3..242a995b0b 100644 --- a/src/mbgl/style/expression/let.cpp +++ b/src/mbgl/style/expression/let.cpp @@ -65,6 +65,17 @@ ParseResult Let::parse(const Convertible& value, ParsingContext& ctx) { return ParseResult(std::make_unique(std::move(bindings_), std::move(*result_))); } +mbgl::Value Let::serialize() const { + std::vector serialized; + serialized.emplace_back(getOperator()); + for (auto entry : bindings) { + serialized.emplace_back(entry.first); + serialized.emplace_back(entry.second->serialize()); + } + serialized.emplace_back(result->serialize()); + return serialized; +} + EvaluationResult Var::evaluate(const EvaluationContext& params) const { return value->evaluate(params); } @@ -95,6 +106,10 @@ ParseResult Var::parse(const Convertible& value_, ParsingContext& ctx) { return ParseResult(std::make_unique(name_, std::move(*bindingValue))); } +mbgl::Value Var::serialize() const { + return std::vector{{ getOperator(), name }}; +} + } // namespace expression } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/expression/literal.cpp b/src/mbgl/style/expression/literal.cpp index 7e79fcbfe6..8a63980dba 100644 --- a/src/mbgl/style/expression/literal.cpp +++ b/src/mbgl/style/expression/literal.cpp @@ -102,6 +102,14 @@ ParseResult Literal::parse(const Convertible& value, ParsingContext& ctx) { } } +mbgl::Value Literal::serialize() const { + if (getType().is() || getType().is()) { + return std::vector{{ getOperator(), *fromExpressionValue(value) }}; + } else { + return *fromExpressionValue(value); + } +} + } // namespace expression } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/expression/match.cpp b/src/mbgl/style/expression/match.cpp index 0b2790b688..3d41f0bdd3 100644 --- a/src/mbgl/style/expression/match.cpp +++ b/src/mbgl/style/expression/match.cpp @@ -40,6 +40,42 @@ std::vector> Match::possibleOutputs() const { return result; } +template +mbgl::Value Match::serialize() const { + std::vector serialized; + serialized.emplace_back(getOperator()); + serialized.emplace_back(input->serialize()); + + // Sort so serialization has an arbitrary defined order, even though branch order doesn't affect evaluation + std::map> sortedBranches(branches.begin(), branches.end()); + + // Group branches by unique match expression to support condensed serializations + // of the form [case1, case2, ...] -> matchExpression + std::map outputLookup; + std::vector>> groupedByOutput; + for (auto& entry : sortedBranches) { + auto outputIndex = outputLookup.find(entry.second.get()); + if (outputIndex == outputLookup.end()) { + // First time seeing this output, add it to the end of the grouped list + outputLookup[entry.second.get()] = groupedByOutput.size(); + groupedByOutput.emplace_back(entry.second.get(), std::vector{{entry.first}}); + } else { + // We've seen this expression before, add the label to that output's group + groupedByOutput[outputIndex->second].second.emplace_back(entry.first); + } + }; + + for (auto& entry : groupedByOutput) { + entry.second.size() == 1 + ? serialized.emplace_back(entry.second[0]) // Only a single label matches this output expression + : serialized.emplace_back(entry.second); // Array of literal labels pointing to this output expression + serialized.emplace_back(entry.first->serialize()); // The output expression itself + } + + serialized.emplace_back(otherwise->serialize()); + return serialized; +} + template<> EvaluationResult Match::evaluate(const EvaluationContext& params) const { const EvaluationResult inputValue = input->evaluate(params); diff --git a/src/mbgl/style/expression/step.cpp b/src/mbgl/style/expression/step.cpp index 34537d48ae..ddaf9417cb 100644 --- a/src/mbgl/style/expression/step.cpp +++ b/src/mbgl/style/expression/step.cpp @@ -168,6 +168,18 @@ ParseResult Step::parse(const mbgl::style::conversion::Convertible& value, Parsi return ParseResult(std::make_unique(*outputType, std::move(*input), std::move(stops))); } +mbgl::Value Step::serialize() const { + std::vector serialized; + serialized.emplace_back(getOperator()); + serialized.emplace_back(input->serialize()); + for (auto& entry : stops) { + if (entry.first > -std::numeric_limits::infinity()) { + serialized.emplace_back(entry.first); + } + serialized.emplace_back(entry.second->serialize()); + } + return serialized; +} } // namespace expression } // namespace style diff --git a/src/mbgl/style/expression/value.cpp b/src/mbgl/style/expression/value.cpp index faa44e78aa..72779d4956 100644 --- a/src/mbgl/style/expression/value.cpp +++ b/src/mbgl/style/expression/value.cpp @@ -103,6 +103,37 @@ Value ValueConverter::toExpressionValue(const mbgl::Value& value) { return mbgl::Value::visit(value, FromMBGLValue()); } +mbgl::Value ValueConverter::fromExpressionValue(const Value& value) { + return value.match( + [&](const Color& color)->mbgl::Value { + return std::vector{ + std::string("rgba"), + double(255 * color.r / color.a), + double(255 * color.g / color.a), + double(255 * color.b / color.a), + double(color.a) + }; + }, + [&](const std::vector& values)->mbgl::Value { + std::vector converted; + converted.reserve(values.size()); + for (const Value& v : values) { + converted.emplace_back(fromExpressionValue(v)); + } + return converted; + }, + [&](const std::unordered_map& values)->mbgl::Value { + std::unordered_map converted; + converted.reserve(values.size()); + for(const auto& entry : values) { + converted.emplace(entry.first, fromExpressionValue(entry.second)); + } + return converted; + }, + [&](const auto& a)->mbgl::Value { return a; } + ); +} + Value ValueConverter::toExpressionValue(const float value) { return static_cast(value); } @@ -237,7 +268,7 @@ template <> type::Type valueTypeToExpressionType() { return typ template Value toExpressionValue(const mbgl::Value&); - +template optional fromExpressionValue(const Value&); // for to_rgba expression template type::Type valueTypeToExpressionType>(); -- cgit v1.2.1 From fdd9890461b417385e68db0453ec9fff5368f7b3 Mon Sep 17 00:00:00 2001 From: Ivo van Dongen Date: Mon, 19 Feb 2018 18:24:04 +0200 Subject: [core] check opengl error state after custom layer invocations --- src/mbgl/renderer/layers/render_custom_layer.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/mbgl/renderer/layers/render_custom_layer.cpp b/src/mbgl/renderer/layers/render_custom_layer.cpp index adafd8583f..a429b8d82e 100644 --- a/src/mbgl/renderer/layers/render_custom_layer.cpp +++ b/src/mbgl/renderer/layers/render_custom_layer.cpp @@ -5,6 +5,7 @@ #include #include #include +#include namespace mbgl { @@ -46,11 +47,11 @@ void RenderCustomLayer::render(PaintParameters& paintParameters, RenderSource*) if (context != impl().context || !initialized) { //If the context changed, deinitialize the previous one before initializing the new one. if (context && !contextDestroyed && impl().deinitializeFn) { - impl().deinitializeFn(context); + MBGL_CHECK_ERROR(impl().deinitializeFn(context)); } context = impl().context; assert(impl().initializeFn); - impl().initializeFn(impl().context); + MBGL_CHECK_ERROR(impl().initializeFn(impl().context)); initialized = true; } @@ -75,7 +76,7 @@ void RenderCustomLayer::render(PaintParameters& paintParameters, RenderSource*) parameters.fieldOfView = state.getFieldOfView(); assert(impl().renderFn); - impl().renderFn(context, parameters); + MBGL_CHECK_ERROR(impl().renderFn(context, parameters)); // Reset the view back to our original one, just in case the CustomLayer changed // the viewport or Framebuffer. -- cgit v1.2.1 From 1f73c9999eff11091e2c0c1fb375d2d35673d428 Mon Sep 17 00:00:00 2001 From: Ivo van Dongen Date: Mon, 19 Feb 2018 18:33:15 +0200 Subject: [core] blacklist vao usage on mali t720 (sapphire 650) Avoids problems on (amongst others) Samsung Galaxy J3 --- src/mbgl/gl/context.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/mbgl/gl/context.cpp b/src/mbgl/gl/context.cpp index f40cfa1f2c..ba44adb42b 100644 --- a/src/mbgl/gl/context.cpp +++ b/src/mbgl/gl/context.cpp @@ -281,10 +281,17 @@ UniqueTexture Context::createTexture() { bool Context::supportsVertexArrays() const { static bool blacklisted = []() { - // Blacklist Adreno 2xx, 3xx as it crashes on glBuffer(Sub)Data const std::string renderer = reinterpret_cast(glGetString(GL_RENDERER)); + + Log::Info(Event::General, "GPU Identifier: %s", renderer.c_str()); + + // Blacklist Adreno 2xx, 3xx as it crashes on glBuffer(Sub)Data + // Blacklist ARM Mali-T720 (in some MT8163 chipsets) as it crashes on glBindVertexArray return renderer.find("Adreno (TM) 2") != std::string::npos - || renderer.find("Adreno (TM) 3") != std::string::npos; + || renderer.find("Adreno (TM) 3") != std::string::npos + || renderer.find("Mali-T720") != std::string::npos + || renderer.find("Sapphire 650") != std::string::npos; + }(); return !blacklisted && -- cgit v1.2.1 From 15ba70b5ff018ec2285e0459bf557d1f61240c41 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 28 Feb 2018 15:03:47 -0800 Subject: Infer type assertions for object Cross-ports https://github.com/mapbox/mapbox-gl-js/pull/6243. --- src/mbgl/style/expression/parsing_context.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/mbgl/style/expression/parsing_context.cpp b/src/mbgl/style/expression/parsing_context.cpp index 0215982209..880772d926 100644 --- a/src/mbgl/style/expression/parsing_context.cpp +++ b/src/mbgl/style/expression/parsing_context.cpp @@ -147,7 +147,7 @@ ParseResult ParsingContext::parse(const Convertible& value, TypeAnnotationOption }; const type::Type actual = (*parsed)->getType(); - if ((*expected == type::String || *expected == type::Number || *expected == type::Boolean) && actual == type::Value) { + if ((*expected == type::String || *expected == type::Number || *expected == type::Boolean || *expected == type::Object) && actual == type::Value) { if (typeAnnotationOption == includeTypeAnnotations) { parsed = { std::make_unique(*expected, array(std::move(*parsed))) }; } -- cgit v1.2.1 From 54f3dc961e3a9e668312d7210706cddcf19676f7 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 1 Mar 2018 16:02:56 -0800 Subject: Relax type checking for "length" --- src/mbgl/style/expression/compound_expression.cpp | 7 --- src/mbgl/style/expression/length.cpp | 66 +++++++++++++++++++++++ src/mbgl/style/expression/parsing_context.cpp | 2 + 3 files changed, 68 insertions(+), 7 deletions(-) create mode 100644 src/mbgl/style/expression/length.cpp (limited to 'src') diff --git a/src/mbgl/style/expression/compound_expression.cpp b/src/mbgl/style/expression/compound_expression.cpp index 86d968c521..b1c2d74a9f 100644 --- a/src/mbgl/style/expression/compound_expression.cpp +++ b/src/mbgl/style/expression/compound_expression.cpp @@ -271,13 +271,6 @@ std::unordered_map initiali return object.at(key); }); - define("length", [](const std::vector& arr) -> Result { - return arr.size(); - }); - define("length", [] (const std::string s) -> Result { - return s.size(); - }); - define("properties", [](const EvaluationContext& params) -> Result> { if (!params.feature) { return EvaluationError { diff --git a/src/mbgl/style/expression/length.cpp b/src/mbgl/style/expression/length.cpp new file mode 100644 index 0000000000..258353ae4e --- /dev/null +++ b/src/mbgl/style/expression/length.cpp @@ -0,0 +1,66 @@ +#include +#include + +namespace mbgl { +namespace style { +namespace expression { + +Length::Length(std::unique_ptr input_) + : Expression(type::Number), + input(std::move(input_)) { +} + +EvaluationResult Length::evaluate(const EvaluationContext& params) const { + const EvaluationResult value = input->evaluate(params); + if (!value) return value; + return value->match( + [] (const std::string& s) { + return EvaluationResult { double(s.size()) }; + }, + [] (const std::vector& v) { + return EvaluationResult { double(v.size()) }; + }, + [&] (const auto&) -> EvaluationResult { + return EvaluationError { "Expected value to be of type string or array, but found " + toString(typeOf(*value)) + " instead." }; + }); +} + +void Length::eachChild(const std::function& visit) const { + visit(*input); +} + +bool Length::operator==(const Expression& e) const { + if (auto eq = dynamic_cast(&e)) { + return *eq->input == *input; + } + return false; +} + +std::vector> Length::possibleOutputs() const { + return { nullopt }; +} + +using namespace mbgl::style::conversion; +ParseResult Length::parse(const Convertible& value, ParsingContext& ctx) { + std::size_t length = arrayLength(value); + + if (length != 2) { + ctx.error("Expected one argument, but found " + util::toString(length) + " instead."); + return ParseResult(); + } + + ParseResult input = ctx.parse(arrayMember(value, 1), 1); + if (!input) return ParseResult(); + + type::Type type = (*input)->getType(); + if (!type.is() && !type.is() && !type.is()) { + ctx.error("Expected argument of type string or array, but found " + toString(type) + " instead."); + return ParseResult(); + } + + return ParseResult(std::make_unique(std::move(*input))); +} + +} // namespace expression +} // namespace style +} // namespace mbgl diff --git a/src/mbgl/style/expression/parsing_context.cpp b/src/mbgl/style/expression/parsing_context.cpp index 880772d926..364c3f740a 100644 --- a/src/mbgl/style/expression/parsing_context.cpp +++ b/src/mbgl/style/expression/parsing_context.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -89,6 +90,7 @@ const ExpressionRegistry& getExpressionRegistry() { {"case", Case::parse}, {"coalesce", Coalesce::parse}, {"interpolate", parseInterpolate}, + {"length", Length::parse}, {"let", Let::parse}, {"literal", Literal::parse}, {"match", parseMatch}, -- cgit v1.2.1 From 8ed540ea7b90d29e1d0947f9a421f36ab9bc66b2 Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Fri, 2 Mar 2018 16:38:46 -0500 Subject: Port fix for "at" expression off-by-1 error (#11375) * Port fix for "at" expression off-by-1 error Refs https://github.com/mapbox/mapbox-gl-js/pull/6269 * Consistency * Add ignores for symbol positiong differences in real-world tests --- src/mbgl/style/expression/at.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/mbgl/style/expression/at.cpp b/src/mbgl/style/expression/at.cpp index e447d33bc7..725e5ddb51 100644 --- a/src/mbgl/style/expression/at.cpp +++ b/src/mbgl/style/expression/at.cpp @@ -19,15 +19,21 @@ EvaluationResult At::evaluate(const EvaluationContext& params) const { const auto i = evaluatedIndex->get(); const auto inputArray = evaluatedInput->get>(); - if (i < 0 || i >= inputArray.size()) { + if (i < 0) { return EvaluationError { - "Array index out of bounds: " + stringify(i) + - " > " + util::toString(inputArray.size()) + "." + "Array index out of bounds: " + util::toString(i) + " < 0." + }; + } + + if (i >= inputArray.size()) { + return EvaluationError { + "Array index out of bounds: " + util::toString(i) + + " > " + util::toString(inputArray.size() - 1) + "." }; } if (i != std::floor(i)) { return EvaluationError { - "Array index must be an integer, but found " + stringify(i) + " instead." + "Array index must be an integer, but found " + util::toString(i) + " instead." }; } return inputArray[static_cast(i)]; -- cgit v1.2.1 From dfdfbc09902cdb592d217d3e264f736ce5217926 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Paczos?= Date: Thu, 8 Mar 2018 15:55:47 +0100 Subject: [android] calculating camera's LatLng for bounds without map padding (#11410) --- src/mbgl/map/map.cpp | 1 - 1 file changed, 1 deletion(-) (limited to 'src') diff --git a/src/mbgl/map/map.cpp b/src/mbgl/map/map.cpp index 947973415a..a7beb0f1e2 100644 --- a/src/mbgl/map/map.cpp +++ b/src/mbgl/map/map.cpp @@ -446,7 +446,6 @@ CameraOptions Map::cameraForGeometry(const Geometry& geometry, const Edg latLngs.push_back({ pt.y, pt.x }); }); return cameraForLatLngs(latLngs, padding, bearing); - } LatLngBounds Map::latLngBoundsForCamera(const CameraOptions& camera) const { -- cgit v1.2.1 From 8a3deab73373c8f3182e4337a3619ebcc58ecc2c Mon Sep 17 00:00:00 2001 From: Lucas Wojciechowski Date: Thu, 8 Mar 2018 15:48:08 -0800 Subject: [core] Add expression filter support (#11251) * WIP * WIP * WIP * Remove Filter::operator()(const Feature&) * WIP * WIP * WIP * WIP * Hook up expression filter evaluator * Replace `shared_ptr` with &reference * Fill in implementation of `void operator()(const ExpressionFilter&)` * Fix failing tests * Switch back to a shared_ptr per chat with @anandthakker * Fix benchmark compilation * Shot in the dark to fix CI * Shot in the dark to fix CI (part 2) * Shot in the dark to fix CI (part 3) * In src/mbgl/style/conversion/filter.cpp, add a port of isExpressionFilter and use it to decide in Converter::operator() whether to parse the incoming JSON as an ExpressionFilter or one of the legacy filter types * Remove bool Filter::operator()(const GeometryTileFeature&) const * Ensure the map zoom is passed into filtering operations wherever applicable * Add expression filter tests * Addressed PR feedback * Implement `NSPredicate *operator()(mbgl::style::ExpressionFilter filter)` * Fix formatting& nit --- src/mbgl/geometry/feature_index.cpp | 2 +- src/mbgl/layout/symbol_layout.cpp | 2 +- src/mbgl/style/conversion/filter.cpp | 65 ++++++++- src/mbgl/style/conversion/stringify.hpp | 4 + src/mbgl/style/filter.cpp | 13 ++ src/mbgl/style/filter_evaluator.cpp | 225 ++++++++++++++++++++++++++++++++ src/mbgl/tile/custom_geometry_tile.cpp | 2 +- src/mbgl/tile/geojson_tile.cpp | 2 +- src/mbgl/tile/geometry_tile.cpp | 2 +- src/mbgl/tile/geometry_tile_worker.cpp | 2 +- 10 files changed, 307 insertions(+), 12 deletions(-) create mode 100644 src/mbgl/style/filter.cpp create mode 100644 src/mbgl/style/filter_evaluator.cpp (limited to 'src') diff --git a/src/mbgl/geometry/feature_index.cpp b/src/mbgl/geometry/feature_index.cpp index 3b5e12b54a..6fb0d5e446 100644 --- a/src/mbgl/geometry/feature_index.cpp +++ b/src/mbgl/geometry/feature_index.cpp @@ -125,7 +125,7 @@ void FeatureIndex::addFeature( continue; } - if (options.filter && !(*options.filter)(*geometryTileFeature)) { + if (options.filter && !(*options.filter)(style::expression::EvaluationContext { static_cast(tileID.z), geometryTileFeature.get() })) { continue; } diff --git a/src/mbgl/layout/symbol_layout.cpp b/src/mbgl/layout/symbol_layout.cpp index a41a98fcaf..3bf85407c6 100644 --- a/src/mbgl/layout/symbol_layout.cpp +++ b/src/mbgl/layout/symbol_layout.cpp @@ -100,7 +100,7 @@ SymbolLayout::SymbolLayout(const BucketParameters& parameters, const size_t featureCount = sourceLayer->featureCount(); for (size_t i = 0; i < featureCount; ++i) { auto feature = sourceLayer->getFeature(i); - if (!leader.filter(feature->getType(), feature->getID(), [&] (const auto& key) { return feature->getValue(key); })) + if (!leader.filter(expression::EvaluationContext { this->zoom, feature.get() })) continue; SymbolFeature ft(std::move(feature)); diff --git a/src/mbgl/style/conversion/filter.cpp b/src/mbgl/style/conversion/filter.cpp index bb7bb6ea98..fba149da12 100644 --- a/src/mbgl/style/conversion/filter.cpp +++ b/src/mbgl/style/conversion/filter.cpp @@ -1,11 +1,52 @@ #include #include +#include +#include +#include namespace mbgl { namespace style { namespace conversion { + +using GeometryValue = mapbox::geometry::value; -static optional normalizeValue(const optional& value, Error& error) { +// This is a port from https://github.com/mapbox/mapbox-gl-js/blob/master/src/style-spec/feature_filter/index.js +static bool isExpressionFilter(const Convertible& filter) { + if (!isArray(filter) || arrayLength(filter) == 0) { + return false; + } + + optional op = toString(arrayMember(filter, 0)); + + if (!op) { + return false; + + } else if (*op == "has") { + if (arrayLength(filter) < 2) return false; + optional operand = toString(arrayMember(filter, 1)); + return operand && *operand != "$id" && *operand != "$type"; + + } else if (*op == "in" || *op == "!in" || *op == "!has" || *op == "none") { + return false; + + } else if (*op == "==" || *op == "!=" || *op == ">" || *op == ">=" || *op == "<" || *op == "<=") { + return arrayLength(filter) == 3 && (isArray(arrayMember(filter, 1)) || isArray(arrayMember(filter, 2))); + + } else if (*op == "any" || *op == "all") { + for (std::size_t i = 1; i < arrayLength(filter); i++) { + Convertible f = arrayMember(filter, i); + if (!isExpressionFilter(f) && !toBool(f)) { + return false; + } + } + return true; + + } else { + return true; + } +} + +static optional normalizeValue(const optional& value, Error& error) { if (!value) { error = { "filter expression value must be a boolean, number, or string" }; return {}; @@ -32,7 +73,7 @@ static optional toFeatureType(const Convertible& value, Error& erro } static optional toFeatureIdentifier(const Convertible& value, Error& error) { - optional identifier = toValue(value); + optional identifier = toValue(value); if (!identifier) { error = { "filter expression value must be a boolean, number, or string" }; return {}; @@ -99,7 +140,7 @@ optional convertEqualityFilter(const Convertible& value, Error& error) { return { IdentifierFilterType { *filterValue } }; } else { - optional filterValue = normalizeValue(toValue(arrayMember(value, 2)), error); + optional filterValue = normalizeValue(toValue(arrayMember(value, 2)), error); if (!filterValue) { return {}; } @@ -121,7 +162,7 @@ optional convertBinaryFilter(const Convertible& value, Error& error) { return {}; } - optional filterValue = normalizeValue(toValue(arrayMember(value, 2)), error); + optional filterValue = normalizeValue(toValue(arrayMember(value, 2)), error); if (!filterValue) { return {}; } @@ -167,9 +208,9 @@ optional convertSetFilter(const Convertible& value, Error& error) { return { IdentifierFilterType { std::move(values) } }; } else { - std::vector values; + std::vector values; for (std::size_t i = 2; i < arrayLength(value); ++i) { - optional filterValue = normalizeValue(toValue(arrayMember(value, i)), error); + optional filterValue = normalizeValue(toValue(arrayMember(value, i)), error); if (!filterValue) { return {}; } @@ -193,8 +234,20 @@ optional convertCompoundFilter(const Convertible& value, Error& error) { return { FilterType { std::move(filters) } }; } + +optional convertExpressionFilter(const Convertible& value, Error& error) { + optional> expression = convert>(value, error, expression::type::Boolean); + if (!expression) { + return {}; + } + return { ExpressionFilter { std::move(*expression) } }; +} optional Converter::operator()(const Convertible& value, Error& error) const { + if (isExpressionFilter(value)) { + return convertExpressionFilter(value, error); + } + if (!isArray(value)) { error = { "filter expression must be an array" }; return {}; diff --git a/src/mbgl/style/conversion/stringify.hpp b/src/mbgl/style/conversion/stringify.hpp index 6ae6fede42..7924a442c4 100644 --- a/src/mbgl/style/conversion/stringify.hpp +++ b/src/mbgl/style/conversion/stringify.hpp @@ -225,6 +225,10 @@ public: void operator()(const NotHasIdentifierFilter&) { stringifyUnaryFilter("!has", "$id"); } + + void operator()(const ExpressionFilter& filter) { + stringify(writer, filter.expression->serialize()); + } private: template diff --git a/src/mbgl/style/filter.cpp b/src/mbgl/style/filter.cpp new file mode 100644 index 0000000000..51aa6bcf82 --- /dev/null +++ b/src/mbgl/style/filter.cpp @@ -0,0 +1,13 @@ +#include +#include +#include + +namespace mbgl { +namespace style { + +bool Filter::operator()(const expression::EvaluationContext &context) const { + return FilterBase::visit(*this, FilterEvaluator { context }); +} + +} // namespace style +} // namespace mbgl diff --git a/src/mbgl/style/filter_evaluator.cpp b/src/mbgl/style/filter_evaluator.cpp new file mode 100644 index 0000000000..72022172f4 --- /dev/null +++ b/src/mbgl/style/filter_evaluator.cpp @@ -0,0 +1,225 @@ +#include +#include +#include + +namespace mbgl { +namespace style { + +template +struct Comparator { + const Op& op; + + template + bool operator()(const T& lhs, const T& rhs) const { + return op(lhs, rhs); + } + + template + auto operator()(const T0& lhs, const T1& rhs) const + -> typename std::enable_if_t::value && !std::is_same::value && + std::is_arithmetic::value && !std::is_same::value, bool> { + return op(double(lhs), double(rhs)); + } + + template + auto operator()(const T0&, const T1&) const + -> typename std::enable_if_t::value || std::is_same::value || + !std::is_arithmetic::value || std::is_same::value, bool> { + return false; + } + + bool operator()(const NullValue&, + const NullValue&) const { + // Should be unreachable; null is not currently allowed by the style specification. + assert(false); + return false; + } + + bool operator()(const std::vector&, + const std::vector&) const { + // Should be unreachable; nested values are not currently allowed by the style specification. + assert(false); + return false; + } + + bool operator()(const PropertyMap&, + const PropertyMap&) const { + // Should be unreachable; nested values are not currently allowed by the style specification. + assert(false); + return false; + } +}; + +template +bool compare(const Value& lhs, const Value& rhs, const Op& op) { + return Value::binary_visit(lhs, rhs, Comparator { op }); +} + +bool equal(const Value& lhs, const Value& rhs) { + return compare(lhs, rhs, [] (const auto& lhs_, const auto& rhs_) { return lhs_ == rhs_; }); +} + +bool FilterEvaluator::operator()(const NullFilter&) const { + return true; +} + +bool FilterEvaluator::operator()(const EqualsFilter& filter) const { + optional actual = context.feature->getValue(filter.key); + return actual && equal(*actual, filter.value); +} + +bool FilterEvaluator::operator()(const NotEqualsFilter& filter) const { + optional actual = context.feature->getValue(filter.key); + return !actual || !equal(*actual, filter.value); +} + +bool FilterEvaluator::operator()(const LessThanFilter& filter) const { + optional actual = context.feature->getValue(filter.key); + return actual && compare(*actual, filter.value, [] (const auto& lhs_, const auto& rhs_) { return lhs_ < rhs_; }); +} + +bool FilterEvaluator::operator()(const LessThanEqualsFilter& filter) const { + optional actual = context.feature->getValue(filter.key); + return actual && compare(*actual, filter.value, [] (const auto& lhs_, const auto& rhs_) { return lhs_ <= rhs_; }); +} + +bool FilterEvaluator::operator()(const GreaterThanFilter& filter) const { + optional actual = context.feature->getValue(filter.key); + return actual && compare(*actual, filter.value, [] (const auto& lhs_, const auto& rhs_) { return lhs_ > rhs_; }); +} + +bool FilterEvaluator::operator()(const GreaterThanEqualsFilter& filter) const { + optional actual = context.feature->getValue(filter.key); + return actual && compare(*actual, filter.value, [] (const auto& lhs_, const auto& rhs_) { return lhs_ >= rhs_; }); +} + +bool FilterEvaluator::operator()(const InFilter& filter) const { + optional actual = context.feature->getValue(filter.key); + if (!actual) + return false; + for (const auto& v: filter.values) { + if (equal(*actual, v)) { + return true; + } + } + return false; +} + +bool FilterEvaluator::operator()(const NotInFilter& filter) const { + optional actual = context.feature->getValue(filter.key); + if (!actual) + return true; + for (const auto& v: filter.values) { + if (equal(*actual, v)) { + return false; + } + } + return true; +} + +bool FilterEvaluator::operator()(const AnyFilter& filter) const { + for (const auto& f: filter.filters) { + if (Filter::visit(f, *this)) { + return true; + } + } + return false; +} + +bool FilterEvaluator::operator()(const AllFilter& filter) const { + for (const auto& f: filter.filters) { + if (!Filter::visit(f, *this)) { + return false; + } + } + return true; +} + +bool FilterEvaluator::operator()(const NoneFilter& filter) const { + for (const auto& f: filter.filters) { + if (Filter::visit(f, *this)) { + return false; + } + } + return true; +} + +bool FilterEvaluator::operator()(const HasFilter& filter) const { + return bool(context.feature->getValue(filter.key)); +} + +bool FilterEvaluator::operator()(const NotHasFilter& filter) const { + return !context.feature->getValue(filter.key); +} + +bool FilterEvaluator::operator()(const TypeEqualsFilter& filter) const { + return context.feature->getType() == filter.value; +} + +bool FilterEvaluator::operator()(const TypeNotEqualsFilter& filter) const { + return context.feature->getType() != filter.value; +} + +bool FilterEvaluator::operator()(const TypeInFilter& filter) const { + for (const auto& v: filter.values) { + if (context.feature->getType() == v) { + return true; + } + } + return false; +} + +bool FilterEvaluator::operator()(const TypeNotInFilter& filter) const { + for (const auto& v: filter.values) { + if (context.feature->getType() == v) { + return false; + } + } + return true; +} + +bool FilterEvaluator::operator()(const IdentifierEqualsFilter& filter) const { + return context.feature->getID() == filter.value; +} + +bool FilterEvaluator::operator()(const IdentifierNotEqualsFilter& filter) const { + return context.feature->getID() != filter.value; +} + +bool FilterEvaluator::operator()(const IdentifierInFilter& filter) const { + for (const auto& v: filter.values) { + if (context.feature->getID() == v) { + return true; + } + } + return false; +} + +bool FilterEvaluator::operator()(const IdentifierNotInFilter& filter) const { + for (const auto& v: filter.values) { + if (context.feature->getID() == v) { + return false; + } + } + return true; +} + +bool FilterEvaluator::operator()(const HasIdentifierFilter&) const { + return bool(context.feature->getID()); +} + +bool FilterEvaluator::operator()(const NotHasIdentifierFilter&) const { + return !context.feature->getID(); +} + +bool FilterEvaluator::operator()(const ExpressionFilter& filter) const { + const expression::EvaluationResult result = filter.expression->evaluate(context); + if (result) { + const optional typed = expression::fromExpressionValue(*result); + return typed ? *typed : false; + } + return false; +} + +} // namespace style +} // namespace mbgl diff --git a/src/mbgl/tile/custom_geometry_tile.cpp b/src/mbgl/tile/custom_geometry_tile.cpp index 33962ad87d..a2fefcfa9f 100644 --- a/src/mbgl/tile/custom_geometry_tile.cpp +++ b/src/mbgl/tile/custom_geometry_tile.cpp @@ -79,7 +79,7 @@ void CustomGeometryTile::querySourceFeatures( auto feature = layer->getFeature(i); // Apply filter, if any - if (queryOptions.filter && !(*queryOptions.filter)(*feature)) { + if (queryOptions.filter && !(*queryOptions.filter)(style::expression::EvaluationContext { static_cast(id.overscaledZ), feature.get() })) { continue; } diff --git a/src/mbgl/tile/geojson_tile.cpp b/src/mbgl/tile/geojson_tile.cpp index bbec899950..f211c03569 100644 --- a/src/mbgl/tile/geojson_tile.cpp +++ b/src/mbgl/tile/geojson_tile.cpp @@ -30,7 +30,7 @@ void GeoJSONTile::querySourceFeatures( auto feature = layer->getFeature(i); // Apply filter, if any - if (options.filter && !(*options.filter)(*feature)) { + if (options.filter && !(*options.filter)(style::expression::EvaluationContext { static_cast(this->id.overscaledZ), feature.get() })) { continue; } diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp index a58c744065..82d0c91806 100644 --- a/src/mbgl/tile/geometry_tile.cpp +++ b/src/mbgl/tile/geometry_tile.cpp @@ -281,7 +281,7 @@ void GeometryTile::querySourceFeatures( auto feature = layer->getFeature(i); // Apply filter, if any - if (options.filter && !(*options.filter)(*feature)) { + if (options.filter && !(*options.filter)(style::expression::EvaluationContext { static_cast(this->id.overscaledZ), feature.get() })) { continue; } diff --git a/src/mbgl/tile/geometry_tile_worker.cpp b/src/mbgl/tile/geometry_tile_worker.cpp index 24841dd125..8b6f1f63bf 100644 --- a/src/mbgl/tile/geometry_tile_worker.cpp +++ b/src/mbgl/tile/geometry_tile_worker.cpp @@ -339,7 +339,7 @@ void GeometryTileWorker::redoLayout() { for (std::size_t i = 0; !obsolete && i < geometryLayer->featureCount(); i++) { std::unique_ptr feature = geometryLayer->getFeature(i); - if (!filter(feature->getType(), feature->getID(), [&] (const auto& key) { return feature->getValue(key); })) + if (!filter(expression::EvaluationContext { static_cast(this->id.overscaledZ), feature.get() })) continue; GeometryCollection geometries = feature->getGeometries(); -- cgit v1.2.1 From a57be420b0a90760d505d8e1df3e0d42ac1aa10d Mon Sep 17 00:00:00 2001 From: Asheem Mamoowala Date: Thu, 8 Mar 2018 12:35:33 -0800 Subject: [core] TileJSON conversion clamps bounds longitude to [-180,180], per spec --- src/mbgl/style/conversion/tileset.cpp | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src') diff --git a/src/mbgl/style/conversion/tileset.cpp b/src/mbgl/style/conversion/tileset.cpp index 6d89cef944..a2c4aa80b3 100644 --- a/src/mbgl/style/conversion/tileset.cpp +++ b/src/mbgl/style/conversion/tileset.cpp @@ -103,6 +103,8 @@ optional Converter::operator()(const Convertible& value, Error error = { "bounds left longitude should be less than right longitude" }; return {}; } + *left = util::max(-180.0, *left); + *right = util::min(180.0, *right); result.bounds = LatLngBounds::hull({ *bottom, *left }, { *top, *right }); } -- cgit v1.2.1 From 2912a1423d7b1141e6f77b3c29f350065a084598 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Mon, 19 Feb 2018 13:09:31 -0800 Subject: [core] Stringify expression syntax, not function syntax --- src/mbgl/style/conversion/stringify.hpp | 131 ++------------------------------ 1 file changed, 6 insertions(+), 125 deletions(-) (limited to 'src') diff --git a/src/mbgl/style/conversion/stringify.hpp b/src/mbgl/style/conversion/stringify.hpp index 7924a442c4..7b7727d7c4 100644 --- a/src/mbgl/style/conversion/stringify.hpp +++ b/src/mbgl/style/conversion/stringify.hpp @@ -290,138 +290,19 @@ void stringify(Writer& writer, const Undefined&) { writer.Null(); } -template -void stringify(Writer& writer, const CategoricalValue& v) { - CategoricalValue::visit(v, [&] (const auto& v_) { stringify(writer, v_); }); -} - -template -class StringifyStops { -public: - Writer& writer; - - template - void operator()(const ExponentialStops& f) { - writer.Key("type"); - writer.String("exponential"); - writer.Key("base"); - writer.Double(f.base); - writer.Key("stops"); - stringifyStops(f.stops); - } - - template - void operator()(const IntervalStops& f) { - writer.Key("type"); - writer.String("interval"); - writer.Key("stops"); - stringifyStops(f.stops); - } - - template - void operator()(const CategoricalStops& f) { - writer.Key("type"); - writer.String("categorical"); - writer.Key("stops"); - stringifyStops(f.stops); - } - - template - void operator()(const IdentityStops&) { - writer.Key("type"); - writer.String("identity"); - } - - template - void operator()(const CompositeExponentialStops& f) { - writer.Key("type"); - writer.String("exponential"); - writer.Key("base"); - writer.Double(f.base); - writer.Key("stops"); - stringifyCompositeStops(f.stops); - } - - template - void operator()(const CompositeIntervalStops& f) { - writer.Key("type"); - writer.String("interval"); - writer.Key("stops"); - stringifyCompositeStops(f.stops); - } - - template - void operator()(const CompositeCategoricalStops& f) { - writer.Key("type"); - writer.String("categorical"); - writer.Key("stops"); - stringifyCompositeStops(f.stops); - } - -private: - template - void stringifyStops(const std::map& stops) { - writer.StartArray(); - for (const auto& stop : stops) { - writer.StartArray(); - stringify(writer, stop.first); - stringify(writer, stop.second); - writer.EndArray(); - } - writer.EndArray(); - } - - template - void stringifyCompositeStops(const std::map& stops) { - writer.StartArray(); - for (const auto& outer : stops) { - for (const auto& inner : outer.second) { - writer.StartArray(); - writer.StartObject(); - writer.Key("zoom"); - writer.Double(outer.first); - writer.Key("value"); - stringify(writer, inner.first); - writer.EndObject(); - stringify(writer, inner.second); - writer.EndArray(); - } - } - writer.EndArray(); - } -}; - template -void stringify(Writer& writer, const CameraFunction& f) { - writer.StartObject(); - CameraFunction::Stops::visit(f.stops, StringifyStops { writer }); - writer.EndObject(); +void stringify(Writer& writer, const CameraFunction& fn) { + stringify(writer, fn.getExpression().serialize()); } template -void stringify(Writer& writer, const SourceFunction& f) { - writer.StartObject(); - writer.Key("property"); - writer.String(f.property); - SourceFunction::Stops::visit(f.stops, StringifyStops { writer }); - if (f.defaultValue) { - writer.Key("default"); - stringify(writer, *f.defaultValue); - } - writer.EndObject(); +void stringify(Writer& writer, const SourceFunction& fn) { + stringify(writer, fn.getExpression().serialize()); } template -void stringify(Writer& writer, const CompositeFunction& f) { - writer.StartObject(); - writer.Key("property"); - writer.String(f.property); - CompositeFunction::Stops::visit(f.stops, StringifyStops { writer }); - if (f.defaultValue) { - writer.Key("default"); - stringify(writer, *f.defaultValue); - } - writer.EndObject(); +void stringify(Writer& writer, const CompositeFunction& fn) { + stringify(writer, fn.getExpression().serialize()); } template -- cgit v1.2.1 From 449c68e329448196c976fade12a8c0d58a0c7af9 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Mon, 19 Feb 2018 13:09:31 -0800 Subject: [core] Update mapbox-gl-js --- src/mbgl/shaders/line_pattern.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/mbgl/shaders/line_pattern.cpp b/src/mbgl/shaders/line_pattern.cpp index f8d785ade9..be88255e3c 100644 --- a/src/mbgl/shaders/line_pattern.cpp +++ b/src/mbgl/shaders/line_pattern.cpp @@ -215,8 +215,14 @@ void main() { float x_a = mod(v_linesofar / u_pattern_size_a.x, 1.0); float x_b = mod(v_linesofar / u_pattern_size_b.x, 1.0); - float y_a = 0.5 + (v_normal.y * v_width2.s / u_pattern_size_a.y); - float y_b = 0.5 + (v_normal.y * v_width2.s / u_pattern_size_b.y); + + // v_normal.y is 0 at the midpoint of the line, -1 at the lower edge, 1 at the upper edge + // we clamp the line width outset to be between 0 and half the pattern height plus padding (2.0) + // to ensure we don't sample outside the designated symbol on the sprite sheet. + // 0.5 is added to shift the component to be bounded between 0 and 1 for interpolation of + // the texture coordinate + float y_a = 0.5 + (v_normal.y * clamp(v_width2.s, 0.0, (u_pattern_size_a.y + 2.0) / 2.0) / u_pattern_size_a.y); + float y_b = 0.5 + (v_normal.y * clamp(v_width2.s, 0.0, (u_pattern_size_b.y + 2.0) / 2.0) / u_pattern_size_b.y); vec2 pos_a = mix(u_pattern_tl_a / u_texsize, u_pattern_br_a / u_texsize, vec2(x_a, y_a)); vec2 pos_b = mix(u_pattern_tl_b / u_texsize, u_pattern_br_b / u_texsize, vec2(x_b, y_b)); -- cgit v1.2.1 From ef1dcc0d5b5ae23b303ca736168f1452ea36ae77 Mon Sep 17 00:00:00 2001 From: Ansis Brammanis Date: Wed, 21 Feb 2018 11:36:40 -0500 Subject: [core] don't hide icons if text is an empty string --- src/mbgl/text/placement.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/mbgl/text/placement.cpp b/src/mbgl/text/placement.cpp index fc24c78902..54b2b7539b 100644 --- a/src/mbgl/text/placement.cpp +++ b/src/mbgl/text/placement.cpp @@ -95,9 +95,6 @@ void Placement::placeLayerBucket( auto partiallyEvaluatedTextSize = bucket.textSizeBinder->evaluateForZoom(state.getZoom()); auto partiallyEvaluatedIconSize = bucket.iconSizeBinder->evaluateForZoom(state.getZoom()); - const bool iconWithoutText = !bucket.hasTextData() || bucket.layout.get(); - const bool textWithoutIcon = !bucket.hasIconData() || bucket.layout.get(); - for (auto& symbolInstance : bucket.symbolInstances) { if (seenCrossTileIDs.count(symbolInstance.crossTileID) == 0) { @@ -140,6 +137,9 @@ void Placement::placeLayerBucket( offscreen &= placed.second; } + const bool iconWithoutText = !symbolInstance.hasText || bucket.layout.get(); + const bool textWithoutIcon = !symbolInstance.hasIcon || bucket.layout.get(); + // combine placements for icon and text if (!iconWithoutText && !textWithoutIcon) { placeText = placeIcon = placeText && placeIcon; -- cgit v1.2.1 From 4dc191b8da139b19c5aa5a705169647786516a20 Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Tue, 20 Mar 2018 10:06:35 -0700 Subject: [core] Avoid flashing on pitched overzoomed tiles. Clamps perspective ratios in shaders. Fixes issue #11487. Port of GL JS PR #6365. --- src/mbgl/shaders/collision_box.cpp | 5 ++++- src/mbgl/shaders/symbol_icon.cpp | 5 ++++- src/mbgl/shaders/symbol_sdf.cpp | 5 ++++- 3 files changed, 12 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/mbgl/shaders/collision_box.cpp b/src/mbgl/shaders/collision_box.cpp index 9d11640bf4..bc5d9bc6f9 100644 --- a/src/mbgl/shaders/collision_box.cpp +++ b/src/mbgl/shaders/collision_box.cpp @@ -22,7 +22,10 @@ varying float v_notUsed; void main() { vec4 projectedPoint = u_matrix * vec4(a_anchor_pos, 0, 1); highp float camera_to_anchor_distance = projectedPoint.w; - highp float collision_perspective_ratio = 0.5 + 0.5 * (u_camera_to_center_distance / camera_to_anchor_distance); + highp float collision_perspective_ratio = clamp( + 0.5 + 0.5 * (u_camera_to_center_distance / camera_to_anchor_distance), + 0.0, // Prevents oversized near-field boxes in pitched/overzoomed tiles + 4.0); gl_Position = u_matrix * vec4(a_pos, 0.0, 1.0); gl_Position.xy += a_extrude * u_extrude_scale * gl_Position.w * collision_perspective_ratio; diff --git a/src/mbgl/shaders/symbol_icon.cpp b/src/mbgl/shaders/symbol_icon.cpp index f5c2bbe22d..da31d36291 100644 --- a/src/mbgl/shaders/symbol_icon.cpp +++ b/src/mbgl/shaders/symbol_icon.cpp @@ -80,7 +80,10 @@ void main() { highp float distance_ratio = u_pitch_with_map ? camera_to_anchor_distance / u_camera_to_center_distance : u_camera_to_center_distance / camera_to_anchor_distance; - highp float perspective_ratio = 0.5 + 0.5 * distance_ratio; + highp float perspective_ratio = clamp( + 0.5 + 0.5 * distance_ratio, + 0.0, // Prevents oversized near-field symbols in pitched/overzoomed tiles + 4.0); size *= perspective_ratio; diff --git a/src/mbgl/shaders/symbol_sdf.cpp b/src/mbgl/shaders/symbol_sdf.cpp index 441eaf7aac..46bab10be5 100644 --- a/src/mbgl/shaders/symbol_sdf.cpp +++ b/src/mbgl/shaders/symbol_sdf.cpp @@ -156,7 +156,10 @@ void main() { highp float distance_ratio = u_pitch_with_map ? camera_to_anchor_distance / u_camera_to_center_distance : u_camera_to_center_distance / camera_to_anchor_distance; - highp float perspective_ratio = 0.5 + 0.5 * distance_ratio; + highp float perspective_ratio = clamp( + 0.5 + 0.5 * distance_ratio, + 0.0, // Prevents oversized near-field symbols in pitched/overzoomed tiles + 4.0); size *= perspective_ratio; -- cgit v1.2.1 From 673f9e364804b4c3f449c8619e767a7b40e5350e Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Thu, 22 Mar 2018 12:19:11 -0700 Subject: [core] Double maximum label width by reducing glyph precision. Fixes issue #11507. Port of GL JS PR #6375. --- src/mbgl/programs/symbol_program.hpp | 4 ++-- src/mbgl/shaders/symbol_icon.cpp | 2 +- src/mbgl/shaders/symbol_sdf.cpp | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/mbgl/programs/symbol_program.hpp b/src/mbgl/programs/symbol_program.hpp index a14afac702..9b5037ed9f 100644 --- a/src/mbgl/programs/symbol_program.hpp +++ b/src/mbgl/programs/symbol_program.hpp @@ -61,8 +61,8 @@ struct SymbolLayoutAttributes : gl::Attributes< {{ static_cast(labelAnchor.x), static_cast(labelAnchor.y), - static_cast(::round(o.x * 64)), // use 1/64 pixels for placement - static_cast(::round((o.y + glyphOffsetY) * 64)) + static_cast(::round(o.x * 32)), // use 1/32 pixels for placement + static_cast(::round((o.y + glyphOffsetY) * 32)) }}, {{ tx, diff --git a/src/mbgl/shaders/symbol_icon.cpp b/src/mbgl/shaders/symbol_icon.cpp index da31d36291..c037c81005 100644 --- a/src/mbgl/shaders/symbol_icon.cpp +++ b/src/mbgl/shaders/symbol_icon.cpp @@ -105,7 +105,7 @@ void main() { mat2 rotation_matrix = mat2(angle_cos, -1.0 * angle_sin, angle_sin, angle_cos); vec4 projected_pos = u_label_plane_matrix * vec4(a_projected_pos.xy, 0.0, 1.0); - gl_Position = u_gl_coord_matrix * vec4(projected_pos.xy / projected_pos.w + rotation_matrix * (a_offset / 64.0 * fontScale), 0.0, 1.0); + gl_Position = u_gl_coord_matrix * vec4(projected_pos.xy / projected_pos.w + rotation_matrix * (a_offset / 32.0 * fontScale), 0.0, 1.0); v_tex = a_tex / u_texsize; vec2 fade_opacity = unpack_opacity(a_fade_opacity); diff --git a/src/mbgl/shaders/symbol_sdf.cpp b/src/mbgl/shaders/symbol_sdf.cpp index 46bab10be5..b584c00315 100644 --- a/src/mbgl/shaders/symbol_sdf.cpp +++ b/src/mbgl/shaders/symbol_sdf.cpp @@ -183,7 +183,7 @@ void main() { mat2 rotation_matrix = mat2(angle_cos, -1.0 * angle_sin, angle_sin, angle_cos); vec4 projected_pos = u_label_plane_matrix * vec4(a_projected_pos.xy, 0.0, 1.0); - gl_Position = u_gl_coord_matrix * vec4(projected_pos.xy / projected_pos.w + rotation_matrix * (a_offset / 64.0 * fontScale), 0.0, 1.0); + gl_Position = u_gl_coord_matrix * vec4(projected_pos.xy / projected_pos.w + rotation_matrix * (a_offset / 32.0 * fontScale), 0.0, 1.0); float gamma_scale = gl_Position.w; vec2 tex = a_tex / u_texsize; -- cgit v1.2.1 From 9499a2bda7eed68f8a11cdecce0130b1be2054a4 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 22 Mar 2018 16:44:12 -0700 Subject: [core] Don't resolve tokens after evaluating a text-field or icon-image expression (#11509) --- src/mbgl/layout/symbol_layout.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/mbgl/layout/symbol_layout.cpp b/src/mbgl/layout/symbol_layout.cpp index 3bf85407c6..82a9255824 100644 --- a/src/mbgl/layout/symbol_layout.cpp +++ b/src/mbgl/layout/symbol_layout.cpp @@ -126,7 +126,7 @@ SymbolLayout::SymbolLayout(const BucketParameters& parameters, if (hasText) { std::string u8string = layout.evaluate(zoom, ft); - if (layout.get().isConstant()) { + if (layout.get().isConstant() && !leader.layout.get().isExpression()) { u8string = util::replaceTokens(u8string, getValue); } @@ -159,7 +159,7 @@ SymbolLayout::SymbolLayout(const BucketParameters& parameters, if (hasIcon) { std::string icon = layout.evaluate(zoom, ft); - if (layout.get().isConstant()) { + if (layout.get().isConstant() && !leader.layout.get().isExpression()) { icon = util::replaceTokens(icon, getValue); } ft.icon = icon; -- cgit v1.2.1 From f9b6d5f09c844ad367c54d34677d71c9da757bba Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Wed, 28 Mar 2018 14:11:55 -0700 Subject: [core] Fix potential race condition crash in symbol querying.. First half of fix for issue #11538. Testing `if (pendingData)` didn't work if there _was_ a data update, but the update was "the data for this tile is now null". In that case, the tile's FeatureIndex would be updated, but the tile's data member would remain unchanged. In the case of a tile's data being deleted, the matching FeatureIndex would have an empty set of bucketLayerIDs, but the _old_ data would still be in place for querying, and the symbolBuckets might not be updated yet (pending onPlacement). In this case `bucketLayerIDs.at(indexedFeature.bucketName)` could throw an out-of-range exception. --- src/mbgl/tile/geometry_tile.cpp | 10 ++++++---- src/mbgl/tile/geometry_tile.hpp | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp index 82d0c91806..7fb0c9922d 100644 --- a/src/mbgl/tile/geometry_tile.cpp +++ b/src/mbgl/tile/geometry_tile.cpp @@ -130,8 +130,8 @@ void GeometryTile::onLayout(LayoutResult result, const uint64_t resultCorrelatio // replacing a tile at a different zoom that _did_ have symbols. (void)resultCorrelationID; nonSymbolBuckets = std::move(result.nonSymbolBuckets); - pendingFeatureIndex = std::move(result.featureIndex); - pendingData = std::move(result.tileData); + pendingFeatureIndex = { std::move(result.featureIndex) }; + pendingData = { std::move(result.tileData) }; observer->onTileChanged(*this); } @@ -215,10 +215,12 @@ Bucket* GeometryTile::getBucket(const Layer::Impl& layer) const { void GeometryTile::commitFeatureIndex() { if (pendingFeatureIndex) { - featureIndex = std::move(pendingFeatureIndex); + featureIndex = std::move(*pendingFeatureIndex); + pendingFeatureIndex = nullopt; } if (pendingData) { - data = std::move(pendingData); + data = std::move(*pendingData); + pendingData = nullopt; } } diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp index 00a4aafadf..0c6adf0337 100644 --- a/src/mbgl/tile/geometry_tile.hpp +++ b/src/mbgl/tile/geometry_tile.hpp @@ -125,9 +125,9 @@ private: std::unordered_map> nonSymbolBuckets; std::unique_ptr featureIndex; - std::unique_ptr pendingFeatureIndex; + optional> pendingFeatureIndex; std::unique_ptr data; - std::unique_ptr pendingData; + optional> pendingData; optional glyphAtlasImage; optional iconAtlasImage; -- cgit v1.2.1 From e5a3a7a4510bbbf2fc97363ab0084fb4bc3dd448 Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Wed, 28 Mar 2018 15:40:57 -0700 Subject: [core] Fix potential race condition crash in symbol querying.. Second half of fix for issue #11538. If a global placement took place between the time a tile's non-symbol layout updated and the time new symbol buckets arrived, the tile's new FeatureIndex would be committed, but the global CollisionIndex would be generated against the old symbolBuckets. The mismatch could cause the CollisionIndex to contain indices that didn't exist in the new FeatureIndex, and attempting to access them would generate an out-of-bounds exception. --- src/mbgl/tile/geometry_tile.cpp | 29 +++++++++++++++++++++++------ src/mbgl/tile/geometry_tile.hpp | 4 ++-- 2 files changed, 25 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp index 7fb0c9922d..0c8a92d8b7 100644 --- a/src/mbgl/tile/geometry_tile.cpp +++ b/src/mbgl/tile/geometry_tile.cpp @@ -130,8 +130,10 @@ void GeometryTile::onLayout(LayoutResult result, const uint64_t resultCorrelatio // replacing a tile at a different zoom that _did_ have symbols. (void)resultCorrelationID; nonSymbolBuckets = std::move(result.nonSymbolBuckets); - pendingFeatureIndex = { std::move(result.featureIndex) }; - pendingData = { std::move(result.tileData) }; + // It is possible for multiple onLayouts to be called before an onPlacement is called, in which + // case we will discard previous pending data + pendingFeatureIndex = {{ false, std::move(result.featureIndex) }}; + pendingData = {{ false, std::move(result.tileData) }}; observer->onTileChanged(*this); } @@ -142,6 +144,17 @@ void GeometryTile::onPlacement(PlacementResult result, const uint64_t resultCorr pending = false; } symbolBuckets = std::move(result.symbolBuckets); + // When symbolBuckets arrive, mark pendingData/FeatureIndex as "ready for commit" at the + // time of the next global placement. We are counting on these symbolBuckets being + // in sync with the data/index in the last `onLayout` call, even though the correlation IDs + // may not be the same (e.g. "setShowCollisionBoxes" could bump the correlation ID while + // waiting for glyph dependencies) + if (pendingData) { + pendingData->first = true; + } + if (pendingFeatureIndex) { + pendingFeatureIndex->first = true; + } if (result.glyphAtlasImage) { glyphAtlasImage = std::move(*result.glyphAtlasImage); } @@ -214,12 +227,16 @@ Bucket* GeometryTile::getBucket(const Layer::Impl& layer) const { } void GeometryTile::commitFeatureIndex() { - if (pendingFeatureIndex) { - featureIndex = std::move(*pendingFeatureIndex); + // We commit our pending FeatureIndex and GeometryTileData when: + // 1) An `onPlacement` result has delivered us updated symbolBuckets since we received the pending data + // 2) A global placement has run, synchronizing the global CollisionIndex with the latest + // symbolBuckets (and thus with the latest FeatureIndex/GeometryTileData) + if (pendingFeatureIndex && pendingFeatureIndex->first) { + featureIndex = std::move(pendingFeatureIndex->second); pendingFeatureIndex = nullopt; } - if (pendingData) { - data = std::move(*pendingData); + if (pendingData && pendingData->first) { + data = std::move(pendingData->second); pendingData = nullopt; } } diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp index 0c6adf0337..8843d6e579 100644 --- a/src/mbgl/tile/geometry_tile.hpp +++ b/src/mbgl/tile/geometry_tile.hpp @@ -125,9 +125,9 @@ private: std::unordered_map> nonSymbolBuckets; std::unique_ptr featureIndex; - optional> pendingFeatureIndex; + optional>> pendingFeatureIndex; std::unique_ptr data; - optional> pendingData; + optional>> pendingData; optional glyphAtlasImage; optional iconAtlasImage; -- cgit v1.2.1 From f4d9fb816d4a22bcc17a07452a926f4f06e75d3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguye=CC=82=CC=83n?= Date: Thu, 29 Mar 2018 12:55:29 -0700 Subject: [core] Round-trip linear interpolators --- src/mbgl/style/expression/interpolate.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/mbgl/style/expression/interpolate.cpp b/src/mbgl/style/expression/interpolate.cpp index 30b2cba81b..daad8523f2 100644 --- a/src/mbgl/style/expression/interpolate.cpp +++ b/src/mbgl/style/expression/interpolate.cpp @@ -223,7 +223,11 @@ mbgl::Value Interpolate::serialize() const { interpolator.match( [&](const ExponentialInterpolator& exponential) { - serialized.emplace_back(std::vector{{ std::string("exponential"), exponential.base }}); + if (exponential.base == 1) { + serialized.emplace_back(std::vector{{ std::string("linear") }}); + } else { + serialized.emplace_back(std::vector{{ std::string("exponential"), exponential.base }}); + } }, [&](const CubicBezierInterpolator& cubicBezier) { static const std::string cubicBezierTag("cubic-bezier"); -- cgit v1.2.1 From d066e94103e7e21dbd795a986659fc729aa53a4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguy=E1=BB=85n?= Date: Fri, 30 Mar 2018 03:21:44 -0700 Subject: [core] Fixed to-rgba with zero opacity --- src/mbgl/style/expression/compound_expression.cpp | 7 +------ src/mbgl/style/expression/value.cpp | 9 +++++---- src/mbgl/util/color.cpp | 22 ++++++++++++++++++---- 3 files changed, 24 insertions(+), 14 deletions(-) (limited to 'src') diff --git a/src/mbgl/style/expression/compound_expression.cpp b/src/mbgl/style/expression/compound_expression.cpp index b1c2d74a9f..77b294b380 100644 --- a/src/mbgl/style/expression/compound_expression.cpp +++ b/src/mbgl/style/expression/compound_expression.cpp @@ -209,12 +209,7 @@ std::unordered_map initiali ); }); define("to-rgba", [](const Color& color) -> Result> { - return std::array {{ - 255 * color.r / color.a, - 255 * color.g / color.a, - 255 * color.b / color.a, - color.a - }}; + return color.toArray(); }); define("rgba", rgba); diff --git a/src/mbgl/style/expression/value.cpp b/src/mbgl/style/expression/value.cpp index 72779d4956..1b3257c755 100644 --- a/src/mbgl/style/expression/value.cpp +++ b/src/mbgl/style/expression/value.cpp @@ -106,12 +106,13 @@ Value ValueConverter::toExpressionValue(const mbgl::Value& value) { mbgl::Value ValueConverter::fromExpressionValue(const Value& value) { return value.match( [&](const Color& color)->mbgl::Value { + std::array array = color.toArray(); return std::vector{ std::string("rgba"), - double(255 * color.r / color.a), - double(255 * color.g / color.a), - double(255 * color.b / color.a), - double(color.a) + array[0], + array[1], + array[2], + array[3], }; }, [&](const std::vector& values)->mbgl::Value { diff --git a/src/mbgl/util/color.cpp b/src/mbgl/util/color.cpp index c8145d36e7..55f1b65436 100644 --- a/src/mbgl/util/color.cpp +++ b/src/mbgl/util/color.cpp @@ -23,11 +23,25 @@ optional Color::parse(const std::string& s) { } std::string Color::stringify() const { + std::array array = toArray(); return "rgba(" + - util::toString(r * 255 / a) + "," + - util::toString(g * 255 / a) + "," + - util::toString(b * 255 / a) + "," + - util::toString(a) + ")"; + util::toString(array[0]) + "," + + util::toString(array[1]) + "," + + util::toString(array[2]) + "," + + util::toString(array[3]) + ")"; +} + +std::array Color::toArray() const { + if (a == 0) { + return {{ 0, 0, 0, 0 }}; + } else { + return {{ + r * 255 / a, + g * 255 / a, + b * 255 / a, + a, + }}; + } } } // namespace mbgl -- cgit v1.2.1 From 7f42e3970ee8c6a57fbb4dba7c564ff17c2eb6bb Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Fri, 30 Mar 2018 16:02:56 -0700 Subject: [core] Convert GeometryTileWorker to "one-phase" loading Modest simplification refactoring (issue #10457). Also, fixes issue #11538, which was caused in part by a hole in the vestigial two-phase loading. --- src/mbgl/tile/geometry_tile.cpp | 46 ++------- src/mbgl/tile/geometry_tile.hpp | 33 +++--- src/mbgl/tile/geometry_tile_worker.cpp | 177 ++++++++++++++++++++------------- src/mbgl/tile/geometry_tile_worker.hpp | 14 ++- 4 files changed, 141 insertions(+), 129 deletions(-) (limited to 'src') diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp index 0c8a92d8b7..f95d7220bd 100644 --- a/src/mbgl/tile/geometry_tile.cpp +++ b/src/mbgl/tile/geometry_tile.cpp @@ -125,36 +125,17 @@ void GeometryTile::setShowCollisionBoxes(const bool showCollisionBoxes_) { } void GeometryTile::onLayout(LayoutResult result, const uint64_t resultCorrelationID) { - // Don't mark ourselves loaded or renderable until the first successful placement - // TODO: Ideally we'd render this tile without symbols as long as this tile wasn't - // replacing a tile at a different zoom that _did_ have symbols. - (void)resultCorrelationID; - nonSymbolBuckets = std::move(result.nonSymbolBuckets); - // It is possible for multiple onLayouts to be called before an onPlacement is called, in which - // case we will discard previous pending data - pendingFeatureIndex = {{ false, std::move(result.featureIndex) }}; - pendingData = {{ false, std::move(result.tileData) }}; - observer->onTileChanged(*this); -} - -void GeometryTile::onPlacement(PlacementResult result, const uint64_t resultCorrelationID) { loaded = true; renderable = true; if (resultCorrelationID == correlationID) { pending = false; } + + nonSymbolBuckets = std::move(result.nonSymbolBuckets); symbolBuckets = std::move(result.symbolBuckets); - // When symbolBuckets arrive, mark pendingData/FeatureIndex as "ready for commit" at the - // time of the next global placement. We are counting on these symbolBuckets being - // in sync with the data/index in the last `onLayout` call, even though the correlation IDs - // may not be the same (e.g. "setShowCollisionBoxes" could bump the correlation ID while - // waiting for glyph dependencies) - if (pendingData) { - pendingData->first = true; - } - if (pendingFeatureIndex) { - pendingFeatureIndex->first = true; - } + + dataPendingCommit = {{ std::move(result.tileData), std::move(result.featureIndex) }}; + if (result.glyphAtlasImage) { glyphAtlasImage = std::move(*result.glyphAtlasImage); } @@ -227,17 +208,12 @@ Bucket* GeometryTile::getBucket(const Layer::Impl& layer) const { } void GeometryTile::commitFeatureIndex() { - // We commit our pending FeatureIndex and GeometryTileData when: - // 1) An `onPlacement` result has delivered us updated symbolBuckets since we received the pending data - // 2) A global placement has run, synchronizing the global CollisionIndex with the latest - // symbolBuckets (and thus with the latest FeatureIndex/GeometryTileData) - if (pendingFeatureIndex && pendingFeatureIndex->first) { - featureIndex = std::move(pendingFeatureIndex->second); - pendingFeatureIndex = nullopt; - } - if (pendingData && pendingData->first) { - data = std::move(pendingData->second); - pendingData = nullopt; + // We commit our pending FeatureIndex and GeometryTileData when a global placement has run, + // synchronizing the global CollisionIndex with the latest symbolBuckets/FeatureIndex/GeometryTileData + if (dataPendingCommit) { + data = std::move(dataPendingCommit->first); + featureIndex = std::move(dataPendingCommit->second); + dataPendingCommit = nullopt; } } diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp index 8843d6e579..98b5f0c490 100644 --- a/src/mbgl/tile/geometry_tile.hpp +++ b/src/mbgl/tile/geometry_tile.hpp @@ -68,30 +68,24 @@ public: std::unordered_map> nonSymbolBuckets; std::unique_ptr featureIndex; std::unique_ptr tileData; + std::unordered_map> symbolBuckets; + optional glyphAtlasImage; + optional iconAtlasImage; LayoutResult(std::unordered_map> nonSymbolBuckets_, std::unique_ptr featureIndex_, - std::unique_ptr tileData_) + std::unique_ptr tileData_, + std::unordered_map> symbolBuckets_, + optional glyphAtlasImage_, + optional iconAtlasImage_) : nonSymbolBuckets(std::move(nonSymbolBuckets_)), featureIndex(std::move(featureIndex_)), - tileData(std::move(tileData_)) {} - }; - void onLayout(LayoutResult, uint64_t correlationID); - - class PlacementResult { - public: - std::unordered_map> symbolBuckets; - optional glyphAtlasImage; - optional iconAtlasImage; - - PlacementResult(std::unordered_map> symbolBuckets_, - optional glyphAtlasImage_, - optional iconAtlasImage_) - : symbolBuckets(std::move(symbolBuckets_)), + tileData(std::move(tileData_)), + symbolBuckets(std::move(symbolBuckets_)), glyphAtlasImage(std::move(glyphAtlasImage_)), iconAtlasImage(std::move(iconAtlasImage_)) {} }; - void onPlacement(PlacementResult, uint64_t correlationID); + void onLayout(LayoutResult, uint64_t correlationID); void onError(std::exception_ptr, uint64_t correlationID); @@ -124,16 +118,15 @@ private: uint64_t correlationID = 0; std::unordered_map> nonSymbolBuckets; + std::unordered_map> symbolBuckets; + + optional, std::unique_ptr>> dataPendingCommit; std::unique_ptr featureIndex; - optional>> pendingFeatureIndex; std::unique_ptr data; - optional>> pendingData; optional glyphAtlasImage; optional iconAtlasImage; - std::unordered_map> symbolBuckets; - const MapMode mode; bool showCollisionBoxes; diff --git a/src/mbgl/tile/geometry_tile_worker.cpp b/src/mbgl/tile/geometry_tile_worker.cpp index 8b6f1f63bf..482cbe0053 100644 --- a/src/mbgl/tile/geometry_tile_worker.cpp +++ b/src/mbgl/tile/geometry_tile_worker.cpp @@ -41,48 +41,73 @@ GeometryTileWorker::GeometryTileWorker(ActorRef self_, GeometryTileWorker::~GeometryTileWorker() = default; /* - NOTE: The comments below are technically correct, but currently - conceptually misleading. The change to foreground label placement - means that: - (1) "placement" here is a misnomer: the remaining role of - "attemptPlacement" is symbol buffer generation - (2) Once a tile has completed layout, we will only run - "attemptPlacement" once - (3) Tiles won't be rendered until "attemptPlacement" has run once - - TODO: Simplify GeometryTileWorker to fit its new role - https://github.com/mapbox/mapbox-gl-native/issues/10457 - GeometryTileWorker is a state machine. This is its transition diagram. States are indicated by [state], lines are transitions triggered by messages, (parentheses) are actions taken on transition. - [idle] <----------------------------. - | | - set{Data,Layers,Placement}, symbolDependenciesChanged | - | | - (do layout/placement; self-send "coalesced") | - v | - [coalescing] --- coalesced ------------. - | | - .-----------------. .---------------. + [Idle] <-------------------------. + | | + set{Data,Layers}, symbolDependenciesChanged, | + setShowCollisionBoxes | + | | + (do parse and/or symbol layout; self-send "coalesced") | + v | + [Coalescing] --- coalesced ---------. + | | + .-----------. .---------------------. | | - .--- set{Data,Layers} setPlacement -----. - | | | | - | v v | - .-- [need layout] <-- set{Data,Layers} -- [need placement] --. + .--- set{Data,Layers} setShowCollisionBoxes, + | | symbolDependenciesChanged --. + | | | | + | v v | + .-- [NeedsParse] <-- set{Data,Layers} -- [NeedsSymbolLayout] ---. | | coalesced coalesced | | v v - (do layout or placement; self-send "coalesced"; goto [coalescing]) - - The idea is that in the [idle] state, layout or placement happens immediately - in response to a "set" message. During this processing, multiple "set" messages - might get queued in the mailbox. At the end of processing, we self-send "coalesced", - read all the queued messages until we get to "coalesced", and then redo either - layout or placement if there were one or more "set"s (with layout taking priority, - since it will trigger placement when complete), or return to the [idle] state if not. + (do parse or symbol layout; self-send "coalesced"; goto [coalescing]) + + The idea is that in the [idle] state, parsing happens immediately in response to + a "set" message, and symbol layout happens once all symbol dependencies are met. + During this processing, multiple "set" messages might get queued in the mailbox. + At the end of processing, we self-send "coalesced", read all the queued messages + until we get to "coalesced", and then re-parse if there were one or more "set"s or + return to the [idle] state if not. + + One important goal of the design is to prevent starvation. Under heavy load new + requests for tiles should not prevent in progress request from completing. + It is nevertheless possible to restart an in-progress request: + + - [Idle] setData -> parse() + sends getGlyphs, hasPendingSymbolDependencies() is true + enters [Coalescing], sends coalesced + - [Coalescing] coalesced -> [Idle] + - [Idle] setData -> new parse(), interrupts old parse() + sends getGlyphs, hasPendingSymbolDependencies() is true + enters [Coalescing], sends coalesced + - [Coalescing] onGlyphsAvailable -> [NeedsSymbolLayout] + hasPendingSymbolDependencies() may or may not be true + - [NeedsSymbolLayout] coalesced -> performSymbolLayout() + Generates result depending on whether dependencies are met + -> [Idle] + + In this situation, we are counting on the idea that even with rapid changes to + the tile's data, the set of glyphs/images it requires will not keep growing without + limit. + + Although parsing (which populates all non-symbol buckets and requests dependencies + for symbol buckets) is internally separate from symbol layout, we only return + results to the foreground when we have completed both steps. Because we _move_ + the result buckets to the foreground, it is necessary to re-generate all buckets from + scratch for `setShowCollisionBoxes`, even though it only affects symbol layers. + + The GL JS equivalent (in worker_tile.js and vector_tile_worker_source.js) + is somewhat simpler because it relies on getGlyphs/getImages calls that transfer + an entire set of glyphs/images on every tile load, while the native logic + maintains a local state that can be incrementally updated. Because each tile load + call becomes self-contained, the equivalent of the coalescing logic is handled by + 'reloadTile' queueing a single extra 'reloadTile' callback to run after the next + completed parse. */ void GeometryTileWorker::setData(std::unique_ptr data_, uint64_t correlationID_) { @@ -92,14 +117,14 @@ void GeometryTileWorker::setData(std::unique_ptr data_, switch (state) { case Idle: - redoLayout(); + parse(); coalesce(); break; case Coalescing: - case NeedLayout: - case NeedPlacement: - state = NeedLayout; + case NeedsParse: + case NeedsSymbolLayout: + state = NeedsParse; break; } } catch (...) { @@ -114,16 +139,16 @@ void GeometryTileWorker::setLayers(std::vector> layers_, switch (state) { case Idle: - redoLayout(); + parse(); coalesce(); break; case Coalescing: - case NeedPlacement: - state = NeedLayout; + case NeedsSymbolLayout: + state = NeedsParse; break; - case NeedLayout: + case NeedsParse: break; } } catch (...) { @@ -138,16 +163,20 @@ void GeometryTileWorker::setShowCollisionBoxes(bool showCollisionBoxes_, uint64_ switch (state) { case Idle: - attemptPlacement(); - coalesce(); + if (!hasPendingParseResult()) { + // Trigger parse if nothing is in flight, otherwise symbol layout will automatically + // pick up the change + parse(); + coalesce(); + } break; case Coalescing: - state = NeedPlacement; + state = NeedsSymbolLayout; break; - case NeedPlacement: - case NeedLayout: + case NeedsSymbolLayout: + case NeedsParse: break; } } catch (...) { @@ -160,19 +189,23 @@ void GeometryTileWorker::symbolDependenciesChanged() { switch (state) { case Idle: if (symbolLayoutsNeedPreparation) { - attemptPlacement(); + // symbolLayoutsNeedPreparation can only be set true by parsing + // and the parse result can only be cleared by performSymbolLayout + // which also clears symbolLayoutsNeedPreparation + assert(hasPendingParseResult()); + performSymbolLayout(); coalesce(); } break; case Coalescing: if (symbolLayoutsNeedPreparation) { - state = NeedPlacement; + state = NeedsSymbolLayout; } break; - case NeedPlacement: - case NeedLayout: + case NeedsSymbolLayout: + case NeedsParse: break; } } catch (...) { @@ -191,13 +224,16 @@ void GeometryTileWorker::coalesced() { state = Idle; break; - case NeedLayout: - redoLayout(); + case NeedsParse: + parse(); coalesce(); break; - case NeedPlacement: - attemptPlacement(); + case NeedsSymbolLayout: + // We may have entered NeedsSymbolLayout while coalescing + // after a performSymbolLayout. In that case, we need to + // start over with parsing in order to do another layout. + hasPendingParseResult() ? performSymbolLayout() : parse(); coalesce(); break; } @@ -279,7 +315,7 @@ static std::vector> toRenderLayers(const std::vecto return renderLayers; } -void GeometryTileWorker::redoLayout() { +void GeometryTileWorker::parse() { if (!data || !layers) { return; } @@ -292,8 +328,8 @@ void GeometryTileWorker::redoLayout() { } std::unordered_map> symbolLayoutMap; - std::unordered_map> buckets; - auto featureIndex = std::make_unique(); + nonSymbolBuckets.clear(); + featureIndex = std::make_unique(); BucketParameters parameters { id, mode, pixelRatio }; GlyphDependencies glyphDependencies; @@ -352,7 +388,7 @@ void GeometryTileWorker::redoLayout() { } for (const auto& layer : group) { - buckets.emplace(layer->getID(), bucket); + nonSymbolBuckets.emplace(layer->getID(), bucket); } } } @@ -368,13 +404,7 @@ void GeometryTileWorker::redoLayout() { requestNewGlyphs(glyphDependencies); requestNewImages(imageDependencies); - parent.invoke(&GeometryTile::onLayout, GeometryTile::LayoutResult { - std::move(buckets), - std::move(featureIndex), - *data ? (*data)->clone() : nullptr, - }, correlationID); - - attemptPlacement(); + performSymbolLayout(); } bool GeometryTileWorker::hasPendingSymbolDependencies() const { @@ -385,9 +415,13 @@ bool GeometryTileWorker::hasPendingSymbolDependencies() const { } return !pendingImageDependencies.empty(); } + +bool GeometryTileWorker::hasPendingParseResult() const { + return bool(featureIndex); +} -void GeometryTileWorker::attemptPlacement() { - if (!data || !layers || hasPendingSymbolDependencies()) { +void GeometryTileWorker::performSymbolLayout() { + if (!data || !layers || !hasPendingParseResult() || hasPendingSymbolDependencies()) { return; } @@ -435,11 +469,14 @@ void GeometryTileWorker::attemptPlacement() { } firstLoad = false; - - parent.invoke(&GeometryTile::onPlacement, GeometryTile::PlacementResult { + + parent.invoke(&GeometryTile::onLayout, GeometryTile::LayoutResult { + std::move(nonSymbolBuckets), + std::move(featureIndex), + *data ? (*data)->clone() : nullptr, std::move(buckets), std::move(glyphAtlasImage), - std::move(iconAtlasImage), + std::move(iconAtlasImage) }, correlationID); } diff --git a/src/mbgl/tile/geometry_tile_worker.hpp b/src/mbgl/tile/geometry_tile_worker.hpp index 0276392679..458ec76988 100644 --- a/src/mbgl/tile/geometry_tile_worker.hpp +++ b/src/mbgl/tile/geometry_tile_worker.hpp @@ -8,6 +8,8 @@ #include #include #include +#include +#include #include #include @@ -43,8 +45,8 @@ public: private: void coalesced(); - void redoLayout(); - void attemptPlacement(); + void parse(); + void performSymbolLayout(); void coalesce(); @@ -53,6 +55,7 @@ private: void symbolDependenciesChanged(); bool hasPendingSymbolDependencies() const; + bool hasPendingParseResult() const; ActorRef self; ActorRef parent; @@ -62,12 +65,15 @@ private: const std::atomic& obsolete; const MapMode mode; const float pixelRatio; + + std::unique_ptr featureIndex; + std::unordered_map> nonSymbolBuckets; enum State { Idle, Coalescing, - NeedLayout, - NeedPlacement + NeedsParse, + NeedsSymbolLayout }; State state = Idle; -- cgit v1.2.1 From 11961d5c31e6ec245823b583a4dc5e1b12dd7f5a Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Sat, 31 Mar 2018 18:17:16 -0700 Subject: [core] Consolidate GeometryTile symbol/nonSymbolBuckets Conversion to one-phase tile loading removed any need to track them separately. --- src/mbgl/tile/geometry_tile.cpp | 12 +++--------- src/mbgl/tile/geometry_tile.hpp | 12 ++++-------- src/mbgl/tile/geometry_tile_worker.cpp | 9 +++------ src/mbgl/tile/geometry_tile_worker.hpp | 2 +- 4 files changed, 11 insertions(+), 24 deletions(-) (limited to 'src') diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp index f95d7220bd..5089f95022 100644 --- a/src/mbgl/tile/geometry_tile.cpp +++ b/src/mbgl/tile/geometry_tile.cpp @@ -131,8 +131,7 @@ void GeometryTile::onLayout(LayoutResult result, const uint64_t resultCorrelatio pending = false; } - nonSymbolBuckets = std::move(result.nonSymbolBuckets); - symbolBuckets = std::move(result.symbolBuckets); + buckets = std::move(result.buckets); dataPendingCommit = {{ std::move(result.tileData), std::move(result.featureIndex) }}; @@ -177,11 +176,7 @@ void GeometryTile::upload(gl::Context& context) { } }; - for (auto& entry : nonSymbolBuckets) { - uploadFn(*entry.second); - } - - for (auto& entry : symbolBuckets) { + for (auto& entry : buckets) { uploadFn(*entry.second); } @@ -197,7 +192,6 @@ void GeometryTile::upload(gl::Context& context) { } Bucket* GeometryTile::getBucket(const Layer::Impl& layer) const { - const auto& buckets = layer.type == LayerType::Symbol ? symbolBuckets : nonSymbolBuckets; const auto it = buckets.find(layer.id); if (it == buckets.end()) { return nullptr; @@ -209,7 +203,7 @@ Bucket* GeometryTile::getBucket(const Layer::Impl& layer) const { void GeometryTile::commitFeatureIndex() { // We commit our pending FeatureIndex and GeometryTileData when a global placement has run, - // synchronizing the global CollisionIndex with the latest symbolBuckets/FeatureIndex/GeometryTileData + // synchronizing the global CollisionIndex with the latest buckets/FeatureIndex/GeometryTileData if (dataPendingCommit) { data = std::move(dataPendingCommit->first); featureIndex = std::move(dataPendingCommit->second); diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp index 98b5f0c490..0161e00efd 100644 --- a/src/mbgl/tile/geometry_tile.hpp +++ b/src/mbgl/tile/geometry_tile.hpp @@ -65,23 +65,20 @@ public: class LayoutResult { public: - std::unordered_map> nonSymbolBuckets; + std::unordered_map> buckets; std::unique_ptr featureIndex; std::unique_ptr tileData; - std::unordered_map> symbolBuckets; optional glyphAtlasImage; optional iconAtlasImage; - LayoutResult(std::unordered_map> nonSymbolBuckets_, + LayoutResult(std::unordered_map> buckets_, std::unique_ptr featureIndex_, std::unique_ptr tileData_, - std::unordered_map> symbolBuckets_, optional glyphAtlasImage_, optional iconAtlasImage_) - : nonSymbolBuckets(std::move(nonSymbolBuckets_)), + : buckets(std::move(buckets_)), featureIndex(std::move(featureIndex_)), tileData(std::move(tileData_)), - symbolBuckets(std::move(symbolBuckets_)), glyphAtlasImage(std::move(glyphAtlasImage_)), iconAtlasImage(std::move(iconAtlasImage_)) {} }; @@ -117,8 +114,7 @@ private: uint64_t correlationID = 0; - std::unordered_map> nonSymbolBuckets; - std::unordered_map> symbolBuckets; + std::unordered_map> buckets; optional, std::unique_ptr>> dataPendingCommit; std::unique_ptr featureIndex; diff --git a/src/mbgl/tile/geometry_tile_worker.cpp b/src/mbgl/tile/geometry_tile_worker.cpp index 482cbe0053..f57732117b 100644 --- a/src/mbgl/tile/geometry_tile_worker.cpp +++ b/src/mbgl/tile/geometry_tile_worker.cpp @@ -328,7 +328,7 @@ void GeometryTileWorker::parse() { } std::unordered_map> symbolLayoutMap; - nonSymbolBuckets.clear(); + buckets.clear(); featureIndex = std::make_unique(); BucketParameters parameters { id, mode, pixelRatio }; @@ -388,7 +388,7 @@ void GeometryTileWorker::parse() { } for (const auto& layer : group) { - nonSymbolBuckets.emplace(layer->getID(), bucket); + buckets.emplace(layer->getID(), bucket); } } } @@ -448,8 +448,6 @@ void GeometryTileWorker::performSymbolLayout() { symbolLayoutsNeedPreparation = false; } - std::unordered_map> buckets; - for (auto& symbolLayout : symbolLayouts) { if (obsolete) { return; @@ -471,10 +469,9 @@ void GeometryTileWorker::performSymbolLayout() { firstLoad = false; parent.invoke(&GeometryTile::onLayout, GeometryTile::LayoutResult { - std::move(nonSymbolBuckets), + std::move(buckets), std::move(featureIndex), *data ? (*data)->clone() : nullptr, - std::move(buckets), std::move(glyphAtlasImage), std::move(iconAtlasImage) }, correlationID); diff --git a/src/mbgl/tile/geometry_tile_worker.hpp b/src/mbgl/tile/geometry_tile_worker.hpp index 458ec76988..b5417c8114 100644 --- a/src/mbgl/tile/geometry_tile_worker.hpp +++ b/src/mbgl/tile/geometry_tile_worker.hpp @@ -67,7 +67,7 @@ private: const float pixelRatio; std::unique_ptr featureIndex; - std::unordered_map> nonSymbolBuckets; + std::unordered_map> buckets; enum State { Idle, -- cgit v1.2.1 From a2399a57a22aa453dbc5a9b16244bbc953e2d632 Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Sat, 31 Mar 2018 19:47:40 -0700 Subject: [core] Make FeatureIndex own GeometryTileData. Prevents querying a FeatureIndex built against a separate set of data, which can lead to invalid index exceptions. The GeometryTileWorker 'data' member can still change independently of the data in the feature index, at the time 'setData' is called. The GeometryTileWorker maintains ownership of its local data (which may be used to re-parse) and clones the data for use by the FeatureIndex in the foreground. --- src/mbgl/geometry/feature_index.cpp | 17 ++++++++++------- src/mbgl/geometry/feature_index.hpp | 7 ++++--- src/mbgl/tile/geometry_tile.cpp | 22 ++++++++++------------ src/mbgl/tile/geometry_tile.hpp | 8 ++------ src/mbgl/tile/geometry_tile_worker.cpp | 3 +-- 5 files changed, 27 insertions(+), 30 deletions(-) (limited to 'src') diff --git a/src/mbgl/geometry/feature_index.cpp b/src/mbgl/geometry/feature_index.cpp index 6fb0d5e446..c67786274a 100644 --- a/src/mbgl/geometry/feature_index.cpp +++ b/src/mbgl/geometry/feature_index.cpp @@ -17,8 +17,9 @@ namespace mbgl { -FeatureIndex::FeatureIndex() - : grid(util::EXTENT, util::EXTENT, util::EXTENT / 16) { // 16x16 grid -> 32px cell +FeatureIndex::FeatureIndex(std::unique_ptr tileData_) + : grid(util::EXTENT, util::EXTENT, util::EXTENT / 16) // 16x16 grid -> 32px cell + , tileData(std::move(tileData_)) { } void FeatureIndex::insert(const GeometryCollection& geometries, @@ -47,12 +48,15 @@ void FeatureIndex::query( const double tileSize, const double scale, const RenderedQueryOptions& queryOptions, - const GeometryTileData& geometryTileData, const UnwrappedTileID& tileID, const std::string& sourceID, const std::vector& layers, const CollisionIndex& collisionIndex, const float additionalQueryRadius) const { + + if (!tileData) { + return; + } // Determine query radius const float pixelsToTileUnits = util::EXTENT / tileSize / scale; @@ -72,13 +76,13 @@ void FeatureIndex::query( if (indexedFeature.sortIndex == previousSortIndex) continue; previousSortIndex = indexedFeature.sortIndex; - addFeature(result, indexedFeature, queryGeometry, queryOptions, geometryTileData, tileID.canonical, layers, bearing, pixelsToTileUnits); + addFeature(result, indexedFeature, queryGeometry, queryOptions, tileID.canonical, layers, bearing, pixelsToTileUnits); } std::vector symbolFeatures = collisionIndex.queryRenderedSymbols(queryGeometry, tileID, sourceID); std::sort(symbolFeatures.begin(), symbolFeatures.end(), topDownSymbols); for (const auto& symbolFeature : symbolFeatures) { - addFeature(result, symbolFeature, queryGeometry, queryOptions, geometryTileData, tileID.canonical, layers, bearing, pixelsToTileUnits); + addFeature(result, symbolFeature, queryGeometry, queryOptions, tileID.canonical, layers, bearing, pixelsToTileUnits); } } @@ -87,7 +91,6 @@ void FeatureIndex::addFeature( const IndexedSubfeature& indexedFeature, const GeometryCoordinates& queryGeometry, const RenderedQueryOptions& options, - const GeometryTileData& geometryTileData, const CanonicalTileID& tileID, const std::vector& layers, const float bearing, @@ -113,7 +116,7 @@ void FeatureIndex::addFeature( } if (!geometryTileFeature) { - sourceLayer = geometryTileData.getLayer(indexedFeature.sourceLayerName); + sourceLayer = tileData->getLayer(indexedFeature.sourceLayerName); assert(sourceLayer); geometryTileFeature = sourceLayer->getFeature(indexedFeature.index); diff --git a/src/mbgl/geometry/feature_index.hpp b/src/mbgl/geometry/feature_index.hpp index e95bb94da6..9e0c172342 100644 --- a/src/mbgl/geometry/feature_index.hpp +++ b/src/mbgl/geometry/feature_index.hpp @@ -50,8 +50,10 @@ public: class FeatureIndex { public: - FeatureIndex(); + FeatureIndex(std::unique_ptr tileData_); + const GeometryTileData* getData() { return tileData.get(); } + void insert(const GeometryCollection&, std::size_t index, const std::string& sourceLayerName, const std::string& bucketName); void query( @@ -61,7 +63,6 @@ public: const double tileSize, const double scale, const RenderedQueryOptions& options, - const GeometryTileData&, const UnwrappedTileID&, const std::string&, const std::vector&, @@ -83,7 +84,6 @@ private: const IndexedSubfeature&, const GeometryCoordinates& queryGeometry, const RenderedQueryOptions& options, - const GeometryTileData&, const CanonicalTileID&, const std::vector&, const float bearing, @@ -93,5 +93,6 @@ private: unsigned int sortIndex = 0; std::unordered_map> bucketLayerIDs; + std::unique_ptr tileData; }; } // namespace mbgl diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp index 5089f95022..a99cb91d26 100644 --- a/src/mbgl/tile/geometry_tile.cpp +++ b/src/mbgl/tile/geometry_tile.cpp @@ -133,7 +133,7 @@ void GeometryTile::onLayout(LayoutResult result, const uint64_t resultCorrelatio buckets = std::move(result.buckets); - dataPendingCommit = {{ std::move(result.tileData), std::move(result.featureIndex) }}; + featureIndexPendingCommit = { std::move(result.featureIndex) }; if (result.glyphAtlasImage) { glyphAtlasImage = std::move(*result.glyphAtlasImage); @@ -202,12 +202,11 @@ Bucket* GeometryTile::getBucket(const Layer::Impl& layer) const { } void GeometryTile::commitFeatureIndex() { - // We commit our pending FeatureIndex and GeometryTileData when a global placement has run, - // synchronizing the global CollisionIndex with the latest buckets/FeatureIndex/GeometryTileData - if (dataPendingCommit) { - data = std::move(dataPendingCommit->first); - featureIndex = std::move(dataPendingCommit->second); - dataPendingCommit = nullopt; + // We commit our pending FeatureIndex when a global placement has run, + // synchronizing the global CollisionIndex with the latest buckets/FeatureIndex + if (featureIndexPendingCommit) { + featureIndex = std::move(*featureIndexPendingCommit); + featureIndexPendingCommit = nullopt; } } @@ -219,7 +218,7 @@ void GeometryTile::queryRenderedFeatures( const RenderedQueryOptions& options, const CollisionIndex& collisionIndex) { - if (!featureIndex || !data) return; + if (!getData()) return; // Determine the additional radius needed factoring in property functions float additionalRadius = 0; @@ -236,7 +235,6 @@ void GeometryTile::queryRenderedFeatures( util::tileSize * id.overscaleFactor(), std::pow(2, transformState.getZoom() - id.overscaledZ), options, - *data, id.toUnwrapped(), sourceID, layers, @@ -248,8 +246,8 @@ void GeometryTile::querySourceFeatures( std::vector& result, const SourceQueryOptions& options) { - // Data not yet available - if (!data) { + // Data not yet available, or tile is empty + if (!getData()) { return; } @@ -262,7 +260,7 @@ void GeometryTile::querySourceFeatures( for (auto sourceLayer : *options.sourceLayers) { // Go throught all sourceLayers, if any // to gather all the features - auto layer = data->getLayer(sourceLayer); + auto layer = getData()->getLayer(sourceLayer); if (layer) { auto featureCount = layer->featureCount(); diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp index 0161e00efd..418db4a0b2 100644 --- a/src/mbgl/tile/geometry_tile.hpp +++ b/src/mbgl/tile/geometry_tile.hpp @@ -67,18 +67,15 @@ public: public: std::unordered_map> buckets; std::unique_ptr featureIndex; - std::unique_ptr tileData; optional glyphAtlasImage; optional iconAtlasImage; LayoutResult(std::unordered_map> buckets_, std::unique_ptr featureIndex_, - std::unique_ptr tileData_, optional glyphAtlasImage_, optional iconAtlasImage_) : buckets(std::move(buckets_)), featureIndex(std::move(featureIndex_)), - tileData(std::move(tileData_)), glyphAtlasImage(std::move(glyphAtlasImage_)), iconAtlasImage(std::move(iconAtlasImage_)) {} }; @@ -95,7 +92,7 @@ public: protected: const GeometryTileData* getData() { - return data.get(); + return featureIndex ? featureIndex->getData() : nullptr; } private: @@ -116,9 +113,8 @@ private: std::unordered_map> buckets; - optional, std::unique_ptr>> dataPendingCommit; + optional> featureIndexPendingCommit; std::unique_ptr featureIndex; - std::unique_ptr data; optional glyphAtlasImage; optional iconAtlasImage; diff --git a/src/mbgl/tile/geometry_tile_worker.cpp b/src/mbgl/tile/geometry_tile_worker.cpp index f57732117b..1378ad5d3a 100644 --- a/src/mbgl/tile/geometry_tile_worker.cpp +++ b/src/mbgl/tile/geometry_tile_worker.cpp @@ -329,7 +329,7 @@ void GeometryTileWorker::parse() { std::unordered_map> symbolLayoutMap; buckets.clear(); - featureIndex = std::make_unique(); + featureIndex = std::make_unique(*data ? (*data)->clone() : nullptr); BucketParameters parameters { id, mode, pixelRatio }; GlyphDependencies glyphDependencies; @@ -471,7 +471,6 @@ void GeometryTileWorker::performSymbolLayout() { parent.invoke(&GeometryTile::onLayout, GeometryTile::LayoutResult { std::move(buckets), std::move(featureIndex), - *data ? (*data)->clone() : nullptr, std::move(glyphAtlasImage), std::move(iconAtlasImage) }, correlationID); -- cgit v1.2.1 From 099fcc3b3c72b117e93d4da39ee6f70c6c5a2bc8 Mon Sep 17 00:00:00 2001 From: Asheem Mamoowala Date: Mon, 2 Apr 2018 16:15:25 -0700 Subject: Use a host interface for CustomLayer instead of function pointers (#11553) Use a host interface for CustomLayer instead of function pointers Co-authored-by: Julian Rex --- src/mbgl/renderer/layers/render_custom_layer.cpp | 29 +++++++++++------------- src/mbgl/renderer/layers/render_custom_layer.hpp | 3 +-- src/mbgl/style/layers/custom_layer.cpp | 16 ++----------- src/mbgl/style/layers/custom_layer_impl.cpp | 12 ++-------- src/mbgl/style/layers/custom_layer_impl.hpp | 14 ++++-------- 5 files changed, 22 insertions(+), 52 deletions(-) (limited to 'src') diff --git a/src/mbgl/renderer/layers/render_custom_layer.cpp b/src/mbgl/renderer/layers/render_custom_layer.cpp index a429b8d82e..85ca5b074e 100644 --- a/src/mbgl/renderer/layers/render_custom_layer.cpp +++ b/src/mbgl/renderer/layers/render_custom_layer.cpp @@ -12,17 +12,17 @@ namespace mbgl { using namespace style; RenderCustomLayer::RenderCustomLayer(Immutable _impl) - : RenderLayer(LayerType::Custom, _impl) { + : RenderLayer(LayerType::Custom, _impl), host(_impl->host) { + assert(BackendScope::exists()); + host->initialize(); } RenderCustomLayer::~RenderCustomLayer() { assert(BackendScope::exists()); - if (initialized) { - if (contextDestroyed && impl().contextLostFn ) { - impl().contextLostFn(impl().context); - } else if (!contextDestroyed && impl().deinitializeFn) { - impl().deinitializeFn(impl().context); - } + if (contextDestroyed) { + host->contextLost(); + } else { + host->deinitialize(); } } @@ -44,15 +44,13 @@ std::unique_ptr RenderCustomLayer::createBucket(const BucketParameters&, } void RenderCustomLayer::render(PaintParameters& paintParameters, RenderSource*) { - if (context != impl().context || !initialized) { + if (host != impl().host) { //If the context changed, deinitialize the previous one before initializing the new one. - if (context && !contextDestroyed && impl().deinitializeFn) { - MBGL_CHECK_ERROR(impl().deinitializeFn(context)); + if (host && !contextDestroyed) { + MBGL_CHECK_ERROR(host->deinitialize()); } - context = impl().context; - assert(impl().initializeFn); - MBGL_CHECK_ERROR(impl().initializeFn(impl().context)); - initialized = true; + host = impl().host; + MBGL_CHECK_ERROR(host->initialize()); } gl::Context& glContext = paintParameters.context; @@ -75,8 +73,7 @@ void RenderCustomLayer::render(PaintParameters& paintParameters, RenderSource*) parameters.pitch = state.getPitch(); parameters.fieldOfView = state.getFieldOfView(); - assert(impl().renderFn); - MBGL_CHECK_ERROR(impl().renderFn(context, parameters)); + MBGL_CHECK_ERROR(host->render(parameters)); // Reset the view back to our original one, just in case the CustomLayer changed // the viewport or Framebuffer. diff --git a/src/mbgl/renderer/layers/render_custom_layer.hpp b/src/mbgl/renderer/layers/render_custom_layer.hpp index 6d1fea99d3..971d8d8f42 100644 --- a/src/mbgl/renderer/layers/render_custom_layer.hpp +++ b/src/mbgl/renderer/layers/render_custom_layer.hpp @@ -24,9 +24,8 @@ public: }; private: - bool initialized = false; bool contextDestroyed = false; - void * context = nullptr; + std::shared_ptr host; }; template <> diff --git a/src/mbgl/style/layers/custom_layer.cpp b/src/mbgl/style/layers/custom_layer.cpp index 854c771847..0e51a70e50 100644 --- a/src/mbgl/style/layers/custom_layer.cpp +++ b/src/mbgl/style/layers/custom_layer.cpp @@ -6,20 +6,8 @@ namespace mbgl { namespace style { CustomLayer::CustomLayer(const std::string& layerID, - CustomLayerInitializeFunction init, - CustomLayerRenderFunction render, - CustomLayerContextLostFunction contextLost, - CustomLayerDeinitializeFunction deinit, - void* context) - : Layer(makeMutable(layerID, init, render, contextLost, deinit, context)) { -} - -CustomLayer::CustomLayer(const std::string& layerID, - CustomLayerInitializeFunction init, - CustomLayerRenderFunction render, - CustomLayerDeinitializeFunction deinit, - void* context) - : Layer(makeMutable(layerID, init, render, nullptr, deinit, context)) { + std::unique_ptr host) + : Layer(makeMutable(layerID, std::move(host))) { } CustomLayer::~CustomLayer() = default; diff --git a/src/mbgl/style/layers/custom_layer_impl.cpp b/src/mbgl/style/layers/custom_layer_impl.cpp index 1de268d2e2..05c41623c4 100644 --- a/src/mbgl/style/layers/custom_layer_impl.cpp +++ b/src/mbgl/style/layers/custom_layer_impl.cpp @@ -4,17 +4,9 @@ namespace mbgl { namespace style { CustomLayer::Impl::Impl(const std::string& id_, - CustomLayerInitializeFunction initializeFn_, - CustomLayerRenderFunction renderFn_, - CustomLayerContextLostFunction contextLostFn_, - CustomLayerDeinitializeFunction deinitializeFn_, - void* context_) + std::unique_ptr host_) : Layer::Impl(LayerType::Custom, id_, std::string()) { - initializeFn = initializeFn_; - renderFn = renderFn_; - deinitializeFn = deinitializeFn_; - contextLostFn = contextLostFn_; - context = context_; + host = std::move(host_); } bool CustomLayer::Impl::hasLayoutDifference(const Layer::Impl&) const { diff --git a/src/mbgl/style/layers/custom_layer_impl.hpp b/src/mbgl/style/layers/custom_layer_impl.hpp index 62efbbe15b..a41962c276 100644 --- a/src/mbgl/style/layers/custom_layer_impl.hpp +++ b/src/mbgl/style/layers/custom_layer_impl.hpp @@ -3,6 +3,8 @@ #include #include +#include + namespace mbgl { class TransformState; @@ -12,20 +14,12 @@ namespace style { class CustomLayer::Impl : public Layer::Impl { public: Impl(const std::string& id, - CustomLayerInitializeFunction, - CustomLayerRenderFunction, - CustomLayerContextLostFunction, - CustomLayerDeinitializeFunction, - void* context); + std::unique_ptr host); bool hasLayoutDifference(const Layer::Impl&) const override; void stringifyLayout(rapidjson::Writer&) const override; - CustomLayerInitializeFunction initializeFn = nullptr; - CustomLayerRenderFunction renderFn = nullptr; - CustomLayerContextLostFunction contextLostFn = nullptr; - CustomLayerDeinitializeFunction deinitializeFn = nullptr; - void* context = nullptr; + std::shared_ptr host; }; } // namespace style -- cgit v1.2.1 From dc495e1c9d831ccf928381845ffcb409c5d91271 Mon Sep 17 00:00:00 2001 From: Asheem Mamoowala Date: Wed, 4 Apr 2018 15:08:16 -0700 Subject: [core] Add Projection matrix to CustomLayerRenderParameters to transform web mercator world coordinates to GL coordinates. --- src/mbgl/renderer/layers/render_custom_layer.cpp | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src') diff --git a/src/mbgl/renderer/layers/render_custom_layer.cpp b/src/mbgl/renderer/layers/render_custom_layer.cpp index 85ca5b074e..be9f64d9eb 100644 --- a/src/mbgl/renderer/layers/render_custom_layer.cpp +++ b/src/mbgl/renderer/layers/render_custom_layer.cpp @@ -6,6 +6,7 @@ #include #include #include +#include namespace mbgl { @@ -72,6 +73,9 @@ void RenderCustomLayer::render(PaintParameters& paintParameters, RenderSource*) parameters.bearing = -state.getAngle() * util::RAD2DEG; parameters.pitch = state.getPitch(); parameters.fieldOfView = state.getFieldOfView(); + mat4 projMatrix; + state.getProjMatrix(projMatrix); + parameters.projectionMatrix = projMatrix; MBGL_CHECK_ERROR(host->render(parameters)); -- cgit v1.2.1 From 6f639dba2028e02c85a28f32fcc854ee353f5b25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Paczos?= Date: Fri, 6 Apr 2018 16:28:36 +0200 Subject: [core] re-bind uniform locations after re-linking program (#11618) - On some gl implementations the uniform locations are shifted after linking the program a second time, resulting in errors when using any uniform. On some implementations they are actually doubled. Re-binding the uniforms after selectively binding the vertex attributes prevents this. (cherry picked from commit 4a0e025) --- src/mbgl/gl/program.hpp | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src') diff --git a/src/mbgl/gl/program.hpp b/src/mbgl/gl/program.hpp index 3b54ec194a..af02ad3d54 100644 --- a/src/mbgl/gl/program.hpp +++ b/src/mbgl/gl/program.hpp @@ -36,8 +36,13 @@ public: context.createShader(ShaderType::Fragment, fragmentSource))), uniformsState((context.linkProgram(program), Uniforms::bindLocations(program))), attributeLocations(Attributes::bindLocations(program)) { + // Re-link program after manually binding only active attributes in Attributes::bindLocations context.linkProgram(program); + + // We have to re-initialize the uniforms state from the bindings as the uniform locations + // get shifted on some implementations + uniformsState = Uniforms::bindLocations(program); } template -- cgit v1.2.1 From 8d26f7409106c94f37689d206f9a68aac57572a4 Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Mon, 9 Apr 2018 17:59:18 -0400 Subject: Fix style parsing bug for constant expressions (#11606) * Fix style parsing bug for constant expressions Closes #10849 * Ignore tests for unported GL JS change Refs https://github.com/mapbox/mapbox-gl-js/pull/6429 * Fuller fix * Update mapbox-gl-js --- src/mbgl/style/expression/parsing_context.cpp | 41 ++++++++++++++++++--------- 1 file changed, 27 insertions(+), 14 deletions(-) (limited to 'src') diff --git a/src/mbgl/style/expression/parsing_context.cpp b/src/mbgl/style/expression/parsing_context.cpp index 364c3f740a..713c0b5d27 100644 --- a/src/mbgl/style/expression/parsing_context.cpp +++ b/src/mbgl/style/expression/parsing_context.cpp @@ -32,23 +32,36 @@ namespace style { namespace expression { bool isConstant(const Expression& expression) { - if (dynamic_cast(&expression)) { - return false; + if (auto varExpression = dynamic_cast(&expression)) { + return isConstant(*varExpression->getBoundExpression()); } - + if (auto compound = dynamic_cast(&expression)) { if (compound->getName() == "error") { return false; } } + + bool isTypeAnnotation = dynamic_cast(&expression) || + dynamic_cast(&expression) || + dynamic_cast(&expression); - bool literalArgs = true; + bool childrenConstant = true; expression.eachChild([&](const Expression& child) { - if (!dynamic_cast(&child)) { - literalArgs = false; + // We can _almost_ assume that if `expressions` children are constant, + // they would already have been evaluated to Literal values when they + // were parsed. Type annotations are the exception, because they might + // have been inferred and added after a child was parsed. + + // So we recurse into isConstant() for the children of type annotations, + // but otherwise simply check whether they are Literals. + if (isTypeAnnotation) { + childrenConstant = childrenConstant && isConstant(child); + } else { + childrenConstant = childrenConstant && dynamic_cast(&child); } }); - if (!literalArgs) { + if (!childrenConstant) { return false; } @@ -141,13 +154,13 @@ ParseResult ParsingContext::parse(const Convertible& value, TypeAnnotationOption return parsed; } - if (expected) { - auto array = [&](std::unique_ptr expression) { - std::vector> args; - args.push_back(std::move(expression)); - return args; - }; + auto array = [&](std::unique_ptr expression) { + std::vector> args; + args.push_back(std::move(expression)); + return args; + }; + if (expected) { const type::Type actual = (*parsed)->getType(); if ((*expected == type::String || *expected == type::Number || *expected == type::Boolean || *expected == type::Object) && actual == type::Value) { if (typeAnnotationOption == includeTypeAnnotations) { @@ -169,7 +182,7 @@ ParseResult ParsingContext::parse(const Convertible& value, TypeAnnotationOption } } - // If an expression's arguments are all literals, we can evaluate + // If an expression's arguments are all constant, we can evaluate // it immediately and replace it with a literal value in the // parsed result. if (!dynamic_cast(parsed->get()) && isConstant(**parsed)) { -- cgit v1.2.1 From f0f67b4788f967ee34f307312db302cecb68dd0b Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Wed, 11 Apr 2018 12:03:56 -0400 Subject: Add abs, round, floor, ceil operators (#11653) * Add abs, round, floor, ceil operators Port of https://github.com/mapbox/mapbox-gl-js/pull/6496 * [ios, macos] Simplified abs, ceiling, floor expressions * [ios, macos] Added rounding expression function * [android] - binding integration for round, ceil, floor and abs expressions * Update mapbox-gl-js to include non-integer rounding test * Drop extra braces * mapbox-gl-js -> master * Update style-spec docs -> PropertyFactory.java --- src/mbgl/style/expression/compound_expression.cpp | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src') diff --git a/src/mbgl/style/expression/compound_expression.cpp b/src/mbgl/style/expression/compound_expression.cpp index 77b294b380..c36ffa33e3 100644 --- a/src/mbgl/style/expression/compound_expression.cpp +++ b/src/mbgl/style/expression/compound_expression.cpp @@ -6,6 +6,7 @@ #include #include #include +#include namespace mbgl { namespace style { @@ -362,6 +363,11 @@ std::unordered_map initiali return result; }); + define("round", [](double x) -> Result { return std::round(x); }); + define("floor", [](double x) -> Result { return std::floor(x); }); + define("ceil", [](double x) -> Result { return std::ceil(x); }); + define("abs", [](double x) -> Result { return std::abs(x); }); + define(">", [](double lhs, double rhs) -> Result { return lhs > rhs; }); define(">", [](const std::string& lhs, const std::string& rhs) -> Result { return lhs > rhs; }); define(">=", [](double lhs, double rhs) -> Result { return lhs >= rhs; }); -- cgit v1.2.1 From 22b4ef1f4b0ea053068d45bd2b4cffd7f935a250 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Paczos?= Date: Thu, 12 Apr 2018 10:42:14 +0200 Subject: Update layer immediately when changing its max/min zoom level (#11399) * [android][core] update layer immediately when changing it's max/min zoom * [core] node bindings for layer zoom range --- src/mbgl/style/layers/background_layer.cpp | 2 ++ src/mbgl/style/layers/circle_layer.cpp | 2 ++ src/mbgl/style/layers/fill_extrusion_layer.cpp | 2 ++ src/mbgl/style/layers/fill_layer.cpp | 2 ++ src/mbgl/style/layers/heatmap_layer.cpp | 2 ++ src/mbgl/style/layers/hillshade_layer.cpp | 2 ++ src/mbgl/style/layers/layer.cpp.ejs | 2 ++ src/mbgl/style/layers/line_layer.cpp | 2 ++ src/mbgl/style/layers/raster_layer.cpp | 2 ++ src/mbgl/style/layers/symbol_layer.cpp | 2 ++ 10 files changed, 20 insertions(+) (limited to 'src') diff --git a/src/mbgl/style/layers/background_layer.cpp b/src/mbgl/style/layers/background_layer.cpp index d4ead18816..66ab46c078 100644 --- a/src/mbgl/style/layers/background_layer.cpp +++ b/src/mbgl/style/layers/background_layer.cpp @@ -53,12 +53,14 @@ void BackgroundLayer::setMinZoom(float minZoom) { auto impl_ = mutableImpl(); impl_->minZoom = minZoom; baseImpl = std::move(impl_); + observer->onLayerChanged(*this); } void BackgroundLayer::setMaxZoom(float maxZoom) { auto impl_ = mutableImpl(); impl_->maxZoom = maxZoom; baseImpl = std::move(impl_); + observer->onLayerChanged(*this); } // Layout properties diff --git a/src/mbgl/style/layers/circle_layer.cpp b/src/mbgl/style/layers/circle_layer.cpp index 9854932699..6dd744df1f 100644 --- a/src/mbgl/style/layers/circle_layer.cpp +++ b/src/mbgl/style/layers/circle_layer.cpp @@ -81,12 +81,14 @@ void CircleLayer::setMinZoom(float minZoom) { auto impl_ = mutableImpl(); impl_->minZoom = minZoom; baseImpl = std::move(impl_); + observer->onLayerChanged(*this); } void CircleLayer::setMaxZoom(float maxZoom) { auto impl_ = mutableImpl(); impl_->maxZoom = maxZoom; baseImpl = std::move(impl_); + observer->onLayerChanged(*this); } // Layout properties diff --git a/src/mbgl/style/layers/fill_extrusion_layer.cpp b/src/mbgl/style/layers/fill_extrusion_layer.cpp index 62f92cef75..c5b4ef0ef3 100644 --- a/src/mbgl/style/layers/fill_extrusion_layer.cpp +++ b/src/mbgl/style/layers/fill_extrusion_layer.cpp @@ -81,12 +81,14 @@ void FillExtrusionLayer::setMinZoom(float minZoom) { auto impl_ = mutableImpl(); impl_->minZoom = minZoom; baseImpl = std::move(impl_); + observer->onLayerChanged(*this); } void FillExtrusionLayer::setMaxZoom(float maxZoom) { auto impl_ = mutableImpl(); impl_->maxZoom = maxZoom; baseImpl = std::move(impl_); + observer->onLayerChanged(*this); } // Layout properties diff --git a/src/mbgl/style/layers/fill_layer.cpp b/src/mbgl/style/layers/fill_layer.cpp index 65975752db..99a2a51ed0 100644 --- a/src/mbgl/style/layers/fill_layer.cpp +++ b/src/mbgl/style/layers/fill_layer.cpp @@ -81,12 +81,14 @@ void FillLayer::setMinZoom(float minZoom) { auto impl_ = mutableImpl(); impl_->minZoom = minZoom; baseImpl = std::move(impl_); + observer->onLayerChanged(*this); } void FillLayer::setMaxZoom(float maxZoom) { auto impl_ = mutableImpl(); impl_->maxZoom = maxZoom; baseImpl = std::move(impl_); + observer->onLayerChanged(*this); } // Layout properties diff --git a/src/mbgl/style/layers/heatmap_layer.cpp b/src/mbgl/style/layers/heatmap_layer.cpp index 4989ff15f1..3f7881ddd3 100644 --- a/src/mbgl/style/layers/heatmap_layer.cpp +++ b/src/mbgl/style/layers/heatmap_layer.cpp @@ -85,12 +85,14 @@ void HeatmapLayer::setMinZoom(float minZoom) { auto impl_ = mutableImpl(); impl_->minZoom = minZoom; baseImpl = std::move(impl_); + observer->onLayerChanged(*this); } void HeatmapLayer::setMaxZoom(float maxZoom) { auto impl_ = mutableImpl(); impl_->maxZoom = maxZoom; baseImpl = std::move(impl_); + observer->onLayerChanged(*this); } // Layout properties diff --git a/src/mbgl/style/layers/hillshade_layer.cpp b/src/mbgl/style/layers/hillshade_layer.cpp index ea736af1ad..e352ae090c 100644 --- a/src/mbgl/style/layers/hillshade_layer.cpp +++ b/src/mbgl/style/layers/hillshade_layer.cpp @@ -59,12 +59,14 @@ void HillshadeLayer::setMinZoom(float minZoom) { auto impl_ = mutableImpl(); impl_->minZoom = minZoom; baseImpl = std::move(impl_); + observer->onLayerChanged(*this); } void HillshadeLayer::setMaxZoom(float maxZoom) { auto impl_ = mutableImpl(); impl_->maxZoom = maxZoom; baseImpl = std::move(impl_); + observer->onLayerChanged(*this); } // Layout properties diff --git a/src/mbgl/style/layers/layer.cpp.ejs b/src/mbgl/style/layers/layer.cpp.ejs index 657a7f5a8a..6d748311bf 100644 --- a/src/mbgl/style/layers/layer.cpp.ejs +++ b/src/mbgl/style/layers/layer.cpp.ejs @@ -108,12 +108,14 @@ void <%- camelize(type) %>Layer::setMinZoom(float minZoom) { auto impl_ = mutableImpl(); impl_->minZoom = minZoom; baseImpl = std::move(impl_); + observer->onLayerChanged(*this); } void <%- camelize(type) %>Layer::setMaxZoom(float maxZoom) { auto impl_ = mutableImpl(); impl_->maxZoom = maxZoom; baseImpl = std::move(impl_); + observer->onLayerChanged(*this); } // Layout properties diff --git a/src/mbgl/style/layers/line_layer.cpp b/src/mbgl/style/layers/line_layer.cpp index 1c7f0d28ee..56eac34c00 100644 --- a/src/mbgl/style/layers/line_layer.cpp +++ b/src/mbgl/style/layers/line_layer.cpp @@ -82,12 +82,14 @@ void LineLayer::setMinZoom(float minZoom) { auto impl_ = mutableImpl(); impl_->minZoom = minZoom; baseImpl = std::move(impl_); + observer->onLayerChanged(*this); } void LineLayer::setMaxZoom(float maxZoom) { auto impl_ = mutableImpl(); impl_->maxZoom = maxZoom; baseImpl = std::move(impl_); + observer->onLayerChanged(*this); } // Layout properties diff --git a/src/mbgl/style/layers/raster_layer.cpp b/src/mbgl/style/layers/raster_layer.cpp index a9a8d273fa..36b2e3e027 100644 --- a/src/mbgl/style/layers/raster_layer.cpp +++ b/src/mbgl/style/layers/raster_layer.cpp @@ -59,12 +59,14 @@ void RasterLayer::setMinZoom(float minZoom) { auto impl_ = mutableImpl(); impl_->minZoom = minZoom; baseImpl = std::move(impl_); + observer->onLayerChanged(*this); } void RasterLayer::setMaxZoom(float maxZoom) { auto impl_ = mutableImpl(); impl_->maxZoom = maxZoom; baseImpl = std::move(impl_); + observer->onLayerChanged(*this); } // Layout properties diff --git a/src/mbgl/style/layers/symbol_layer.cpp b/src/mbgl/style/layers/symbol_layer.cpp index d1a1ba246e..c940f3b00a 100644 --- a/src/mbgl/style/layers/symbol_layer.cpp +++ b/src/mbgl/style/layers/symbol_layer.cpp @@ -82,12 +82,14 @@ void SymbolLayer::setMinZoom(float minZoom) { auto impl_ = mutableImpl(); impl_->minZoom = minZoom; baseImpl = std::move(impl_); + observer->onLayerChanged(*this); } void SymbolLayer::setMaxZoom(float maxZoom) { auto impl_ = mutableImpl(); impl_->maxZoom = maxZoom; baseImpl = std::move(impl_); + observer->onLayerChanged(*this); } // Layout properties -- cgit v1.2.1 From 352d63d2ce98286cd854b11e85d66440501a6fd0 Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Thu, 12 Apr 2018 16:02:48 -0400 Subject: Don't enforce ["zoom"] constraints for filters (#11672) * Don't enforce ["zoom"] constraints for filters Fixes #11594 * Add a couple of comments * Fix ambiguous constructor call * ParsingContext(optional) => ParsingContext(Type) --- src/mbgl/style/conversion/filter.cpp | 7 +++++-- src/mbgl/style/expression/parsing_context.cpp | 27 ++++++++++++++++++++++++--- 2 files changed, 29 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/mbgl/style/conversion/filter.cpp b/src/mbgl/style/conversion/filter.cpp index fba149da12..3c941945fd 100644 --- a/src/mbgl/style/conversion/filter.cpp +++ b/src/mbgl/style/conversion/filter.cpp @@ -2,7 +2,7 @@ #include #include #include -#include +#include namespace mbgl { namespace style { @@ -236,10 +236,13 @@ optional convertCompoundFilter(const Convertible& value, Error& error) { } optional convertExpressionFilter(const Convertible& value, Error& error) { - optional> expression = convert>(value, error, expression::type::Boolean); + expression::ParsingContext ctx(expression::type::Boolean); + expression::ParseResult expression = ctx.parseExpression(value); if (!expression) { + error = { ctx.getCombinedErrors() }; return {}; } + return { ExpressionFilter { std::move(*expression) } }; } diff --git a/src/mbgl/style/expression/parsing_context.cpp b/src/mbgl/style/expression/parsing_context.cpp index 713c0b5d27..b522aeff9a 100644 --- a/src/mbgl/style/expression/parsing_context.cpp +++ b/src/mbgl/style/expression/parsing_context.cpp @@ -206,8 +206,16 @@ ParseResult ParsingContext::parse(const Convertible& value, TypeAnnotationOption } } - // if this is the root expression, enforce constraints on the use ["zoom"]. - if (key.size() == 0 && !isZoomConstant(**parsed)) { + return parsed; +} + +ParseResult ParsingContext::parseExpression(const Convertible& value, TypeAnnotationOption typeAnnotationOption) { + return parse(value, typeAnnotationOption); +} + +ParseResult ParsingContext::parseLayerPropertyExpression(const Convertible& value, TypeAnnotationOption typeAnnotationOption) { + ParseResult parsed = parse(value, typeAnnotationOption); + if (parsed && !isZoomConstant(**parsed)) { optional> zoomCurve = findZoomCurve(parsed->get()); if (!zoomCurve) { error(R"("zoom" expression may only be used as input to a top-level "step" or "interpolate" expression.)"); @@ -217,10 +225,23 @@ ParseResult ParsingContext::parse(const Convertible& value, TypeAnnotationOption return ParseResult(); } } - return parsed; } +const std::string ParsingContext::getCombinedErrors() const { + std::string combinedError; + for (const ParsingError& parsingError : *errors) { + if (combinedError.size() > 0) { + combinedError += "\n"; + } + if (parsingError.key.size() > 0) { + combinedError += parsingError.key + ": "; + } + combinedError += parsingError.message; + } + return combinedError; +} + optional ParsingContext::checkType(const type::Type& t) { assert(expected); optional err = type::checkSubtype(*expected, t); -- cgit v1.2.1 From b414c607c3369641fca23c79dfaff231a66a89cf Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Wed, 18 Apr 2018 17:44:24 -0700 Subject: Don't crash on placing symbols with 0 renderable glyphs Fixes issue #11729. Close relative of issue #10956. It is possible for us to receive Glyphs from the server that are valid, but have an invalid bitmap. In that case, the glyphs will be present in the `GlyphMap` used for shaping, but not present in the `GlyphPositions` used in `getGlyphQuads`. `SymbolInstance::hasText` looked at the shaping instead of the actual quads. `symbol_projection.cpp` should never try to project a label without any quads, but we'll also try to make it so that it doesn't crash if it does. --- src/mbgl/layout/symbol_instance.cpp | 4 +++- src/mbgl/layout/symbol_projection.cpp | 6 +++++- 2 files changed, 8 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/mbgl/layout/symbol_instance.cpp b/src/mbgl/layout/symbol_instance.cpp index 6e152349ca..7dfa8edf43 100644 --- a/src/mbgl/layout/symbol_instance.cpp +++ b/src/mbgl/layout/symbol_instance.cpp @@ -27,7 +27,7 @@ SymbolInstance::SymbolInstance(Anchor& anchor_, anchor(anchor_), line(line_), index(index_), - hasText(shapedTextOrientations.first || shapedTextOrientations.second), + hasText(false), hasIcon(shapedIcon), // Create the collision features that will be used to check whether this symbol instance can be placed @@ -48,6 +48,8 @@ SymbolInstance::SymbolInstance(Anchor& anchor_, if (shapedTextOrientations.second) { verticalGlyphQuads = getGlyphQuads(shapedTextOrientations.second, layout, textPlacement, positions); } + // 'hasText' depends on finding at least one glyph in the shaping that's also in the GlyphPositionMap + hasText = horizontalGlyphQuads.size() > 0 || verticalGlyphQuads.size() > 0; if (shapedTextOrientations.first && shapedTextOrientations.second) { writingModes = WritingModeType::Horizontal | WritingModeType::Vertical; diff --git a/src/mbgl/layout/symbol_projection.cpp b/src/mbgl/layout/symbol_projection.cpp index 9e077e2532..ef669c6e19 100644 --- a/src/mbgl/layout/symbol_projection.cpp +++ b/src/mbgl/layout/symbol_projection.cpp @@ -240,7 +240,11 @@ namespace mbgl { const PlacedSymbol& symbol, const mat4& labelPlaneMatrix, const bool returnTileDistance) { - + if (symbol.glyphOffsets.empty()) { + assert(false); + return optional>(); + } + const float firstGlyphOffset = symbol.glyphOffsets.front(); const float lastGlyphOffset = symbol.glyphOffsets.back();; -- cgit v1.2.1