Fix icon text fit

src/symbol/placement.js

@@ -237,7 +237,7 @@ export class Placement {
     attemptAnchorPlacement(anchor: TextAnchor, textBox: SingleCollisionBox, width: number, height: number,
                            textBoxScale: number, rotateWithMap: boolean,
                            pitchWithMap: boolean, textPixelRatio: number, posMatrix: mat4, collisionGroup: CollisionGroup,
-                           textAllowOverlap: boolean, symbolInstance: SymbolInstance, bucket: SymbolBucket, orientation: number): ?{ box: Array<number>, offscreen: boolean }  {
+                           textAllowOverlap: boolean, symbolInstance: SymbolInstance, bucket: SymbolBucket, orientation: number): ?{ shift: Point, placedGlyphBoxes: { box: Array<number>, offscreen: boolean } }  {
 
         const textOffset = [symbolInstance.textOffset0, symbolInstance.textOffset1];
         const shift = calculateVariableLayoutShift(anchor, width, height, textOffset, textBoxScale);
@@ -274,7 +274,7 @@ export class Placement {
                 this.placedOrientations[symbolInstance.crossTileID] = orientation;
             }
 
-            return placedGlyphBoxes;
+            return {shift, placedGlyphBoxes};
         }
     }
 
@@ -308,6 +308,7 @@ export class Placement {
 
         const rotateWithMap = layout.get('text-rotation-alignment') === 'map';
         const pitchWithMap = layout.get('text-pitch-alignment') === 'map';
+        const hasIconTextFit = layout.get('icon-text-fit') !== 'none';
         const zOrderByViewportY = layout.get('symbol-z-order') === 'viewport-y';
 
         if (!bucket.collisionArrays && collisionBoxArray) {
@@ -326,6 +327,7 @@ export class Placement {
             let placeText = false;
             let placeIcon = false;
             let offscreen = true;
+            let shift = null;
 
             let placed = {box: null, offscreen: null};
             let placedVertical = {box: null, offscreen: null};
@@ -426,14 +428,18 @@ export class Placement {
                         for (let i = 0; i < placementAttempts; ++i) {
                             const anchor = anchors[i % anchors.length];
                             const allowOverlap = (i >= anchors.length);
-                            placedBox = this.attemptAnchorPlacement(
+                            const result = this.attemptAnchorPlacement(
                                 anchor, collisionTextBox, width, height,
                                 textBoxScale, rotateWithMap, pitchWithMap, textPixelRatio, posMatrix,
                                 collisionGroup, allowOverlap, symbolInstance, bucket, orientation);
 
-                            if (placedBox && placedBox.box && placedBox.box.length) {
-                                placeText = true;
-                                break;
+                            if (result) {
+                                placedBox = result.placedGlyphBoxes;
+                                if (placedBox && placedBox.box && placedBox.box.length) {
+                                    placeText = true;
+                                    shift = result.shift;
+                                    break;
+                                }
                             }
                         }
 
@@ -508,7 +514,14 @@ export class Placement {
                 iconFeatureIndex = collisionArrays.iconFeatureIndex;
             }
             if (collisionArrays.iconBox) {
-                placedIconBoxes = this.collisionIndex.placeCollisionBox(collisionArrays.iconBox,
+
+                const iconBox = hasIconTextFit && shift ?
+                    shiftVariableCollisionBox(
+                        collisionArrays.iconBox, shift.x, shift.y,
+                        rotateWithMap, pitchWithMap, this.transform.angle) :
+                    collisionArrays.iconBox;
+
+                placedIconBoxes = this.collisionIndex.placeCollisionBox(iconBox,
                         layout.get('icon-allow-overlap'), textPixelRatio, posMatrix, collisionGroup.predicate);
                 placeIcon = placedIconBoxes.box.length > 0;
                 offscreen = offscreen && placedIconBoxes.offscreen;
@@ -709,6 +722,7 @@ export class Placement {
         const variablePlacement = layout.get('text-variable-anchor');
         const rotateWithMap = layout.get('text-rotation-alignment') === 'map';
         const pitchWithMap = layout.get('text-pitch-alignment') === 'map';
+        const hasIconTextFit = layout.get('icon-text-fit') !== 'none';
         // If allow-overlap is true, we can show symbols before placement runs on them
         // But we have to wait for placement if we potentially depend on a paired icon/text
         // with allow-overlap: false.
@@ -802,8 +816,8 @@ export class Placement {
                 bucket.hasTextCollisionBoxData() || bucket.hasTextCollisionCircleData()) {
                 const collisionArrays = bucket.collisionArrays[s];
                 if (collisionArrays) {
+                    let shift = new Point(0, 0);
                     if (collisionArrays.textBox) {
-                        let shift = new Point(0, 0);
                         let used = true;
                         if (variablePlacement) {
                             const variableOffset = this.variableOffsets[crossTileID];
@@ -832,7 +846,9 @@ export class Placement {
                     }
 
                     if (collisionArrays.iconBox) {
-                        updateCollisionVertices(bucket.iconCollisionBox.collisionVertexArray, opacityState.icon.placed, false);
+                        updateCollisionVertices(bucket.iconCollisionBox.collisionVertexArray, opacityState.icon.placed, false,
+                            hasIconTextFit ? shift.x : 0,
+                            hasIconTextFit ? shift.y : 0);
                     }
 
                     const textCircles = collisionArrays.textCircles;

src/symbol/quads.js

@@ -44,58 +44,35 @@ export type SymbolQuad = {
  * Create the quads used for rendering an icon.
  * @private
  */
-export function getIconQuads(anchor: Anchor,
+export function getIconQuads(
                       shapedIcon: PositionedIcon,
-                      layer: SymbolStyleLayer,
-                      alongLine: boolean,
-                      shapedText: Shaping | null,
-                      feature: Feature): Array<SymbolQuad> {
+                      iconRotate: number): Array<SymbolQuad> {
     const image = shapedIcon.image;
-    const layout = layer.layout;
 
     // If you have a 10px icon that isn't perfectly aligned to the pixel grid it will cover 11 actual
     // pixels. The quad needs to be padded to account for this, otherwise they'll look slightly clipped
     // on one edge in some cases.
     const border = 1;
 
-    const top = shapedIcon.top - border / image.pixelRatio;
-    const left = shapedIcon.left - border / image.pixelRatio;
-    const bottom = shapedIcon.bottom + border / image.pixelRatio;
-    const right = shapedIcon.right + border / image.pixelRatio;
-    let tl, tr, br, bl;
-
-    // text-fit mode
-    if (layout.get('icon-text-fit') !== 'none' && shapedText) {
-        const iconWidth = (right - left),
-            iconHeight = (bottom - top),
-            size = layout.get('text-size').evaluate(feature, {}) / 24,
-            textLeft = shapedText.left * size,
-            textRight = shapedText.right * size,
-            textTop = shapedText.top * size,
-            textBottom = shapedText.bottom * size,
-            textWidth = textRight - textLeft,
-            textHeight = textBottom - textTop,
-            padT = layout.get('icon-text-fit-padding')[0],
-            padR = layout.get('icon-text-fit-padding')[1],
-            padB = layout.get('icon-text-fit-padding')[2],
-            padL = layout.get('icon-text-fit-padding')[3],
-            offsetY = layout.get('icon-text-fit') === 'width' ? (textHeight - iconHeight) * 0.5 : 0,
-            offsetX = layout.get('icon-text-fit') === 'height' ? (textWidth - iconWidth) * 0.5 : 0,
-            width = layout.get('icon-text-fit') === 'width' || layout.get('icon-text-fit') === 'both' ? textWidth : iconWidth,
-            height = layout.get('icon-text-fit') === 'height' || layout.get('icon-text-fit') === 'both' ? textHeight : iconHeight;
-        tl = new Point(textLeft + offsetX - padL,         textTop + offsetY - padT);
-        tr = new Point(textLeft + offsetX + padR + width, textTop + offsetY - padT);
-        br = new Point(textLeft + offsetX + padR + width, textTop + offsetY + padB + height);
-        bl = new Point(textLeft + offsetX - padL,         textTop + offsetY + padB + height);
-    // Normal icon size mode
-    } else {
-        tl = new Point(left, top);
-        tr = new Point(right, top);
-        br = new Point(right, bottom);
-        bl = new Point(left, bottom);
-    }
-
-    const angle = layer.layout.get('icon-rotate').evaluate(feature, {}) * Math.PI / 180;
+    // Expand the box to respect the 1 pixel border in the atlas image. We're using `image.paddedRect - border`
+    // instead of image.displaySize because we only pad with one pixel for retina images as well, and the
+    // displaySize uses the logical dimensions, not the physical pixel dimensions.
+    const iconWidth = shapedIcon.right - shapedIcon.left;
+    const expandX = (iconWidth * image.paddedRect.w / (image.paddedRect.w - 2 * border) - iconWidth) / 2;
+    const left = shapedIcon.left - expandX;
+    const right = shapedIcon.right + expandX;
+
+    const iconHeight = shapedIcon.bottom - shapedIcon.top;
+    const expandY = (iconHeight * image.paddedRect.h / (image.paddedRect.h - 2 * border) - iconHeight) / 2;
+    const top = shapedIcon.top - expandY;
+    const bottom = shapedIcon.bottom + expandY;
+
+    const tl = new Point(left, top);
+    const tr = new Point(right, top);
+    const br = new Point(right, bottom);
+    const bl = new Point(left, bottom);
+
+    const angle = iconRotate * Math.PI / 180;
 
     if (angle) {
         const sin = Math.sin(angle),

src/symbol/shaping.js

@@ -1,5 +1,6 @@
 // @flow
 
+import assert from 'assert';
 import {
     charHasUprightVerticalOrientation,
     charAllowsIdeographicBreaking,
@@ -19,7 +20,7 @@ const WritingMode = {
     horizontalOnly: 3
 };
 
-export {shapeText, shapeIcon, getAnchorAlignment, WritingMode};
+export {shapeText, shapeIcon, fitIconToText, getAnchorAlignment, WritingMode};
 
 // The position of a glyph relative to the text's anchor point.
 export type PositionedGlyph = {
@@ -581,3 +582,46 @@ function shapeIcon(image: ImagePosition, iconOffset: [number, number], iconAncho
     const y2 = y1 + image.displaySize[1];
     return {image, top: y1, bottom: y2, left: x1, right: x2};
 }
+
+function fitIconToText(shapedIcon: PositionedIcon, shapedText: Shaping,
+                       textFit: string,
+                       padding: [ number, number, number, number ],
+                       iconOffset: [ number, number ], fontScale: number): PositionedIcon {
+    assert(textFit !== 'none');
+    assert(Array.isArray(padding) && padding.length === 4);
+    assert(Array.isArray(iconOffset) && iconOffset.length === 2);
+
+    const image = shapedIcon.image;
+
+    // We don't respect the icon-anchor, because icon-text-fit is set. Instead,
+    // the icon will be centered on the text, then stretched in the given
+    // dimensions.
+
+    const textLeft = shapedText.left * fontScale;
+    const textRight = shapedText.right * fontScale;
+
+    let top, right, bottom, left;
+    if (textFit === 'width' || textFit === 'both') {
+        // Stretched horizontally to the text width
+        left = iconOffset[0] + textLeft - padding[3];
+        right = iconOffset[0] + textRight + padding[1];
+    } else {
+        // Centered on the text
+        left = iconOffset[0] + (textLeft + textRight - image.displaySize[0]) / 2;
+        right = left + image.displaySize[0];
+    }
+
+    const textTop = shapedText.top * fontScale;
+    const textBottom = shapedText.bottom * fontScale;
+    if (textFit === 'height' || textFit === 'both') {
+        // Stretched vertically to the text height
+        top = iconOffset[1] + textTop - padding[0];
+        bottom = iconOffset[1] + textBottom + padding[2];
+    } else {
+        // Centered on the text
+        top = iconOffset[1] + (textTop + textBottom - image.displaySize[1]) / 2;
+        bottom = top + image.displaySize[1];
+    }
+
+    return {image, top, right, bottom, left};
+}

src/symbol/symbol_layout.js

@@ -4,7 +4,7 @@ import Anchor from './anchor';
 
 import {getAnchors, getCenterAnchor} from './get_anchors';
 import clipLine from './clip_line';
-import {shapeText, shapeIcon, WritingMode} from './shaping';
+import {shapeText, shapeIcon, WritingMode, fitIconToText} from './shaping';
 import {getGlyphQuads, getIconQuads} from './quads';
 import CollisionFeature from './collision_feature';
 import {warnOnce} from '../util/util';
@@ -377,6 +377,15 @@ function addFeature(bucket: SymbolBucket,
         symbolPlacement = layout.get('symbol-placement'),
         textRepeatDistance = symbolMinDistance / 2;
 
+    const iconTextFit = layout.get('icon-text-fit');
+    // Adjust shaped icon size when icon-text-fit is used.
+    if (shapedIcon && iconTextFit !== 'none') {
+        if (defaultHorizontalShaping) {
+            shapedIcon = fitIconToText(shapedIcon, defaultHorizontalShaping, iconTextFit,
+                                       layout.get('icon-text-fit-padding'), iconOffset, fontScale);
+        }
+    }
+
     const addSymbolAtAnchor = (line, anchor) => {
         if (anchor.x < 0 || anchor.x >= EXTENT || anchor.y < 0 || anchor.y >= EXTENT) {
             // Symbol layers are drawn across tile boundaries, We filter out symbols
@@ -577,10 +586,8 @@ function addSymbol(bucket: SymbolBucket,
     //If the style specifies an `icon-text-fit` then the icon would have to shift along with it.
     // For more info check `updateVariableAnchors` in `draw_symbol.js` .
     if (shapedIcon) {
-        const iconQuads = getIconQuads(anchor, shapedIcon, layer,
-                            iconAlongLine, getDefaultHorizontalShaping(shapedTextOrientations.horizontal),
-                            feature);
         const iconRotate = layer.layout.get('icon-rotate').evaluate(feature, {});
+        const iconQuads = getIconQuads(shapedIcon, iconRotate);
         iconCollisionFeature = new CollisionFeature(collisionBoxArray, line, anchor, featureIndex, sourceLayerIndex, bucketIndex, shapedIcon, iconBoxScale, iconPadding, /*align boxes to line*/false, bucket.overscaling, iconRotate);
 
         numIconVertices = iconQuads.length * 4;

test/ignores.json

@@ -13,19 +13,7 @@
   "render-tests/mixed-zoom/z10-z11": "current behavior conflicts with https://github.com/mapbox/mapbox-gl-js/pull/6803. can be fixed when https://github.com/mapbox/api-maps/issues/1480 is done",
   "render-tests/fill-extrusion-pattern/tile-buffer": "https://github.com/mapbox/mapbox-gl-js/issues/4403",
   "render-tests/symbol-sort-key/text-ignore-placement": "skip - text drawn over icons",
-  "render-tests/icon-text-fit/both-padding": "https://github.com/mapbox/mapbox-gl-js/issues/8583",
-  "render-tests/icon-text-fit/both": "https://github.com/mapbox/mapbox-gl-js/issues/8583",
-  "render-tests/icon-text-fit/height-padding": "https://github.com/mapbox/mapbox-gl-js/issues/8583",
-  "render-tests/icon-text-fit/height": "https://github.com/mapbox/mapbox-gl-js/issues/8583",
-  "render-tests/icon-text-fit/placement-line": "https://github.com/mapbox/mapbox-gl-js/issues/8583",
-  "render-tests/icon-text-fit/width-padding": "https://github.com/mapbox/mapbox-gl-js/issues/8583",
-  "render-tests/icon-text-fit/width": "https://github.com/mapbox/mapbox-gl-js/issues/8583",
-  "render-tests/icon-text-fit/both-collision-variable-anchor-text-fit": "https://github.com/mapbox/mapbox-gl-js/issues/8583",
-  "render-tests/icon-text-fit/both-collision-variable-anchor": "https://github.com/mapbox/mapbox-gl-js/issues/8583",
-  "render-tests/icon-text-fit/both-collision": "https://github.com/mapbox/mapbox-gl-js/issues/8583",
-  "render-tests/regressions/mapbox-gl-js#5631": "https://github.com/mapbox/mapbox-gl-js/issues/8583",
-  "render-tests/text-variable-anchor/all-anchors-icon-text-fit": "https://github.com/mapbox/mapbox-gl-js/issues/8583",
-  "render-tests/text-variable-anchor/icon-text-fit-collision-box": "https://github.com/mapbox/mapbox-gl-js/issues/8583",
+  "render-tests/text-variable-anchor/remember-last-placement": "skip - not sure this is correct behavior",
   "render-tests/text-writing-mode/point_label/cjk-variable-anchors-vertical-horizontal-mode-icon-text-fit": "https://github.com/mapbox/mapbox-gl-js/issues/8583",
   "render-tests/text-writing-mode/point_label/mixed-multiline-vertical-horizontal-mode-icon-text-fit": "https://github.com/mapbox/mapbox-gl-js/issues/8583",
   "render-tests/icon-image/icon-sdf-non-sdf-one-layer": "skip - render sdf icon and normal icon in one layer"

test/integration/geojson/anchors.json

@@ -0,0 +1,85 @@
+{
+    "type": "FeatureCollection",
+    "features": [{
+        "type": "Feature",
+        "properties": {
+            "anchor": "center"
+        },
+        "geometry": {
+            "type": "Point", 
+            "coordinates": [ 0, 0 ]
+        }
+    }, {
+        "type": "Feature",
+        "properties": {
+            "anchor": "left"
+        },
+        "geometry": {
+            "type": "Point", 
+            "coordinates": [ 30, 0 ]
+        }
+    }, {
+        "type": "Feature",
+        "properties": {
+            "anchor": "top-left"
+        },
+        "geometry": {
+            "type": "Point", 
+            "coordinates": [ 20, -15 ]
+        }
+    }, {
+        "type": "Feature",
+        "properties": {
+            "anchor": "top"
+        },
+        "geometry": {
+            "type": "Point", 
+            "coordinates": [ 0, -25 ]
+        }
+    }, {
+        "type": "Feature",
+        "properties": {
+            "anchor": "top-right"
+        },
+        "geometry": {
+            "type": "Point", 
+            "coordinates": [ -20, -15 ]
+        }
+    }, {
+        "type": "Feature",
+        "properties": {
+            "anchor": "right"
+        },
+        "geometry": {
+            "type": "Point", 
+            "coordinates": [ -30, 0 ]
+        }
+    }, {
+        "type": "Feature",
+        "properties": {
+            "anchor": "bottom-left"
+        },
+        "geometry": {
+            "type": "Point", 
+            "coordinates": [ 20, 15 ]
+        }
+    }, {
+        "type": "Feature",
+        "properties": {
+            "anchor": "bottom"
+        },
+        "geometry": {
+            "type": "Point", 
+            "coordinates": [ 0, 25 ]
+        }
+    }, {
+        "type": "Feature",
+        "properties": {
+            "anchor": "bottom-right"
+        },
+        "geometry": {
+            "type": "Point", 
+            "coordinates": [ -20, 15 ]
+        }
+    }]
+}