This repository has been archived by the owner on Aug 8, 2023. It is now read-only.
Monitor annotation tiles #2869
Merged
Monitor annotation tiles #2869
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
jfirebaugh
force-pushed
the
1688-monitor-annotation-tiles
branch
from
774aa39
to
b611131
Compare
|
The |
|
It's failing because |
| @@ -91,11 +90,8 @@ void TileWorker::redoPlacement( | |||
| // Reset the collision tile so we have a clean slate; we're placing all features anyway. | |||
| collisionTile = std::make_unique<CollisionTile>(config); | |||
|
|
|||
| for (auto i = layers.rbegin(); i != layers.rend(); i++) { | |||
There was a problem hiding this comment.
@kkaefer Do you see any issues here with iterating buckets directly, rather than layers?
There was a problem hiding this comment.
Ah, yeah, feature placement is order dependent, of course.
jfirebaugh
force-pushed
the
1688-monitor-annotation-tiles
branch
2 times, most recently
from
f0d5948
to
8522c1f
Compare
| @@ -287,6 +287,6 @@ void TileWorker::createSymbolBucket(const GeometryTileLayer& layer, | |||
|
|
|||
| void TileWorker::insertBucket(const std::string& name, std::unique_ptr<Bucket> bucket) { | |||
| if (bucket->hasData()) { | |||
| result.buckets.emplace_back(name, std::move(bucket)); | |||
| result.buckets.emplace(name, std::move(bucket)); | |||
There was a problem hiding this comment.
Are we ever overwriting buckets here?
There was a problem hiding this comment.
I don't think so -- if we are, there's a bug in the bucket de-duping step. The name is the bucket name, which should be unique.
A tile may go from having particular layers, to not having them. (Annotation tiles in particular.)
This fixes adding shape annotations after VectorTileData objects have been created for annotations already, and will also be necessary for the dynamic Style API.
jfirebaugh
force-pushed
the
1688-monitor-annotation-tiles
branch
from
8522c1f
to
8c3b3cb
Compare
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.
Fixes #1688.
👀 @kkaefer