Symbols: fix translation, variable-anchors and collision boxes (#4071)

* Import mercator symbol render tests from globe branch * Import src changes to symbols from globe branch * Add forgotten file change from globe branch * Use a reduced version of projection, adapt changes for main branch * Update build size * Fix failing unit test * Minor refactor * Rename ProjectionArgs to SymbolProjectionContext * Refactor posMatrix in draw_collision_debug * Add "_" to projectTruncatedLineSegment function name * Refactor collision box passing into a function * Remove unused code from placement.ts * collision_index: use else-if * collision_index: avoid creating copies of points * collision_index: refactor AABB projection using map() calls * collision_index: remove unused code branch * Fix merge * Change changelog formulation * Symbols: fix pitchWithMap texts sometimes having a wrongly shifted collision box * Symbols: fix pitchWithMap symbol collision boxes sometimes becoming oversized when zooming in * Symbols: update render tests * Revert oversized collisionbox fix * Symbols: more render tests with collisions enabled * Remove unused arguments from drawCollisionDebug * Refactor bounding box projection * Use explicit type for projection result in symbol projection * Add render test for distant pitch-aligned text collision bugfix * Add render test variants without bounding box corners * Fix missing docs * Add symbol box collisions benchmark * Only measure collision box handling, not grid performance * Minor refactor * Fix benchmark name being inconsistent with file name
maplibre · May 15, 2024 · a548083 · a548083
1 parent c8b27f2
commit a548083
Show file tree

Hide file tree

Showing 98 changed files with 7,356 additions and 412 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,11 @@
 - Fix normalizeSpriteURL before transformRequest throwing an Error with relative URLs ([#3897](https://github.com/maplibre/maplibre-gl-js/issues/3897))
 - Fix return type of map.cameraForBounds ([#3760](https://github.com/maplibre/maplibre-gl-js/issues/3760))
 - Fix to run benchmark with MAPLIBRE_STYLES environment variable ([#2122](https://github.com/maplibre/maplibre-gl-js/issues/2122))
+- Fix symbol collision debug view (`showCollisionBoxes`) not showing the actual bounding boxes used for collision and click areas. The displayed boxes now match actual collision boxes exactly.
+- Fix symbol collisions using inaccurate and sometimes entirely wrong collision boxes when the map is pitched or rotated ([#210](https://github.com/maplibre/maplibre-gl-js/issues/210))
+- Fix symbol collision boxes not being accurate for variable-anchor symbols.
+- Fix icon collision boxes using `text-translate` property for translation instead of the correct `icon-translate`.
+- Fix `text-translate` and `icon-translate` behaving weirdly and inconsistently with other `-translate` properties ([#3456](https://github.com/maplibre/maplibre-gl-js/issues/3456))
 - _...Add new stuff here..._
 
 ## 4.2.0

diff --git a/src/data/bucket/symbol_attributes.ts b/src/data/bucket/symbol_attributes.ts
@@ -16,7 +16,8 @@ export const placementOpacityAttributes = createLayout([
 
 export const collisionVertexAttributes = createLayout([
     {name: 'a_placed', components: 2, type: 'Uint8'},
-    {name: 'a_shift', components: 2, type: 'Float32'}
+    {name: 'a_shift', components: 2, type: 'Float32'},
+    {name: 'a_box_real', components: 2, type: 'Int16'},
 ]);
 
 export const collisionBox = createLayout([

diff --git a/src/data/bucket/symbol_bucket.test.ts b/src/data/bucket/symbol_bucket.test.ts
@@ -18,6 +18,7 @@ import {IndexedFeature, PopulateParameters} from '../bucket';
 import {StyleImage} from '../../style/style_image';
 import glyphs from '../../../test/unit/assets/fontstack-glyphs.json' with {type: 'json'};
 import {StyleGlyph} from '../../style/style_glyph';
+import {createProjection} from '../../geo/projection/projection';
 
 // Load a point feature from fixture tile.
 const vt = new VectorTile(new Protobuf(fs.readFileSync(path.resolve(__dirname, '../../../test/unit/assets/mbsv5-6-18-23.vector.pbf'))));
@@ -64,7 +65,7 @@ describe('SymbolBucket', () => {
         const bucketA = bucketSetup() as any as SymbolBucket;
         const bucketB = bucketSetup() as any as SymbolBucket;
         const options = {iconDependencies: {}, glyphDependencies: {}} as PopulateParameters;
-        const placement = new Placement(transform, undefined as any, 0, true);
+        const placement = new Placement(transform, createProjection(), undefined as any, 0, true);
         const tileID = new OverscaledTileID(0, 0, 0, 0, 0);
         const crossTileSymbolIndex = new CrossTileSymbolIndex();
 

diff --git a/src/geo/projection/projection.ts b/src/geo/projection/projection.ts
@@ -0,0 +1,72 @@
+import type {Tile} from '../../source/tile';
+import {pixelsToTileUnits} from '../../source/pixels_to_tile_units';
+import type {PointProjection} from '../../symbol/projection';
+
+/**
+ * A greatly reduced version of the `Projection` interface from the globe branch,
+ * used to port symbol bugfixes over to the main branch. Will be replaced with
+ * the proper interface once globe is merged.
+ */
+export type Projection = {
+    useSpecialProjectionForSymbols: boolean;
+    isOccluded(_x, _y, _t): boolean;
+    projectTileCoordinates(_x, _y, _t, _ele): PointProjection;
+    getPitchedTextCorrection(_transform, _anchor, _tile): number;
+    translatePosition(transform: { angle: number; zoom: number }, tile: Tile, translate: [number, number], translateAnchor: 'map' | 'viewport'): [number, number];
+    getCircleRadiusCorrection(tr: any): number;
+};
+
+export function createProjection(): Projection {
+    return {
+        isOccluded(_x: any, _y: any, _t: any) {
+            return false;
+        },
+        getPitchedTextCorrection(_transform: any, _anchor: any, _tile: any) {
+            return 1.0;
+        },
+        get useSpecialProjectionForSymbols() { return false; },
+        projectTileCoordinates(_x, _y, _t, _ele) {
+            // This function should only be used when useSpecialProjectionForSymbols is set to true.
+            throw new Error('Not implemented.');
+        },
+        translatePosition(transform, tile, translate, translateAnchor) {
+            return translatePosition(transform, tile, translate, translateAnchor);
+        },
+        getCircleRadiusCorrection(_: any) {
+            return 1.0;
+        }
+    };
+}
+
+/**
+ * Returns a translation in tile units that correctly incorporates the view angle and the *-translate and *-translate-anchor properties.
+ * @param inViewportPixelUnitsUnits - True when the units accepted by the matrix are in viewport pixels instead of tile units.
+ *
+ * Temporarily imported from globe branch.
+ */
+function translatePosition(
+    transform: { angle: number; zoom: number },
+    tile: Tile,
+    translate: [number, number],
+    translateAnchor: 'map' | 'viewport',
+    inViewportPixelUnitsUnits: boolean = false
+): [number, number] {
+    if (!translate[0] && !translate[1]) return [0, 0];
+
+    const angle = inViewportPixelUnitsUnits ?
+        (translateAnchor === 'map' ? transform.angle : 0) :
+        (translateAnchor === 'viewport' ? -transform.angle : 0);
+
+    if (angle) {
+        const sinA = Math.sin(angle);
+        const cosA = Math.cos(angle);
+        translate = [
+            translate[0] * cosA - translate[1] * sinA,
+            translate[0] * sinA + translate[1] * cosA
+        ];
+    }
+
+    return [
+        inViewportPixelUnitsUnits ? translate[0] : pixelsToTileUnits(tile, translate[0], transform.zoom),
+        inViewportPixelUnitsUnits ? translate[1] : pixelsToTileUnits(tile, translate[1], transform.zoom)];
+}
diff --git a/src/render/draw_collision_debug.ts b/src/render/draw_collision_debug.ts
@@ -7,7 +7,6 @@ import {DepthMode} from '../gl/depth_mode';
 import {StencilMode} from '../gl/stencil_mode';
 import {CullFaceMode} from '../gl/cull_face_mode';
 import {collisionUniformValues, collisionCircleUniformValues} from './program/collision_program';
-
 import {QuadTriangleArray, CollisionCircleLayoutArray} from '../data/array_types.g';
 import {collisionCircleLayout} from '../data/bucket/symbol_attributes';
 import {SegmentVector} from '../data/segment';
@@ -25,7 +24,7 @@ type TileBatch = {
 
 let quadTriangles: QuadTriangleArray;
 
-export function drawCollisionDebug(painter: Painter, sourceCache: SourceCache, layer: StyleLayer, coords: Array<OverscaledTileID>, translate: [number, number], translateAnchor: 'map' | 'viewport', isText: boolean) {
+export function drawCollisionDebug(painter: Painter, sourceCache: SourceCache, layer: StyleLayer, coords: Array<OverscaledTileID>, isText: boolean) {
     const context = painter.context;
     const gl = context.gl;
     const program = painter.useProgram('collisionBox');
@@ -38,10 +37,6 @@ export function drawCollisionDebug(painter: Painter, sourceCache: SourceCache, l
         const tile = sourceCache.getTile(coord);
         const bucket: SymbolBucket = (tile.getBucket(layer) as any);
         if (!bucket) continue;
-        let posMatrix = coord.posMatrix;
-        if (translate[0] !== 0 || translate[1] !== 0) {
-            posMatrix = painter.translatePosMatrix(coord.posMatrix, tile, translate, translateAnchor);
-        }
         const buffers = isText ? bucket.textCollisionBox : bucket.iconCollisionBox;
         // Get collision circle data of this bucket
         const circleArray: Array<number> = bucket.collisionCircleArray;
@@ -50,31 +45,32 @@ export function drawCollisionDebug(painter: Painter, sourceCache: SourceCache, l
             // This might vary between buckets as the symbol placement is a continuous process. This matrix is
             // required for transforming points from previous screen space to the current one
             const invTransform = mat4.create();
-            const transform = posMatrix;
 
             mat4.mul(invTransform, bucket.placementInvProjMatrix, painter.transform.glCoordMatrix);
             mat4.mul(invTransform, invTransform, bucket.placementViewportMatrix);
 
             tileBatches.push({
                 circleArray,
                 circleOffset,
-                transform,
+                transform: coord.posMatrix, // Ignore translation
                 invTransform,
                 coord
             });
 
             circleCount += circleArray.length / 4;  // 4 values per circle
             circleOffset = circleCount;
         }
-        if (!buffers) continue;
+
+        // Draw collision boxes
+        if (!buffers) {
+            continue;
+        }
+
         program.draw(context, gl.LINES,
             DepthMode.disabled, StencilMode.disabled,
             painter.colorModeForRenderPass(),
             CullFaceMode.disabled,
-            collisionUniformValues(
-                posMatrix,
-                painter.transform,
-                tile),
+            collisionUniformValues(painter.transform, coord.posMatrix),
             painter.style.map.terrain && painter.style.map.terrain.getTerrainData(coord),
             layer.id, buffers.layoutVertexBuffer, buffers.indexBuffer,
             buffers.segments, null, painter.transform.zoom, null, null,

diff --git a/src/render/draw_symbol.test.ts b/src/render/draw_symbol.test.ts
@@ -156,7 +156,7 @@ describe('drawSymbol', () => {
         const spy = jest.spyOn(symbolProjection, 'updateLineLabels');
         drawSymbols(painterMock, sourceCacheMock, layer, [tileId], null);
 
-        expect(spy.mock.calls[0][9]).toBeFalsy(); // rotateToLine === false
+        expect(spy.mock.calls[0][8]).toBeFalsy(); // rotateToLine === false
     });
 
     test('transparent tile optimization should prevent program.draw from being called', () => {