[core] Skip duplicated IDs when querying point annotations #6655
Conversation
|
@brunoabinader, thanks for your PR! By analyzing the history of the files in this pull request, we identified @1ec5, @jfirebaugh and @kkaefer to be potential reviewers. |
| @@ -755,7 +755,10 @@ AnnotationIDs Map::queryPointAnnotations(const ScreenBox& box) { | |||
| for (auto &feature : features) { | |||
| assert(feature.id); | |||
| assert(*feature.id <= std::numeric_limits<AnnotationID>::max()); | |||
| ids.push_back(static_cast<AnnotationID>(feature.id->get<uint64_t>())); | |||
| AnnotationID id = static_cast<AnnotationID>(feature.id->get<uint64_t>()); | |||
| if (std::find(ids.begin(), ids.end(), id) == ids.end()) { | |||
There was a problem hiding this comment.
This is an O(n²) algorithm. Better use a set to do deduplication.
There was a problem hiding this comment.
Now using AnnotationIDs as a typedef for std::set<AnnotationID>. This involves updating platform code that uses Map::queryPointAnnotations().
There was a problem hiding this comment.
Now using AnnotationIDs as a typedef for std::set.
It turned out that platform code logic depended on particularities of std::vector e.g. ability to reassign member values via std::remove_if that would end up adding more code complexity by converting it to an approach in which std::set is compatible with.
Thus, I'm keeping AnnotationIDs as a std::vector. I'm using std::set internally when issuing the query and later moving those values to a std::vector, preserving linear complexity.
brunoabinader
force-pushed
the
filter-query-point-annotations
branch
6 times, most recently
from
325b48f
to
862c198
Compare
brunoabinader
force-pushed
the
filter-query-point-annotations
branch
from
862c198
to
7707646
Compare
Extracted out of #3563.
/cc @ivovandongen