[core] fix collisionBox alignment when Icon/text translation is enabl…

…ed (#15467) * add initial fix * fix bug for collision circle * refind code structure * fix indentation * update test * refind code structure * Add changelog * Add comment for boolean
mapbox · Aug 30, 2019 · 6ef2710 · 6ef2710
1 parent 29b11a5
commit 6ef2710
Show file tree

Hide file tree

Showing 10 changed files with 205 additions and 143 deletions.
diff --git a/platform/android/CHANGELOG.md b/platform/android/CHANGELOG.md
@@ -10,6 +10,9 @@ Mapbox welcomes participation and contributions from everyone.  If you'd like to
  ### Performance improvements
  - Mark used offline region resources in batches. [#15521](https://github.com/mapbox/mapbox-gl-native/pull/15521)
 
+### Bug fixes
+ - Fixed a rendering issue of `collisionBox` when `text-translate` or `icon-translate` is enabled. [#15467](https://github.com/mapbox/mapbox-gl-native/pull/15467)
+
 ## 8.3.0 - August 28, 2019
 [Changes](https://github.com/mapbox/mapbox-gl-native/compare/android-v8.3.0-beta.1...android-v8.3.0) since [Mapbox Maps SDK for Android v8.3.0-beta.1](https://github.com/mapbox/mapbox-gl-native/releases/tag/android-v8.3.0-beta.1):
 

diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md
@@ -6,6 +6,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
 
 * Fixed an issue that caused the tilt gesture to trigger too easily and conflict with pinch or pan gestures. ([#15349](https://github.com/mapbox/mapbox-gl-native/pull/15349))
 * Fixed a bug with annotation view positions after camera transitions. ([#15122](https://github.com/mapbox/mapbox-gl-native/pull/15122/))
+* Fixed a rendering issue of `collisionBox` when `text-translate` or `icon-translate` is enabled. ([#15467](https://github.com/mapbox/mapbox-gl-native/pull/15467))
 
  ### Performance improvements
 

diff --git a/platform/macos/CHANGELOG.md b/platform/macos/CHANGELOG.md
@@ -14,6 +14,7 @@
 * Fixed rendering and collision detection issues with using `text-variable-anchor` and `icon-text-fit` properties on the same layer. ([#15367](https://github.com/mapbox/mapbox-gl-native/pull/15367))
 * Fixed symbol overlap when zooming out quickly. ([15416](https://github.com/mapbox/mapbox-gl-native/pull/15416))
 * Fixed a rendering issue that non-SDF icon would be treated as SDF icon if they are in the same layer. ([#15456](https://github.com/mapbox/mapbox-gl-native/pull/15456))
+* Fixed a rendering issue of `collisionBox` when `text-translate` or `icon-translate` is enabled. ([#15467](https://github.com/mapbox/mapbox-gl-native/pull/15467))
 
 ### Styles and rendering
 

diff --git a/platform/node/CHANGELOG.md b/platform/node/CHANGELOG.md
@@ -2,6 +2,7 @@
 * Introduce `text-writing-mode` layout property for symbol layer ([#14932](https://github.com/mapbox/mapbox-gl-native/pull/14932)). The `text-writing-mode` layout property allows control over symbol's preferred writing mode. The new property value is an array, whose values are enumeration values from a ( `horizontal` | `vertical` ) set.
 * Fixed rendering and collision detection issues with using `text-variable-anchor` and `icon-text-fit` properties on the same layer ([#15367](https://github.com/mapbox/mapbox-gl-native/pull/15367)).
 * Fixed a rendering issue that non-SDF icon would be treated as SDF icon if they are in the same layer. ([#15456](https://github.com/mapbox/mapbox-gl-native/pull/15456))
+* Fixed a rendering issue of `collisionBox` when `text-translate` or `icon-translate` is enabled. ([#15467](https://github.com/mapbox/mapbox-gl-native/pull/15467))
 
 # 4.2.0
 - Add an option to set whether or not an image should be treated as a SDF ([#15054](https://github.com/mapbox/mapbox-gl-native/issues/15054))

diff --git a/src/mbgl/layout/symbol_layout.cpp b/src/mbgl/layout/symbol_layout.cpp
@@ -812,12 +812,15 @@ void SymbolLayout::addToDebugBuffers(SymbolBucket& bucket) {
         return;
     }
 
-    for (const SymbolInstance &symbolInstance : symbolInstances) {
-        auto populateCollisionBox = [&](const auto& feature) {
-            SymbolBucket::CollisionBuffer& collisionBuffer = feature.alongLine ?
-                static_cast<SymbolBucket::CollisionBuffer&>(bucket.getOrCreateCollisionCircleBuffer()) :
-                static_cast<SymbolBucket::CollisionBuffer&>(bucket.getOrCreateCollisionBox());
-            for (const CollisionBox &box : feature.boxes) {
+    for (const SymbolInstance& symbolInstance : symbolInstances) {
+        auto populateCollisionBox = [&](const auto& feature, bool isText) {
+            SymbolBucket::CollisionBuffer& collisionBuffer =
+                feature.alongLine ? (isText ? static_cast<SymbolBucket::CollisionBuffer&>(bucket.getOrCreateTextCollisionCircleBuffer())
+                                            : static_cast<SymbolBucket::CollisionBuffer&>(bucket.getOrCreateIconCollisionCircleBuffer()))
+                                  : (isText ? static_cast<SymbolBucket::CollisionBuffer&>(bucket.getOrCreateTextCollisionBox())
+                                            : static_cast<SymbolBucket::CollisionBuffer&>(bucket.getOrCreateIconCollisionBox()));
+
+            for (const CollisionBox& box : feature.boxes) {
                 auto& anchor = box.anchor;
 
                 Point<float> tl{box.x1, box.y1};
@@ -829,8 +832,11 @@ void SymbolLayout::addToDebugBuffers(SymbolBucket& bucket) {
                 const std::size_t indexLength = feature.alongLine ? 6 : 8;
 
                 if (collisionBuffer.segments.empty() || collisionBuffer.segments.back().vertexLength + vertexLength > std::numeric_limits<uint16_t>::max()) {
-                    collisionBuffer.segments.emplace_back(collisionBuffer.vertices.elements(),
-                      feature.alongLine ? bucket.collisionCircle->triangles.elements() : bucket.collisionBox->lines.elements());
+                    collisionBuffer.segments.emplace_back(
+                        collisionBuffer.vertices.elements(),
+                        feature.alongLine
+                            ? (isText ? bucket.textCollisionCircle->triangles.elements() : bucket.iconCollisionCircle->triangles.elements())
+                            : (isText ? bucket.textCollisionBox->lines.elements() : bucket.iconCollisionBox->lines.elements()));
                 }
 
                 auto& segment = collisionBuffer.segments.back();
@@ -850,27 +856,29 @@ void SymbolLayout::addToDebugBuffers(SymbolBucket& bucket) {
                 collisionBuffer.dynamicVertices.emplace_back(dynamicVertex);
 
                 if (feature.alongLine) {
-                    bucket.collisionCircle->triangles.emplace_back(index, index + 1, index + 2);
-                    bucket.collisionCircle->triangles.emplace_back(index, index + 2, index + 3);
+                    auto& collisionCircle = (isText ? bucket.textCollisionCircle : bucket.iconCollisionCircle);
+                    collisionCircle->triangles.emplace_back(index, index + 1, index + 2);
+                    collisionCircle->triangles.emplace_back(index, index + 2, index + 3);
                 } else {
-                    bucket.collisionBox->lines.emplace_back(index + 0, index + 1);
-                    bucket.collisionBox->lines.emplace_back(index + 1, index + 2);
-                    bucket.collisionBox->lines.emplace_back(index + 2, index + 3);
-                    bucket.collisionBox->lines.emplace_back(index + 3, index + 0);
+                    auto& collisionBox = (isText ? bucket.textCollisionBox : bucket.iconCollisionBox);
+                    collisionBox->lines.emplace_back(index + 0, index + 1);
+                    collisionBox->lines.emplace_back(index + 1, index + 2);
+                    collisionBox->lines.emplace_back(index + 2, index + 3);
+                    collisionBox->lines.emplace_back(index + 3, index + 0);
                 }
 
                 segment.vertexLength += vertexLength;
                 segment.indexLength += indexLength;
             }
         };
-        populateCollisionBox(symbolInstance.textCollisionFeature);
+        populateCollisionBox(symbolInstance.textCollisionFeature, true /*isText*/);
         if (symbolInstance.verticalTextCollisionFeature) {
-            populateCollisionBox(*symbolInstance.verticalTextCollisionFeature);
+            populateCollisionBox(*symbolInstance.verticalTextCollisionFeature, true /*isText*/);
         }
         if (symbolInstance.verticalIconCollisionFeature) {
-            populateCollisionBox(*symbolInstance.verticalIconCollisionFeature);
+            populateCollisionBox(*symbolInstance.verticalIconCollisionFeature, false /*isText*/);
         }
-        populateCollisionBox(symbolInstance.iconCollisionFeature);
+        populateCollisionBox(symbolInstance.iconCollisionFeature, false /*isText*/);
     }
 }
 

diff --git a/src/mbgl/renderer/buckets/symbol_bucket.cpp b/src/mbgl/renderer/buckets/symbol_bucket.cpp
@@ -113,37 +113,50 @@ void SymbolBucket::upload(gfx::UploadPass& uploadPass) {
     if (hasIconData()) {
         updateIconBuffer(icon);
     }
-
     if (hasSdfIconData()) {
         updateIconBuffer(sdfIcon);
     }
 
-    if (hasCollisionBoxData()) {
+    const auto updateCollisionBox = [&](CollisionBoxBuffer& collisionBox) {
         if (!staticUploaded) {
-            collisionBox->indexBuffer = uploadPass.createIndexBuffer(std::move(collisionBox->lines));
-            collisionBox->vertexBuffer = uploadPass.createVertexBuffer(std::move(collisionBox->vertices));
+            collisionBox.indexBuffer = uploadPass.createIndexBuffer(std::move(collisionBox.lines));
+            collisionBox.vertexBuffer = uploadPass.createVertexBuffer(std::move(collisionBox.vertices));
         }
         if (!placementChangesUploaded) {
-            if (!collisionBox->dynamicVertexBuffer) {
-                collisionBox->dynamicVertexBuffer = uploadPass.createVertexBuffer(std::move(collisionBox->dynamicVertices), gfx::BufferUsageType::StreamDraw);
+            if (!collisionBox.dynamicVertexBuffer) {
+                collisionBox.dynamicVertexBuffer = uploadPass.createVertexBuffer(std::move(collisionBox.dynamicVertices), gfx::BufferUsageType::StreamDraw);
             } else {
-                uploadPass.updateVertexBuffer(*collisionBox->dynamicVertexBuffer, std::move(collisionBox->dynamicVertices));
+                uploadPass.updateVertexBuffer(*collisionBox.dynamicVertexBuffer, std::move(collisionBox.dynamicVertices));
             }
         }
+    };
+    if (hasIconCollisionBoxData()) {
+        updateCollisionBox(*iconCollisionBox);
+    }
+
+    if (hasTextCollisionBoxData()) {
+        updateCollisionBox(*textCollisionBox);
     }
 
-    if (hasCollisionCircleData()) {
+    const auto updateCollisionCircle = [&](CollisionCircleBuffer& collisionCircle) {
         if (!staticUploaded) {
-            collisionCircle->indexBuffer = uploadPass.createIndexBuffer(std::move(collisionCircle->triangles));
-            collisionCircle->vertexBuffer = uploadPass.createVertexBuffer(std::move(collisionCircle->vertices));
+            collisionCircle.indexBuffer = uploadPass.createIndexBuffer(std::move(collisionCircle.triangles));
+            collisionCircle.vertexBuffer = uploadPass.createVertexBuffer(std::move(collisionCircle.vertices));
         }
         if (!placementChangesUploaded) {
-            if (!collisionCircle->dynamicVertexBuffer) {
-                collisionCircle->dynamicVertexBuffer = uploadPass.createVertexBuffer(std::move(collisionCircle->dynamicVertices), gfx::BufferUsageType::StreamDraw);
+            if (!collisionCircle.dynamicVertexBuffer) {
+                collisionCircle.dynamicVertexBuffer = uploadPass.createVertexBuffer(std::move(collisionCircle.dynamicVertices), gfx::BufferUsageType::StreamDraw);
             } else {
-                uploadPass.updateVertexBuffer(*collisionCircle->dynamicVertexBuffer, std::move(collisionCircle->dynamicVertices));
+                uploadPass.updateVertexBuffer(*collisionCircle.dynamicVertexBuffer, std::move(collisionCircle.dynamicVertices));
             }
         }
+    };
+    if (hasIconCollisionCircleData()) {
+        updateCollisionCircle(*iconCollisionCircle);
+    }
+
+    if (hasTextCollisionCircleData()) {
+        updateCollisionCircle(*textCollisionCircle);
     }
 
     uploaded = true;
@@ -154,7 +167,8 @@ void SymbolBucket::upload(gfx::UploadPass& uploadPass) {
 }
 
 bool SymbolBucket::hasData() const {
-    return hasTextData() || hasIconData() || hasSdfIconData() || hasCollisionBoxData();
+    return hasTextData() || hasIconData() || hasSdfIconData() || hasIconCollisionBoxData() ||
+           hasTextCollisionBoxData() || hasIconCollisionCircleData() || hasTextCollisionCircleData();
 }
 
 bool SymbolBucket::hasTextData() const {
@@ -169,12 +183,20 @@ bool SymbolBucket::hasSdfIconData() const {
     return !sdfIcon.segments.empty();
 }
 
-bool SymbolBucket::hasCollisionBoxData() const {
-    return collisionBox && !collisionBox->segments.empty();
+bool SymbolBucket::hasIconCollisionBoxData() const {
+    return iconCollisionBox && !iconCollisionBox->segments.empty();
+}
+
+bool SymbolBucket::hasIconCollisionCircleData() const {
+    return iconCollisionCircle && !iconCollisionCircle->segments.empty();
+}
+
+bool SymbolBucket::hasTextCollisionBoxData() const {
+    return textCollisionBox && !textCollisionBox->segments.empty();
 }
 
-bool SymbolBucket::hasCollisionCircleData() const {
-    return collisionCircle && !collisionCircle->segments.empty();
+bool SymbolBucket::hasTextCollisionCircleData() const {
+    return textCollisionCircle && !textCollisionCircle->segments.empty();
 }
 
 void addPlacedSymbol(gfx::IndexVector<gfx::Triangles>& triangles, const PlacedSymbol& placedSymbol) {

diff --git a/src/mbgl/renderer/buckets/symbol_bucket.hpp b/src/mbgl/renderer/buckets/symbol_bucket.hpp
@@ -72,8 +72,10 @@ class SymbolBucket final : public Bucket {
     bool hasTextData() const;
     bool hasIconData() const;
     bool hasSdfIconData() const;
-    bool hasCollisionBoxData() const;
-    bool hasCollisionCircleData() const;
+    bool hasIconCollisionBoxData() const;
+    bool hasIconCollisionCircleData() const;
+    bool hasTextCollisionBoxData() const;
+    bool hasTextCollisionCircleData() const;
     bool hasFormatSectionOverrides() const;
 
 
@@ -138,22 +140,34 @@ class SymbolBucket final : public Bucket {
         gfx::IndexVector<gfx::Lines> lines;
         optional<gfx::IndexBuffer> indexBuffer;
     };
-    std::unique_ptr<CollisionBoxBuffer> collisionBox;
+    std::unique_ptr<CollisionBoxBuffer> iconCollisionBox;
+    std::unique_ptr<CollisionBoxBuffer> textCollisionBox;
 
-    CollisionBoxBuffer& getOrCreateCollisionBox() {
-        if (!collisionBox) collisionBox = std::make_unique<CollisionBoxBuffer>();
-        return *collisionBox;
+    CollisionBoxBuffer& getOrCreateIconCollisionBox() {
+        if (!iconCollisionBox) iconCollisionBox = std::make_unique<CollisionBoxBuffer>();
+        return *iconCollisionBox;
+    }
+
+    CollisionBoxBuffer& getOrCreateTextCollisionBox() {
+        if (!textCollisionBox) textCollisionBox = std::make_unique<CollisionBoxBuffer>();
+        return *textCollisionBox;
     }
 
     struct CollisionCircleBuffer : public CollisionBuffer {
         gfx::IndexVector<gfx::Triangles> triangles;
         optional<gfx::IndexBuffer> indexBuffer;
     };
-    std::unique_ptr<CollisionCircleBuffer> collisionCircle;
+    std::unique_ptr<CollisionCircleBuffer> iconCollisionCircle;
+    std::unique_ptr<CollisionCircleBuffer> textCollisionCircle;
+
+    CollisionCircleBuffer& getOrCreateIconCollisionCircleBuffer() {
+        if (!iconCollisionCircle) iconCollisionCircle = std::make_unique<CollisionCircleBuffer>();
+        return *iconCollisionCircle;
+    }
 
-    CollisionCircleBuffer& getOrCreateCollisionCircleBuffer() {
-        if (!collisionCircle) collisionCircle = std::make_unique<CollisionCircleBuffer>();
-        return *collisionCircle;
+    CollisionCircleBuffer& getOrCreateTextCollisionCircleBuffer() {
+        if (!textCollisionCircle) textCollisionCircle = std::make_unique<CollisionCircleBuffer>();
+        return *textCollisionCircle;
     }
 
     const float tilePixelRatio;