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

[ios] duration to time interval fix #8276

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 5 additions & 5 deletions platform/darwin/src/MGLStyle.mm
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ - (void)setStyleClasses:(NS_ARRAY_OF(NSString *) *)appliedClasses transitionDura
newAppliedClasses.push_back([appliedClass UTF8String]);
}

mbgl::style::TransitionOptions transition { { MGLDurationInSecondsFromTimeInterval(transitionDuration) } };
mbgl::style::TransitionOptions transition { { MGLDurationFromTimeInterval(transitionDuration) } };
self.mapView.mbglMap->setTransitionOptions(transition);
self.mapView.mbglMap->setClasses(newAppliedClasses);
}
Expand Down Expand Up @@ -575,27 +575,27 @@ - (MGLImage *)imageForName:(NSString *)name
- (void)setTransitionDuration:(NSTimeInterval)duration
{
auto transitionOptions = self.mapView.mbglMap->getTransitionOptions();
transitionOptions.duration = MGLDurationInSecondsFromTimeInterval(duration);
transitionOptions.duration = MGLDurationFromTimeInterval(duration);
self.mapView.mbglMap->setTransitionOptions(transitionOptions);
}

- (NSTimeInterval)transitionDuration
{
const mbgl::style::TransitionOptions transitionOptions = self.mapView.mbglMap->getTransitionOptions();
return MGLTimeIntervalFromDurationInSeconds(transitionOptions.duration.value_or(mbgl::Duration::zero()));
return MGLTimeIntervalFromDuration(transitionOptions.duration.value_or(mbgl::Duration::zero()));
}

- (void)setTransitionDelay:(NSTimeInterval)delay
{
auto transitionOptions = self.mapView.mbglMap->getTransitionOptions();
transitionOptions.delay = MGLDurationInSecondsFromTimeInterval(delay);
transitionOptions.delay = MGLDurationFromTimeInterval(delay);
self.mapView.mbglMap->setTransitionOptions(transitionOptions);
}

- (NSTimeInterval)transitionDelay
{
const mbgl::style::TransitionOptions transitionOptions = self.mapView.mbglMap->getTransitionOptions();
return MGLTimeIntervalFromDurationInSeconds(transitionOptions.delay.value_or(mbgl::Duration::zero()));
return MGLTimeIntervalFromDuration(transitionOptions.delay.value_or(mbgl::Duration::zero()));
}

- (NSString *)description
Expand Down
12 changes: 8 additions & 4 deletions platform/darwin/src/NSDate+MGLAdditions.h
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
#import <Foundation/Foundation.h>

#import "MGLFoundation.h"
#include <mbgl/util/chrono.hpp>

@interface NSDate (MGLAdditions)
NS_ASSUME_NONNULL_BEGIN


/// Converts from a duration in seconds to a duration object usable in mbgl.
mbgl::Duration MGLDurationInSecondsFromTimeInterval(NSTimeInterval duration);
MGL_EXPORT
mbgl::Duration MGLDurationFromTimeInterval(NSTimeInterval duration);

/// Converts from an mbgl duration object to a duration in seconds.
NSTimeInterval MGLTimeIntervalFromDurationInSeconds(mbgl::Duration duration);
MGL_EXPORT
NSTimeInterval MGLTimeIntervalFromDuration(mbgl::Duration duration);

@end
NS_ASSUME_NONNULL_END
13 changes: 6 additions & 7 deletions platform/darwin/src/NSDate+MGLAdditions.mm
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
#import "NSDate+MGLAdditions.h"

@implementation NSDate (MGLAdditions)

mbgl::Duration MGLDurationInSecondsFromTimeInterval(NSTimeInterval duration)
mbgl::Duration MGLDurationFromTimeInterval(NSTimeInterval duration)
{
return std::chrono::duration_cast<mbgl::Duration>(std::chrono::duration<NSTimeInterval>(duration));
}

NSTimeInterval MGLTimeIntervalFromDurationInSeconds(mbgl::Duration duration)
NSTimeInterval MGLTimeIntervalFromDuration(mbgl::Duration duration)
{
return duration.count();
std::chrono::nanoseconds nano(duration.count());
Copy link
Contributor

Choose a reason for hiding this comment

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

This implementation makes the unsafe assumption that mbgl::Duration's count is in nanoseconds. mbgl::Duration is a typedef to std::chrono::steady_clock::duration, and std::chrono::steady_clock does not specify a particular tick period.

In addition, this implementation truncates any fractional seconds in the duration. NSTimeInterval is a floating-point type, and so the conversion should be expected to preserve fractional seconds.

I believe the correct conversion is as follows (but don't trust me -- write some more tests 😄):

NSTimeInterval MGLTimeIntervalFromDuration(mbgl::Duration duration)
{
    std::chrono::duration<NSTimeInterval, std::ratio<1>>(duration).count();
}

std::chrono::seconds sec = std::chrono::duration_cast<std::chrono::seconds>(nano);
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, since this is Objective-C++ code, you could condense these lines somewhat by passing duration.count() directly into the duration_cast. You can also use auto instead of declaring the local variable to be of type std::chrono::seconds. What you’ve got is totally reasonable, though.


return sec.count();
}

@end
26 changes: 26 additions & 0 deletions platform/darwin/test/MGLNSDateAdditionsTests.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#import <XCTest/XCTest.h>

#include <mbgl/util/chrono.hpp>
#import "../../darwin/src/NSDate+MGLAdditions.h"

using namespace std::chrono_literals;

@interface MGLNSDateAdditionsTests : XCTestCase
@end

@implementation MGLNSDateAdditionsTests

- (void)testDurationToNSTimeInterval {

NSTimeInterval timeInterval = 5;
mbgl::Duration duration = MGLDurationFromTimeInterval(timeInterval);
NSTimeInterval durationTimeInterval = MGLTimeIntervalFromDuration(duration);

mbgl::Duration expectedDuration = 5s;

XCTAssertEqual(timeInterval, durationTimeInterval);
XCTAssertEqual(timeInterval, MGLTimeIntervalFromDuration(expectedDuration));

}

@end
4 changes: 4 additions & 0 deletions platform/ios/ios.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
/* Begin PBXBuildFile section */
1753ED421E53CE6F00A9FD90 /* MGLConversion.h in Headers */ = {isa = PBXBuildFile; fileRef = 1753ED411E53CE6F00A9FD90 /* MGLConversion.h */; };
1753ED431E53CE6F00A9FD90 /* MGLConversion.h in Headers */ = {isa = PBXBuildFile; fileRef = 1753ED411E53CE6F00A9FD90 /* MGLConversion.h */; };
1F131E341E6A35EC0055AF5B /* MGLNSDateAdditionsTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 1F131E331E6A35EC0055AF5B /* MGLNSDateAdditionsTests.mm */; };
30E578171DAA85520050F07E /* UIImage+MGLAdditions.h in Headers */ = {isa = PBXBuildFile; fileRef = 30E578111DAA7D690050F07E /* UIImage+MGLAdditions.h */; };
30E578181DAA85520050F07E /* UIImage+MGLAdditions.h in Headers */ = {isa = PBXBuildFile; fileRef = 30E578111DAA7D690050F07E /* UIImage+MGLAdditions.h */; };
30E578191DAA855E0050F07E /* UIImage+MGLAdditions.mm in Sources */ = {isa = PBXBuildFile; fileRef = 30E578121DAA7D690050F07E /* UIImage+MGLAdditions.mm */; };
Expand Down Expand Up @@ -526,6 +527,7 @@

/* Begin PBXFileReference section */
1753ED411E53CE6F00A9FD90 /* MGLConversion.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLConversion.h; sourceTree = "<group>"; };
1F131E331E6A35EC0055AF5B /* MGLNSDateAdditionsTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MGLNSDateAdditionsTests.mm; sourceTree = "<group>"; };
20DABE861DF78148007AC5FF /* zh-Hans */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "zh-Hans"; path = "zh-Hans.lproj/Foundation.strings"; sourceTree = "<group>"; };
20DABE881DF78148007AC5FF /* zh-Hans */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "zh-Hans"; path = "zh-Hans.lproj/Localizable.strings"; sourceTree = "<group>"; };
20DABE8A1DF78149007AC5FF /* zh-Hans */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "zh-Hans"; path = "zh-Hans.lproj/Root.strings"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1175,6 +1177,7 @@
DA0CD58F1CF56F6A00A5F5A5 /* MGLFeatureTests.mm */,
DA2E885C1CC0382C00F24E7B /* MGLGeometryTests.mm */,
35E208A61D24210F00EC9A46 /* MGLNSDataAdditionsTests.m */,
1F131E331E6A35EC0055AF5B /* MGLNSDateAdditionsTests.mm */,
DAE7DEC11E245455007505A6 /* MGLNSStringAdditionsTests.m */,
DA2E885D1CC0382C00F24E7B /* MGLOfflinePackTests.m */,
DA2E885E1CC0382C00F24E7B /* MGLOfflineRegionTests.m */,
Expand Down Expand Up @@ -2070,6 +2073,7 @@
40CFA6511D7875BB008103BD /* MGLShapeSourceTests.mm in Sources */,
DA35A2C51CCA9F8300E826B2 /* MGLClockDirectionFormatterTests.m in Sources */,
35B8E08C1D6C8B5100E768D2 /* MGLPredicateTests.mm in Sources */,
1F131E341E6A35EC0055AF5B /* MGLNSDateAdditionsTests.mm in Sources */,
DD58A4C61D822BD000E1F038 /* MGLExpressionTests.mm in Sources */,
3575798B1D502B0C000B822E /* MGLBackgroundStyleLayerTests.mm in Sources */,
DA2E88621CC0382C00F24E7B /* MGLOfflinePackTests.m in Sources */,
Expand Down
24 changes: 12 additions & 12 deletions platform/ios/src/MGLMapView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1237,7 +1237,7 @@ - (void)handlePanGesture:(UIPanGestureRecognizer *)pan
if (![self.delegate respondsToSelector:@selector(mapView:shouldChangeFromCamera:toCamera:)] ||
[self.delegate mapView:self shouldChangeFromCamera:oldCamera toCamera:toCamera])
{
_mbglMap->moveBy({ offset.x, offset.y }, MGLDurationInSecondsFromTimeInterval(self.decelerationRate));
_mbglMap->moveBy({ offset.x, offset.y }, MGLDurationFromTimeInterval(self.decelerationRate));
}
}

Expand Down Expand Up @@ -1346,7 +1346,7 @@ - (void)handlePinchGesture:(UIPinchGestureRecognizer *)pinch
} else {
if (drift)
{
_mbglMap->setScale(newScale, mbgl::ScreenCoordinate { centerPoint.x, centerPoint.y }, MGLDurationInSecondsFromTimeInterval(duration));
_mbglMap->setScale(newScale, mbgl::ScreenCoordinate { centerPoint.x, centerPoint.y }, MGLDurationFromTimeInterval(duration));
}
}

Expand Down Expand Up @@ -1418,7 +1418,7 @@ - (void)handleRotateGesture:(UIRotationGestureRecognizer *)rotate
if (![self.delegate respondsToSelector:@selector(mapView:shouldChangeFromCamera:toCamera:)] ||
[self.delegate mapView:self shouldChangeFromCamera:oldCamera toCamera:toCamera])
{
_mbglMap->setBearing(newDegrees, mbgl::ScreenCoordinate { centerPoint.x, centerPoint.y }, MGLDurationInSecondsFromTimeInterval(decelerationRate));
_mbglMap->setBearing(newDegrees, mbgl::ScreenCoordinate { centerPoint.x, centerPoint.y }, MGLDurationFromTimeInterval(decelerationRate));

[self notifyGestureDidEndWithDrift:YES];

Expand Down Expand Up @@ -1558,7 +1558,7 @@ - (void)handleDoubleTapGesture:(UITapGestureRecognizer *)doubleTap
[self trackGestureEvent:MGLEventGestureDoubleTap forRecognizer:doubleTap];

mbgl::ScreenCoordinate center(gesturePoint.x, gesturePoint.y);
_mbglMap->setZoom(newZoom, center, MGLDurationInSecondsFromTimeInterval(MGLAnimationDuration));
_mbglMap->setZoom(newZoom, center, MGLDurationFromTimeInterval(MGLAnimationDuration));

__weak MGLMapView *weakSelf = self;

Expand Down Expand Up @@ -1596,7 +1596,7 @@ - (void)handleTwoFingerTapGesture:(UITapGestureRecognizer *)twoFingerTap
[self.delegate mapView:self shouldChangeFromCamera:oldCamera toCamera:toCamera])
{
mbgl::ScreenCoordinate center(gesturePoint.x, gesturePoint.y);
_mbglMap->setZoom(newZoom, center, MGLDurationInSecondsFromTimeInterval(MGLAnimationDuration));
_mbglMap->setZoom(newZoom, center, MGLDurationFromTimeInterval(MGLAnimationDuration));

__weak MGLMapView *weakSelf = self;

Expand Down Expand Up @@ -2426,7 +2426,7 @@ - (void)_setCenterCoordinate:(CLLocationCoordinate2D)centerCoordinate edgePaddin
mbgl::AnimationOptions animationOptions;
if (duration)
{
animationOptions.duration.emplace(MGLDurationInSecondsFromTimeInterval(duration));
animationOptions.duration.emplace(MGLDurationFromTimeInterval(duration));
animationOptions.easing.emplace(MGLUnitBezierForMediaTimingFunction(function));
}
if (completion)
Expand Down Expand Up @@ -2481,7 +2481,7 @@ - (void)setZoomLevel:(double)zoomLevel animated:(BOOL)animated

_mbglMap->setZoom(zoomLevel,
MGLEdgeInsetsFromNSEdgeInsets(self.contentInset),
MGLDurationInSecondsFromTimeInterval(duration));
MGLDurationFromTimeInterval(duration));
}

- (void)setMinimumZoomLevel:(double)minimumZoomLevel
Expand Down Expand Up @@ -2590,7 +2590,7 @@ - (void)_setVisibleCoordinates:(const CLLocationCoordinate2D *)coordinates count
mbgl::AnimationOptions animationOptions;
if (duration > 0)
{
animationOptions.duration.emplace(MGLDurationInSecondsFromTimeInterval(duration));
animationOptions.duration.emplace(MGLDurationFromTimeInterval(duration));
animationOptions.easing.emplace(MGLUnitBezierForMediaTimingFunction(function));
}
if (completion)
Expand Down Expand Up @@ -2653,13 +2653,13 @@ - (void)_setDirection:(CLLocationDirection)direction animated:(BOOL)animated
{
_mbglMap->setBearing(direction,
MGLEdgeInsetsFromNSEdgeInsets(self.contentInset),
MGLDurationInSecondsFromTimeInterval(duration));
MGLDurationFromTimeInterval(duration));
}
else
{
CGPoint centerPoint = self.userLocationAnnotationViewCenter;
_mbglMap->setBearing(direction, mbgl::ScreenCoordinate { centerPoint.x, centerPoint.y },
MGLDurationInSecondsFromTimeInterval(duration));
MGLDurationFromTimeInterval(duration));
}
}

Expand Down Expand Up @@ -2704,7 +2704,7 @@ - (void)setCamera:(MGLMapCamera *)camera withDuration:(NSTimeInterval)duration a
mbgl::AnimationOptions animationOptions;
if (duration > 0)
{
animationOptions.duration.emplace(MGLDurationInSecondsFromTimeInterval(duration));
animationOptions.duration.emplace(MGLDurationFromTimeInterval(duration));
animationOptions.easing.emplace(MGLUnitBezierForMediaTimingFunction(function));
}
if (completion)
Expand Down Expand Up @@ -2754,7 +2754,7 @@ - (void)_flyToCamera:(MGLMapCamera *)camera edgePadding:(UIEdgeInsets)insets wit
mbgl::AnimationOptions animationOptions;
if (duration >= 0)
{
animationOptions.duration = MGLDurationInSecondsFromTimeInterval(duration);
animationOptions.duration = MGLDurationFromTimeInterval(duration);
}
if (peakAltitude >= 0)
{
Expand Down
26 changes: 26 additions & 0 deletions platform/ios/test/MGLNSDateAdditionsTests.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#import <XCTest/XCTest.h>

#include <mbgl/util/chrono.hpp>
#import "../../darwin/src/NSDate+MGLAdditions.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Since NSDate+MGLAdditions.h is shared between iOS and macOS, the tests should be shared as well. Can you move this file to platform/darwin/test/ and add it to macos.xcodeproj?


using namespace std::chrono_literals;

@interface MGLNSDateAdditionsTests : XCTestCase
@end

@implementation MGLNSDateAdditionsTests

- (void)testDurationToNSTimeInterval {

NSTimeInterval timeInterval = 5;
mbgl::Duration duration = MGLDurationFromTimeInterval(timeInterval);
NSTimeInterval durationTimeInterval = MGLTimeIntervalFromDuration(duration);

mbgl::Duration expectedDuration = 5s;

XCTAssertEqual(timeInterval, durationTimeInterval);
XCTAssertEqual(timeInterval, MGLTimeIntervalFromDuration(expectedDuration));

}

@end
4 changes: 4 additions & 0 deletions platform/macos/macos.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

/* Begin PBXBuildFile section */
1753ED401E53CE6100A9FD90 /* MGLConversion.h in Headers */ = {isa = PBXBuildFile; fileRef = 1753ED3F1E53CE5200A9FD90 /* MGLConversion.h */; };
1F131E361E6A36170055AF5B /* MGLNSDateAdditionsTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 1F131E351E6A36170055AF5B /* MGLNSDateAdditionsTests.mm */; };
30E5781B1DAA857E0050F07E /* NSImage+MGLAdditions.h in Headers */ = {isa = PBXBuildFile; fileRef = 30E578141DAA7D920050F07E /* NSImage+MGLAdditions.h */; };
3508EC641D749D39009B0EE4 /* NSExpression+MGLAdditions.h in Headers */ = {isa = PBXBuildFile; fileRef = 3508EC621D749D39009B0EE4 /* NSExpression+MGLAdditions.h */; };
3508EC651D749D39009B0EE4 /* NSExpression+MGLAdditions.mm in Sources */ = {isa = PBXBuildFile; fileRef = 3508EC631D749D39009B0EE4 /* NSExpression+MGLAdditions.mm */; };
Expand Down Expand Up @@ -263,6 +264,7 @@

/* Begin PBXFileReference section */
1753ED3F1E53CE5200A9FD90 /* MGLConversion.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLConversion.h; sourceTree = "<group>"; };
1F131E351E6A36170055AF5B /* MGLNSDateAdditionsTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MGLNSDateAdditionsTests.mm; sourceTree = "<group>"; };
30E578141DAA7D920050F07E /* NSImage+MGLAdditions.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = "NSImage+MGLAdditions.h"; path = "src/NSImage+MGLAdditions.h"; sourceTree = SOURCE_ROOT; };
3508EC621D749D39009B0EE4 /* NSExpression+MGLAdditions.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "NSExpression+MGLAdditions.h"; sourceTree = "<group>"; };
3508EC631D749D39009B0EE4 /* NSExpression+MGLAdditions.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = "NSExpression+MGLAdditions.mm"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -944,6 +946,7 @@
DA35A2A71CC9F41600E826B2 /* MGLCoordinateFormatterTests.m */,
DA2987591E1A4290002299F5 /* MGLDocumentationExampleTests.swift */,
DD58A4C71D822C6200E1F038 /* MGLExpressionTests.mm */,
1F131E351E6A36170055AF5B /* MGLNSDateAdditionsTests.mm */,
35C6DF861E214C1800ACA483 /* MGLDistanceFormatterTests.m */,
DA0CD58D1CF56F5800A5F5A5 /* MGLFeatureTests.mm */,
DAE6C3C81CC34BD800DB3429 /* MGLGeometryTests.mm */,
Expand Down Expand Up @@ -1403,6 +1406,7 @@
DAE6C3D21CC34C9900DB3429 /* MGLGeometryTests.mm in Sources */,
DA87A9A41DCACC5000810D09 /* MGLSymbolStyleLayerTests.mm in Sources */,
40E1601D1DF217D6005EA6D9 /* MGLStyleLayerTests.m in Sources */,
1F131E361E6A36170055AF5B /* MGLNSDateAdditionsTests.mm in Sources */,
DA87A9A61DCACC5000810D09 /* MGLCircleStyleLayerTests.mm in Sources */,
DA87A99E1DC9DC2100810D09 /* MGLPredicateTests.mm in Sources */,
DD58A4C91D822C6700E1F038 /* MGLExpressionTests.mm in Sources */,
Expand Down
Loading