Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core] Polymorphic bucket creation #2873

Closed
wants to merge 6 commits into from

Conversation

jfirebaugh
Copy link
Contributor

Fifth in a series of PRs that builds towards a full-fledged programmatic style API. This is a big chunk of work, but I'm trying to break it up step by step and not create a long-lived branch with a lot of changes.

This episode moves further towards being able to remove the generic ClassProperties in favor of concrete paint and layout classes like those found in style_properties.hpp. The way that I'm going to do that is to continue refactoring until all usage of ClassProperties is in layer subclasses, and then substitute the concrete per-layer replacements.

This PR eliminates StyleBucket and moves bucket creation to layer subclasses. Along the way I believe to have fixed #2820, by restoring the previous use of a partialParse boolean.

👀 @kkaefer @brunoabinader

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.
@kkaefer
Copy link
Member

kkaefer commented Oct 30, 2015

restoring the previous use of a partialParse boolean.

This means that we don't parse any symbol buckets that are after one that is missing. We could still parse them, but not place them.

@jfirebaugh
Copy link
Contributor Author

I think it's set up so that they are parsed, but not placed: SymbolLayer::createBucket calls parseFeatures unconditionally, but addFeatures only if partialParse is false:

https://github.com/mapbox/mapbox-gl-native/pull/2873/files#diff-511e12d83fd87b94fa15dcdbf0d27bbcR159

Then TileWorker adds the bucket to the pending set if partialParse is true and it's a symbol bucket:

if (layer.type == StyleLayerType::Symbol && partialParse) {

Let me know if that looks correct.

@kkaefer
Copy link
Member

kkaefer commented Oct 30, 2015

There are actually three steps:

  • parseFeatures: determines the text we need to display by replacing tokens and applying text transformations, and the glyphs we need
  • addFeatures: determines the potential position of labels
  • placeFeatures: actually places them and adds them to a buffer

placeFeatures is currently called as the last step in addFeatures, but there's no good reason to; we could separate that out and call the addFeatures without placing as soon as we have the symbols. However, that means we'd have to introduce a third step beyond pending.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants