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

[core] Introduce StyleLayer subclasses#2730

Merged
jfirebaugh merged 1 commit intomasterfrom
style-api
Oct 22, 2015
Merged

[core] Introduce StyleLayer subclasses#2730
jfirebaugh merged 1 commit intomasterfrom
style-api

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

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.

Copy link
Contributor

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
Contributor

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