Fix for global-state doesn't work with icon-size/text-size layout property of symbol layer (#6321)

* render tests to reproduce issue of `icon-size` and `text-size` properties ignoring `global-state`

#6308

* style layer `update` has been expecting an object as `parameters` for 7 years

* add reference to `global-state` to style layer and use it when recalculating layers

* add `global-state` reference to all layout properties at the point of evaluation

* changelog

---------

Co-authored-by: Natalia Kowalczyk <melitele@code42day.com>
Co-authored-by: Harel M <harel.mazor@gmail.com>

Author: 3 people

CHANGELOG.md

@@ -8,6 +8,7 @@
 - Fix evaluating `global-state` in paint `...-pattern` properties ([6301](https://github.com/maplibre/maplibre-gl-js/pull/6301))
 - Fix evaluation of `text-color` when using `format` within `step` ([#5833](https://github.com/maplibre/maplibre-gl-js/issues/5432))
 - Fix regression in `mergeSourceDiffs`: handle add/remove/removeAll ([6342](https://github.com/maplibre/maplibre-gl-js/pull/6342))
+- Fix evaluating `global-state` in layout properties `icon-size` and `text-size` ([#6308](https://github.com/maplibre/maplibre-gl-js/issues/6308))
 - _...Add new stuff here..._
 
 ## 5.7.0

src/source/worker_tile.test.ts

@@ -6,6 +6,9 @@ import {StyleLayerIndex} from '../style/style_layer_index';
 import {type WorkerTileParameters} from './worker_source';
 import {type VectorTile} from '@mapbox/vector-tile';
 import {SubdivisionGranularitySetting} from '../render/subdivision_granularity_settings';
+import {type EvaluationParameters} from '../style/evaluation_parameters';
+import {type PossiblyEvaluated} from '../style/properties';
+import {type SymbolLayoutProps, type SymbolLayoutPropsPossiblyEvaluated} from '../style/style_layer/symbol_style_layer_properties.g';
 
 function createWorkerTile(params?: {globalState?: Record<string, any>}): WorkerTile {
     return new WorkerTile({
@@ -286,4 +289,26 @@ describe('worker tile', () => {
         expect(sendAsync).toHaveBeenCalledWith(expect.objectContaining({data: expect.objectContaining({'icons': ['hello'], 'type': 'patterns'})}), expect.any(Object));
         expect(sendAsync).toHaveBeenCalledWith(expect.objectContaining({data: expect.objectContaining({'source': 'source', 'type': 'glyphs', 'stacks': {'StandardFont-Bold': [101, 115, 116]}})}), expect.any(Object));
     });
+
+    test('WorkerTile.parse passes global-state to layers', async () => {
+        const layerIndex = new StyleLayerIndex([
+            {
+                id: 'layer-id',
+                type: 'symbol',
+                source: 'source',
+                layout: {
+                    'text-size': ['global-state', 'size']
+                }
+            }
+        ]);
+
+        const globalState = {} as any;
+        const tile = createWorkerTile({globalState});
+        globalState.size = 12;
+        await tile.parse(createLineWrapper(), layerIndex, [], {} as any, SubdivisionGranularitySetting.noSubdivision);
+        const layer = layerIndex._layers['layer-id'];
+        layer.recalculate({} as EvaluationParameters, []);
+        const layout = layer.layout as PossiblyEvaluated<SymbolLayoutProps, SymbolLayoutPropsPossiblyEvaluated>;
+        expect(layout.get('text-size').evaluate({} as any, {})).toBe(12);
+    });
 });

src/source/worker_tile.ts

@@ -206,8 +206,9 @@ export class WorkerTile {
 
 function recalculateLayers(layers: ReadonlyArray<StyleLayer>, zoom: number, availableImages: Array<string>, globalState: Record<string, any>) {
     // Layers are shared and may have been used by a WorkerTile with a different zoom.
-    const parameters = new EvaluationParameters(zoom, {globalState});
+    const parameters = new EvaluationParameters(zoom);
     for (const layer of layers) {
+        layer.setGlobalState(globalState);
         layer.recalculate(parameters, availableImages);
     }
 }

src/style/properties.test.ts

@@ -0,0 +1,24 @@
+import {describe, test, expect} from 'vitest';
+import {Layout, PropertyValue} from './properties';
+import symbolProperties from './style_layer/symbol_style_layer_properties.g';
+import {type EvaluationParameters} from './evaluation_parameters';
+
+describe('PropertyValue', () => {
+    test('set global state', () => {
+        const propertyValue = new PropertyValue(symbolProperties.layout.properties['text-size'], ['global-state', 'size']);
+        propertyValue.setGlobalState({size: 17});
+        expect(propertyValue.expression.evaluate({} as EvaluationParameters)).toBe(17);
+    });
+});
+
+describe('Layout', () => {
+    test('set global state', () => {
+        const layout = new Layout(symbolProperties.layout);
+        layout.setValue('text-size', ['global-state', 'textSize']);
+        layout.setGlobalState({textSize: 15, textTransform: 'uppercase'});
+        layout.setValue('text-transform', ['global-state', 'textTransform']);
+        const _layout = layout.possiblyEvaluate({} as EvaluationParameters);
+        expect(_layout.get('text-size').evaluate()).toBe(15);
+        expect(_layout.get('text-transform').evaluate()).toBe('uppercase');
+    });
+});

src/style/properties.ts

@@ -2,6 +2,7 @@ import {clone, extend, easeCubicInOut} from '../util/util';
 import {interpolates, type Color, type StylePropertySpecification, normalizePropertyExpression,
     type Feature,
     type FeatureState,
+    type GlobalProperties,
     type StylePropertyExpression,
     type SourceExpression,
     type CompositeExpression, type TransitionSpecification,
@@ -71,11 +72,13 @@ export class PropertyValue<T, R> {
     property: Property<T, R>;
     value: PropertyValueSpecification<T> | void;
     expression: StylePropertyExpression;
+    private _evaluate: (parameters: EvaluationParameters) => R; // original evaluate function
 
     constructor(property: Property<T, R>, value: PropertyValueSpecification<T> | void) {
         this.property = property;
         this.value = value;
         this.expression = normalizePropertyExpression(value === undefined ? property.specification.default : value, property.specification);
+        this._evaluate = this.expression.evaluate;
     }
 
     isDataDriven(): boolean {
@@ -93,6 +96,16 @@ export class PropertyValue<T, R> {
     ): R {
         return this.property.possiblyEvaluate(this, parameters, canonical, availableImages);
     }
+
+    setGlobalState(globalState: Record<string, any>) {
+        //@ts-ignore
+        this.expression.evaluate = (globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, canonical?: ICanonicalTileID, availableImages?: Array<string>, formattedSection?: FormattedSection) => {
+            // force the use of global state object when evaluating any expression
+            //@ts-ignore
+            globals.globalState = globalState;
+            return this._evaluate.call(this.expression, globals, feature, featureState, canonical, availableImages, formattedSection);
+        };
+    }
 }
 
 export type TransitionParameters = {
@@ -315,10 +328,12 @@ export class Transitioning<Props> {
 export class Layout<Props> {
     _properties: Properties<Props>;
     _values: {[K in keyof Props]: PropertyValue<any, PossiblyEvaluatedPropertyValue<any>>};
+    private _globalState: Record<string, any>; // reference to global state
 
     constructor(properties: Properties<Props>) {
         this._properties = properties;
         this._values = (Object.create(properties.defaultPropertyValues) as any);
+        this._globalState = {};
     }
 
     hasValue<S extends keyof Props>(name: S) {
@@ -331,6 +346,7 @@ export class Layout<Props> {
 
     setValue<S extends keyof Props>(name: S, value: any) {
         this._values[name] = new PropertyValue(this._values[name].property, value === null ? undefined : clone(value)) as any;
+        this._values[name].setGlobalState(this._globalState);
     }
 
     serialize() {
@@ -355,6 +371,13 @@ export class Layout<Props> {
         }
         return result;
     }
+
+    setGlobalState(globalState: Record<string, any>) {
+        this._globalState = globalState;
+        for (const value of Object.values(this._values)) {
+            (value as PropertyValue<any, PossiblyEvaluatedPropertyValue<any>>).setGlobalState(globalState);
+        }
+    }
 }
 
 // ------- PossiblyEvaluated -------

src/style/style.test.ts

@@ -11,14 +11,16 @@ import {OverscaledTileID} from '../source/tile_id';
 import {fakeServer, type FakeServer} from 'nise';
 
 import {type EvaluationParameters} from './evaluation_parameters';
-import {type LayerSpecification, type GeoJSONSourceSpecification, type FilterSpecification, type SourceSpecification, type StyleSpecification, type SymbolLayerSpecification, type SkySpecification} from '@maplibre/maplibre-gl-style-spec';
+import {type Feature, type LayerSpecification, type GeoJSONSourceSpecification, type FilterSpecification, type SourceSpecification, type StyleSpecification, type SymbolLayerSpecification, type SkySpecification} from '@maplibre/maplibre-gl-style-spec';
 import {type GeoJSONSource} from '../source/geojson_source';
 import {StubMap, sleep, waitForEvent} from '../util/test/util';
 import {RTLPluginLoadedEventName} from '../source/rtl_text_plugin_status';
 import {MessageType} from '../util/actor_messages';
 import {MercatorTransform} from '../geo/projection/mercator_transform';
 import {type Tile} from '../source/tile';
 import type Point from '@mapbox/point-geometry';
+import {type PossiblyEvaluated} from './properties';
+import {type SymbolLayoutProps, type SymbolLayoutPropsPossiblyEvaluated} from './style_layer/symbol_style_layer_properties.g';
 
 function createStyleJSON(properties?): StyleSpecification {
     return extend({
@@ -469,6 +471,37 @@ describe('Style.loadJSON', () => {
         expect(style.stylesheet.layers[0].id).toBe(previousStyle.layers[0].id);
         expect(style.stylesheet.layers).toHaveLength(1);
     });
+
+    test('propagates global state object to layers', async () => {
+        const style = new Style(getStubMap());
+        style.loadJSON(
+            createStyleJSON({
+                sources: {
+                    'source-id': createGeoJSONSource()
+                },
+                layers: [
+                    {
+                        id: 'layer-id',
+                        type: 'symbol',
+                        source: 'source-id',
+                        layout: {
+                            'text-size': ['global-state', 'size']
+                        }
+                    }
+                ]
+            })
+        );
+        await style.once('style.load');
+        // tests that reference to globalState is propagated to layers
+        // by changing globalState property and checking if the changed value
+        // was used when evaluating the layer
+        const globalState = style.getGlobalState();
+        globalState.size = 12;
+        const layer = style.getLayer('layer-id');
+        layer.recalculate({} as EvaluationParameters, []);
+        const layout = layer.layout as PossiblyEvaluated<SymbolLayoutProps, SymbolLayoutPropsPossiblyEvaluated>;
+        expect(layout.get('text-size').evaluate({} as Feature, {})).toBe(12);
+    });
 });
 
 describe('Style._load', () => {
@@ -1090,7 +1123,7 @@ describe('Style.removeSource', () => {
             }]
         }));
         await style.once('style.load');
-        style.update(1 as any as EvaluationParameters);
+        style.update({zoom: 1} as EvaluationParameters);
         return style;
     }
 
@@ -2344,7 +2377,7 @@ describe('Style.setPaintProperty', () => {
         tr.resize(512, 512);
 
         await style.once('style.load');
-        style.update(tr.zoom as any as EvaluationParameters);
+        style.update({zoom: tr.zoom} as EvaluationParameters);
         const sourceCache = style.sourceCaches['geojson'];
         const source = style.getSource('geojson') as GeoJSONSource;
 
@@ -2739,7 +2772,7 @@ describe('Style.setLayerZoomRange', () => {
         vi.spyOn(style, '_reloadSource');
 
         style.setLayerZoomRange('raster', 5, 12);
-        style.update(0 as any as EvaluationParameters);
+        style.update({zoom: 0} as EvaluationParameters);
         expect(style._reloadSource).not.toHaveBeenCalled();
     });
 });
@@ -2888,7 +2921,7 @@ describe('Style.queryRenderedFeatures', () => {
             style.sourceCaches.mapLibre.transform = transform;
             style.sourceCaches.other.transform = transform;
 
-            style.update(0 as any as EvaluationParameters);
+            style.update({zoom: 0} as EvaluationParameters);
             style._updateSources(transform);
             callback();
         });

src/style/style.ts

@@ -447,8 +447,6 @@ export class Style extends Evented {
 
         this.map.setTerrain(this.stylesheet.terrain ?? null);
 
-        this.setGlobalState(this.stylesheet.state ?? null);
-
         this.fire(new Event('data', {dataType: 'style'}));
         this.fire(new Event('style.load'));
     }
@@ -463,11 +461,14 @@ export class Style extends Evented {
         this._order = dereferencedLayers.map((layer) => layer.id);
         this._layers = {};
 
+        this.setGlobalState(this.stylesheet.state ?? null);
+
         // reset serialization field, to be populated only when needed
         this._serializedLayers = null;
         for (const layer of dereferencedLayers) {
             const styledLayer = createStyleLayer(layer);
             styledLayer.setEventedParent(this, {layer: {id: layer.id}});
+            styledLayer.setGlobalState(this._globalState);
             this._layers[layer.id] = styledLayer;
         }
     }

src/style/style_layer.test.ts

@@ -452,3 +452,23 @@ describe('StyleLayer.serialize', () => {
         expect(layer.paint).toBeTruthy();
     });
 });
+
+describe('StyleLayer.globalState', () => {
+    test('uses layer global state when recalculating layout properties', () => {
+        const layer = createStyleLayer({
+            id: 'symbol',
+            type: 'symbol',
+            layout: {
+                'text-field': '{text}',
+                'text-size': ['global-state', 'textSize'],
+                'text-transform': ['global-state', 'textTransform']
+            }
+        }  as LayerSpecification) as SymbolStyleLayer;
+        layer.setGlobalState({textSize: 15, textTransform: 'uppercase'});
+
+        layer.recalculate({zoom: 0, zoomHistory: {}, globalState: {textSize: 13, textTransform: 'lowercase'} as Record<string, any>} as EvaluationParameters, undefined);
+
+        expect(layer.layout.get('text-size').evaluate(undefined, {})).toBe(15);
+        expect(layer.layout.get('text-transform').evaluate(undefined, {})).toBe('uppercase');
+    });
+});

src/style/style_layer.ts

@@ -106,6 +106,8 @@ export abstract class StyleLayer extends Evented {
     queryIntersectsFeature?(params: QueryIntersectsFeatureParams): boolean | number;
     createBucket?(parameters: BucketParameters<any>): Bucket;
 
+    private _globalState: Record<string, any>; // reference to global state
+
     constructor(layer: LayerSpecification | CustomLayerInterface, properties: Readonly<{
         layout?: Properties<any>;
         paint?: Properties<any>;
@@ -124,6 +126,8 @@ export abstract class StyleLayer extends Evented {
         this.minzoom = layer.minzoom;
         this.maxzoom = layer.maxzoom;
 
+        this._globalState = {};
+
         if (layer.type !== 'background') {
             this.source = layer.source;
             this.sourceLayer = layer['source-layer'];
@@ -295,6 +299,7 @@ export abstract class StyleLayer extends Evented {
     }
 
     recalculate(parameters: EvaluationParameters, availableImages: Array<string>) {
+        parameters.globalState = this._globalState;
         if (parameters.getCrossfadeParameters) {
             this._crossfadeParameters = parameters.getCrossfadeParameters();
         }
@@ -306,6 +311,13 @@ export abstract class StyleLayer extends Evented {
         (this as any).paint = this._transitioningPaint.possiblyEvaluate(parameters, undefined, availableImages);
     }
 
+    setGlobalState(globalState: Record<string, any>) {
+        this._globalState = globalState;
+        if (this._unevaluatedLayout) {
+            this._unevaluatedLayout.setGlobalState(globalState);
+        }
+    }
+
     serialize(): LayerSpecification {
         const output: LayerSpecification = {
             'id': this.id,