[core] Fix placement for updated buckets

include/mbgl/storage/resource.hpp

@@ -1,11 +1,10 @@
 #pragma once
 
 #include <mbgl/storage/response.hpp>
+#include <mbgl/util/bitmask_operations.hpp>
 #include <mbgl/util/optional.hpp>
 #include <mbgl/util/font_stack.hpp>
 #include <mbgl/util/tileset.hpp>
-#include <mbgl/util/util.hpp>
-#include <mbgl/util/traits.hpp>
 
 #include <string>
 
@@ -98,21 +97,8 @@ class Resource {
     std::shared_ptr<const std::string> priorData;
 };
 
-
-MBGL_CONSTEXPR Resource::LoadingMethod operator|(Resource::LoadingMethod a, Resource::LoadingMethod b) {
-    return Resource::LoadingMethod(mbgl::underlying_type(a) | mbgl::underlying_type(b));
-}
-
-MBGL_CONSTEXPR Resource::LoadingMethod& operator|=(Resource::LoadingMethod& a, Resource::LoadingMethod b) {
-    return (a = a | b);
-}
-
-MBGL_CONSTEXPR Resource::LoadingMethod operator&(Resource::LoadingMethod a, Resource::LoadingMethod b) {
-    return Resource::LoadingMethod(mbgl::underlying_type(a) & mbgl::underlying_type(b));
-}
-
 inline bool Resource::hasLoadingMethod(Resource::LoadingMethod method) {
-    return (loadingMethod & method) != Resource::LoadingMethod::None;
+    return (loadingMethod & method);
 }
 
 } // namespace mbgl

include/mbgl/util/bitmask_operations.hpp

@@ -0,0 +1,33 @@
+#pragma once
+
+#include <mbgl/util/traits.hpp>
+#include <mbgl/util/util.hpp>
+
+namespace mbgl {
+
+template<typename Enum>
+MBGL_CONSTEXPR Enum operator|(Enum a, Enum b) {
+    static_assert(std::is_enum<Enum>::value, "Enum must be an enum type");
+    return Enum(mbgl::underlying_type(a) | mbgl::underlying_type(b));
+}
+
+template<typename Enum>
+MBGL_CONSTEXPR Enum& operator|=(Enum& a, Enum b) {
+    static_assert(std::is_enum<Enum>::value, "Enum must be an enum type");
+    return (a = a | b);
+}
+
+template<typename Enum>
+MBGL_CONSTEXPR bool operator&(Enum a, Enum b) {
+    static_assert(std::is_enum<Enum>::value, "Enum must be an enum type");
+    return bool(mbgl::underlying_type(a) & mbgl::underlying_type(b));
+}
+
+template<typename Enum>
+MBGL_CONSTEXPR Enum operator~(Enum value) {
+    static_assert(std::is_enum<Enum>::value, "Enum must be an enum type");
+    return Enum(~mbgl::underlying_type(value));
+}
+
+
+} // namespace mbgl

platform/android/CHANGELOG.md

@@ -11,6 +11,9 @@ Mapbox welcomes participation and contributions from everyone.  If you'd like to
 ## 8.4.0-alpha.2 - September 11, 2019
 [Changes](https://github.com/mapbox/mapbox-gl-native/compare/android-v8.4.0-alpha.1...android-v8.4.0-alpha.2) since [Mapbox Maps SDK for Android v8.4.0](https://github.com/mapbox/mapbox-gl-native/releases/tag/android-v8.4.0-alpha.1): 
 
+### Performance improvements
+ - Newly loaded labels appear faster on the screen. [#15308](https://github.com/mapbox/mapbox-gl-native/pull/15308)
+
 ### Bug fixes
  - Fixed an issue of integer overflow when converting `tileCoordinates` to `LatLon`, which caused issues such as `queryRenderedFeatures` and `querySourceFeatures` returning incorrect coordinates at zoom levels 20 and higher. [#15560](https://github.com/mapbox/mapbox-gl-native/pull/15560)
 

platform/ios/CHANGELOG.md

@@ -2,6 +2,12 @@
 
 Mapbox welcomes participation and contributions from everyone. Please read [CONTRIBUTING.md](../../CONTRIBUTING.md) to get started.
 
+## master
+
+### Performance improvements
+
+* Newly loaded labels appear faster on the screen. ([#15308](https://github.com/mapbox/mapbox-gl-native/pull/15308))
+
 ## 5.4.0
 
 ### Styles and rendering

src/core-files.json

@@ -456,6 +456,7 @@
         "mbgl/tile/tile_necessity.hpp": "include/mbgl/tile/tile_necessity.hpp",
         "mbgl/util/async_request.hpp": "include/mbgl/util/async_request.hpp",
         "mbgl/util/async_task.hpp": "include/mbgl/util/async_task.hpp",
+        "mbgl/util/bitmask_operations.hpp": "include/mbgl/util/bitmask_operations.hpp",
         "mbgl/util/char_array_buffer.hpp": "include/mbgl/util/char_array_buffer.hpp",
         "mbgl/util/chrono.hpp": "include/mbgl/util/chrono.hpp",
         "mbgl/util/color.hpp": "include/mbgl/util/color.hpp",

src/mbgl/layout/symbol_instance.hpp

@@ -4,8 +4,7 @@
 #include <mbgl/text/glyph_atlas.hpp>
 #include <mbgl/text/collision_feature.hpp>
 #include <mbgl/style/layers/symbol_layer_properties.hpp>
-#include <mbgl/util/traits.hpp>
-#include <mbgl/util/util.hpp>
+#include <mbgl/util/bitmask_operations.hpp>
 
 namespace mbgl {
 
@@ -50,18 +49,6 @@ enum class SymbolContent : uint8_t {
     IconSDF = 1 << 2
 };
 
-MBGL_CONSTEXPR SymbolContent operator|(SymbolContent a, SymbolContent b) {
-    return SymbolContent(mbgl::underlying_type(a) | mbgl::underlying_type(b));
-}
-
-MBGL_CONSTEXPR SymbolContent& operator|=(SymbolContent& a, SymbolContent b) {
-    return (a = a | b);
-}
-
-MBGL_CONSTEXPR SymbolContent operator&(SymbolContent a, SymbolContent b) {
-    return SymbolContent(mbgl::underlying_type(a) & mbgl::underlying_type(b));
-}
-
 class SymbolInstance {
 public:
     SymbolInstance(Anchor& anchor_,

src/mbgl/renderer/render_layer.cpp

@@ -32,7 +32,7 @@ const std::string& RenderLayer::getID() const {
 }
 
 bool RenderLayer::hasRenderPass(RenderPass pass) const {
-    return bool(passes & pass);
+    return passes & pass;
 }
 
 bool RenderLayer::needsRendering() const {

src/mbgl/renderer/render_orchestrator.cpp

@@ -360,27 +360,32 @@ std::unique_ptr<RenderTree> RenderOrchestrator::createRenderTree(const UpdatePar
             crossTileSymbolIndex.reset();
         }
 
+        bool symbolBucketsAdded = false;
         bool symbolBucketsChanged = false;
+        for (auto it = layersNeedPlacement.rbegin(); it != layersNeedPlacement.rend(); ++it) {
+            auto result = crossTileSymbolIndex.addLayer(*it, updateParameters.transformState.getLatLng().longitude()); 
+            symbolBucketsAdded = symbolBucketsAdded || (result & CrossTileSymbolIndex::AddLayerResult::BucketsAdded);
+            symbolBucketsChanged = symbolBucketsChanged || (result != CrossTileSymbolIndex::AddLayerResult::NoChanges);
+        }
+        // We want new symbols to show up faster, however simple setting `placementChanged` to `true` would
+        // initiate placement too often as new buckets ususally come from several rendered tiles in a row within
+        // a short period of time. Instead, we squeeze placement update period to coalesce buckets updates from several tiles.
+        if (symbolBucketsAdded) placement->setMaximumUpdatePeriod(Milliseconds(30));
         renderTreeParameters->placementChanged = !placement->stillRecent(updateParameters.timePoint, updateParameters.transformState.getZoom());
+
         std::set<std::string> usedSymbolLayers;
         if (renderTreeParameters->placementChanged) {
             placement = std::make_unique<Placement>(
                 updateParameters.transformState, updateParameters.mode,
                 updateParameters.transitionOptions, updateParameters.crossSourceCollisions,
                 std::move(placement));
-        }
 
-        for (auto it = layersNeedPlacement.rbegin(); it != layersNeedPlacement.rend(); ++it) {
-            const RenderLayer& layer = *it;
-            if (crossTileSymbolIndex.addLayer(layer, updateParameters.transformState.getLatLng().longitude())) symbolBucketsChanged = true;
-
-            if (renderTreeParameters->placementChanged) {
+            for (auto it = layersNeedPlacement.rbegin(); it != layersNeedPlacement.rend(); ++it) {
+                const RenderLayer& layer = *it;
                 usedSymbolLayers.insert(layer.getID());
                 placement->placeLayer(layer, renderTreeParameters->transformParams.projMatrix, updateParameters.debugOptions & MapDebugOptions::Collision);
             }
-        }
 
-        if (renderTreeParameters->placementChanged) {
             placement->commit(updateParameters.timePoint, updateParameters.transformState.getZoom());
             crossTileSymbolIndex.pruneUnusedLayers(usedSymbolLayers);
             for (const auto& entry : renderSources) {

src/mbgl/renderer/render_pass.hpp

@@ -1,7 +1,6 @@
 #pragma once
 
-#include <mbgl/util/traits.hpp>
-#include <mbgl/util/util.hpp>
+#include <mbgl/util/bitmask_operations.hpp>
 
 #include <cstdint>
 
@@ -14,18 +13,6 @@ enum class RenderPass : uint8_t {
     Pass3D = 1 << 2,
 };
 
-MBGL_CONSTEXPR RenderPass operator|(RenderPass a, RenderPass b) {
-    return RenderPass(mbgl::underlying_type(a) | mbgl::underlying_type(b));
-}
-
-MBGL_CONSTEXPR RenderPass& operator|=(RenderPass& a, RenderPass b) {
-    return (a = a | b);
-}
-
-MBGL_CONSTEXPR RenderPass operator&(RenderPass a, RenderPass b) {
-    return RenderPass(mbgl::underlying_type(a) & mbgl::underlying_type(b));
-}
-
 // Defines whether the overdraw shaders should be used instead of the regular shaders.
 enum class PaintMode : bool {
     Regular = false,

src/mbgl/text/cross_tile_symbol_index.cpp

@@ -163,27 +163,26 @@ bool CrossTileSymbolLayerIndex::removeStaleBuckets(const std::unordered_set<uint
 
 CrossTileSymbolIndex::CrossTileSymbolIndex() = default;
 
-bool CrossTileSymbolIndex::addLayer(const RenderLayer& layer, float lng) {
+auto CrossTileSymbolIndex::addLayer(const RenderLayer& layer, float lng) -> AddLayerResult {
     auto& layerIndex = layerIndexes[layer.getID()];
 
-    bool symbolBucketsChanged = false;
+    AddLayerResult result = AddLayerResult::NoChanges;
     std::unordered_set<uint32_t> currentBucketIDs;
 
     layerIndex.handleWrapJump(lng);
 
     for (const auto& item : layer.getPlacementData()) {
         const RenderTile& renderTile = item.tile;
         Bucket& bucket = item.bucket;
-        auto result = bucket.registerAtCrossTileIndex(layerIndex, renderTile.getOverscaledTileID(), maxCrossTileID);
-        assert(result.first != 0u);
-        symbolBucketsChanged = symbolBucketsChanged || result.second;
-        currentBucketIDs.insert(result.first);
+        auto pair = bucket.registerAtCrossTileIndex(layerIndex, renderTile.getOverscaledTileID(), maxCrossTileID);
+        assert(pair.first != 0u);
+        if (pair.second) result |= AddLayerResult::BucketsAdded;
+        currentBucketIDs.insert(pair.first);
     }
 
-    if (layerIndex.removeStaleBuckets(currentBucketIDs)) {
-        symbolBucketsChanged = true;
-    }
-    return symbolBucketsChanged;
+    if (layerIndex.removeStaleBuckets(currentBucketIDs)) result |= AddLayerResult::BucketsRemoved;
+
+    return result;
 }
 
 void CrossTileSymbolIndex::pruneUnusedLayers(const std::set<std::string>& usedLayers) {

src/mbgl/text/cross_tile_symbol_index.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <mbgl/tile/tile_id.hpp>
+#include <mbgl/util/bitmask_operations.hpp>
 #include <mbgl/util/geometry.hpp>
 #include <mbgl/util/constants.hpp>
 #include <mbgl/util/optional.hpp>
@@ -58,7 +59,13 @@ class CrossTileSymbolIndex {
 public:
     CrossTileSymbolIndex();
 
-    bool addLayer(const RenderLayer& layer, float lng);
+    enum class AddLayerResult : uint8_t {
+        NoChanges = 0,
+        BucketsAdded = 1 << 0,
+        BucketsRemoved = 1 << 1
+    };
+
+    AddLayerResult addLayer(const RenderLayer& layer, float lng);
     void pruneUnusedLayers(const std::set<std::string>&);
 
     void reset();

src/mbgl/text/glyph.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <mbgl/text/glyph_range.hpp>
+#include <mbgl/util/bitmask_operations.hpp>
 #include <mbgl/util/font_stack.hpp>
 #include <mbgl/util/rect.hpp>
 #include <mbgl/util/traits.hpp>
@@ -99,26 +100,6 @@ enum class WritingModeType : uint8_t {
     Vertical = 1 << 1,
 };
 
-MBGL_CONSTEXPR WritingModeType operator|(WritingModeType a, WritingModeType b) {
-    return WritingModeType(mbgl::underlying_type(a) | mbgl::underlying_type(b));
-}
-
-MBGL_CONSTEXPR WritingModeType& operator|=(WritingModeType& a, WritingModeType b) {
-    return (a = a | b);
-}
-
-MBGL_CONSTEXPR bool operator&(WritingModeType lhs, WritingModeType rhs) {
-    return mbgl::underlying_type(lhs) & mbgl::underlying_type(rhs);
-}
-
-MBGL_CONSTEXPR WritingModeType& operator&=(WritingModeType& lhs, WritingModeType rhs) {
-    return (lhs = WritingModeType(mbgl::underlying_type(lhs) & mbgl::underlying_type(rhs)));
-}
-
-MBGL_CONSTEXPR WritingModeType operator~(WritingModeType value) {
-    return WritingModeType(~mbgl::underlying_type(value));
-}
-
 using GlyphDependencies = std::map<FontStack, GlyphIDs>;
 using GlyphRangeDependencies = std::map<FontStack, std::unordered_set<GlyphRange>>;
 

src/mbgl/text/placement.cpp

@@ -699,7 +699,7 @@ void Placement::updateBucketOpacities(SymbolBucket& bucket, const TransformState
     if (bucket.hasTextCollisionBoxData()) bucket.textCollisionBox->dynamicVertices.clear();
     if (bucket.hasTextCollisionCircleData()) bucket.textCollisionCircle->dynamicVertices.clear();
 
-    JointOpacityState duplicateOpacityState(false, false, true);
+    const JointOpacityState duplicateOpacityState(false, false, true);
 
     const bool textAllowOverlap = bucket.layout->get<style::TextAllowOverlap>();
     const bool iconAllowOverlap = bucket.layout->get<style::IconAllowOverlap>();
@@ -712,7 +712,7 @@ void Placement::updateBucketOpacities(SymbolBucket& bucket, const TransformState
     // But we have to wait for placement if we potentially depend on a paired icon/text
     // with allow-overlap: false.
     // See https://github.com/mapbox/mapbox-gl-native/issues/12483
-    JointOpacityState defaultOpacityState(
+    const JointOpacityState defaultOpacityState(
             textAllowOverlap && (iconAllowOverlap || !(bucket.hasIconData() || bucket.hasSdfIconData()) || bucket.layout->get<style::IconOptional>()),
             iconAllowOverlap && (textAllowOverlap || !bucket.hasTextData() || bucket.layout->get<style::TextOptional>()),
             true);
@@ -728,10 +728,6 @@ void Placement::updateBucketOpacities(SymbolBucket& bucket, const TransformState
             opacityState = it->second;
         }
 
-        if (it == opacities.end()) {
-            opacities.emplace(symbolInstance.crossTileID, defaultOpacityState);
-        }
-
         seenCrossTileIDs.insert(symbolInstance.crossTileID);
 
         if (symbolInstance.hasText()) {
@@ -968,6 +964,17 @@ float Placement::zoomAdjustment(const float zoom) const {
     return std::max(0.0, (placementZoom - zoom) / 1.5);
 }
 
+Duration Placement::getUpdatePeriod(const float zoom) const {
+    // Even if transitionOptions.duration is set to a value < 300ms, we still wait for this default transition duration
+    // before attempting another placement operation.
+    const auto fadeDuration = std::max(util::DEFAULT_TRANSITION_DURATION, transitionOptions.duration.value_or(util::DEFAULT_TRANSITION_DURATION));
+    const auto adjustedDuration = std::chrono::duration_cast<Duration>(fadeDuration * (1.0 - zoomAdjustment(zoom)));
+    if (maximumUpdatePeriod) {
+        return std::min(*maximumUpdatePeriod, adjustedDuration);
+    }
+    return adjustedDuration;
+}
+
 bool Placement::hasTransitions(TimePoint now) const {
     if (mapMode == MapMode::Continuous && transitionOptions.enablePlacementTransitions) {
         return stale || std::chrono::duration<float>(now - fadeStartTime) < transitionOptions.duration.value_or(util::DEFAULT_TRANSITION_DURATION);
@@ -977,12 +984,13 @@ bool Placement::hasTransitions(TimePoint now) const {
 }
 
 bool Placement::stillRecent(TimePoint now, const float zoom) const {
-    // Even if transitionOptions.duration is set to a value < 300ms, we still wait for this default transition duration
-    // before attempting another placement operation.
-    const auto fadeDuration = std::max(util::DEFAULT_TRANSITION_DURATION, transitionOptions.duration.value_or(util::DEFAULT_TRANSITION_DURATION));
     return mapMode == MapMode::Continuous &&
         transitionOptions.enablePlacementTransitions &&
-        commitTime + fadeDuration * (1.0 - zoomAdjustment(zoom)) > now;
+        commitTime + getUpdatePeriod(zoom) > now;
+}
+
+void Placement::setMaximumUpdatePeriod(Duration duration) {
+    maximumUpdatePeriod = duration;
 }
 
 void Placement::setStale() {

src/mbgl/text/placement.hpp

@@ -110,7 +110,7 @@ class Placement {
     const CollisionIndex& getCollisionIndex() const;
 
     bool stillRecent(TimePoint now, const float zoom) const;
-    void setRecent(TimePoint now);
+    void setMaximumUpdatePeriod(Duration);
     void setStale();
 
     const RetainedQueryData& getQueryData(uint32_t bucketInstanceId) const;
@@ -126,6 +126,7 @@ class Placement {
     void markUsedJustification(SymbolBucket&, style::TextVariableAnchorType, const SymbolInstance&, style::TextWritingModeType orientation);
     void markUsedOrientation(SymbolBucket&, style::TextWritingModeType, const SymbolInstance&);
     float zoomAdjustment(const float zoom) const;
+    Duration getUpdatePeriod(const float zoom) const;
 
     CollisionIndex collisionIndex;
 
@@ -147,6 +148,7 @@ class Placement {
     std::unordered_map<uint32_t, RetainedQueryData> retainedQueryData;
     CollisionGroups collisionGroups;
     std::unique_ptr<Placement> prevPlacement;
+    optional<Duration> maximumUpdatePeriod;
 
     // Used for debug purposes.
     std::unordered_map<const CollisionFeature*, std::vector<ProjectedCollisionBox>> collisionCircles;