From 6ad90f1c424da755bca1593343b8ebcc6e88751a Mon Sep 17 00:00:00 2001 From: Eric Date: Wed, 26 Aug 2026 20:17:00 -0400 Subject: [PATCH 1/2] Fix crash when a symbol layer without feature-state paint gets a paint update Tile#updateBuckets picks bucket.stateDependentLayers whenever withStateUpdates is true, but enters the update block under separately-checked conditions (hasPaintUpdate, needsSymbolUBOUpdate). If a source carries feature state but none of a bucket's layers actually read it in their paint (isStateDependent), stateDependentLayers is empty, so a concurrent paint-property change (e.g. setFeatureState followed by setPaintProperty) passes an empty layers array into SymbolBucket#update. Its UBO-based update path unconditionally casts layers[0] to SymbolStyleLayer and dereferences .paint, throwing when the array is empty. Factor the repeated withStateUpdates && stateDependentLayers.length !== 0 check into one hasStateDependentLayers value, reused for both the layers selection and all three guards that depend on it, so the array chosen can never disagree with the condition that lets it be used. Fixes https://github.com/mapbox/mapbox-gl-js/issues/13714 Co-Authored-By: Claude Sonnet 5 --- src/source/tile.ts | 15 +++++++++----- test/unit/source/tile.test.ts | 38 +++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/src/source/tile.ts b/src/source/tile.ts index 0f29cc2b594..e9eefb58a01 100644 --- a/src/source/tile.ts +++ b/src/source/tile.ts @@ -797,8 +797,14 @@ class Tile { const imagePositions: SpritePositions = this.imageAtlas ? Object.fromEntries(this.imageAtlas.patternPositions) : {}; const withStateUpdates = Object.keys(sourceLayerStates).length > 0 && !isBrightnessChanged; bucket.hasAppearances = bucket.layers.some(layer => layer.appearances && layer.appearances.length > 0); - const layers = withStateUpdates ? bucket.stateDependentLayers : bucket.layers; - if ((withStateUpdates && bucket.stateDependentLayers.length !== 0) || isBrightnessChanged || hasPaintUpdate || needsSymbolUBOUpdate) { + // withStateUpdates only means the source carries feature state; it says nothing about + // whether this bucket's layers actually read it (isStateDependent). A bucket can have + // withStateUpdates true and an empty stateDependentLayers, so the two must be checked + // together wherever stateDependentLayers is used, or bucket.update() below can be + // called with an empty layers array and crash on layers[0]. + const hasStateDependentLayers = withStateUpdates && bucket.stateDependentLayers.length !== 0; + const layers = hasStateDependentLayers ? bucket.stateDependentLayers : bucket.layers; + if (hasStateDependentLayers || isBrightnessChanged || hasPaintUpdate || needsSymbolUBOUpdate) { const vtLayers = this.latestFeatureIndex.loadVTLayers(); const sourceLayer = vtLayers[sourceLayerId]; bucket.update(sourceLayerStates, sourceLayer, images, imagePositions, layers, isBrightnessChanged, brightness, this.tileID.canonical); @@ -849,15 +855,14 @@ class Tile { } } } - if ((withStateUpdates && bucket.stateDependentLayers.length !== 0) || isBrightnessChanged || bucket.hasAppearances) { + if (hasStateDependentLayers || isBrightnessChanged || bucket.hasAppearances) { const globalProperties = { zoom: painter.transform.zoom, pitch: painter.transform.pitch, brightness: painter.style.getBrightness() || 0, worldview: painter.worldview }; - const featureStateChanged = withStateUpdates && bucket.stateDependentLayers.length !== 0; - const result = bucket.updateAppearances(this.tileID.canonical, sourceLayerStates, images, globalProperties, painter.imageManager, featureStateChanged); + const result = bucket.updateAppearances(this.tileID.canonical, sourceLayerStates, images, globalProperties, painter.imageManager, hasStateDependentLayers); if (result && result.hasUboChanges) { const context = painter.context; if (bucket instanceof SymbolBucket && bucket.text && bucket.text.uboBinder) { diff --git a/test/unit/source/tile.test.ts b/test/unit/source/tile.test.ts index f92e3dc411e..4258ddf711b 100644 --- a/test/unit/source/tile.test.ts +++ b/test/unit/source/tile.test.ts @@ -284,6 +284,44 @@ describe('rtl text detection', () => { ); }); +describe('Tile#updateBuckets', () => { + test('does not throw when feature state and a paint update land on a symbol layer with no state-dependent paint', () => { + const tile = new Tile(new OverscaledTileID(1, 0, 1, 1, 1), 512, 22); + const symbolBucket = createSymbolBucket('test', 'Test', 'test', new CollisionBoxArray()); + // Force UBO binder creation: the crash only happens on the UBO-based update path in + // SymbolBucket#update, which is skipped entirely when text/icon.uboBinder is null. + symbolBucket.createArrays(); + + const layer = symbolBucket.layers[0]; + const painter = createPainter({ + hasLayer: () => true, + listImages: () => [], + getBrightness: () => 0, + getLayerSourceCache: () => undefined, + getLayer: () => layer, + getOwnLayer: () => layer + }); + + tile.loadVectorData( + createVectorData({rawTileData: rawTileData as ArrayBuffer, buckets: [symbolBucket]}), + painter + ); + + // createSymbolBucket() gives the layer only layout properties (text-font, text-field); + // its paint is left at defaults, so it never reads feature-state and stateDependentLayers + // stays empty even though the source below carries feature state. + expect(tile.buckets[layer.fqid].stateDependentLayers).toEqual([]); + + expect(() => tile.updateBuckets( + painter, + false, // isBrightnessChanged + {_geojsonTileLayer: {'1': {hover: true}}}, // states -> withStateUpdates = true + false, // needsSymbolUBOUpdate + new Set([layer.fqid]) // updatedPaintProps -> hasPaintUpdate = true + )).not.toThrow(); + }); +}); + function createVectorData(options?: {buckets?: Bucket[]; rawTileData?: ArrayBuffer}): WorkerSourceVectorTileResult { const collisionBoxArray = new CollisionBoxArray(); return ({collisionBoxArray: deserialize(serialize(collisionBoxArray)), From 5b5cf4be2febea66365ee97f9198286d2d77aa1d Mon Sep 17 00:00:00 2001 From: Eric Date: Wed, 26 Aug 2026 20:26:58 -0400 Subject: [PATCH 2/2] Remove explanatory comment from tile.ts fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The invariant it described belongs in the PR description, not the code — it referenced the fix history rather than a non-obvious runtime constraint. Co-Authored-By: Claude Sonnet 5 --- src/source/tile.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/source/tile.ts b/src/source/tile.ts index e9eefb58a01..2612d8512fa 100644 --- a/src/source/tile.ts +++ b/src/source/tile.ts @@ -797,11 +797,6 @@ class Tile { const imagePositions: SpritePositions = this.imageAtlas ? Object.fromEntries(this.imageAtlas.patternPositions) : {}; const withStateUpdates = Object.keys(sourceLayerStates).length > 0 && !isBrightnessChanged; bucket.hasAppearances = bucket.layers.some(layer => layer.appearances && layer.appearances.length > 0); - // withStateUpdates only means the source carries feature state; it says nothing about - // whether this bucket's layers actually read it (isStateDependent). A bucket can have - // withStateUpdates true and an empty stateDependentLayers, so the two must be checked - // together wherever stateDependentLayers is used, or bucket.update() below can be - // called with an empty layers array and crash on layers[0]. const hasStateDependentLayers = withStateUpdates && bucket.stateDependentLayers.length !== 0; const layers = hasStateDependentLayers ? bucket.stateDependentLayers : bucket.layers; if (hasStateDependentLayers || isBrightnessChanged || hasPaintUpdate || needsSymbolUBOUpdate) {