fix(IconManager) Fix icon frame x/y for auto packed icons (#9928)

Author: gabeboning

modules/layers/src/icon-layer/icon-layer.ts

@@ -309,8 +309,13 @@ export default class IconLayer<DataT = any, ExtraPropsT extends {} = {}> extends
     });
   }
 
-  private _onUpdate(): void {
-    this.setNeedsRedraw();
+  private _onUpdate(didFrameChange: boolean): void {
+    if (didFrameChange) {
+      this.getAttributeManager()?.invalidate('getIcon');
+      this.setNeedsUpdate();
+    } else {
+      this.setNeedsRedraw();
+    }
   }
 
   private _onError(evt: LoadIconErrorContext): void {

modules/layers/src/icon-layer/icon-manager.ts

@@ -296,7 +296,7 @@ export function getDiffIcons(
 export default class IconManager {
   device: Device;
 
-  private onUpdate: () => void;
+  private onUpdate: (didFrameChange: boolean) => void;
   private onError: (context: LoadIconErrorContext) => void;
   private _loadOptions: any = null;
   private _texture: Texture | null = null;
@@ -326,7 +326,7 @@ export default class IconManager {
       onError = noop
     }: {
       /** Callback when the texture updates */
-      onUpdate: () => void;
+      onUpdate: (didFrameChange: boolean) => void;
       /** Callback when an error is encountered */
       onError: (context: LoadIconErrorContext) => void;
     }
@@ -435,7 +435,7 @@ export default class IconManager {
         );
       }
 
-      this.onUpdate();
+      this.onUpdate(true);
 
       // load images
       this._canvas = this._canvas || document.createElement('canvas');
@@ -461,7 +461,7 @@ export default class IconManager {
           const id = getIconId(icon);
 
           const iconDef = this._mapping[id];
-          const {x, y, width: maxWidth, height: maxHeight} = iconDef;
+          const {x: initialX, y: initialY, width: maxWidth, height: maxHeight} = iconDef;
 
           const {image, width, height} = resizeImage(
             ctx,
@@ -470,20 +470,25 @@ export default class IconManager {
             maxHeight
           );
 
+          const x = initialX + (maxWidth - width) / 2;
+          const y = initialY + (maxHeight - height) / 2;
+
           this._texture?.copyExternalImage({
             image,
-            x: x + (maxWidth - width) / 2,
-            y: y + (maxHeight - height) / 2,
+            x,
+            y,
             width,
             height
           });
+          iconDef.x = x;
+          iconDef.y = y;
           iconDef.width = width;
           iconDef.height = height;
 
           // Call to regenerate mipmaps after modifying texture(s)
           this._texture?.generateMipmapsWebGL();
 
-          this.onUpdate();
+          this.onUpdate(width !== maxWidth || height !== maxHeight);
         })
         .catch(error => {
           this.onError({

test/modules/layers/icon-manager.spec.ts

@@ -251,16 +251,30 @@ test('IconManager#resize', t => {
   const testImage =
     'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAAAAAA6mKC9AAAACXBIWXMAAD2EAAA9hAHVrK90AAAAjElEQVQYlXWPMQrCQBBFn7NJI9oobraz1kqv4zk9h4iCEDsLq4gBR4LgGi3WNRhxmj88Zv6f6Sz5LuEPSNMWsLYFnHs3SRBjMY8I+iPoCpMKCiUBHUwFGFPvNKwcynkPuK40ml5ygFyblAzvyZoUcec1M7etAbMAhrfN3R+FKk6UJ+C5Nx+PcFKQn29fOzIjztSX8AwAAAAASUVORK5CYII=';
   let updateCount = 0;
+
+  const icons = [
+    {id: 'no-resize', width: 16, height: 16},
+    {id: 'down-size', width: 12, height: 12},
+    {id: 'preserve-aspect-ratio-landscape', width: 32, height: 24},
+    {id: 'preserve-aspect-ratio-portrait', width: 12, height: 16}
+  ];
+
+  const assertIconFrame = (id, expected) => {
+    const mapping = iconManager.getIconMapping({id});
+    t.is(mapping.x, expected.x, `${id} x`);
+    t.is(mapping.y, expected.y, `${id} y`);
+    t.is(mapping.width, expected.width, `${id} width`);
+    t.is(mapping.height, expected.height, `${id} height`);
+  };
+
   const onUpdate = () => {
     updateCount++;
-    if (updateCount > 3) {
-      t.is(iconManager.getIconMapping({id: 'no-resize'}).width, 16, 'no-resize');
-      t.is(iconManager.getIconMapping({id: 'down-size'}).width, 12, 'down-size');
-      t.is(
-        iconManager.getIconMapping({id: 'preserve-aspect-ratio'}).width,
-        24,
-        'preserve-aspect-ratio'
-      );
+    if (updateCount > icons.length) {
+      assertIconFrame('no-resize', {x: 0, y: 0, width: 16, height: 16});
+      assertIconFrame('down-size', {x: 20, y: 0, width: 12, height: 12});
+      assertIconFrame('preserve-aspect-ratio-landscape', {x: 40, y: 0, width: 24, height: 24});
+      assertIconFrame('preserve-aspect-ratio-portrait', {x: 72, y: 2, width: 12, height: 12});
+
       t.end();
     }
   };
@@ -275,15 +289,8 @@ test('IconManager#resize', t => {
   iconManager.setProps({
     autoPacking: true
   });
-  iconManager.packIcons(
-    [
-      {id: 'no-resize', width: 16, height: 16},
-      {id: 'down-size', width: 12, height: 12},
-      {id: 'preserve-aspect-ratio', width: 32, height: 24}
-    ],
-    d => ({
-      ...d,
-      url: testImage
-    })
-  );
+  iconManager.packIcons(icons, d => ({
+    ...d,
+    url: testImage
+  }));
 });