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

[core] Introduce StyleLayer subclasses #2730

Merged
merged 1 commit into from
Oct 22, 2015
Merged

[core] Introduce StyleLayer subclasses #2730

merged 1 commit into from
Oct 22, 2015

Conversation

jfirebaugh
Copy link
Contributor

Second 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 change introduces subclasses of StyleLayer, one for each layer type. Each subclass holds an instance of the appropriate ___PaintProperties that we introduced in the previous commit. That means we can eliminate the StyleProperties variant. (Well, not completely: ShapeAnnotation still uses a version reduced to varying between LinePaintProperties and FillPaintProperties.)

We can also move the explicit instantiations of StyleLayer::applyStyleProperties (one for each layer type) into pure virtual StyleLayer::applyStyleProperties and subclass implementations. Future commits in this series will make similar changes, replacing switch or template polymorphism with good old type polymorphism.

👀 @kkaefer @brunoabinader

@@ -64,29 +65,6 @@ void TileWorker::redoPlacement(float angle, float pitch, bool collisionDebug) {
}

template <typename T>
struct PropertyEvaluator {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PropertyEvaluator was duplicated in two places and another part of this commit is pulling it out to mbgl/style/property_evaluator.hpp.


inline bool isVisible() const {
return opacity > 0;
}
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about adding a PaintProperties that every ____PaintProperties inherits containing isVisible as a pure virtual?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd want to do that only if we actually needed runtime polymorphism from PaintProperties. So far I don't see the need -- everywhere isVisible() is currently used, it's with a concrete type.

@brunoabinader
Copy link
Member

This is great stuff! 👍 particularly excited about the way how StyleLayer code is now much more readable.

@jfirebaugh jfirebaugh merged commit 3afc8b4 into master Oct 22, 2015
@jfirebaugh jfirebaugh deleted the style-api branch October 22, 2015 23:01
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