maplibre / maplibre-gl-js

MapLibre GL JS - Interactive vector tile maps in the browser
https://maplibre.org/maplibre-gl-js/docs/
Other
6.49k stars 695 forks source link

setFeatureState changes type of properties from feature and changes evaluation of unrelated properties. #4499

Open navidevongit2 opened 2 months ago

navidevongit2 commented 2 months ago

maplibre-gl-js version: 4.5.1

browser: Google Chrome 127.0.6533.73 (Official Build) (arm64)

Steps to Trigger Behavior

  1. Create a GeoJSON feature on a map. Give it a property called "attributeIds" and put some number in it, it is then a numbers array. Also create a "case" in the Paint Properties so it changes color accordingly to those number values in attributeIds array. Also create a paint rule which colors the feature in a different color, when a feature state is enabled.
  2. When you load the map, everything works quite as expected.
  3. Change the feature state of the element with setFeatureState to something. It works fine as expected. Then change the feature state again to "default" behavior.
  4. The evaluation of the "in" expression based on attributeIds works now different then expected. It looks like the "attributeIds" number array is now interpreted as string ("in" expression works also on strings and substrings within it).

I provide a demonstration, where a Feature has the attributesIds array of [1, 37]. On load it works fine, the feature is colored as it should be (blue-ish), cause no rule matches, especially not ["in", 7, ["get", "attributeIds"]], which would color it in purple. But after change the featureState with setFeatureState this rule matches, as it founds a "7" in the attribute array, cause likely it is now interpreted as string object and not as number array as before.

Link to Demonstration

https://jsbin.com/luravug/edit?html,output

Expected Behavior

The color of the top line should revert to blue-ish color, when this feature was first modified feature-state by "Select line 1" and then modified again the feature-state with"Unselect line 2"

Actual Behavior

The line goes purple instead of blue-ish, cause now the expression ["in", 7, ["get", "attributeIds"]] matches cause of attributeIds 37.

HarelM commented 2 months ago

Thanks for taking the time to report this issue! Feel free to investigate and open a PR. set feature state and arrays in properties are error prone in general...

pcardinal commented 1 month ago

Does somebody has an idea (a hint) where in the code the line ["in", 7, ["get", "attributeIds"]] is evaluated to determine the "line-color" ? In fact ["in", 7, ["get", "attributeIds"]]should return the value of the property "attributeIds" and check if 7 is an element of the array. I don't find where in the code the colour "#FF00FF" is chosen to draw the line in the tile.

map.on('load', () => {
          map.addSource('lines', {
        'type': 'geojson',
        'data': {
            'type': 'FeatureCollection',
            'features': [
                {
                'type': 'Feature',
                'properties': {
                  'attributeIds': [1, 7]
                },

                'geometry': {
                  'type': 'LineString',
                  'coordinates': [
                      [
                        3.048769463967318,
                        39.27890998502306
                      ],
                      [
                        3.084156033986659,
                        39.283959301334505
                    ]
                  ]
                }
            }
            ]
        }
    });

    map.addLayer({
        'id': 'routex',
        'type': 'line',
        'source': "lines",
        'layout': {'line-cap': 'round'},
        'paint': {
            'line-color': ["case",
                           ["in", 7, ["get", "attributeIds"]],
                           "#FF00FF",  
                           "#007296"  
                          ],

            'line-width': 4
        }
    });

image

HarelM commented 1 month ago

In the maplibre spec package, it's a different repo.

pcardinal commented 1 month ago

Yes, in fact, but the piece of code I am looking for should also be in my build file ../dist/maplibre-gl.js (in which I am debugging)

HarelM commented 1 month ago

You can use the -dev.js version for a file that can be debugged better. And yes it will be in that file as everything is bundled into it.

pcardinal commented 1 month ago

Thanks for your input and, effectively, I am using the maplibre-gl-dev.js for debugging. I have found where to look at about the line ["in", 7, ["get", "attributeIds"]] when using the event listener for map.setFeatureState (following code)


 'get': {
        type: ValueType,
        overloads: [
            [
                [StringType],
                (ctx, [key]) => get(key.evaluate(ctx), ctx.properties())
            ], [
                [StringType, ObjectType],
                (ctx, [key, obj]) => get(key.evaluate(ctx), obj.evaluate(ctx))
            ]
        ]
    },

but these lines of code are not used for the first time the map is displayed.

I am looking around the lines of code where the Class In of the "case" is evaluated but still not see where the code gets the value for the property "attributeIds" to check if the number 7 is one of the elements of the array in property "attributeIds", also I don't find the code where the expression["in", 7, ["get", "attributeIds"]] returns true or false for returning the colour of the line.

class In {
    constructor(needle, haystack) {
        this.type = BooleanType;
        this.needle = needle;
        this.haystack = haystack;
    }
    static parse(args, context) {
        if (args.length !== 3) {
            return context.error(`Expected 2 arguments, but found ${args.length - 1} instead.`);
        }
        const needle = context.parse(args[1], 1, ValueType);
        const haystack = context.parse(args[2], 2, ValueType);
        if (!needle || !haystack)
            return null;
        if (!isValidType(needle.type, [BooleanType, StringType, NumberType, NullType, ValueType])) {
            return context.error(`Expected first argument to be of type boolean, string, number or null, but found ${toString$1(needle.type)} instead`);
        }
       return new In(needle, haystack);
    }
    evaluate(ctx) { .....

Also, as I understand, there is not worker involved (map._sourcesDirty === false) for what I am looking for now.

pcardinal commented 3 weeks ago

This case is a "big" PBF problem. A string and an array are save in vector tile the same way. Ex. "attributeIds" = [1,37] and "attributeIds" = '[1,37]' will have the same bytes representation in the vector tile data (Pbf). Is the solution to this problem is to forbidden arrays in feature properties, if not then the Pdf reading/writing should be recoded. Any idea?

HarelM commented 3 weeks ago

There are no plans to change the pbf encoding or decoding. This is a "fixed" format and had its limitations. We are working on a new spec, but it might take a long time to have it integrated into the core. I would advise to avoid using arrays...