Fix appearance text-color and icon-image cross-fade

ibesora · github-actions[bot] · commit 289b9657a361 · 2026-06-12T14:29:08.000Z
GitOrigin-RevId: 513541cd4c665e7463fc02888ae1595ec7fcf991
diff --git a/src/data/bucket/symbol_bucket.ts b/src/data/bucket/symbol_bucket.ts
@@ -107,6 +107,7 @@ export type AppearanceFeatureData = {
     // Optional because these are only ever assigned on the main thread; the
     // worker doesn't need to serialize empty placeholders.
     layoutBasedIconVertexData?: Uint16Array;
+    layoutBasedIconTransitioningVertexData?: Uint16Array;
     layoutBasedTextVertexData?: Uint16Array;
 
     // Mutable render state — updated every frame by updateAppearances() on the main thread.
@@ -299,6 +300,9 @@ function containsRTLText(formattedText: Formatted): boolean {
 // Layout: [tileAnchorX, tileAnchorY, ox, oy, tx, ty, aSizeX, aSizeY, pixelOffsetX, pixelOffsetY, minFontScaleX, minFontScaleY]
 const SYMBOL_VERTEX_STRIDE = 12;
 
+// Number of uint16 values per vertex in SymbolIconTransitioningArray (a_texb: [tx, ty]).
+const ICON_TRANSITIONING_STRIDE = 2;
+
 export class SymbolBuffers {
     layoutVertexArray: SymbolLayoutArray;
     layoutVertexBuffer: VertexBuffer;
@@ -431,6 +435,15 @@ export class SymbolBuffers {
         this.layoutVertexArray.uint16.set(snapshot, offset * SYMBOL_VERTEX_STRIDE);
     }
 
+    snapshotIconTransitioningVertexData(offset: number, numVertices: number): Uint16Array {
+        const start = offset * ICON_TRANSITIONING_STRIDE;
+        return this.iconTransitioningVertexArray.uint16.slice(start, start + numVertices * ICON_TRANSITIONING_STRIDE);
+    }
+
+    restoreIconTransitioningVertexData(offset: number, snapshot: Uint16Array) {
+        this.iconTransitioningVertexArray.uint16.set(snapshot, offset * ICON_TRANSITIONING_STRIDE);
+    }
+
     updateSymbolVertexData(vertexIndex: number, anchorX: number, anchorY: number, newOx: number, newOy: number, newTx: number, newTy: number, newSizeX: number, newSizeY: number, pixelOffsetX: number, pixelOffsetY: number, minFontScaleX: number, minFontScaleY: number) {
         assert(vertexIndex >= 0 && vertexIndex < this.layoutVertexArray.length, 'Invalid vertex start index');
 
@@ -1054,9 +1067,13 @@ class SymbolBucket implements Bucket {
                 appearances.forEach(a => {
                     const iconImage = a.getLayoutProperty('icon-image');
                     if (!iconImage) return;
-                    const iconPrimary = this.getCombinedIconPrimary(a, symbolLayer, evaluationFeature, canonical, availableImages, symbolFeature, iconScaleFactor);
+                    const {iconPrimary, iconSecondary} = this.getCombinedIconVariants(a, symbolLayer, evaluationFeature, canonical, availableImages, symbolFeature, iconScaleFactor);
                     if (!iconPrimary) return;
                     addImageVariantToIcons(iconPrimary);
+                    if (iconSecondary) {
+                        this.hasAnySecondaryIcon = true;
+                        addImageVariantToIcons(iconSecondary);
+                    }
                 });
             }
 
@@ -1107,11 +1124,10 @@ class SymbolBucket implements Bucket {
         }
     }
 
-    getCombinedIconPrimary(appearance: SymbolAppearance, layer: SymbolStyleLayer, evaluationFeature: EvaluationFeature, canonical: CanonicalTileID, availableImages: ImageId[],
+    getCombinedIconVariants(appearance: SymbolAppearance, layer: SymbolStyleLayer, evaluationFeature: EvaluationFeature, canonical: CanonicalTileID, availableImages: ImageId[],
         symbolFeature: SymbolFeature, iconScaleFactor: number
-    ): ImageVariant | undefined {
+    ): {iconPrimary: ImageVariant | undefined; iconSecondary: ImageVariant | undefined} {
         let icon: ResolvedImage;
-        let iconPrimary: ImageVariant;
         if (appearance.hasLayoutProperty('icon-image')) {
             const resolvedTokens = layer.getAppearanceValueAndResolveTokens(appearance, 'icon-image', evaluationFeature, canonical, availableImages);
             icon = this.getResolvedImageFromTokens(resolvedTokens as string);
@@ -1125,11 +1141,17 @@ class SymbolBucket implements Bucket {
                 appearance.getUnevaluatedLayoutProperty('icon-size') :
                 layer._unevaluatedLayout._values['icon-size']) as PropertyValue<number, PossiblyEvaluatedPropertyValue<number>>;
             const iconSizeData = getSizeData(this.zoom, unevaluatedIconSize, this.worldview, availableImages);
-            const imageVariant = getScaledImageVariant(icon, iconSizeData, unevaluatedIconSize, canonical, this.zoom, symbolFeature, this.pixelRatio, iconScaleFactor, this.worldview, availableImages);
-            iconPrimary = imageVariant.iconPrimary;
+            const {iconPrimary, iconSecondary} = getScaledImageVariant(icon, iconSizeData, unevaluatedIconSize, canonical, this.zoom, symbolFeature, this.pixelRatio, iconScaleFactor, this.worldview, availableImages);
+            return {iconPrimary, iconSecondary};
         }
 
-        return iconPrimary;
+        return {iconPrimary: undefined, iconSecondary: undefined};
+    }
+
+    getCombinedIconPrimary(appearance: SymbolAppearance, layer: SymbolStyleLayer, evaluationFeature: EvaluationFeature, canonical: CanonicalTileID, availableImages: ImageId[],
+        symbolFeature: SymbolFeature, iconScaleFactor: number
+    ): ImageVariant | undefined {
+        return this.getCombinedIconVariants(appearance, layer, evaluationFeature, canonical, availableImages, symbolFeature, iconScaleFactor).iconPrimary;
     }
 
     private updateSymbolInstanceIconVertices(
@@ -1163,7 +1185,7 @@ class SymbolBucket implements Bucket {
                 id: featureData.id
             };
 
-            const iconPrimary = this.getCombinedIconPrimary(activeAppearance, layer, evaluationFeature, canonical, availableImages, minimalFeature, iconScaleFactor);
+            const {iconPrimary, iconSecondary} = this.getCombinedIconVariants(activeAppearance, layer, evaluationFeature, canonical, availableImages, minimalFeature, iconScaleFactor);
             if (!iconPrimary) return {vertexOffsetDelta: 0, hasChanges: false};
 
             const primaryImageSerialized = iconPrimary.toString();
@@ -1173,7 +1195,10 @@ class SymbolBucket implements Bucket {
                 // Get values from appearance and fallback to layout ones
                 const {appearanceIconOffset: iconOffset, appearanceIconRotate: iconRotate} = getAppearanceIconValues(activeAppearance, layer, evaluationFeature as SymbolFeature, canonical, layoutIconOffset, layoutIconRotate, layoutIconSize, iconScaleFactor);
                 const iconAnchor = layer.layout.get('icon-anchor').evaluate(evaluationFeature, featureState, canonical);
-                let shapedIcon = shapeIcon(position, undefined, iconOffset, iconAnchor);
+
+                // Resolve secondary position for cross-fade support
+                const secondaryPosition = iconSecondary ? (this.iconAtlasPositions && this.iconAtlasPositions.get(iconSecondary.toString())) : undefined;
+                let shapedIcon = shapeIcon(position, secondaryPosition, iconOffset, iconAnchor);
 
                 const isSDFIcon = position.sdf;
                 // Get icon-text-fit from layout since we don't support it in appearances
@@ -1201,17 +1226,23 @@ class SymbolBucket implements Bucket {
                 // Generate new icon quads with updated texture coordinates and size
                 const iconQuads = getIconQuads(shapedIcon, iconRotate, isSDFIcon, iconTextFit !== 'none', iconScaleFactor);
 
+                const hasTransitioning = this.icon.iconTransitioningVertexArray.length > 0;
+
                 // Store the layout-based vertex data to restore it later if it's the first time we use an appearance for this feature
                 if (!featureData.isUsingAppearanceIconVertexData) {
                     featureData.isUsingAppearanceIconVertexData = true;
                     featureData.layoutBasedIconVertexData = this.icon.snapshotSymbolVertexData(vertexOffset, symbolInstance.numIconVertices);
+                    if (hasTransitioning) {
+                        featureData.layoutBasedIconTransitioningVertexData = this.icon.snapshotIconTransitioningVertexData(vertexOffset, symbolInstance.numIconVertices);
+                    }
                 }
 
                 // Update vertex data - only update as many quads as were allocated during layout
                 const maxQuads = Math.floor(symbolInstance.numIconVertices / 4);
                 const quadsToUpdate = Math.min(iconQuads.length, maxQuads);
 
                 let currentVertexOffset = vertexOffset;
+                const transitioningUint16 = hasTransitioning ? this.icon.iconTransitioningVertexArray.uint16 : null;
                 for (let j = 0; j < quadsToUpdate; ++j) {
                     const quad = iconQuads[j];
 
@@ -1231,6 +1262,21 @@ class SymbolBucket implements Bucket {
                     this.icon.updateSymbolVertexData(currentVertexOffset + 1, anchorX, anchorY, Math.round(quad.tr.x * 32), Math.round(quad.tr.y * 32), quad.texPrimary.x + quad.texPrimary.w, quad.texPrimary.y, newSizeX, newSizeY, pixelOffsetBRX, pixelOffsetTLY, minFontScaleX, minFontScaleY);
                     this.icon.updateSymbolVertexData(currentVertexOffset + 2, anchorX, anchorY, Math.round(quad.bl.x * 32), Math.round(quad.bl.y * 32), quad.texPrimary.x, quad.texPrimary.y + quad.texPrimary.h, newSizeX, newSizeY, pixelOffsetTLX, pixelOffsetBRY, minFontScaleX, minFontScaleY);
                     this.icon.updateSymbolVertexData(currentVertexOffset + 3, anchorX, anchorY, Math.round(quad.br.x * 32), Math.round(quad.br.y * 32), quad.texPrimary.x + quad.texPrimary.w, quad.texPrimary.y + quad.texPrimary.h, newSizeX, newSizeY, pixelOffsetBRX, pixelOffsetBRY, minFontScaleX, minFontScaleY);
+
+                    // Update secondary (cross-fade) texture coords: fall back to primary when no secondary.
+                    if (transitioningUint16) {
+                        const tex = quad.texSecondary ? quad.texSecondary : quad.texPrimary;
+                        const base = currentVertexOffset * ICON_TRANSITIONING_STRIDE;
+                        transitioningUint16[base]     = tex.x;
+                        transitioningUint16[base + 1] = tex.y;
+                        transitioningUint16[base + 2] = tex.x + tex.w;
+                        transitioningUint16[base + 3] = tex.y;
+                        transitioningUint16[base + 4] = tex.x;
+                        transitioningUint16[base + 5] = tex.y + tex.h;
+                        transitioningUint16[base + 6] = tex.x + tex.w;
+                        transitioningUint16[base + 7] = tex.y + tex.h;
+                    }
+
                     currentVertexOffset += 4;
                 }
 
@@ -1255,6 +1301,9 @@ class SymbolBucket implements Bucket {
         } else if (featureData.isUsingAppearanceIconVertexData) {
             // No active appearance but vertex buffer still holds appearance data — restore original layout vertices.
             this.icon.restoreSymbolVertexData(vertexOffset, featureData.layoutBasedIconVertexData);
+            if (featureData.layoutBasedIconTransitioningVertexData) {
+                this.icon.restoreIconTransitioningVertexData(vertexOffset, featureData.layoutBasedIconTransitioningVertexData);
+            }
             featureData.isUsingAppearanceIconVertexData = false;
             return {vertexOffsetDelta: featureData.layoutBasedIconVertexData.length / SYMBOL_VERTEX_STRIDE, hasChanges: true};
         }
@@ -1671,6 +1720,12 @@ class SymbolBucket implements Bucket {
             }
         }
 
+        if (hasIconChanges && this.icon.iconTransitioningVertexBuffer && this.icon.iconTransitioningVertexArray.length > 0) {
+            if (this.icon.iconTransitioningVertexArray.length === this.icon.iconTransitioningVertexBuffer.length) {
+                this.icon.iconTransitioningVertexBuffer.updateData(this.icon.iconTransitioningVertexArray);
+            }
+        }
+
         if (hasTextChanges && this.text.layoutVertexBuffer && this.text.layoutVertexArray.arrayBuffer !== null) {
             if (this.text.layoutVertexArray.length === this.text.layoutVertexBuffer.length) {
                 this.text.layoutVertexBuffer.updateData(this.text.layoutVertexArray);
diff --git a/src/data/bucket/symbol_property_binder_ubo.ts b/src/data/bucket/symbol_property_binder_ubo.ts
@@ -132,11 +132,12 @@ export class SymbolPropertyBinderUBO {
     maxUniformBufferBindings: number;
 
     // Per-feature tracking, in insertion (populate) order. One entry per populateUBO call;
-    // the entry's index IS the feature's global index (see _writeFeatureBlock), so batch/local
-    // need not be stored. These two parallel arrays are the only feature-tracking state
-    // transferred worker→main; the lookup maps below are rebuilt lazily on the main thread.
+    // the entry's index IS the feature's global index (see _writeFeatureBlock).
+    // These parallel arrays are the only feature-tracking state transferred worker→main,
+    // the lookup maps below are rebuilt lazily on the main thread.
     allFeatureVtIndices: number[];                          // vector-tile feature index per entry
     allFeatureIds: Array<string | number | undefined>;      // feature id per entry (if any)
+    allFormattedSections: Array<FormattedSection | null>;   // formatted section per entry (for per-section color overrides)
 
     // Lazily built on the main thread (omitted from serialization). Map a featureId / vtFeatureIndex
     // to the positions in allFeature* that reference it, for O(1) targeted updates.
@@ -196,6 +197,7 @@ export class SymbolPropertyBinderUBO {
 
         this.allFeatureVtIndices = [];
         this.allFeatureIds = [];
+        this.allFormattedSections = [];
         this.featureVertexRangesFromId = null;
         this.featureVertexRangesFromVtIndex = null;
         this.ubos = [];
@@ -343,14 +345,24 @@ export class SymbolPropertyBinderUBO {
     /**
      * Resolve a paint property by name, preferring the active appearance's override when it
      * defines that property, otherwise the layer's paint. Shared by all three evaluate paths.
+     *
+     * When formattedSection is provided and the layer's property has a section override for it
+     * (e.g. per-section text-color in a formatted text-field), the section's explicit value
+     * takes precedence over the appearance. Return the layer's prop so its FormatSectionOverride
+     * correctly returns the section color.
      */
-    private _resolveProp<T>(propName: string, activeAppearance: SymbolAppearance | null | undefined): PossiblyEvaluatedPropertyValue<T> | undefined {
+    private _resolveProp<T>(propName: string, activeAppearance: SymbolAppearance | null | undefined,
+        formattedSection?: FormattedSection): PossiblyEvaluatedPropertyValue<T> | undefined {
+        const paint = this.layer.paint;
+        const layerProp = paint.get(propName as keyof typeof paint._values) as unknown as PossiblyEvaluatedPropertyValue<T>;
         const appearanceName = propName as keyof AppearancePaintProps;
         if (activeAppearance && activeAppearance.hasPaintProperty(appearanceName)) {
+            if (formattedSection && layerProp && layerProp.property.overrides && layerProp.property.overrides.hasOverride(formattedSection)) {
+                return layerProp;
+            }
             return activeAppearance.paintProperties.get(appearanceName) as unknown as PossiblyEvaluatedPropertyValue<T> | undefined;
         }
-        const paint = this.layer.paint;
-        return paint.get(propName as keyof typeof paint._values) as unknown as PossiblyEvaluatedPropertyValue<T> | undefined;
+        return layerProp;
     }
 
     /** Evaluate a property at the given zoom params, with the verbose shared argument list filled in. */
@@ -372,7 +384,7 @@ export class SymbolPropertyBinderUBO {
         ctx: EvaluationContext,
         flatOffset: number
     ): void {
-        const prop = this._resolveProp<Color>(propName, ctx.activeAppearance);
+        const prop = this._resolveProp<Color>(propName, ctx.activeAppearance, ctx.formattedSection);
 
         if (!prop) {
             evalFlatScratch[flatOffset] = 0;
@@ -638,7 +650,8 @@ export class SymbolPropertyBinderUBO {
         };
 
         const activeAppearance = this.activeAppearanceByVtIndex ? this.activeAppearanceByVtIndex.get(vtFeatureIndex) : undefined;
-        const allValues = this.evaluateAllProperties(feature, featureState, canonical, availableImages, brightness, undefined, activeAppearance);
+        const formattedSection = this.allFormattedSections ? this.allFormattedSections[i] : undefined;
+        const allValues = this.evaluateAllProperties(feature, featureState, canonical, availableImages, brightness, formattedSection || undefined, activeAppearance);
         this._writeFeatureBlock(i, allValues);
     }
 
@@ -723,6 +736,7 @@ export class SymbolPropertyBinderUBO {
         // _writeFeatureBlock re-derives batch/local; the lookup maps are built lazily on the main thread.
         this.allFeatureVtIndices.push(vtFeatureIndex);
         this.allFeatureIds.push(featureId);
+        this.allFormattedSections.push(formattedSection || null);
 
         return localIndex;
     }
@@ -806,11 +820,13 @@ export class SymbolPropertyBinderUBO {
         const positions = this.featureVertexRangesFromVtIndex.get(vtFeatureIndex);
         if (!positions) return false;
 
-        // All positions for this vtFeatureIndex evaluate identically (same feature/state/appearance),
-        // so evaluate once and write the shared result into each slot.
-        const allValues = this.evaluateAllProperties(feature, featureState, canonical, availableImages, brightness, undefined, activeAppearance);
+        // Evaluate per slot: sections of a formatted text-field each have their own UBO entry
+        // and may have per-section paint overrides (e.g. text-color from format expression).
+        // Re-use the stored formattedSection so those overrides take precedence over the appearance.
         let wrote = false;
         for (const i of positions) {
+            const formattedSection = this.allFormattedSections ? this.allFormattedSections[i] : undefined;
+            const allValues = this.evaluateAllProperties(feature, featureState, canonical, availableImages, brightness, formattedSection || undefined, activeAppearance);
             wrote = this._writeFeatureBlock(i, allValues) || wrote;
         }
         return wrote;
diff --git a/src/symbol/symbol_layout.ts b/src/symbol/symbol_layout.ts
@@ -872,7 +872,11 @@ export function postRasterizationSymbolLayout(bucket: SymbolBucket, bucketData:
     availableImages: ImageId[], canonical: CanonicalTileID, tileZoom: number, projection: Projection, brightness: number | null, imageMap: StyleImageMap<StringifiedImageVariant>, imageAtlas: {iconPositions: ImagePositionMap}) {
 
     bucket.iconAtlasPositions = imageAtlas.iconPositions;
-    const {featureData, hasAnySecondaryIcon, sizes, textAlongLine, symbolPlacement, coverageFrcMask, coveragePolygons, coverageTileZoom, symbolAnchorInFrcCoverage} = bucketData;
+    const {featureData, sizes, textAlongLine, symbolPlacement, coverageFrcMask, coveragePolygons, coverageTileZoom, symbolAnchorInFrcCoverage} = bucketData;
+    // An appearance may introduce a secondary icon even when the layout icon has
+    // no secondary. Include bucket.hasAnySecondaryIcon so transitioning vertices are created and the
+    // iconTransitioningVertexBuffer is populated for appearance-driven cross-fades.
+    const hasAnySecondaryIcon = bucketData.hasAnySecondaryIcon || bucket.hasAnySecondaryIcon;
 
     for (const data of featureData) {
         const {shapedIcon, verticallyShapedIcon, feature, shapedTextOrientations, shapedText, layoutTextSize, layoutIconSize,
diff --git a/test/ignores/all.js b/test/ignores/all.js
@@ -298,13 +298,6 @@ const skip = [
     "render-tests/hd-sd-transition/hd-on-remove-import",
     "render-tests/hd-sd-transition/sd-on-steady",
 
-    // https://mapbox.atlassian.net/browse/GLJS-1855
-    "render-tests/appearance/text-field",
-
-    // https://mapbox.atlassian.net/browse/GLJS-1856
-    "render-tests/appearance/icon-image-cross-fade/two-to-two",
-    "render-tests/appearance/icon-image-cross-fade/one-to-two",
-    "render-tests/appearance/icon-image-cross-fade/two-to-one"
 ];
 
 export default {todo, skip};
