Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Evaluate query results on queryRenderedFeatures #5677 #5692

Closed
wants to merge 1 commit into from

Conversation

Syntaf
Copy link
Contributor

@Syntaf Syntaf commented Nov 16, 2017

Preface: the code in this PR likely needs to change, but I wanted to show a 'working' example and mention a possible fix if i'm on the right track.

If I understand the issue correctly, this PR will 'fix' #5677, and properly evaluate the current values before showing them.

The solution I wrote however is a bit hacky, as I realized a couple of things:

  • Proper evaluation of the layout and paint declaration rely on the global property zoom, which is not available during the serialize method.
  • Modifying serialize to use the global property zoom and pass it along during the mapping of layout and paint would solve one problem and create another, as serialize is also used to grab non-evaluated results.

I think filterMatching needs to separate some logic out for queryRenderedFeatures somehow, such that you don't need to check when the global context is valid to pass or not.

My first impression is to maybe create a flag within filterMatching which defines whether to serialize and evaluate, or just serialize. That way it won't have to rely on checking whether this.coord.z is defined or not to determine if the serialization should also perform evaluation.

Something along the lines of:

    filterMatching(
        result: {[string]: Array<{ featureIndex: number, feature: GeoJSONFeature }>},
        matching: Array<any>,
        array: any,
        queryGeometry: Array<Array<Point>>,
        filter: FeatureFilter,
        filterLayerIDs: Array<string>,
        styleLayers: {[string]: StyleLayer},
        bearing: number,
        pixelsToTileUnits: number,
        serializeAndEvaluate?: boolean // make optional maybe?
    ) {

            // ....

           // if asked to evaluate, we know the global context is defined and valid
           if (serializeAndEvaluate) {
               (geojsonFeature: any).layer = styleLayer.serializeWithEvaluation(globals);
           } else {
               // otherwise, perform a normal serialization
               (geojsonFeature: any).layer = styleLayer.serialize();
           }

Thoughts?

P.S. I used this example from the ticket to test this.

@ryanhamley
Copy link
Contributor

This section of the library has changed significantly since this PR was submitted so I'm going to close it. We'd be happy to revisit a new PR. Sorry for the lack of response.

@ryanhamley ryanhamley closed this Sep 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants