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

Layer ownership refactor #6904

Merged
merged 5 commits into from
Nov 4, 2016
Merged

Layer ownership refactor #6904

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4903b19
[ios, macos wip] initial refactor for MGLFillStyleLayer
fabian-guerra Nov 3, 2016
ec37fba
[ios, macos wip] ownership refactor for MGLBackgroundStyleLayer and M…
fabian-guerra Nov 4, 2016
478f7a5
[ios, macos wip] addToMapView became private, MGLStyleLayer.mm.ejs up…
fabian-guerra Nov 4, 2016
20c1ba8
[ios, macos wip] added missing selectors to MGLStyleLayer.mm.ejs temp…
fabian-guerra Nov 4, 2016
8fba1ac
[ios, macos wip] commonInit pattern removed, standardized the way to …
fabian-guerra Nov 4, 2016
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
3 changes: 3 additions & 0 deletions platform/darwin/src/MGLFillStyleLayer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ typedef NS_ENUM(NSUInteger, MGLFillTranslateAnchor) {
*/
@property (nonatomic, null_resettable) MGLStyleValue<NSString *> *fillPattern;


- (void)addToMapView:(MGLMapView *)mapView;
Copy link
Contributor

Choose a reason for hiding this comment

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

This method needs to be private. We want developers to continue to call -[MGLStyle addLayer:] or -[MGLStyle insertLayer:belowLayer:] as appropriate. To avoid having to create a separate private header for each layer subclass, declare it in the private header for MGLStyle.

Copy link
Contributor

Choose a reason for hiding this comment

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

Er, I meant the private header for MGLStyleLayer, MGLStyleLayer_Private.h.


@end

NS_ASSUME_NONNULL_END
20 changes: 19 additions & 1 deletion platform/darwin/src/MGLFillStyleLayer.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Edit platform/darwin/scripts/generate-style-code.js, then run `make style-code-darwin`.

#import "MGLSource.h"
#import "MGLMapView_Private.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

These changes will eventually need to happen in all the style layer subclasses. We have a script (mentioned above) that automatically generates these headers. So when you're ready, update the templates that the script uses (the .ejs files in this directory) and run make style-code-darwin. Don't do that until you've updated the templates with your changes, though!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated the script to import MGLMapView_Private.h

#import "NSPredicate+MGLAdditions.h"
#import "MGLStyleLayer_Private.h"
#import "MGLStyleValue_Private.h"
Expand All @@ -16,15 +17,25 @@ @interface MGLFillStyleLayer ()
@end

@implementation MGLFillStyleLayer
{
std::unique_ptr<mbgl::style::FillLayer> _pendingLayer;
}

- (instancetype)initWithIdentifier:(NSString *)identifier source:(MGLSource *)source
{
if (self = [super initWithIdentifier:identifier source:source]) {
_layer = new mbgl::style::FillLayer(identifier.UTF8String, source.identifier.UTF8String);
[self commonInit:identifier source:source];
}
return self;
}

- (void)commonInit:(NSString *)identifier source:(MGLSource *)source
{
auto layer = std::make_unique<mbgl::style::FillLayer>(identifier.UTF8String, source.identifier.UTF8String);
_pendingLayer = std::move(layer);
self.layer = _pendingLayer.get();
}

- (NSString *)sourceLayerIdentifier
{
auto layerID = self.layer->getSourceLayer();
Expand Down Expand Up @@ -118,4 +129,11 @@ - (void)setFillPattern:(MGLStyleValue<NSString *> *)fillPattern {
return MGLStyleValueTransformer<std::string, NSString *>().toStyleValue(propertyValue);
}

#pragma mark -

- (void)addToMapView:(MGLMapView *)mapView
{
mapView.mbglMap->addLayer(std::move(_pendingLayer));
}

@end
11 changes: 11 additions & 0 deletions platform/darwin/src/MGLStyleLayer_Private.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,20 @@

#include <mbgl/style/layer.hpp>

@class MGLMapView;

@interface MGLStyleLayer (Private)

@property (nonatomic, readwrite, copy) NSString *identifier;
@property (nonatomic) mbgl::style::Layer *layer;
Copy link
Contributor

Choose a reason for hiding this comment

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

For future reference, I think we should add documentation for this property like we did in MGLSource_Private.h for the rawSource property. When coming back to this code (or looking at it for the first time) it is not immediately obvious that this is a raw pointer that is useable even after layer ownership is transferred in addToMapView:. Also, for consistency, it might be worth considering renaming this to rawLayer (or naming [MGLSource rawSource] to [MGLSource source] (again)).


/**
Adds the mbgl style layer that this object represents to the mbgl map.

Once a mbgl style layer is added, ownership of the object is transferred to the
`mbgl::Map` and this object no longer has an active unique_ptr reference to the
`mbgl::style::Layer`.
*/
- (void)addToMapView:(MGLMapView *)mapView;

@end
3 changes: 2 additions & 1 deletion platform/ios/app/MBXViewController.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -768,7 +768,8 @@ - (void)styleGeoJSONSource

MGLFillStyleLayer *fillLayer = [[MGLFillStyleLayer alloc] initWithIdentifier:@"test" source:source];
fillLayer.fillColor = [MGLStyleValue<UIColor *> valueWithRawValue:[UIColor purpleColor]];
[self.mapView.style addLayer:fillLayer];
[fillLayer addToMapView:self.mapView];

}

- (void)styleSymbolLayer
Expand Down