diff options
| -rw-r--r-- | platform/ios/CHANGELOG.md | 1 | ||||
| -rw-r--r-- | platform/ios/src/MGLMapView.mm | 21 | ||||
| -rw-r--r-- | platform/ios/src/MGLScaleBar.mm | 221 | ||||
| -rw-r--r-- | platform/ios/test/MGLMapViewLayoutTests.m | 85 | ||||
| -rw-r--r-- | platform/ios/test/MGLMapViewScaleBarTests.m | 15 |
5 files changed, 269 insertions, 74 deletions
diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md index 458e533442..d636d94811 100644 --- a/platform/ios/CHANGELOG.md +++ b/platform/ios/CHANGELOG.md @@ -13,6 +13,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT ### Other changes * Suppress network requests for expired tiles update, if these tiles are invisible. ([#15741](https://github.com/mapbox/mapbox-gl-native/pull/15741)) +* Fixed an issue that caused `MGLScaleBar` to have an incorrect size when resizing or rotating. ([#15703](https://github.com/mapbox/mapbox-gl-native/pull/15703)) ## 5.4.0 - September 25, 2019 diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm index 62b943fd3d..76cec479bb 100644 --- a/platform/ios/src/MGLMapView.mm +++ b/platform/ios/src/MGLMapView.mm @@ -852,9 +852,11 @@ public: break; } - [updatedConstraints addObject:[view.widthAnchor constraintEqualToConstant:size.width]]; - [updatedConstraints addObject:[view.heightAnchor constraintEqualToConstant:size.height]]; - + if (!CGSizeEqualToSize(size, CGSizeZero)) { + [updatedConstraints addObject:[view.widthAnchor constraintEqualToConstant:size.width]]; + [updatedConstraints addObject:[view.heightAnchor constraintEqualToConstant:size.height]]; + } + [NSLayoutConstraint deactivateConstraints:constraints]; [constraints removeAllObjects]; [NSLayoutConstraint activateConstraints:updatedConstraints]; @@ -883,7 +885,7 @@ public: [self updateConstraintsForOrnament:self.scaleBar constraints:self.scaleBarConstraints position:self.scaleBarPosition - size:self.scaleBar.intrinsicContentSize + size:CGSizeZero margins:self.scaleBarMargins]; } @@ -929,15 +931,14 @@ public: // This gets called when the view dimension changes, e.g. because the device is being rotated. - (void)layoutSubviews { + [super layoutSubviews]; + // Calling this here instead of in the scale bar itself because if this is done in the // scale bar instance, it triggers a call to this `layoutSubviews` method that calls // `_mbglMap->setSize()` just below that triggers rendering update which triggers // another scale bar update which causes a rendering update loop and a major performace - // degradation. The only time the scale bar's intrinsic content size _must_ invalidated - // is here as a reaction to this object's view dimension changes. + // degradation. [self.scaleBar invalidateIntrinsicContentSize]; - - [super layoutSubviews]; [self adjustContentInset]; @@ -6642,11 +6643,7 @@ public: // setting this property. if ( ! self.scaleBar.hidden) { - CGSize originalSize = self.scaleBar.intrinsicContentSize; [(MGLScaleBar *)self.scaleBar setMetersPerPoint:[self metersPerPointAtLatitude:self.centerCoordinate.latitude]]; - if ( ! CGSizeEqualToSize(originalSize, self.scaleBar.intrinsicContentSize)) { - [self installScaleBarConstraints]; - } } } diff --git a/platform/ios/src/MGLScaleBar.mm b/platform/ios/src/MGLScaleBar.mm index 8525881da7..3efa80013f 100644 --- a/platform/ios/src/MGLScaleBar.mm +++ b/platform/ios/src/MGLScaleBar.mm @@ -82,16 +82,20 @@ static const MGLRow MGLImperialTable[] ={ @property (nonatomic, assign) MGLRow row; @property (nonatomic) UIColor *primaryColor; @property (nonatomic) UIColor *secondaryColor; -@property (nonatomic) CALayer *borderLayer; @property (nonatomic, assign) CGFloat borderWidth; @property (nonatomic) NSMutableDictionary* labelImageCache; @property (nonatomic) MGLScaleBarLabel* prototypeLabel; @property (nonatomic) CGFloat lastLabelWidth; - +@property (nonatomic) CGSize size; +@property (nonatomic) BOOL recalculateSize; +@property (nonatomic) BOOL shouldLayoutBars; +@property (nonatomic) NSNumber *testingRightToLeftOverride; @end static const CGFloat MGLBarHeight = 4; static const CGFloat MGLFeetPerMeter = 3.28084; +static const CGFloat MGLScaleBarLabelWidthHint = 30.0; +static const CGFloat MGLScaleBarMinimumBarWidth = 30.0; // Arbitrary @interface MGLScaleBarLabel : UILabel @@ -137,6 +141,8 @@ static const CGFloat MGLFeetPerMeter = 3.28084; } - (void)commonInit { + _size = CGSizeZero; + _primaryColor = [UIColor colorWithRed:18.0/255.0 green:45.0/255.0 blue:17.0/255.0 alpha:1]; _secondaryColor = [UIColor colorWithRed:247.0/255.0 green:247.0/255.0 blue:247.0/255.0 alpha:1]; _borderWidth = 1.0f; @@ -144,17 +150,17 @@ static const CGFloat MGLFeetPerMeter = 3.28084; self.clipsToBounds = NO; self.hidden = YES; - _containerView = [[UIView alloc] init]; - _containerView.clipsToBounds = YES; - _containerView.backgroundColor = self.secondaryColor; + _containerView = [[UIView alloc] init]; + _containerView.clipsToBounds = YES; + _containerView.backgroundColor = _secondaryColor; + _containerView.layer.borderColor = _primaryColor.CGColor; + _containerView.layer.borderWidth = _borderWidth / [[UIScreen mainScreen] scale]; + + _containerView.layer.cornerRadius = MGLBarHeight / 2.0; + _containerView.layer.masksToBounds = YES; + [self addSubview:_containerView]; - _borderLayer = [CAShapeLayer layer]; - _borderLayer.borderColor = [self.primaryColor CGColor]; - _borderLayer.borderWidth = 1.0f / [[UIScreen mainScreen] scale]; - - [_containerView.layer addSublayer:_borderLayer]; - _formatter = [[MGLDistanceFormatter alloc] init]; // Image labels are now images @@ -176,6 +182,7 @@ static const CGFloat MGLFeetPerMeter = 3.28084; [self addSubview:view]; } _labelViews = [labelViews copy]; + _lastLabelWidth = MGLScaleBarLabelWidthHint; // Zero is a special case (no formatting) [self addZeroLabel]; @@ -194,16 +201,33 @@ static const CGFloat MGLFeetPerMeter = 3.28084; #pragma mark - Dimensions -- (CGSize)intrinsicContentSize { - return self.actualWidth > 0 ? CGSizeMake(ceil(self.actualWidth + self.lastLabelWidth/2), 16) : CGSizeZero; +- (void)setBorderWidth:(CGFloat)borderWidth { + _borderWidth = borderWidth; + _containerView.layer.borderWidth = borderWidth / [[UIScreen mainScreen] scale]; } +// Determines the width of the bars NOT the size of the entire scale bar, +// which includes space for (half) a label. +// Uses the current set `row` - (CGFloat)actualWidth { - CGFloat width = self.row.distance / [self unitsPerPoint]; - return !isnan(width) ? width : 0; + CGFloat unitsPerPoint = [self unitsPerPoint]; + + if (unitsPerPoint == 0.0) { + return 0.0; + } + + CGFloat width = self.row.distance / unitsPerPoint; + + if (width <= MGLScaleBarMinimumBarWidth) { + return 0.0; + } + + // Round, so that each bar section has an integer width + return self.row.numberOfBars * floor(width/self.row.numberOfBars); } - (CGFloat)maximumWidth { + // TODO: Consider taking Scale Bar margins into account here. CGFloat fullWidth = CGRectGetWidth(self.superview.bounds); return floorf(fullWidth / 2); } @@ -215,6 +239,10 @@ static const CGFloat MGLFeetPerMeter = 3.28084; #pragma mark - Convenience methods - (BOOL)usesRightToLeftLayout { + if (self.testingRightToLeftOverride) { + return [self.testingRightToLeftOverride boolValue]; + } + return [UIView userInterfaceLayoutDirectionForSemanticContentAttribute:self.superview.semanticContentAttribute] == UIUserInterfaceLayoutDirectionRightToLeft; } @@ -265,10 +293,61 @@ static const CGFloat MGLFeetPerMeter = 3.28084; [self updateVisibility]; + self.recalculateSize = YES; + [self invalidateIntrinsicContentSize]; +} + +- (CGSize)intrinsicContentSize { + // Size is calculated elsewhere - since intrinsicContentSize is part of the + // constraint system, this should be done in updateConstraints + if (self.size.width < 0.0) { + return CGSizeZero; + } + return self.size; +} + +/// updateConstraints +/// +/// The primary job of updateConstraints here is to recalculate the +/// intrinsicContentSize: _metersPerPoint and the maximum width determine the +/// current "row", which in turn determines the "actualWidth". To obtain the full +/// width of the scale bar, we also need to include some space for the "last" +/// label + +- (void)updateConstraints { + if (self.isHidden || !self.recalculateSize) { + [super updateConstraints]; + return; + } + + // TODO: Improve this (and the side-effects) self.row = [self preferredRow]; - [self invalidateIntrinsicContentSize]; + NSAssert(self.row.numberOfBars > 0, @""); + + CGFloat totalBarWidth = self.actualWidth; + + if (totalBarWidth <= 0.0) { + [super updateConstraints]; + return; + } + + // Determine the "lastLabelWidth". This has changed to take a maximum of each + // label, to ensure that the size does not change in LTR & RTL layouts, and + // also to stop jiggling when the scale bar is on the right hand of the screen + // This will most likely be a constant, as we take a max using a "hint" for + // the initial value + + if (self.shouldLayoutBars) { + [self updateLabels]; + } + + CGFloat halfLabelWidth = ceil(self.lastLabelWidth/2); + + self.size = CGSizeMake(totalBarWidth + halfLabelWidth, 16); + [self setNeedsLayout]; + [super updateConstraints]; // This calls intrinsicContentSize } - (void)updateVisibility { @@ -297,11 +376,9 @@ static const CGFloat MGLFeetPerMeter = 3.28084; return; } + self.shouldLayoutBars = YES; + _row = row; - [_bars makeObjectsPerformSelector:@selector(removeFromSuperview)]; - _bars = nil; - - [self updateLabels]; } #pragma mark - Views @@ -378,9 +455,9 @@ static const CGFloat MGLFeetPerMeter = 3.28084; CLLocationDistance barDistance = multiplier * i; UIImage *image = [self cachedLabelImageForDistance:barDistance]; - if (i == self.row.numberOfBars) { - self.lastLabelWidth = image.size.width; - } + + self.lastLabelWidth = MAX(self.lastLabelWidth, image.size.width); + labelView.layer.contents = (id)image.CGImage; labelView.layer.contentsScale = image.scale; } @@ -397,53 +474,83 @@ static const CGFloat MGLFeetPerMeter = 3.28084; - (void)layoutSubviews { [super layoutSubviews]; - if (!self.row.numberOfBars) { - // Current distance is not within allowed range + if (!self.recalculateSize) { return; } - [self layoutBars]; - [self layoutLabels]; -} + self.recalculateSize = NO; + + // If size is 0, then we keep the existing layout (which will fade out) + if (self.size.width <= 0.0) { + return; + } + + CGFloat totalBarWidth = self.actualWidth; + + if (totalBarWidth <= 0.0) { + return; + } + + if (self.shouldLayoutBars) { + self.shouldLayoutBars = NO; + [_bars makeObjectsPerformSelector:@selector(removeFromSuperview)]; + _bars = nil; + } + + // Re-layout the component bars and labels of the scale bar + CGFloat intrinsicContentHeight = self.intrinsicContentSize.height; + CGFloat barWidth = totalBarWidth/self.bars.count; -- (void)layoutBars { - CGFloat barWidth = round((self.intrinsicContentSize.width - self.borderWidth * 2.0f) / self.bars.count); + BOOL RTL = [self usesRightToLeftLayout]; + CGFloat halfLabelWidth = ceil(self.lastLabelWidth/2); + CGFloat barOffset = RTL ? halfLabelWidth : 0.0; + self.containerView.frame = CGRectMake(barOffset, + intrinsicContentHeight - MGLBarHeight, + totalBarWidth, + MGLBarHeight); + + [self layoutBarsWithWidth:barWidth]; + + CGFloat yPosition = round(0.5 * ( intrinsicContentHeight - MGLBarHeight)); + CGFloat barDelta = RTL ? -barWidth : barWidth; + [self layoutLabelsWithOffset:barOffset delta:barDelta yPosition:yPosition]; +} + +- (void)layoutBarsWithWidth:(CGFloat)barWidth { NSUInteger i = 0; for (UIView *bar in self.bars) { - CGFloat xPosition = barWidth * i + self.borderWidth; + CGFloat xPosition = barWidth * i; bar.backgroundColor = (i % 2 == 0) ? self.primaryColor : self.secondaryColor; - bar.frame = CGRectMake(xPosition, self.borderWidth, barWidth, MGLBarHeight); + bar.frame = CGRectMake(xPosition, 0, barWidth, MGLBarHeight); i++; } - - self.containerView.frame = CGRectMake(CGRectGetMinX(self.bars.firstObject.frame), - self.intrinsicContentSize.height-MGLBarHeight, - self.actualWidth, - MGLBarHeight+self.borderWidth*2); - - [CATransaction begin]; - [CATransaction setDisableActions:YES]; - self.borderLayer.frame = CGRectInset(self.containerView.bounds, self.borderWidth, self.borderWidth); - self.borderLayer.zPosition = FLT_MAX; - [CATransaction commit]; } -- (void)layoutLabels { - CGFloat barWidth = round(self.actualWidth / self.bars.count); - BOOL RTL = [self usesRightToLeftLayout]; - NSUInteger i = RTL ? self.bars.count : 0; +- (void)layoutLabelsWithOffset:(CGFloat)barOffset delta:(CGFloat)barDelta yPosition:(CGFloat)yPosition { +#if !defined(NS_BLOCK_ASSERTIONS) + NSUInteger countOfVisibleLabels = 0; + for (UIView *view in self.labelViews) { + if (!view.isHidden) { + countOfVisibleLabels++; + } + } + NSAssert(self.bars.count == countOfVisibleLabels - 1, @""); +#endif + + CGFloat xPosition = barOffset; + + if (barDelta < 0) { + xPosition -= (barDelta*self.bars.count); + } + for (UIView *label in self.labelViews) { - CGFloat xPosition = round(barWidth * i - CGRectGetMidX(label.bounds) + self.borderWidth); - CGFloat yPosition = round(0.5 * (self.intrinsicContentSize.height - MGLBarHeight)); - - CGRect frame = label.frame; - frame.origin.x = xPosition; - frame.origin.y = yPosition; - label.frame = frame; - - i = RTL ? i-1 : i+1; + // Label frames have 0 size - though the layer contents use "center" and do + // not clip to bounds. This way we don't need to worry about positioning the + // label. (Though you won't see the label in the view debugger) + label.frame = CGRectMake(xPosition, yPosition, 0.0, 0.0); + + xPosition += barDelta; } } - @end diff --git a/platform/ios/test/MGLMapViewLayoutTests.m b/platform/ios/test/MGLMapViewLayoutTests.m index 5d9c0339b5..2a9579818a 100644 --- a/platform/ios/test/MGLMapViewLayoutTests.m +++ b/platform/ios/test/MGLMapViewLayoutTests.m @@ -3,6 +3,7 @@ #import "MGLMapViewDelegate.h" #import "MGLAccountManager.h" +#import "MGLScaleBar.h" @interface MGLOrnamentTestData : NSObject @@ -24,6 +25,14 @@ @end +@interface MGLScaleBar (Tests) +@property (nonatomic, readonly) NSArray<UIView *> *labelViews; +@property (nonatomic, readonly) NSArray<UIView *> *bars; +@property (nonatomic, readonly) UIView *containerView; +@property (nonatomic, readonly) CGSize size; +@property (nonatomic) NSNumber *testingRightToLeftOverride; +@end + @interface MGLMapViewLayoutTests : XCTestCase<MGLMapViewDelegate> @@ -140,7 +149,7 @@ expectedOrigin:CGPointMake(margin, margin)], [MGLOrnamentTestData createWithPosition:MGLOrnamentPositionTopRight offset:CGPointMake(margin, margin) - expectedOrigin:CGPointMake(CGRectGetMaxX(self.mapView.bounds) - margin - CGRectGetWidth(view.frame), 4)], + expectedOrigin:CGPointMake(CGRectGetMaxX(self.mapView.bounds) - margin - CGRectGetWidth(view.frame), margin)], [MGLOrnamentTestData createWithPosition:MGLOrnamentPositionBottomLeft offset:CGPointMake(margin, margin) expectedOrigin:CGPointMake(margin, CGRectGetMaxY(self.mapView.bounds) - margin - bottomSafeAreaInset - CGRectGetHeight(view.frame))], @@ -176,6 +185,8 @@ CGFloat margin = 4.0; UIView *scaleBar = self.mapView.scaleBar; + XCTAssertFalse(CGSizeEqualToSize(scaleBar.bounds.size, CGSizeZero)); + NSArray *testDataList = [self makeTestDataListWithView:scaleBar margin:margin]; for (MGLOrnamentTestData *testData in testDataList) { @@ -191,6 +202,78 @@ } } +// This test checks the frames of the scalebar's subviews, based on the positions +// as above, but also with forced Right-to-Left reading, and modifying zoom levels. +- (void)testScalebarSubviewPlacement { + double accuracy = 0.01; + CGFloat margin = 20.0; + + MGLScaleBar *scaleBar = (MGLScaleBar*)self.mapView.scaleBar; + XCTAssertFalse(CGSizeEqualToSize(scaleBar.bounds.size, CGSizeZero)); + + for (NSInteger rtl = 0; rtl <= 1; rtl++) { + scaleBar.testingRightToLeftOverride = @((BOOL)rtl); + + NSString *positions[] = { + @"MGLOrnamentPositionTopLeft", + @"MGLOrnamentPositionTopRight", + @"MGLOrnamentPositionBottomLeft", + @"MGLOrnamentPositionBottomRight" + }; + + for (CGFloat zoomLevel = 0; zoomLevel < 20; zoomLevel++) + { + self.mapView.zoomLevel = zoomLevel; + [self.superView setNeedsLayout]; + [self.superView layoutIfNeeded]; + + // Following method assumes scaleBar has an up-to-date frame, based + // on the current zoom level. Modifying the position and margins + // should not affect the overall size of the scalebar. + + NSArray *testDataList = [self makeTestDataListWithView:scaleBar margin:margin]; + + CGSize initialSize = scaleBar.intrinsicContentSize; + XCTAssert(CGSizeEqualToSize(initialSize, scaleBar.bounds.size)); + + for (MGLOrnamentTestData *testData in testDataList) { + self.mapView.scaleBarPosition = testData.position; + self.mapView.scaleBarMargins = testData.offset; + + [self.superView setNeedsLayout]; + [self.superView layoutIfNeeded]; + + XCTAssert(CGSizeEqualToSize(initialSize, scaleBar.bounds.size)); + + NSString *activityName = [NSString stringWithFormat: + @"Scalebar subview tests: RTL=%@, Zoom=%ld, POS=%@, Visible=%@", + (rtl == 0 ? @"NO" : @"YES"), + (long)zoomLevel, + positions[testData.position], + scaleBar.alpha > 0.0 ? @"YES" : @"NO"]; + + [XCTContext runActivityNamed:activityName + block:^(id<XCTActivity> activity) { + + // Check the subviews + XCTAssertEqualWithAccuracy(CGRectGetMinX(scaleBar.frame), testData.expectedOrigin.x, accuracy); + XCTAssertEqualWithAccuracy(CGRectGetMinY(scaleBar.frame), testData.expectedOrigin.y, accuracy); + + XCTAssertTrue(CGRectContainsRect(scaleBar.bounds, scaleBar.containerView.frame)); + for (UIView *bar in scaleBar.bars) { + XCTAssertTrue(CGRectContainsRect(scaleBar.containerView.bounds, bar.frame)); + } + for (UIView *label in scaleBar.labelViews) { + if (!label.isHidden) { + XCTAssertTrue(CGRectContainsRect(scaleBar.bounds, label.frame)); + } + } + }]; + } + } + } +} + - (void)testAttributionButtonPlacement { double accuracy = 0.01; CGFloat margin = 4.0; diff --git a/platform/ios/test/MGLMapViewScaleBarTests.m b/platform/ios/test/MGLMapViewScaleBarTests.m index 29acc75d7f..b4f81ef62b 100644 --- a/platform/ios/test/MGLMapViewScaleBarTests.m +++ b/platform/ios/test/MGLMapViewScaleBarTests.m @@ -36,11 +36,14 @@ XCTAssertFalse(scaleBar.hidden); // Scale bar should not be visible at default zoom (~z0), but it should be ready. - XCTAssertFalse(CGSizeEqualToSize(scaleBar.intrinsicContentSize, CGSizeZero)); + // Size is not a measure of readiness here though. + XCTAssertTrue(CGSizeEqualToSize(scaleBar.intrinsicContentSize, CGSizeZero)); XCTAssertEqual(scaleBar.alpha, 0); self.mapView.zoomLevel = 15; + [self.mapView layoutIfNeeded]; XCTAssertGreaterThan(scaleBar.alpha, 0); + XCTAssertFalse(CGSizeEqualToSize(scaleBar.intrinsicContentSize, CGSizeZero)); } - (void)testDirectlySettingScaleBarViewHiddenProperty { @@ -54,10 +57,14 @@ // ... but triggering any camera event will update it. self.mapView.zoomLevel = 1; - XCTAssertFalse(CGSizeEqualToSize(scaleBar.intrinsicContentSize, CGSizeZero)); + [self.mapView layoutIfNeeded]; + + XCTAssertTrue(CGSizeEqualToSize(scaleBar.intrinsicContentSize, CGSizeZero)); XCTAssertEqual(scaleBar.alpha, 0); self.mapView.zoomLevel = 15; + [self.mapView layoutIfNeeded]; + XCTAssertGreaterThan(scaleBar.alpha, 0); -} -@end + XCTAssertFalse(CGSizeEqualToSize(scaleBar.intrinsicContentSize, CGSizeZero)); +}@end |