-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[ios, macos] Light property implementation in MGLStyle #9043
Changes from 8 commits
0b1c506
d237f2a
792f1c5
09007d8
44793f6
d7fcb6a
2ff6da1
db42cf9
5d6ccbf
25cc109
0ef7221
6ac4629
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,109 @@ | ||
#import <CoreGraphics/CoreGraphics.h> | ||
|
||
#import "MGLFoundation.h" | ||
#import "MGLStyleValue.h" | ||
|
||
NS_ASSUME_NONNULL_BEGIN | ||
|
||
|
||
/** Options to specify extruded geometries are lit relative to the map or viewport. */ | ||
typedef NS_ENUM(NSUInteger, MGLLightAnchor) { | ||
/** The position of the light source is aligned to the rotation of the map. */ | ||
MGLLightAnchorTypeMap, | ||
/** The position of the light source is aligned to the rotation of the viewport. */ | ||
MGLLightAnchorTypeViewport | ||
}; | ||
|
||
/** | ||
A structure containing information about the position of the light source | ||
relative to lit geometries. | ||
*/ | ||
typedef struct MGLLightPosition { | ||
/** Distance from the center of the base of an object to its light. */ | ||
CGFloat radial; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What unit is this distance in? If this distance is measured in meters, its type should be CLLocationDistance. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. → #9234 |
||
/** Position of the light relative to 0° (0° when `lightAnchor` is set to viewport corresponds | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The reference to lightAnchor needs to be MGLLight.lightAnchor in order for jazzy to automatically link to the property's documentation. |
||
to the top of the viewport, or 0° when `lightAnchor` is set to map corresponds to due north, | ||
and degrees proceed clockwise). */ | ||
CGFloat azimuthal; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This field's type should be CLLocationDirection, which is measured in degrees clockwise from north (or up). |
||
/** Indicates the height of the light (from 0°, directly above, to 180°, directly below). */ | ||
CGFloat polar; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly, if this height is measured in meters, the type should be CLLocationDistance. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, actually, based on the documentation, this field appears to be measured in degrees as well. It isn't clear to me whether it goes clockwise; if it does, then the appropriate type would be CLLocationDirection. |
||
} MGLLightPosition; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As tail work, maybe we could consider naming this struct MGLSphericalPosition, so it emphasizes the nature of the value instead of how it's used. |
||
|
||
/** | ||
Creates a new `MGLLightPosition` from the given radial, azimuthal, polar. | ||
|
||
@param radial The radial coordinate. | ||
@param azimuthal The azimuthal angle. | ||
@param polar The polar angle. | ||
|
||
@return Returns a `MGLLightPosition` struct containing the position attributes. | ||
*/ | ||
NS_INLINE MGLLightPosition MGLLightPositionMake(CGFloat radial, CGFloat azimuthal, CGFloat polar) { | ||
MGLLightPosition position; | ||
position.radial = radial; | ||
position.azimuthal = azimuthal; | ||
position.polar = polar; | ||
|
||
return position; | ||
} | ||
|
||
/** | ||
An `MGLLight` object represents the light source for extruded geometries in `MGLStyle`. | ||
*/ | ||
MGL_EXPORT | ||
@interface MGLLight : NSObject | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This class any any other new public symbols will need documentation comments. The style specification documentation can be a starting point, although we’ll need to tailor a few details to make sense in Objective-C and Swift. Any new top-level symbols, such as MGLLight, should also be added to both platforms’ jazzy table of contents. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. → #9087 |
||
|
||
/** | ||
`lightAnchorType` Whether extruded geometries are lit relative to the map or viewport. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove "lightAnchorType". |
||
*/ | ||
@property (nonatomic) MGLLightAnchor lightAnchor; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The property should be named anchor, not lightAnchor. There's no ambiguity as to the kind of anchor we're looking at. |
||
|
||
/** | ||
Values describing animated transitions to `lightAnchor` property. | ||
*/ | ||
@property (nonatomic) MGLTransition lightAnchorTransition; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't appear to be possible to customize an anchor transition, probably because an anchor can only be one of two values. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's what I thought as well but looking at light it has a transition for anchor and to keep consistency I kept the property. |
||
|
||
|
||
/** | ||
Position of the light source relative to lit (extruded) geometries. | ||
*/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These documentation comments should cross-reference (and link to) the style specification's entries for the corresponding properties on the Light object. See the MGLSymbolStyleLayer.h documentation comments for examples. This can be tail work. |
||
@property (nonatomic) MGLStyleValue<NSValue *> * position; | ||
|
||
/** | ||
Values describing animated transitions to `position` property. | ||
*/ | ||
@property (nonatomic) MGLTransition positionTransiton; | ||
|
||
|
||
#if TARGET_OS_IPHONE | ||
/** | ||
Color tint for lighting extruded geometries. | ||
*/ | ||
@property (nonatomic) MGLStyleValue<UIColor *> *color; | ||
#else | ||
|
||
/** | ||
Color tint for lighting extruded geometries. | ||
*/ | ||
@property (nonatomic) MGLStyleValue<NSColor *> *color; | ||
#endif | ||
|
||
/** | ||
Values describing animated transitions to `color` property. | ||
*/ | ||
@property (nonatomic) MGLTransition colorTransiton; | ||
|
||
|
||
/** | ||
Intensity of lighting (on a scale from 0 to 1). Higher numbers will present as more extreme contrast. | ||
*/ | ||
@property(nonatomic) MGLStyleValue<NSNumber *> *intensity; | ||
|
||
/** | ||
Values describing animated transitions to `intensity` property. | ||
*/ | ||
@property (nonatomic) MGLTransition intensityTransition; | ||
|
||
@end | ||
|
||
NS_ASSUME_NONNULL_END |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,117 @@ | ||
#import "MGLLight.h" | ||
|
||
#import "MGLTypes.h" | ||
#import "NSDate+MGLAdditions.h" | ||
#import "MGLStyleValue_Private.h" | ||
#import "NSValue+MGLAdditions.h" | ||
|
||
#import <mbgl/style/light.hpp> | ||
#import <mbgl/style/types.hpp> | ||
|
||
namespace mbgl { | ||
|
||
MBGL_DEFINE_ENUM(MGLLightAnchor, { | ||
{ MGLLightAnchorTypeMap, "map" }, | ||
{ MGLLightAnchorTypeViewport, "viewport" }, | ||
}); | ||
|
||
} | ||
|
||
NS_INLINE MGLTransition MGLTransitionFromOptions(const mbgl::style::TransitionOptions& options) { | ||
MGLTransition transition; | ||
transition.duration = MGLTimeIntervalFromDuration(options.duration.value_or(mbgl::Duration::zero())); | ||
transition.delay = MGLTimeIntervalFromDuration(options.delay.value_or(mbgl::Duration::zero())); | ||
|
||
return transition; | ||
} | ||
|
||
NS_INLINE mbgl::style::TransitionOptions MGLOptionsFromTransition(MGLTransition transition) { | ||
mbgl::style::TransitionOptions options { { MGLDurationFromTimeInterval(transition.duration) }, { MGLDurationFromTimeInterval(transition.delay) } }; | ||
return options; | ||
} | ||
|
||
@interface MGLLight() | ||
|
||
@end | ||
|
||
@implementation MGLLight | ||
|
||
- (instancetype)initWithMBGLLight:(const mbgl::style::Light *)mbglLight | ||
{ | ||
if (self = [super init]) { | ||
auto anchor = mbglLight->getAnchor(); | ||
MGLStyleValue<NSValue *> *lightAnchorType; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Call this variable lightAnchorValue or anchorValue. There should be no reason to say "anchor type" anywhere, except where the C++ typed named LightAnchorType forces us to. |
||
if (anchor.isUndefined()) { | ||
mbgl::style::PropertyValue<mbgl::style::LightAnchorType> defaultAnchor = mbglLight->getDefaultAnchor(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: this code would be more readable with the use of auto here and in other situations where we're creating a local variable to hold the return value of a method call. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree but I had a problem in the line below. If I specify it as auto I get There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, makes sense. That's because getDefaultAnchor() returns a raw LightAnchorType and we're relying on this variable declaration to implicitly wrap it in a PropertyValue, PropertyValue being the type that has the toEnunStyleValue() method. |
||
lightAnchorType = MGLStyleValueTransformer<mbgl::style::LightAnchorType, NSValue *, mbgl::style::LightAnchorType, MGLLightAnchor>().toEnumStyleValue(defaultAnchor); | ||
} else { | ||
lightAnchorType = MGLStyleValueTransformer<mbgl::style::LightAnchorType, NSValue *, mbgl::style::LightAnchorType, MGLLightAnchor>().toEnumStyleValue(anchor); | ||
} | ||
|
||
NSAssert([lightAnchorType isKindOfClass:[MGLConstantStyleValue class]], @"Anchor isn’t an enum."); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an assertion that the anchor is constant, not that it's an enumeration (which it always has to be anyways). |
||
NSValue *anchorValue = ((MGLConstantStyleValue *)lightAnchorType).rawValue; | ||
_lightAnchor = [anchorValue MGLLightAnchorValue]; | ||
|
||
_lightAnchorTransition = MGLTransitionFromOptions(mbglLight->getAnchorTransition()); | ||
|
||
auto positionValue = mbglLight->getPosition(); | ||
if (positionValue.isUndefined()) { | ||
_position = MGLStyleValueTransformer<mbgl::style::Position, NSValue *>().toStyleValue(mbglLight->getDefaultPosition()); | ||
} else { | ||
_position = MGLStyleValueTransformer<mbgl::style::Position, NSValue *>().toStyleValue(positionValue); | ||
} | ||
|
||
_positionTransiton = MGLTransitionFromOptions(mbglLight->getPositionTransition()); | ||
|
||
auto colorValue = mbglLight->getColor(); | ||
if (colorValue.isUndefined()) { | ||
_color = MGLStyleValueTransformer<mbgl::Color, MGLColor *>().toStyleValue(mbglLight->getDefaultColor()); | ||
} else { | ||
_color = MGLStyleValueTransformer<mbgl::Color, MGLColor *>().toStyleValue(colorValue); | ||
} | ||
|
||
_colorTransiton = MGLTransitionFromOptions(mbglLight->getColorTransition()); | ||
|
||
auto intensityValue = mbglLight->getIntensity(); | ||
if (intensityValue.isUndefined()) { | ||
_intensity = MGLStyleValueTransformer<float, NSNumber *>().toStyleValue(mbglLight->getDefaultIntensity()); | ||
} else { | ||
_intensity = MGLStyleValueTransformer<float, NSNumber *>().toStyleValue(intensityValue); | ||
} | ||
|
||
_intensityTransition = MGLTransitionFromOptions(mbglLight->getIntensityTransition()); | ||
} | ||
|
||
return self; | ||
} | ||
|
||
- (mbgl::style::Light)mbglLight | ||
{ | ||
mbgl::style::Light mbglLight; | ||
|
||
MGLStyleValue<NSValue *> *lightAnchorType = [MGLStyleValue<NSValue *> valueWithRawValue:[NSValue valueWithMGLLightAnchor:self.lightAnchor]]; | ||
auto anchor = MGLStyleValueTransformer<mbgl::style::LightAnchorType, NSValue *, mbgl::style::LightAnchorType, MGLLightAnchor>().toEnumPropertyValue(lightAnchorType); | ||
mbglLight.setAnchor(anchor); | ||
|
||
|
||
mbglLight.setAnchorTransition(MGLOptionsFromTransition(self.lightAnchorTransition)); | ||
|
||
auto position = MGLStyleValueTransformer<mbgl::style::Position, NSValue *>().toInterpolatablePropertyValue(self.position); | ||
mbglLight.setPosition(position); | ||
|
||
mbglLight.setPositionTransition(MGLOptionsFromTransition(self.positionTransiton)); | ||
|
||
auto color = MGLStyleValueTransformer<mbgl::Color, MGLColor *>().toInterpolatablePropertyValue(self.color); | ||
mbglLight.setColor(color); | ||
|
||
mbglLight.setColorTransition(MGLOptionsFromTransition(self.colorTransiton)); | ||
|
||
auto intensity = MGLStyleValueTransformer<float, NSNumber *>().toInterpolatablePropertyValue(self.intensity); | ||
mbglLight.setIntensity(intensity); | ||
|
||
mbglLight.setIntensityTransition(MGLOptionsFromTransition(self.intensityTransition)); | ||
|
||
return mbglLight; | ||
} | ||
|
||
@end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
#import <Foundation/Foundation.h> | ||
|
||
#import "MGLLight.h" | ||
|
||
#import <mbgl/style/light.hpp> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It may be possible to avoid this import statement too, by forward declaring a class Light inside a namespace style inside a namespace mbgl. See some of the other runtime styling private headers for examples. |
||
|
||
@interface MGLLight (Private) | ||
|
||
/** | ||
Initializes and returns a `MGLLight` associated with a style's light. | ||
*/ | ||
- (instancetype)initWithMBGLLight:(const mbgl::style::Light *)mbglLight; | ||
|
||
/** | ||
Returns an `mbgl::style::Light` representation of the `MGLLight`. | ||
*/ | ||
- (mbgl::style::Light)mbglLight; | ||
|
||
@end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
#import "MGLTypes.h" | ||
|
||
@class MGLSource; | ||
@class MGLLight; | ||
|
||
NS_ASSUME_NONNULL_BEGIN | ||
|
||
|
@@ -564,6 +565,14 @@ MGL_EXPORT | |
*/ | ||
- (void)removeImageForName:(NSString *)name; | ||
|
||
|
||
#pragma mark Managing the Style's Light | ||
|
||
/** | ||
Provides global light source for the style. | ||
*/ | ||
@property (nonatomic, strong) MGLLight *light; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This property is currently under the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Working on it. |
||
|
||
@end | ||
|
||
NS_ASSUME_NONNULL_END |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
#import <Foundation/Foundation.h> | ||
#import <CoreGraphics/CoreGraphics.h> | ||
|
||
#import "MGLFoundation.h" | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these should match the base name so you should remove
Type
. SoMGLLightAnchor
is in bothMGLLightAnchorMap
andMGLLightAnchorViewport