This repository has been archived by the owner on Aug 8, 2023. It is now read-only.
Fix CollisionTile::queryRenderedSymbols query geometry #6553
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@brunoabinader, thanks for your PR! By analyzing the history of the files in this pull request, we identified @jfirebaugh, @ansis and @kkaefer to be potential reviewers. |
|
Next steps:
|
brunoabinader
force-pushed
the
brunoabinader-queryrenderedfeatures
branch
3 times, most recently
from
9484fad
to
86e4098
Compare
|
Commit 86e4098 fixes an issue I found out while testing for queries: when symbols were placed on tile edges e.g. |
|
Heads up: Though the fixes on this PR are valid, I am still obtaining the wrong amount of features while querying for visible features as I simply zoom in and out of the map. I'll continue my investigations around the relations between |
We were erroneously assigning a value to optional<ScreenCoordinate> (null island), causing it to be a valid anchor for Transform::easeTo.
brunoabinader
force-pushed
the
brunoabinader-queryrenderedfeatures
branch
from
86e4098
to
b92af8b
Compare
This provides a means of testing cases where an updated geometry tile would return wrong results for `queryRenderedFeatures`.
This gives the ability to pan the map in a posterior step after initial render for testing purposes.
Edge cases e.g. a point in null island (0, 0) makes the geometry intersectable by the four surrounding tiles. This makes sure only the tile that produces a valid projection in updateLayer() continues.
brunoabinader
force-pushed
the
brunoabinader-queryrenderedfeatures
branch
2 times, most recently
from
fff496d
to
fe86312
Compare
|
This PR is getting too big and addressing many different issues. I'm closing this in favor of separated PRs to handle each bug fix specifically. |
This was referenced Oct 7, 2016
|
This PR has been split in the following PRs and/or issues: Fixed and/or pending review:
What's left:
|
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Due to a bug in
CollisionTile- see #6055 (comment) for details - the results forqueryRenderedFeaturesfor symbol features was wrong in some cases e.g. when map bearing is updated. This PR implements the proposed fix and adds both query and unit tests to verify.In order to support the proposed query tests, two new Node.js functions are proposed:
setCenterandsetBearing. These are exemplified viaoperationsinsidestyle.json.Fixes #6055.