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

[core] Immutable style Impls #8929

Merged
jfirebaugh merged 5 commits into from
May 12, 2017
Merged

[core] Immutable style Impls #8929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6ca4af2
[core] Immutable Impls
jfirebaugh Apr 26, 2017
7d6f79b
[core] Remove unused RenderLayer::clone
jfirebaugh May 8, 2017
04ca556
[core] Return {Source,Layer}::getID by value
jfirebaugh May 11, 2017
31f68b2
[core] Replace createRender{Source,Layer} with Render{Source,Layer}::…
jfirebaugh May 11, 2017
2228ad9
[linux] loop-uv depends on mbgl-core for linking
jfirebaugh May 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions cmake/core-files.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,8 @@ set(MBGL_CORE_FILES
src/mbgl/renderer/render_symbol_layer.hpp
src/mbgl/renderer/render_tile.cpp
src/mbgl/renderer/render_tile.hpp
src/mbgl/renderer/style_diff.cpp
src/mbgl/renderer/style_diff.hpp
src/mbgl/renderer/symbol_bucket.cpp
src/mbgl/renderer/symbol_bucket.hpp
src/mbgl/renderer/tile_parameters.hpp
Expand Down Expand Up @@ -341,8 +343,6 @@ set(MBGL_CORE_FILES
src/mbgl/style/source_observer.hpp
src/mbgl/style/style.cpp
src/mbgl/style/style.hpp
src/mbgl/style/tile_source_impl.cpp
src/mbgl/style/tile_source_impl.hpp
src/mbgl/style/types.cpp
src/mbgl/style/update_batch.hpp

Expand Down Expand Up @@ -510,6 +510,7 @@ set(MBGL_CORE_FILES
include/mbgl/util/geometry.hpp
include/mbgl/util/ignore.hpp
include/mbgl/util/image.hpp
include/mbgl/util/immutable.hpp
include/mbgl/util/indexed_tuple.hpp
include/mbgl/util/interpolate.hpp
include/mbgl/util/logging.hpp
Expand Down Expand Up @@ -559,6 +560,7 @@ set(MBGL_CORE_FILES
src/mbgl/util/io.cpp
src/mbgl/util/io.hpp
src/mbgl/util/logging.cpp
src/mbgl/util/longest_common_subsequence.hpp
src/mbgl/util/mapbox.cpp
src/mbgl/util/mapbox.hpp
src/mbgl/util/mat2.cpp
Expand Down
4 changes: 4 additions & 0 deletions cmake/loop-uv.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,8 @@ target_include_directories(mbgl-loop-uv
PRIVATE src
)

target_link_libraries(mbgl-loop-uv
PRIVATE mbgl-core
)

create_source_groups(mbgl-loop-uv)
33 changes: 18 additions & 15 deletions include/mbgl/style/layer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <mbgl/util/noncopyable.hpp>
#include <mbgl/util/any.hpp>
#include <mbgl/util/immutable.hpp>
#include <mbgl/style/layer_type.hpp>
#include <mbgl/style/types.hpp>

Expand All @@ -19,6 +20,7 @@ class RasterLayer;
class BackgroundLayer;
class CustomLayer;
class FillExtrusionLayer;
class LayerObserver;

/**
* The runtime representation of a [layer](https://www.mapbox.com/mapbox-gl-style-spec/#layers) from the Mapbox Style
Expand All @@ -38,15 +40,6 @@ class FillExtrusionLayer;
*/
class Layer : public mbgl::util::noncopyable {
public:
class Impl;

protected:

const LayerType type;
Layer(LayerType, std::unique_ptr<Impl>);

public:

virtual ~Layer();

// Check whether this layer is of the given subtype.
Expand Down Expand Up @@ -78,7 +71,7 @@ class Layer : public mbgl::util::noncopyable {
//
template <class V>
auto accept(V&& visitor) {
switch (type) {
switch (getType()) {
case LayerType::Fill:
return visitor(*as<FillLayer>());
case LayerType::Line:
Expand All @@ -98,20 +91,30 @@ class Layer : public mbgl::util::noncopyable {
}
}

const std::string& getID() const;
LayerType getType() const;
std::string getID() const;

// Visibility
VisibilityType getVisibility() const;
void setVisibility(VisibilityType);
virtual void setVisibility(VisibilityType) = 0;

// Zoom range
float getMinZoom() const;
void setMinZoom(float) const;
float getMaxZoom() const;
void setMaxZoom(float) const;
virtual void setMinZoom(float) = 0;
virtual void setMaxZoom(float) = 0;

// Private implementation
const std::unique_ptr<Impl> baseImpl;
class Impl;
Immutable<Impl> baseImpl;
Copy link
Copy Markdown
Member

@kkaefer kkaefer May 11, 2017

Choose a reason for hiding this comment

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

I realize this has been public before, but is there a way we can make it private now?

Copy link
Copy Markdown
Contributor Author

@jfirebaugh jfirebaugh May 11, 2017

Choose a reason for hiding this comment

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

Style at a minimum would need to be friended. GeometryTile also uses it; it's possible that use could be eliminated. I'd like to leave this for a followup change though.


Layer(Immutable<Impl>);

// Create a layer, copying all properties except id and paint properties from this layer.
virtual std::unique_ptr<Layer> cloneRef(const std::string& id) const = 0;

LayerObserver* observer = nullptr;
void setObserver(LayerObserver*);

// For use in SDK bindings, which store a reference to a platform-native peer
// object here, so that separately-obtained references to this object share
Expand Down
16 changes: 12 additions & 4 deletions include/mbgl/style/layers/background_layer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ class BackgroundLayer : public Layer {
BackgroundLayer(const std::string& layerID);
~BackgroundLayer() final;

// Visibility
void setVisibility(VisibilityType) final;

// Zoom range
void setMinZoom(float) final;
void setMaxZoom(float) final;

// Paint properties

static PropertyValue<Color> getDefaultBackgroundColor();
Expand All @@ -42,15 +49,16 @@ class BackgroundLayer : public Layer {
// Private implementation

class Impl;
Impl* const impl;
const Impl& impl() const;

BackgroundLayer(const Impl&);
BackgroundLayer(const BackgroundLayer&) = delete;
Mutable<Impl> mutableImpl() const;
BackgroundLayer(Immutable<Impl>);
std::unique_ptr<Layer> cloneRef(const std::string& id) const final;
};

template <>
inline bool Layer::is<BackgroundLayer>() const {
return type == LayerType::Background;
return getType() == LayerType::Background;
}

} // namespace style
Expand Down
16 changes: 12 additions & 4 deletions include/mbgl/style/layers/circle_layer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@ class CircleLayer : public Layer {
void setFilter(const Filter&);
const Filter& getFilter() const;

// Visibility
void setVisibility(VisibilityType) final;

// Zoom range
void setMinZoom(float) final;
void setMaxZoom(float) final;

// Paint properties

static DataDrivenPropertyValue<float> getDefaultCircleRadius();
Expand Down Expand Up @@ -92,15 +99,16 @@ class CircleLayer : public Layer {
// Private implementation

class Impl;
Impl* const impl;
const Impl& impl() const;

CircleLayer(const Impl&);
CircleLayer(const CircleLayer&) = delete;
Mutable<Impl> mutableImpl() const;
CircleLayer(Immutable<Impl>);
std::unique_ptr<Layer> cloneRef(const std::string& id) const final;
};

template <>
inline bool Layer::is<CircleLayer>() const {
return type == LayerType::Circle;
return getType() == LayerType::Circle;
}

} // namespace style
Expand Down
13 changes: 11 additions & 2 deletions include/mbgl/style/layers/custom_layer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,21 @@ class CustomLayer : public Layer {
void* context);
~CustomLayer() final;

// Visibility
void setVisibility(VisibilityType) final;

// Zoom range
void setMinZoom(float) final;
void setMaxZoom(float) final;

// Private implementation

class Impl;
Impl* impl;
const Impl& impl() const;

Mutable<Impl> mutableImpl() const;
std::unique_ptr<Layer> cloneRef(const std::string& id) const final;

CustomLayer(const Impl&);
CustomLayer(const CustomLayer&) = delete;
};

Expand Down
16 changes: 12 additions & 4 deletions include/mbgl/style/layers/fill_extrusion_layer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@ class FillExtrusionLayer : public Layer {
void setFilter(const Filter&);
const Filter& getFilter() const;

// Visibility
void setVisibility(VisibilityType) final;

// Zoom range
void setMinZoom(float) final;
void setMaxZoom(float) final;

// Paint properties

static PropertyValue<float> getDefaultFillExtrusionOpacity();
Expand Down Expand Up @@ -74,15 +81,16 @@ class FillExtrusionLayer : public Layer {
// Private implementation

class Impl;
Impl* const impl;
const Impl& impl() const;

FillExtrusionLayer(const Impl&);
FillExtrusionLayer(const FillExtrusionLayer&) = delete;
Mutable<Impl> mutableImpl() const;
FillExtrusionLayer(Immutable<Impl>);
std::unique_ptr<Layer> cloneRef(const std::string& id) const final;
};

template <>
inline bool Layer::is<FillExtrusionLayer>() const {
return type == LayerType::FillExtrusion;
return getType() == LayerType::FillExtrusion;
}

} // namespace style
Expand Down
16 changes: 12 additions & 4 deletions include/mbgl/style/layers/fill_layer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@ class FillLayer : public Layer {
void setFilter(const Filter&);
const Filter& getFilter() const;

// Visibility
void setVisibility(VisibilityType) final;

// Zoom range
void setMinZoom(float) final;
void setMaxZoom(float) final;

// Paint properties

static PropertyValue<bool> getDefaultFillAntialias();
Expand Down Expand Up @@ -74,15 +81,16 @@ class FillLayer : public Layer {
// Private implementation

class Impl;
Impl* const impl;
const Impl& impl() const;

FillLayer(const Impl&);
FillLayer(const FillLayer&) = delete;
Mutable<Impl> mutableImpl() const;
FillLayer(Immutable<Impl>);
std::unique_ptr<Layer> cloneRef(const std::string& id) const final;
};

template <>
inline bool Layer::is<FillLayer>() const {
return type == LayerType::Fill;
return getType() == LayerType::Fill;
}

} // namespace style
Expand Down
16 changes: 12 additions & 4 deletions include/mbgl/style/layers/layer.hpp.ejs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ public:
<% } -%>

<% } -%>
// Visibility
void setVisibility(VisibilityType) final;
Copy link
Copy Markdown
Member

@kkaefer kkaefer May 11, 2017

Choose a reason for hiding this comment

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

Nit: Can we add override to be consistent with the rest of the codebase?

Copy link
Copy Markdown
Contributor Author

@jfirebaugh jfirebaugh May 11, 2017

Choose a reason for hiding this comment

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

You want to write override final or final override? Neither of those appear elsewhere in the codebase.

Copy link
Copy Markdown
Member

@kkaefer kkaefer May 11, 2017

Choose a reason for hiding this comment

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

I'm specifically interested in override (final override reads better), to make it fully clear that this is a virtual function override.

Copy link
Copy Markdown
Contributor Author

@jfirebaugh jfirebaugh May 11, 2017

Choose a reason for hiding this comment

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

Let's discuss making that style change separately. We already have quite a few uses of final without override.


// Zoom range
void setMinZoom(float) final;
void setMaxZoom(float) final;

<% if (layoutProperties.length) { -%>
// Layout properties

Expand All @@ -67,15 +74,16 @@ public:
// Private implementation

class Impl;
Impl* const impl;
const Impl& impl() const;

<%- camelize(type) %>Layer(const Impl&);
<%- camelize(type) %>Layer(const <%- camelize(type) %>Layer&) = delete;
Mutable<Impl> mutableImpl() const;
<%- camelize(type) %>Layer(Immutable<Impl>);
std::unique_ptr<Layer> cloneRef(const std::string& id) const final;
Copy link
Copy Markdown
Member

@kkaefer kkaefer May 11, 2017

Choose a reason for hiding this comment

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

Same note about privatization here. I know they've been public before, so this may be a follow-up change.

};

template <>
inline bool Layer::is<<%- camelize(type) %>Layer>() const {
return type == LayerType::<%- camelize(type) %>;
return getType() == LayerType::<%- camelize(type) %>;
}

} // namespace style
Expand Down
16 changes: 12 additions & 4 deletions include/mbgl/style/layers/line_layer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@ class LineLayer : public Layer {
void setFilter(const Filter&);
const Filter& getFilter() const;

// Visibility
void setVisibility(VisibilityType) final;

// Zoom range
void setMinZoom(float) final;
void setMaxZoom(float) final;

// Layout properties

static PropertyValue<LineCapType> getDefaultLineCap();
Expand Down Expand Up @@ -112,15 +119,16 @@ class LineLayer : public Layer {
// Private implementation

class Impl;
Impl* const impl;
const Impl& impl() const;

LineLayer(const Impl&);
LineLayer(const LineLayer&) = delete;
Mutable<Impl> mutableImpl() const;
LineLayer(Immutable<Impl>);
std::unique_ptr<Layer> cloneRef(const std::string& id) const final;
};

template <>
inline bool Layer::is<LineLayer>() const {
return type == LayerType::Line;
return getType() == LayerType::Line;
}

} // namespace style
Expand Down
16 changes: 12 additions & 4 deletions include/mbgl/style/layers/raster_layer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,13 @@ class RasterLayer : public Layer {
// Source
const std::string& getSourceID() const;

// Visibility
void setVisibility(VisibilityType) final;

// Zoom range
void setMinZoom(float) final;
void setMaxZoom(float) final;

// Paint properties

static PropertyValue<float> getDefaultRasterOpacity();
Expand Down Expand Up @@ -69,15 +76,16 @@ class RasterLayer : public Layer {
// Private implementation

class Impl;
Impl* const impl;
const Impl& impl() const;

RasterLayer(const Impl&);
RasterLayer(const RasterLayer&) = delete;
Mutable<Impl> mutableImpl() const;
RasterLayer(Immutable<Impl>);
std::unique_ptr<Layer> cloneRef(const std::string& id) const final;
};

template <>
inline bool Layer::is<RasterLayer>() const {
return type == LayerType::Raster;
return getType() == LayerType::Raster;
}

} // namespace style
Expand Down
Loading