Fix icon/text collisionBox translation (mapbox#8659)

* init fix * fix lint error * fix indentation and wrong variable * add line placement translate render test * revert accidently added blank line * update comments * update one test fixture * Add changelog + remove whitespace
stepankuzmin · Sep 9, 2019 · a0d4a23 · a0d4a23
1 parent bd8e91e
commit a0d4a23
Show file tree

Hide file tree

Showing 10 changed files with 233 additions and 39 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@
 ## 🐞 Bug fixes
 
 * Fix bug in `NavigationControl` compass button that prevented it from rotating with the map ([#8605](https://github.com/mapbox/mapbox-gl-js/pull/8605))
+* Fix rendering of `collisionBox` when `text-translate` or `icon-translate` is enabled  ([#8659](https://github.com/mapbox/mapbox-gl-js/pull/8659))
 
 ## 1.2.0
 

diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js
@@ -240,8 +240,10 @@ register('CollisionBuffers', CollisionBuffers);
  *      `this.collisionBoxArray`: collision data for use by foreground
  *      `this.text`: SymbolBuffers for text symbols
  *      `this.icons`: SymbolBuffers for icons
- *      `this.collisionBox`: Debug SymbolBuffers for collision boxes
- *      `this.collisionCircle`: Debug SymbolBuffers for collision circles
+ *      `this.iconCollisionBox`: Debug SymbolBuffers for icon collision boxes
+ *      `this.textCollisionBox`: Debug SymbolBuffers for text collision boxes
+ *      `this.iconCollisionCircle`: Debug SymbolBuffers for icon collision circles
+ *      `this.textCollisionCircle`: Debug SymbolBuffers for text collision circles
  *    The results are sent to the foreground for rendering
  *
  * 4. performSymbolPlacement(bucket, collisionIndex) is run on the foreground,
@@ -289,8 +291,10 @@ class SymbolBucket implements Bucket {
 
     text: SymbolBuffers;
     icon: SymbolBuffers;
-    collisionBox: CollisionBuffers;
-    collisionCircle: CollisionBuffers;
+    textCollisionBox: CollisionBuffers;
+    iconCollisionBox: CollisionBuffers;
+    textCollisionCircle: CollisionBuffers;
+    iconCollisionCircle: CollisionBuffers;
     uploaded: boolean;
     sourceLayerIndex: number;
     sourceID: string;
@@ -341,8 +345,10 @@ class SymbolBucket implements Bucket {
         this.text = new SymbolBuffers(new ProgramConfigurationSet(symbolLayoutAttributes.members, this.layers, this.zoom, property => /^text/.test(property)));
         this.icon = new SymbolBuffers(new ProgramConfigurationSet(symbolLayoutAttributes.members, this.layers, this.zoom, property => /^icon/.test(property)));
 
-        this.collisionBox = new CollisionBuffers(CollisionBoxLayoutArray, collisionBoxLayout.members, LineIndexArray);
-        this.collisionCircle = new CollisionBuffers(CollisionCircleLayoutArray, collisionCircleLayout.members, TriangleIndexArray);
+        this.textCollisionBox = new CollisionBuffers(CollisionBoxLayoutArray, collisionBoxLayout.members, LineIndexArray);
+        this.iconCollisionBox = new CollisionBuffers(CollisionBoxLayoutArray, collisionBoxLayout.members, LineIndexArray);
+        this.textCollisionCircle = new CollisionBuffers(CollisionCircleLayoutArray, collisionCircleLayout.members, TriangleIndexArray);
+        this.iconCollisionCircle = new CollisionBuffers(CollisionCircleLayoutArray, collisionCircleLayout.members, TriangleIndexArray);
 
         this.glyphOffsetArray = new GlyphOffsetArray();
         this.lineVertexArray = new SymbolLineVertexArray();
@@ -476,8 +482,10 @@ class SymbolBucket implements Bucket {
 
     upload(context: Context) {
         if (!this.uploaded) {
-            this.collisionBox.upload(context);
-            this.collisionCircle.upload(context);
+            this.textCollisionBox.upload(context);
+            this.iconCollisionBox.upload(context);
+            this.textCollisionCircle.upload(context);
+            this.iconCollisionCircle.upload(context);
         }
         this.text.upload(context, this.sortFeaturesByY, !this.uploaded, this.text.programConfigurations.needsUpload);
         this.icon.upload(context, this.sortFeaturesByY, !this.uploaded, this.icon.programConfigurations.needsUpload);
@@ -487,8 +495,10 @@ class SymbolBucket implements Bucket {
     destroy() {
         this.text.destroy();
         this.icon.destroy();
-        this.collisionBox.destroy();
-        this.collisionCircle.destroy();
+        this.textCollisionBox.destroy();
+        this.iconCollisionBox.destroy();
+        this.textCollisionCircle.destroy();
+        this.iconCollisionCircle.destroy();
     }
 
     addToLineVertexArray(anchor: Anchor, line: any) {
@@ -659,7 +669,7 @@ class SymbolBucket implements Bucket {
         }
     }
 
-    addDebugCollisionBoxes(startIndex: number, endIndex: number, symbolInstance: SymbolInstance) {
+    addDebugCollisionBoxes(startIndex: number, endIndex: number, symbolInstance: SymbolInstance, isText: boolean) {
         for (let b = startIndex; b < endIndex; b++) {
             const box: CollisionBox = (this.collisionBoxArray.get(b): any);
             const x1 = box.x1;
@@ -670,16 +680,18 @@ class SymbolBucket implements Bucket {
             // If the radius > 0, this collision box is actually a circle
             // The data we add to the buffers is exactly the same, but we'll render with a different shader.
             const isCircle = box.radius > 0;
-            this.addCollisionDebugVertices(x1, y1, x2, y2, isCircle ? this.collisionCircle : this.collisionBox, box.anchorPoint, symbolInstance, isCircle);
+            this.addCollisionDebugVertices(x1, y1, x2, y2, isCircle ?
+                (isText ? this.textCollisionCircle : this.iconCollisionCircle) : (isText ? this.textCollisionBox : this.iconCollisionBox),
+                box.anchorPoint, symbolInstance, isCircle);
         }
     }
 
     generateCollisionDebugBuffers() {
         for (let i = 0; i < this.symbolInstances.length; i++) {
             const symbolInstance = this.symbolInstances.get(i);
-            this.addDebugCollisionBoxes(symbolInstance.textBoxStartIndex, symbolInstance.textBoxEndIndex, symbolInstance);
-            this.addDebugCollisionBoxes(symbolInstance.verticalTextBoxStartIndex, symbolInstance.verticalTextBoxEndIndex, symbolInstance);
-            this.addDebugCollisionBoxes(symbolInstance.iconBoxStartIndex, symbolInstance.iconBoxEndIndex, symbolInstance);
+            this.addDebugCollisionBoxes(symbolInstance.textBoxStartIndex, symbolInstance.textBoxEndIndex, symbolInstance, true);
+            this.addDebugCollisionBoxes(symbolInstance.verticalTextBoxStartIndex, symbolInstance.verticalTextBoxEndIndex, symbolInstance, true);
+            this.addDebugCollisionBoxes(symbolInstance.iconBoxStartIndex, symbolInstance.iconBoxEndIndex, symbolInstance, false);
         }
     }
 
@@ -746,12 +758,20 @@ class SymbolBucket implements Bucket {
         return this.icon.segments.get().length > 0;
     }
 
-    hasCollisionBoxData() {
-        return this.collisionBox.segments.get().length > 0;
+    hasTextCollisionBoxData() {
+        return this.textCollisionBox.segments.get().length > 0;
     }
 
-    hasCollisionCircleData() {
-        return this.collisionCircle.segments.get().length > 0;
+    hasIconCollisionBoxData() {
+        return this.iconCollisionBox.segments.get().length > 0;
+    }
+
+    hasTextCollisionCircleData() {
+        return this.textCollisionCircle.segments.get().length > 0;
+    }
+
+    hasIconCollisionCircleData() {
+        return this.iconCollisionCircle.segments.get().length > 0;
     }
 
     addIndicesForPlacedTextSymbol(placedTextSymbolIndex: number) {

diff --git a/src/render/draw_collision_debug.js b/src/render/draw_collision_debug.js
@@ -12,7 +12,8 @@ import { collisionUniformValues } from './program/collision_program';
 
 export default drawCollisionDebug;
 
-function drawCollisionDebugGeometry(painter: Painter, sourceCache: SourceCache, layer: StyleLayer, coords: Array<OverscaledTileID>, drawCircles: boolean) {
+function drawCollisionDebugGeometry(painter: Painter, sourceCache: SourceCache, layer: StyleLayer, coords: Array<OverscaledTileID>, drawCircles: boolean,
+    translate: [number, number], translateAnchor: 'map' | 'viewport', isText: boolean) {
     const context = painter.context;
     const gl = context.gl;
     const program = drawCircles ? painter.useProgram('collisionCircle') : painter.useProgram('collisionBox');
@@ -22,15 +23,18 @@ function drawCollisionDebugGeometry(painter: Painter, sourceCache: SourceCache,
         const tile = sourceCache.getTile(coord);
         const bucket: ?SymbolBucket = (tile.getBucket(layer): any);
         if (!bucket) continue;
-        const buffers = drawCircles ? bucket.collisionCircle : bucket.collisionBox;
+        const buffers = drawCircles ? (isText ? bucket.textCollisionCircle : bucket.iconCollisionCircle) : (isText ? bucket.textCollisionBox : bucket.iconCollisionBox);
         if (!buffers) continue;
-
+        let posMatrix = coord.posMatrix;
+        if (translate[0] !== 0 || translate[1] !== 0) {
+            posMatrix = painter.translatePosMatrix(coord.posMatrix, tile, translate, translateAnchor);
+        }
         program.draw(context, drawCircles ? gl.TRIANGLES : gl.LINES,
             DepthMode.disabled, StencilMode.disabled,
             painter.colorModeForRenderPass(),
             CullFaceMode.disabled,
             collisionUniformValues(
-                coord.posMatrix,
+                posMatrix,
                 painter.transform,
                 tile),
             layer.id, buffers.layoutVertexBuffer, buffers.indexBuffer,
@@ -39,7 +43,7 @@ function drawCollisionDebugGeometry(painter: Painter, sourceCache: SourceCache,
     }
 }
 
-function drawCollisionDebug(painter: Painter, sourceCache: SourceCache, layer: StyleLayer, coords: Array<OverscaledTileID>) {
-    drawCollisionDebugGeometry(painter, sourceCache, layer, coords, false);
-    drawCollisionDebugGeometry(painter, sourceCache, layer, coords, true);
+function drawCollisionDebug(painter: Painter, sourceCache: SourceCache, layer: StyleLayer, coords: Array<OverscaledTileID>, translate: [number, number], translateAnchor: 'map' | 'viewport', isText: boolean) {
+    drawCollisionDebugGeometry(painter, sourceCache, layer, coords, false, translate, translateAnchor, isText);
+    drawCollisionDebugGeometry(painter, sourceCache, layer, coords, true, translate, translateAnchor, isText);
 }
diff --git a/src/render/draw_symbol.js b/src/render/draw_symbol.js
@@ -79,7 +79,10 @@ function drawSymbols(painter: Painter, sourceCache: SourceCache, layer: SymbolSt
     }
 
     if (sourceCache.map.showCollisionBoxes) {
-        drawCollisionDebug(painter, sourceCache, layer, coords);
+        drawCollisionDebug(painter, sourceCache, layer, coords, layer.paint.get('text-translate'),
+            layer.paint.get('text-translate-anchor'), true);
+        drawCollisionDebug(painter, sourceCache, layer, coords, layer.paint.get('icon-translate'),
+            layer.paint.get('icon-translate-anchor'), false);
     }
 }
 

diff --git a/src/symbol/placement.js b/src/symbol/placement.js
@@ -696,8 +696,10 @@ export class Placement {
     updateBucketOpacities(bucket: SymbolBucket, seenCrossTileIDs: { [string | number]: boolean }, collisionBoxArray: ?CollisionBoxArray) {
         if (bucket.hasTextData()) bucket.text.opacityVertexArray.clear();
         if (bucket.hasIconData()) bucket.icon.opacityVertexArray.clear();
-        if (bucket.hasCollisionBoxData()) bucket.collisionBox.collisionVertexArray.clear();
-        if (bucket.hasCollisionCircleData()) bucket.collisionCircle.collisionVertexArray.clear();
+        if (bucket.hasIconCollisionBoxData()) bucket.iconCollisionBox.collisionVertexArray.clear();
+        if (bucket.hasTextCollisionBoxData()) bucket.textCollisionBox.collisionVertexArray.clear();
+        if (bucket.hasIconCollisionCircleData()) bucket.iconCollisionCircle.collisionVertexArray.clear();
+        if (bucket.hasTextCollisionCircleData()) bucket.textCollisionCircle.collisionVertexArray.clear();
 
         const layout = bucket.layers[0].layout;
         const duplicateOpacityState = new JointOpacityState(null, 0, false, false, true);
@@ -715,7 +717,8 @@ export class Placement {
                 iconAllowOverlap && (textAllowOverlap || !bucket.hasTextData() || layout.get('text-optional')),
                 true);
 
-        if (!bucket.collisionArrays && collisionBoxArray && (bucket.hasCollisionBoxData() || bucket.hasCollisionCircleData())) {
+        if (!bucket.collisionArrays && collisionBoxArray && ((bucket.hasIconCollisionBoxData() || bucket.hasIconCollisionCircleData() ||
+            bucket.hasTextCollisionBoxData() || bucket.hasTextCollisionCircleData()))) {
             bucket.deserializeCollisionBoxes(collisionBoxArray);
         }
 
@@ -794,7 +797,8 @@ export class Placement {
                     (opacityState.icon.isHidden(): any);
             }
 
-            if (bucket.hasCollisionBoxData() || bucket.hasCollisionCircleData()) {
+            if (bucket.hasIconCollisionBoxData() || bucket.hasIconCollisionCircleData() ||
+                bucket.hasTextCollisionBoxData() || bucket.hasTextCollisionCircleData()) {
                 const collisionArrays = bucket.collisionArrays[s];
                 if (collisionArrays) {
                     if (collisionArrays.textBox) {
@@ -823,18 +827,18 @@ export class Placement {
                             }
                         }
 
-                        updateCollisionVertices(bucket.collisionBox.collisionVertexArray, opacityState.text.placed, !used, shift.x, shift.y);
+                        updateCollisionVertices(bucket.textCollisionBox.collisionVertexArray, opacityState.text.placed, !used, shift.x, shift.y);
                     }
 
                     if (collisionArrays.iconBox) {
-                        updateCollisionVertices(bucket.collisionBox.collisionVertexArray, opacityState.icon.placed, false);
+                        updateCollisionVertices(bucket.iconCollisionBox.collisionVertexArray, opacityState.icon.placed, false);
                     }
 
                     const textCircles = collisionArrays.textCircles;
-                    if (textCircles && bucket.hasCollisionCircleData()) {
+                    if (textCircles && bucket.hasTextCollisionCircleData()) {
                         for (let k = 0; k < textCircles.length; k += 5) {
                             const notUsed = isDuplicate || textCircles[k + 4] === 0;
-                            updateCollisionVertices(bucket.collisionCircle.collisionVertexArray, opacityState.text.placed, notUsed);
+                            updateCollisionVertices(bucket.textCollisionCircle.collisionVertexArray, opacityState.text.placed, notUsed);
                         }
                     }
                 }
@@ -852,11 +856,17 @@ export class Placement {
         if (bucket.hasIconData() && bucket.icon.opacityVertexBuffer) {
             bucket.icon.opacityVertexBuffer.updateData(bucket.icon.opacityVertexArray);
         }
-        if (bucket.hasCollisionBoxData() && bucket.collisionBox.collisionVertexBuffer) {
-            bucket.collisionBox.collisionVertexBuffer.updateData(bucket.collisionBox.collisionVertexArray);
+        if (bucket.hasIconCollisionBoxData() && bucket.iconCollisionBox.collisionVertexBuffer) {
+            bucket.iconCollisionBox.collisionVertexBuffer.updateData(bucket.iconCollisionBox.collisionVertexArray);
         }
-        if (bucket.hasCollisionCircleData() && bucket.collisionCircle.collisionVertexBuffer) {
-            bucket.collisionCircle.collisionVertexBuffer.updateData(bucket.collisionCircle.collisionVertexArray);
+        if (bucket.hasTextCollisionBoxData() && bucket.textCollisionBox.collisionVertexBuffer) {
+            bucket.textCollisionBox.collisionVertexBuffer.updateData(bucket.textCollisionBox.collisionVertexArray);
+        }
+        if (bucket.hasIconCollisionCircleData() && bucket.iconCollisionCircle.collisionVertexBuffer) {
+            bucket.iconCollisionCircle.collisionVertexBuffer.updateData(bucket.iconCollisionCircle.collisionVertexArray);
+        }
+        if (bucket.hasTextCollisionCircleData() && bucket.textCollisionCircle.collisionVertexBuffer) {
+            bucket.textCollisionCircle.collisionVertexBuffer.updateData(bucket.textCollisionCircle.collisionVertexArray);
         }
 
         assert(bucket.text.opacityVertexArray.length === bucket.text.layoutVertexArray.length / 4);

diff --git a/.../integration/render-tests/debug/collision-icon-text-line-translate/expected.png b/.../integration/render-tests/debug/collision-icon-text-line-translate/expected.png
diff --git a/test/integration/render-tests/debug/collision-icon-text-line-translate/style.json b/test/integration/render-tests/debug/collision-icon-text-line-translate/style.json
@@ -0,0 +1,81 @@
+{
+  "version": 8,
+  "metadata": {
+    "test": {
+      "collisionDebug": true,
+      "height": 1000,
+      "width" : 1000
+    }
+  },
+  "center": [
+    0,
+    0
+  ],
+  "zoom": 12,
+  "sources": {
+    "geojson": {
+      "type": "geojson",
+      "data": {
+        "type": "LineString",
+        "coordinates": [
+            [
+                0,
+                0
+            ],
+            [
+                0.5,
+                0.5
+            ]
+        ]
+      }
+    }
+  },
+  "glyphs": "local://glyphs/{fontstack}/{range}.pbf",
+  "sprite": "local://sprites/sprite",
+  "layers": [
+    {
+      "id": "background",
+      "type": "background",
+      "paint": {
+          "background-color": "white"
+      }
+    },
+    {
+      "id": "translate",
+      "type": "symbol",
+      "source": "geojson",
+      "layout": {
+        "icon-image": "fav-airport-18",
+        "text-field": "abc",
+        "text-font": [
+          "Open Sans Semibold",
+          "Arial Unicode MS Bold"
+        ],
+        "symbol-placement": "line",
+        "symbol-spacing": 50
+      },
+      "paint": {
+        "icon-opacity": 1,
+        "text-color": "hsl(0, 82%, 48%)",
+        "icon-translate": [
+          "interpolate",
+          ["exponential", 2.0],
+          ["zoom"],
+          9,
+          ["literal", [0, 0]],
+          14,
+          ["literal", [-130, -130]]
+        ],
+        "text-translate": [
+          "interpolate",
+          ["exponential", 1.2],
+          ["zoom"],
+          9,
+          ["literal", [0, 0]],
+          13,
+          ["literal", [0, -130]]
+        ]
+      }
+    }
+  ]
+}
diff --git a/...integration/render-tests/debug/collision-icon-text-point-translate/expected.png b/...integration/render-tests/debug/collision-icon-text-point-translate/expected.png