Fix dynamically added props not appearing in queryRenderedFeatures results #10102
Conversation
|
|
There was a problem hiding this comment.
Thanks for finding this bug and putting together a PR @osvodef. As written, this does not actually fix the issue. The query test you've added is passing incidentally because the line color defaults to black. If you try changing the line color, you can see that the query test doesn't update properly. Additionally, if you set up a debug page with the same code as your JSBin, you'll see that the output is 1, not 0.5.
The crux of the issue seems to be that you're evaluating the property from styleLayerProperties which doesn't have the updated style property added to it. Once we get this fixed and have a working query test, I'd like to run benchmarks on this as well. This area of the code base is sensitive to changes, performance-wise as you can see in the original PR that implements it (#9198).
I also noted that we should pass in the CanonicalTileID to the evaluation of the feature property which was never updated after the canonical argument was added. Let me know if you have questions on the way forward.
| serializedLayer.paint = evaluateProperties(styleLayer._transitionablePaint, styleLayer.paint, feature, featureState, availableImages); | ||
| serializedLayer.layout = evaluateProperties(styleLayer._unevaluatedLayout, styleLayer.layout, feature, featureState, availableImages); |
There was a problem hiding this comment.
Here's where we can pass in the CanonicalTileID by doing (styleLayer._transitionablePaint, styleLayer.paint, feature, featureState, this.tileID.canonical, availableImages).
| const prop = styleLayerProperties instanceof PossiblyEvaluated ? styleLayerProperties.get(key) : null; | ||
| return prop && prop.evaluate ? prop.evaluate(feature, featureState, availableImages) : prop; | ||
| }); | ||
| const value = prop && prop.evaluate ? prop.evaluate(feature, featureState, availableImages) : prop; |
There was a problem hiding this comment.
This needs to be updated or it won't evaluate properly down the line because the property evaluation method takes the arguments (feature, featureState, canonical, availableImage). You can pass the CanonicalTileID into this function above.
mapbox-gl-js/src/style/properties.js
Lines 456 to 458 in 15f0983
|
Since there hasn't been any movement on this in a long time, I've created a ticket for the bug #11584 and we're going to close this PR. Thanks for reporting the issue. If you decide to finish the PR, let us know and we can reopen it. |
Bug description
Optional layout and paint properties don't appear in
queryRenderedFeaturesresults if they're set after layer creation.JSFiddle
https://jsfiddle.net/osvodef/f45dapvu/10/
This sets a
text-opacityproperty to a label and then queries it.Solution
Like in #10074, the bug occurs because of serialized layers caching. I modified
evaluatePropertiesso that it iterates overstyleLayer._transitionablePaint._valuesandstyleLayer._unevaluatedLayout._valuesobjects instead of the outdated serializedlayoutandpaintobjects.Launch Checklist