Fix update placement after changes (internal-8903)

GitOrigin-RevId: d0af91f6de57bd859b23ecf2953868d8e56e5af8

Author: underoot

src/style/pauseable_placement.ts

@@ -102,6 +102,17 @@ class PauseablePlacement {
         return this._forceFullPlacement;
     }
 
+    setStale(): void {
+        if (this.placement) {
+            this.placement.stale = true;
+        }
+    }
+
+    isStale(): boolean {
+        if (!this.placement) return false;
+        return this.placement.stale;
+    }
+
     isDone(): boolean {
         return this._done;
     }

src/style/style.ts

@@ -4180,13 +4180,15 @@ class Style extends Evented<MapEvents> {
         const transformChanged = Boolean(this.placement && !transform.equals(this.placement.transform));
         const replacementSourceChanged = Boolean(this.placement && ((this.placement.lastReplacementSourceUpdateTime !== 0 && !replacementSource) || this.placement.lastReplacementSourceUpdateTime !== replacementSource.updateTime));
 
+        const isDonePlacementConsideredStale = transformChanged || replacementSourceChanged || symbolBucketsChanged;
         // Force full placement when fadeDuration === 0 to ensure that newly loaded
         // tiles will fully display symbols in their first frame.
         // Make sure to not do this for the static camera, otherwise
         // we will do expensive full placements on every frame.
-        const fullFrameUpdateRequired = (transformChanged || replacementSourceChanged || symbolBucketsChanged || (this.placement && this.placement.isStale())) && fadeDuration === 0;
+        const newImmediatePlacementRequired = (isDonePlacementConsideredStale || this.pauseablePlacement.isStale()) && fadeDuration === 0;
+        const newNormalPlacementRequired = this.pauseablePlacement.isDone() && !this.placement.stillRecent(browser.now(), transform.zoom) && fadeDuration !== 0;
 
-        if (this.pauseablePlacement.isFullPlacementRequested() || !this.pauseablePlacement.placement || fullFrameUpdateRequired || (fadeDuration !== 0 && this.pauseablePlacement.isDone() && !this.placement.stillRecent(browser.now(), transform.zoom))) {
+        if (this.pauseablePlacement.isFullPlacementRequested() || !this.pauseablePlacement.placement || newImmediatePlacementRequired || newNormalPlacementRequired) {
             const fogState = this.fog && transform.projection.supportsFog ? this.fog.state : null;
             this.pauseablePlacement = this.pauseablePlacement.startNewPlacement(transform, this._mergedOrder, showCollisionBoxes, fadeDuration, crossSourceCollisions, this.placement, fogState, this._buildingIndex);
         }
@@ -4203,8 +4205,10 @@ class Style extends Evented<MapEvents> {
                 // since the placement gets split over multiple frames it is possible
                 // these buckets were processed before they were changed and so the
                 // placement is already stale while it is in progress
-                this.pauseablePlacement.placement.setStale();
+                this.pauseablePlacement.setStale();
             }
+        } else if (isDonePlacementConsideredStale && fadeDuration !== 0) {
+            this.pauseablePlacement.setStale();
         }
 
         if (placementCommitted || symbolBucketsChanged) {

test/unit/style/style.test.ts

@@ -2856,4 +2856,128 @@ describe('Style#_updatePlacement', () => {
         expect(startNewPlacementSpy).not.toHaveBeenCalled();
         expect(continuePlacementSpy).not.toHaveBeenCalled();
     });
+
+    test('returns true when symbol layer is added after load due to symbolBucketsChanged', async () => {
+        const map = new StubMap();
+        // @ts-expect-error - painter is not part of StubMap but required for _updatePlacement
+        map.painter = {scaleFactor: 1};
+        const replacementSource = {updateTime: 0};
+
+        const style = new Style(map);
+        style.loadJSON({
+            "version": 8,
+            "sources": {
+                "geojson": {
+                    "type": "geojson",
+                    "data": {"type": "FeatureCollection", "features": []}
+                }
+            },
+            "layers": []
+        });
+
+        await waitFor(style, 'style.load');
+
+        const tr = map.transform;
+        tr.resize(512, 512);
+
+        style.update({zoom: tr.zoom, fadeDuration: 0});
+        style._updatePlacement(tr, false, 0, false, replacementSource);
+
+        expect(style.pauseablePlacement.isDone()).toBeTruthy();
+
+        style.addLayer({
+            "id": "symbol",
+            "type": "symbol",
+            "source": "geojson"
+        });
+
+        style.update({zoom: tr.zoom, fadeDuration: 0});
+
+        // Spy on crossTileSymbolIndex.addLayer to return true (simulating new symbol buckets)
+        // This happens in real scenarios when tiles have symbol features
+        vi.spyOn(style.crossTileSymbolIndex, 'addLayer').mockReturnValue(true);
+
+        const result = style._updatePlacement(tr, false, 0, false, replacementSource);
+
+        expect(result).toBeTruthy();
+    });
+
+    test('returns true when transformChanged', async () => {
+        const map = new StubMap();
+        // @ts-expect-error - painter is not part of StubMap but required for _updatePlacement
+        map.painter = {scaleFactor: 1};
+        const replacementSource = {updateTime: 0};
+
+        const style = new Style(map);
+        style.loadJSON({
+            "version": 8,
+            "sources": {
+                "geojson": {
+                    "type": "geojson",
+                    "data": {"type": "FeatureCollection", "features": []}
+                }
+            },
+            "layers": [{
+                "id": "symbol",
+                "type": "symbol",
+                "source": "geojson"
+            }]
+        });
+
+        await waitFor(style, 'style.load');
+
+        const tr = map.transform;
+        tr.resize(512, 512);
+
+        style.update({zoom: tr.zoom, fadeDuration: 0});
+        style._updatePlacement(tr, false, 0, false, replacementSource);
+
+        expect(style.pauseablePlacement.isDone()).toBeTruthy();
+
+        // Change transform to trigger transformChanged
+        tr.zoom = 5;
+
+        // Use fadeDuration > 0 so that setStale() is called when placement is
+        // considered stale due to transform change (line 4210-4211 in style.ts)
+        const result = style._updatePlacement(tr, false, 300, false, replacementSource);
+
+        expect(result).toBeTruthy();
+    });
+
+    test('returns true when replacementSourceChanged', async () => {
+        const map = new StubMap();
+        // @ts-expect-error - painter is not part of StubMap but required for _updatePlacement
+        map.painter = {scaleFactor: 1};
+
+        const style = new Style(map);
+        style.loadJSON({
+            "version": 8,
+            "sources": {
+                "geojson": {
+                    "type": "geojson",
+                    "data": {"type": "FeatureCollection", "features": []}
+                }
+            },
+            "layers": [{
+                "id": "symbol",
+                "type": "symbol",
+                "source": "geojson"
+            }]
+        });
+
+        await waitFor(style, 'style.load');
+
+        const tr = map.transform;
+        tr.resize(512, 512);
+
+        style.update({zoom: tr.zoom, fadeDuration: 0});
+        style._updatePlacement(tr, false, 0, false, {updateTime: 0});
+
+        expect(style.pauseablePlacement.isDone()).toBeTruthy();
+
+        // Change replacementSource updateTime to trigger replacementSourceChanged
+        const result = style._updatePlacement(tr, false, 300, false, {updateTime: 1});
+
+        expect(result).toBeTruthy();
+    });
 });