From 6fd011386f45f2264d699373fbbad911b1ebd3f1 Mon Sep 17 00:00:00 2001 From: Tim Oliver Date: Sun, 12 Jul 2026 00:53:30 +0900 Subject: [PATCH 1/5] Fix angle setter hang, dead gridOverlayHidden setter, and degenerate input guards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Setting an angle of ±360 or beyond infinite-looped on the main thread, since the rotation logic wraps its angle to 0 at a full revolution; the setter now normalizes into that range and compares signed values, which also makes direction changes like 90 -> -90 work - gridOverlayHidden passed the old ivar instead of its parameter, making plain assignment a no-op; the animated variant also honors its flag now - A zero-sized image no longer produces NaN layout geometry, and aspect ratios with a single zero component revert to the image ratio Adds the first real unit tests around TOCropView's public setters. --- .../TOCropViewController/Views/TOCropView.m | 32 +++++++--- .../TOCropViewControllerTests.m | 61 +++++++++++++++++++ 2 files changed, 84 insertions(+), 9 deletions(-) diff --git a/Objective-C/TOCropViewController/Views/TOCropView.m b/Objective-C/TOCropViewController/Views/TOCropView.m index 7e12ca63..ce04bd5f 100644 --- a/Objective-C/TOCropViewController/Views/TOCropView.m +++ b/Objective-C/TOCropViewController/Views/TOCropView.m @@ -267,6 +267,10 @@ - (void)performInitialSetup { - (void)layoutInitialImage { CGSize imageSize = self.imageSize; + // A zero-sized image would produce NaN geometry below, which crashes CALayer + if (imageSize.width < FLT_EPSILON || imageSize.height < FLT_EPSILON) { + return; + } self.scrollView.contentSize = imageSize; CGRect bounds = self.contentBounds; @@ -1138,13 +1142,18 @@ - (void)setTranslucencyAlwaysHidden:(BOOL)translucencyAlwaysHidden { } - (void)setGridOverlayHidden:(BOOL)gridOverlayHidden { - [self setGridOverlayHidden:_gridOverlayHidden animated:NO]; + [self setGridOverlayHidden:gridOverlayHidden animated:NO]; } - (void)setGridOverlayHidden:(BOOL)gridOverlayHidden animated:(BOOL)animated { _gridOverlayHidden = gridOverlayHidden; - self.gridOverlayView.alpha = gridOverlayHidden ? 1.0f : 0.0f; + if (!animated) { + self.gridOverlayView.alpha = gridOverlayHidden ? 0.0f : 1.0f; + return; + } + + self.gridOverlayView.alpha = gridOverlayHidden ? 1.0f : 0.0f; [UIView animateWithDuration:0.4f animations:^{ self.gridOverlayView.alpha = gridOverlayHidden ? 0.0f : 1.0f; @@ -1176,26 +1185,31 @@ - (void)setCanBeReset:(BOOL)canReset { } - (void)setAngle:(NSInteger)angle { - // The initial layout would not have been performed yet. - // Save the value and it will be applied when it has + // Only multiples of 90 degrees are supported NSInteger newAngle = angle; if (angle % 90 != 0) { newAngle = 0; } + // Normalize to the (-360, 360) range the rotation logic produces + // (eg, 360 wraps back around to 0), preserving direction + newAngle %= 360; + + // The initial layout would not have been performed yet. + // Save the value and it will be applied when it has if (!self.initialSetupPerformed) { self.restoreAngle = newAngle; return; } // Negative values are allowed, so rotate clockwise or counter clockwise depending - // on direction + // on direction. Compare the signed values since +90 and -90 are distinct states. if (newAngle >= 0) { - while (labs(self.angle) != labs(newAngle)) { + while (self.angle != newAngle) { [self rotateImageNinetyDegreesAnimated:NO clockwise:YES completion:nil]; } } else { - while (-labs(self.angle) != -labs(newAngle)) { + while (self.angle != newAngle) { [self rotateImageNinetyDegreesAnimated:NO clockwise:NO completion:nil]; } } @@ -1373,8 +1387,8 @@ - (void)setAspectRatio:(CGSize)aspectRatio animated:(BOOL)animated { BOOL zoomOut = NO; - // Passing in an empty size will revert back to the image aspect ratio - if (aspectRatio.width < FLT_EPSILON && aspectRatio.height < FLT_EPSILON) { + // Passing in an empty size (or one with a zero component) will revert back to the image aspect ratio + if (aspectRatio.width < FLT_EPSILON || aspectRatio.height < FLT_EPSILON) { aspectRatio = (CGSize){self.imageSize.width, self.imageSize.height}; zoomOut = YES; // Prevent from steadily zooming in when cycling between alternate aspectRatios and original } diff --git a/Objective-C/TOCropViewControllerTests/TOCropViewControllerTests.m b/Objective-C/TOCropViewControllerTests/TOCropViewControllerTests.m index c23afc79..52b88f07 100644 --- a/Objective-C/TOCropViewControllerTests/TOCropViewControllerTests.m +++ b/Objective-C/TOCropViewControllerTests/TOCropViewControllerTests.m @@ -17,6 +17,67 @@ @interface TOCropViewControllerTests : XCTestCase @implementation TOCropViewControllerTests +#pragma mark - Helpers - + +- (UIImage *)testImageWithSize:(CGSize)size { + UIGraphicsImageRenderer *renderer = [[UIGraphicsImageRenderer alloc] initWithSize:size]; + return [renderer imageWithActions:^(UIGraphicsImageRendererContext *_Nonnull context) { + [[UIColor redColor] setFill]; + [context fillRect:(CGRect){CGPointZero, size}]; + }]; +} + +- (TOCropView *)cropViewWithImageSize:(CGSize)imageSize { + TOCropView *cropView = [[TOCropView alloc] initWithImage:[self testImageWithSize:imageSize]]; + cropView.frame = (CGRect){0, 0, 320, 480}; + [cropView performInitialSetup]; + return cropView; +} + +#pragma mark - Tests - + +- (void)testGridOverlayHiddenSetter { + TOCropView *cropView = [self cropViewWithImageSize:(CGSize){40, 20}]; + cropView.gridOverlayHidden = YES; + XCTAssertTrue(cropView.gridOverlayHidden); + cropView.gridOverlayHidden = NO; + XCTAssertFalse(cropView.gridOverlayHidden); +} + +- (void)testSetAngleAllowsDirectionChanges { + TOCropView *cropView = [self cropViewWithImageSize:(CGSize){40, 20}]; + cropView.angle = 90; + XCTAssertEqual(cropView.angle, 90); + cropView.angle = -90; + XCTAssertEqual(cropView.angle, -90); +} + +- (void)testSetAngleNormalizesFullRotations { + TOCropView *cropView = [self cropViewWithImageSize:(CGSize){40, 20}]; + cropView.angle = 360; + XCTAssertEqual(cropView.angle, 0); + cropView.angle = 450; + XCTAssertEqual(cropView.angle, 90); + cropView.angle = -450; + XCTAssertEqual(cropView.angle, -90); +} + +- (void)testZeroSizeImageDoesNotCrash { + TOCropView *cropView = [[TOCropView alloc] initWithImage:[UIImage new]]; + cropView.frame = (CGRect){0, 0, 320, 480}; + XCTAssertNoThrow([cropView performInitialSetup]); + XCTAssertNoThrow([cropView layoutIfNeeded]); +} + +- (void)testDegenerateAspectRatioIsIgnored { + TOCropView *cropView = [self cropViewWithImageSize:(CGSize){40, 20}]; + CGRect before = cropView.imageCropFrame; + [cropView setAspectRatio:(CGSize){0.0f, 5.0f} animated:NO]; + CGRect after = cropView.imageCropFrame; + XCTAssertEqualWithAccuracy(before.origin.x, after.origin.x, 1.0); + XCTAssertEqualWithAccuracy(before.size.width, after.size.width, 1.0); +} + - (void)testViewControllerInstance { // Create a basic image UIGraphicsImageRenderer *renderer = [[UIGraphicsImageRenderer alloc] initWithSize:CGSizeMake(10, 10)]; From 5aa1c0cb38cf67b83683c51ee1da1850f04adabd Mon Sep 17 00:00:00 2001 From: Tim Oliver Date: Sun, 12 Jul 2026 01:01:53 +0900 Subject: [PATCH 2/5] Fix content offset math when rotating with a restored crop state The focus-point mapping after a 90-degree rotation assumed the zoom always changed by the geometric fitting scale, but the branch that restores a previously edited crop applies a different zoom delta, throwing the offset into the wrong coordinate space. The offset is now mapped by the zoom ratio that was actually applied. Also corrects the post-rotation and device-rotation maximum-offset clamps (both disagreed with the proven form used by the recenter animation) and keeps maximumZoomScale in step wherever minimumZoomScale is rescaled, so extreme-aspect images can always fill the crop box. --- .../TOCropViewController/Views/TOCropView.m | 26 +++++++++++++------ .../TOCropViewControllerTests.m | 13 ++++++++++ 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/Objective-C/TOCropViewController/Views/TOCropView.m b/Objective-C/TOCropViewController/Views/TOCropView.m index ce04bd5f..1eed72f4 100644 --- a/Objective-C/TOCropViewController/Views/TOCropView.m +++ b/Objective-C/TOCropViewController/Views/TOCropView.m @@ -345,6 +345,7 @@ - (void)performRelayoutForRotation { CGFloat scale = MIN(contentFrame.size.width / cropFrame.size.width, contentFrame.size.height / cropFrame.size.height); self.scrollView.minimumZoomScale *= scale; + self.scrollView.maximumZoomScale *= scale; self.scrollView.zoomScale *= scale; // Work out the centered, upscaled version of the crop rectangle @@ -382,8 +383,8 @@ - (void)performRelayoutForRotation { // Nor undershoot the bottom right corner CGPoint maximumOffset = CGPointZero; - maximumOffset.x = (self.bounds.size.width - self.scrollView.contentInset.right) + self.scrollView.contentSize.width; - maximumOffset.y = (self.bounds.size.height - self.scrollView.contentInset.bottom) + self.scrollView.contentSize.height; + maximumOffset.x = self.scrollView.contentSize.width - (self.bounds.size.width - self.scrollView.contentInset.right); + maximumOffset.y = self.scrollView.contentSize.height - (self.bounds.size.height - self.scrollView.contentInset.bottom); offset.x = MIN(offset.x, maximumOffset.x); offset.y = MIN(offset.y, maximumOffset.y); self.scrollView.contentOffset = offset; @@ -1004,6 +1005,7 @@ - (void)setCropBoxFrame:(CGRect)cropBoxFrame { CGSize imageSize = self.backgroundContainerView.bounds.size; CGFloat scale = MAX(cropBoxFrame.size.height / imageSize.height, cropBoxFrame.size.width / imageSize.width); self.scrollView.minimumZoomScale = scale; + self.scrollView.maximumZoomScale = MAX(scale, self.scrollView.maximumZoomScale); // make sure content isn't smaller than the crop box CGSize size = self.scrollView.contentSize; @@ -1555,10 +1557,13 @@ - (void)rotateImageNinetyDegreesAnimated:(BOOL)animated clockwise:(BOOL)clockwis CGPoint cropTargetPoint = (CGPoint){cropMidPoint.x + self.scrollView.contentOffset.x, cropMidPoint.y + self.scrollView.contentOffset.y}; // Work out the dimensions of the crop box when rotated + const CGFloat oldZoomScale = self.scrollView.zoomScale; CGRect newCropFrame = CGRectZero; if (labs(self.angle) == labs(self.cropBoxLastEditedAngle) || (labs(self.angle) * -1) == ((labs(self.cropBoxLastEditedAngle) - 180) % 360)) { newCropFrame.size = self.cropBoxLastEditedSize; + // Raise the ceiling first so restoring the saved zoom is never clamped + self.scrollView.maximumZoomScale = MAX(self.scrollView.maximumZoomScale, self.cropBoxLastEditedZoomScale); self.scrollView.minimumZoomScale = self.cropBoxLastEditedMinZoomScale; self.scrollView.zoomScale = self.cropBoxLastEditedZoomScale; } else { @@ -1566,6 +1571,7 @@ - (void)rotateImageNinetyDegreesAnimated:(BOOL)animated clockwise:(BOOL)clockwis // Re-adjust the scrolling dimensions of the scroll view to match the new size self.scrollView.minimumZoomScale *= scale; + self.scrollView.maximumZoomScale *= scale; self.scrollView.zoomScale *= scale; } @@ -1599,9 +1605,13 @@ - (void)rotateImageNinetyDegreesAnimated:(BOOL)animated clockwise:(BOOL)clockwis [self moveCroppedContentToCenterAnimated:NO]; newCropFrame = self.cropBoxFrame; - // work out how to line up out point of interest into the middle of the crop box - cropTargetPoint.x *= scale; - cropTargetPoint.y *= scale; + // work out how to line up out point of interest into the middle of the crop box. + // Use the zoom delta that was actually applied (including any adjustment made while + // re-centering above), which differs from the geometric scale when a previously + // edited crop state was restored + const CGFloat appliedScale = (oldZoomScale > FLT_EPSILON) ? (self.scrollView.zoomScale / oldZoomScale) : scale; + cropTargetPoint.x *= appliedScale; + cropTargetPoint.y *= appliedScale; // swap the target dimensions to match a 90 degree rotation (clockwise or counterclockwise) CGFloat swap = cropTargetPoint.x; @@ -1620,12 +1630,12 @@ - (void)rotateImageNinetyDegreesAnimated:(BOOL)animated clockwise:(BOOL)clockwis offset.y = floorf(-midPoint.y + cropTargetPoint.y); offset.x = MAX(-self.scrollView.contentInset.left, offset.x); offset.y = MAX(-self.scrollView.contentInset.top, offset.y); - offset.x = MIN(self.scrollView.contentSize.width - (newCropFrame.size.width - self.scrollView.contentInset.right), offset.x); - offset.y = MIN(self.scrollView.contentSize.height - (newCropFrame.size.height - self.scrollView.contentInset.bottom), offset.y); + offset.x = MIN(self.scrollView.contentSize.width - CGRectGetMaxX(newCropFrame), offset.x); + offset.y = MIN(self.scrollView.contentSize.height - CGRectGetMaxY(newCropFrame), offset.y); // if the scroll view's new scale is 1 and the new offset is equal to the old, will not trigger the delegate 'scrollViewDidScroll:' // so we should call the method manually to update the foregroundImageView's frame - if (offset.x == self.scrollView.contentOffset.x && offset.y == self.scrollView.contentOffset.y && scale == 1) { + if (offset.x == self.scrollView.contentOffset.x && offset.y == self.scrollView.contentOffset.y && fabs(appliedScale - 1.0) < FLT_EPSILON) { [self matchForegroundToBackground]; } self.scrollView.contentOffset = offset; diff --git a/Objective-C/TOCropViewControllerTests/TOCropViewControllerTests.m b/Objective-C/TOCropViewControllerTests/TOCropViewControllerTests.m index 52b88f07..827eae1b 100644 --- a/Objective-C/TOCropViewControllerTests/TOCropViewControllerTests.m +++ b/Objective-C/TOCropViewControllerTests/TOCropViewControllerTests.m @@ -62,6 +62,19 @@ - (void)testSetAngleNormalizesFullRotations { XCTAssertEqual(cropView.angle, -90); } +- (void)testNinetyDegreeRotationRoundTrip { + TOCropView *cropView = [self cropViewWithImageSize:(CGSize){40, 20}]; + for (NSInteger i = 0; i < 4; i++) { + [cropView rotateImageNinetyDegreesAnimated:NO clockwise:YES completion:nil]; + } + XCTAssertEqual(cropView.angle, 0); + + // A full revolution should land back on the whole image + CGRect cropFrame = cropView.imageCropFrame; + XCTAssertEqualWithAccuracy(cropFrame.size.width, 40.0, 2.0); + XCTAssertEqualWithAccuracy(cropFrame.size.height, 20.0, 2.0); +} + - (void)testZeroSizeImageDoesNotCrash { TOCropView *cropView = [[TOCropView alloc] initWithImage:[UIImage new]]; cropView.frame = (CGRect){0, 0, 320, 480}; From 049de12e7fc4b520df9647c7fd6dbf795396036c Mon Sep 17 00:00:00 2001 From: Tim Oliver Date: Sun, 12 Jul 2026 02:10:24 +0900 Subject: [PATCH 3/5] Clamp the reported crop rect to the image bounds imageCropFrame floored its origin and ceiled its size independently, so an edge-flush crop at a fractional zoom could overhang the image by a point, which rendered as a black hairline on opaque exports. --- Objective-C/TOCropViewController/Views/TOCropView.m | 6 +++--- .../TOCropViewControllerTests.m | 10 ++++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Objective-C/TOCropViewController/Views/TOCropView.m b/Objective-C/TOCropViewController/Views/TOCropView.m index 1eed72f4..5ee5af66 100644 --- a/Objective-C/TOCropViewController/Views/TOCropView.m +++ b/Objective-C/TOCropViewController/Views/TOCropView.m @@ -1049,9 +1049,9 @@ - (CGRect)imageCropFrame { frame.origin.y = floorf((floorf(contentOffset.y) + edgeInsets.top) * (imageSize.height / contentSize.height)); frame.origin.y = MAX(0, frame.origin.y); - // Calculate the normalized width + // Calculate the normalized width, clamped so the rect never extends past the image frame.size.width = ceilf(cropBoxFrame.size.width * scale); - frame.size.width = MIN(imageSize.width, frame.size.width); + frame.size.width = MIN(imageSize.width - frame.origin.x, frame.size.width); // Calculate normalized height if (floor(cropBoxFrame.size.width) == floor(cropBoxFrame.size.height)) { @@ -1059,7 +1059,7 @@ - (CGRect)imageCropFrame { } else { frame.size.height = ceilf(cropBoxFrame.size.height * scale); } - frame.size.height = MIN(imageSize.height, frame.size.height); + frame.size.height = MIN(imageSize.height - frame.origin.y, frame.size.height); return frame; } diff --git a/Objective-C/TOCropViewControllerTests/TOCropViewControllerTests.m b/Objective-C/TOCropViewControllerTests/TOCropViewControllerTests.m index 827eae1b..c1952682 100644 --- a/Objective-C/TOCropViewControllerTests/TOCropViewControllerTests.m +++ b/Objective-C/TOCropViewControllerTests/TOCropViewControllerTests.m @@ -91,6 +91,16 @@ - (void)testDegenerateAspectRatioIsIgnored { XCTAssertEqualWithAccuracy(before.size.width, after.size.width, 1.0); } +- (void)testImageCropFrameStaysWithinImageBounds { + TOCropView *cropView = [self cropViewWithImageSize:(CGSize){403, 301}]; + for (NSInteger x = 150; x <= 203; x += 13) { + [cropView setImageCropFrame:(CGRect){x, 51, 403 - x, 250}]; + CGRect frame = cropView.imageCropFrame; + XCTAssertLessThanOrEqual(CGRectGetMaxX(frame), 403.0); + XCTAssertLessThanOrEqual(CGRectGetMaxY(frame), 301.0); + } +} + - (void)testViewControllerInstance { // Create a basic image UIGraphicsImageRenderer *renderer = [[UIGraphicsImageRenderer alloc] initWithSize:CGSizeMake(10, 10)]; From 91f3a82bd38fa9678a479d2a59390312d6e1583f Mon Sep 17 00:00:00 2001 From: Tim Oliver Date: Mon, 27 Jul 2026 02:17:12 +0900 Subject: [PATCH 4/5] Address review findings in the rotation and zoom geometry fixes - The scroll view's zoom ceiling is now derived in one place from a new baseMaximumZoomScale that is captured and restored alongside the rest of the rotation state, replacing the ad-hoc MAX() guards that let the configured maximumZoomScale drift after repeated rotations or crop box edits - setAngle: bails out instead of spinning forever when a rotation makes no progress, and the two identical loops are collapsed into one - imageCropFrame clamps its origin inside the image so the size can't go negative during rubber-band overscroll - rotateImageNinetyDegreesAnimated:clockwise:completion: reports back on the un-animated path and when the call is dropped, rather than dropping the handler - -_willRotateToInterfaceOrientation: no longer passes an inverted vertical layout flag when sizing the crop view --- .../TOCropViewController.m | 2 +- .../TOCropViewController/Views/TOCropView.h | 4 +- .../TOCropViewController/Views/TOCropView.m | 70 ++++++++++++++----- .../TOCropViewControllerTests.m | 13 ++++ 4 files changed, 68 insertions(+), 21 deletions(-) diff --git a/Objective-C/TOCropViewController/TOCropViewController.m b/Objective-C/TOCropViewController/TOCropViewController.m index cde5b1ed..c00e47d4 100644 --- a/Objective-C/TOCropViewController/TOCropViewController.m +++ b/Objective-C/TOCropViewController/TOCropViewController.m @@ -554,7 +554,7 @@ - (void)_willRotateToInterfaceOrientation:(UIInterfaceOrientation)toInterfaceOri self.toolbar.alpha = 0.0f; [self.cropView prepareforRotation]; - self.cropView.frame = [self frameForCropViewWithVerticalLayout:!UIInterfaceOrientationIsPortrait(toInterfaceOrientation)]; + self.cropView.frame = [self frameForCropViewWithVerticalLayout:UIInterfaceOrientationIsPortrait(toInterfaceOrientation)]; self.cropView.simpleRenderMode = YES; self.cropView.internalLayoutDisabled = YES; } diff --git a/Objective-C/TOCropViewController/Views/TOCropView.h b/Objective-C/TOCropViewController/Views/TOCropView.h index 497e7e12..38690e42 100644 --- a/Objective-C/TOCropViewController/Views/TOCropView.h +++ b/Objective-C/TOCropViewController/Views/TOCropView.h @@ -140,7 +140,9 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, readonly) BOOL cropBoxAspectRatioIsPortrait; /** - The rotation angle of the crop view (Will always be negative as it rotates in a counter-clockwise direction) + The rotation angle of the crop view, in multiples of 90 degrees. Counter-clockwise + rotations are negative and clockwise ones positive, so the value is in the + range (-360, 360). Values that aren't a multiple of 90 are treated as 0. */ @property (nonatomic, assign) NSInteger angle; diff --git a/Objective-C/TOCropViewController/Views/TOCropView.m b/Objective-C/TOCropViewController/Views/TOCropView.m index 5ee5af66..ce1476e6 100644 --- a/Objective-C/TOCropViewController/Views/TOCropView.m +++ b/Objective-C/TOCropViewController/Views/TOCropView.m @@ -90,6 +90,8 @@ @interface TOCropView () @property (nonatomic, assign) NSInteger cropBoxLastEditedAngle; /* Remember which angle we were at when we saved the editing size */ @property (nonatomic, assign) CGFloat cropBoxLastEditedZoomScale; /* Remember the zoom size when we last edited */ @property (nonatomic, assign) CGFloat cropBoxLastEditedMinZoomScale; /* Remember the minimum size when we last edited. */ +@property (nonatomic, assign) CGFloat cropBoxLastEditedMaxZoomScale; /* Remember the zoom ceiling when we last edited. */ +@property (nonatomic, assign) CGFloat baseMaximumZoomScale; /* The absolute zoom ceiling for the current layout; the scroll view's own maximum is always derived from this */ @property (nonatomic, assign) BOOL rotateAnimationInProgress; /* Disallow any input while the rotation animation is playing */ /* Reset state data */ @@ -299,7 +301,8 @@ - (void)layoutInitialImage { // Configure the scroll view self.scrollView.minimumZoomScale = scale; - self.scrollView.maximumZoomScale = scale * self.maximumZoomScale; + self.baseMaximumZoomScale = scale * self.maximumZoomScale; + [self updateScrollViewMaximumZoomScale]; // Set the crop box to the size we calculated and align in the middle of the screen CGRect frame = CGRectZero; @@ -345,7 +348,8 @@ - (void)performRelayoutForRotation { CGFloat scale = MIN(contentFrame.size.width / cropFrame.size.width, contentFrame.size.height / cropFrame.size.height); self.scrollView.minimumZoomScale *= scale; - self.scrollView.maximumZoomScale *= scale; + self.baseMaximumZoomScale *= scale; + [self updateScrollViewMaximumZoomScale]; self.scrollView.zoomScale *= scale; // Work out the centered, upscaled version of the crop rectangle @@ -757,7 +761,10 @@ - (void)updateToImageCropFrame:(CGRect)imageCropframe { CGRect bounds = self.contentBounds; CGFloat scale = MIN(bounds.size.width / scaledCropSize.width, bounds.size.height / scaledCropSize.height); - // Zoom into the scroll view to the appropriate size + // Zoom into the scroll view to the appropriate size, transiently raising the + // ceiling if the restored crop requires more zoom than is normally allowed + // (the next layout-driven update re-derives the ceiling from the base value) + self.scrollView.maximumZoomScale = MAX(self.scrollView.maximumZoomScale, self.scrollView.minimumZoomScale * scale); self.scrollView.zoomScale = self.scrollView.minimumZoomScale * scale; CGSize contentSize = self.scrollView.contentSize; @@ -928,6 +935,7 @@ - (void)scrollViewDidZoom:(UIScrollView *)scrollView { if (scrollView.isTracking) { self.cropBoxLastEditedZoomScale = scrollView.zoomScale; self.cropBoxLastEditedMinZoomScale = scrollView.minimumZoomScale; + self.cropBoxLastEditedMaxZoomScale = self.baseMaximumZoomScale; } [self matchForegroundToBackground]; @@ -1005,7 +1013,7 @@ - (void)setCropBoxFrame:(CGRect)cropBoxFrame { CGSize imageSize = self.backgroundContainerView.bounds.size; CGFloat scale = MAX(cropBoxFrame.size.height / imageSize.height, cropBoxFrame.size.width / imageSize.width); self.scrollView.minimumZoomScale = scale; - self.scrollView.maximumZoomScale = MAX(scale, self.scrollView.maximumZoomScale); + [self updateScrollViewMaximumZoomScale]; // make sure content isn't smaller than the crop box CGSize size = self.scrollView.contentSize; @@ -1042,12 +1050,13 @@ - (CGRect)imageCropFrame { CGRect frame = CGRectZero; - // Calculate the normalized origin + // Calculate the normalized origin, clamped inside the image so the size + // subtraction below can never go negative (eg, during rubber-band overscroll) frame.origin.x = floorf((floorf(contentOffset.x) + edgeInsets.left) * (imageSize.width / contentSize.width)); - frame.origin.x = MAX(0, frame.origin.x); + frame.origin.x = MAX(0, MIN(frame.origin.x, imageSize.width)); frame.origin.y = floorf((floorf(contentOffset.y) + edgeInsets.top) * (imageSize.height / contentSize.height)); - frame.origin.y = MAX(0, frame.origin.y); + frame.origin.y = MAX(0, MIN(frame.origin.y, imageSize.height)); // Calculate the normalized width, clamped so the rect never extends past the image frame.size.width = ceilf(cropBoxFrame.size.width * scale); @@ -1206,13 +1215,15 @@ - (void)setAngle:(NSInteger)angle { // Negative values are allowed, so rotate clockwise or counter clockwise depending // on direction. Compare the signed values since +90 and -90 are distinct states. - if (newAngle >= 0) { - while (self.angle != newAngle) { - [self rotateImageNinetyDegreesAnimated:NO clockwise:YES completion:nil]; - } - } else { - while (self.angle != newAngle) { - [self rotateImageNinetyDegreesAnimated:NO clockwise:NO completion:nil]; + const BOOL clockwise = (newAngle >= 0); + while (self.angle != newAngle) { + const NSInteger previousAngle = self.angle; + [self rotateImageNinetyDegreesAnimated:NO clockwise:clockwise completion:nil]; + + // Bail out if no rotation was performed (eg, an animated rotation is + // still in flight) rather than spinning forever on the main thread + if (self.angle == previousAngle) { + break; } } } @@ -1497,9 +1508,15 @@ - (void)rotateImageNinetyDegreesAnimated:(BOOL)animated completion:(void (^)(BOO } - (void)rotateImageNinetyDegreesAnimated:(BOOL)animated clockwise:(BOOL)clockwise completion:(void (^)(BOOL completed))completionHandler { - // Only allow one rotation animation at a time - if (self.rotateAnimationInProgress) + // Only allow one rotation animation at a time. Report the dropped call rather + // than swallowing the handler, or callers that gate UI on it (such as the + // toolbar's own rotation buttons) would stay disabled forever. + if (self.rotateAnimationInProgress) { + if (completionHandler) { + completionHandler(NO); + } return; + } // Cancel any pending resizing timers if (self.resetTimer) { @@ -1562,16 +1579,18 @@ - (void)rotateImageNinetyDegreesAnimated:(BOOL)animated clockwise:(BOOL)clockwis if (labs(self.angle) == labs(self.cropBoxLastEditedAngle) || (labs(self.angle) * -1) == ((labs(self.cropBoxLastEditedAngle) - 180) % 360)) { newCropFrame.size = self.cropBoxLastEditedSize; - // Raise the ceiling first so restoring the saved zoom is never clamped - self.scrollView.maximumZoomScale = MAX(self.scrollView.maximumZoomScale, self.cropBoxLastEditedZoomScale); + // Restore the full zoom state, ceiling first so the saved zoom isn't clamped + self.baseMaximumZoomScale = self.cropBoxLastEditedMaxZoomScale; self.scrollView.minimumZoomScale = self.cropBoxLastEditedMinZoomScale; + [self updateScrollViewMaximumZoomScale]; self.scrollView.zoomScale = self.cropBoxLastEditedZoomScale; } else { newCropFrame.size = (CGSize){floorf(self.cropBoxFrame.size.height * scale), floorf(self.cropBoxFrame.size.width * scale)}; // Re-adjust the scrolling dimensions of the scroll view to match the new size self.scrollView.minimumZoomScale *= scale; - self.scrollView.maximumZoomScale *= scale; + self.baseMaximumZoomScale *= scale; + [self updateScrollViewMaximumZoomScale]; self.scrollView.zoomScale *= scale; } @@ -1701,12 +1720,25 @@ - (void)rotateImageNinetyDegreesAnimated:(BOOL)animated clockwise:(BOOL)clockwis } [self checkForCanReset]; + + // The un-animated path has already finished by this point, so there's no + // animation completion block to defer the handler to + if (!animated && completionHandler) { + completionHandler(YES); + } } - (void)captureStateForImageRotation { self.cropBoxLastEditedSize = self.cropBoxFrame.size; self.cropBoxLastEditedZoomScale = self.scrollView.zoomScale; self.cropBoxLastEditedMinZoomScale = self.scrollView.minimumZoomScale; + self.cropBoxLastEditedMaxZoomScale = self.baseMaximumZoomScale; +} + +// The scroll view's zoom ceiling normally sits at the layout's absolute base value, +// but must never fall below the minimum or UIScrollView clamps the zoom beneath it +- (void)updateScrollViewMaximumZoomScale { + self.scrollView.maximumZoomScale = MAX(self.scrollView.minimumZoomScale, self.baseMaximumZoomScale); } #pragma mark - Resettable State - diff --git a/Objective-C/TOCropViewControllerTests/TOCropViewControllerTests.m b/Objective-C/TOCropViewControllerTests/TOCropViewControllerTests.m index c1952682..3120a628 100644 --- a/Objective-C/TOCropViewControllerTests/TOCropViewControllerTests.m +++ b/Objective-C/TOCropViewControllerTests/TOCropViewControllerTests.m @@ -11,6 +11,11 @@ #import "TOCropViewController.h" +// Expose private state so the test can simulate an in-flight rotation +@interface TOCropView (UnitTests) +@property (nonatomic, assign) BOOL rotateAnimationInProgress; +@end + @interface TOCropViewControllerTests : XCTestCase @end @@ -75,6 +80,14 @@ - (void)testNinetyDegreeRotationRoundTrip { XCTAssertEqualWithAccuracy(cropFrame.size.height, 20.0, 2.0); } +- (void)testSetAngleBailsOutDuringAnimatedRotation { + TOCropView *cropView = [self cropViewWithImageSize:(CGSize){40, 20}]; + cropView.rotateAnimationInProgress = YES; + cropView.angle = 90; // spun forever before the no-progress guard + XCTAssertEqual(cropView.angle, 0); + cropView.rotateAnimationInProgress = NO; +} + - (void)testZeroSizeImageDoesNotCrash { TOCropView *cropView = [[TOCropView alloc] initWithImage:[UIImage new]]; cropView.frame = (CGRect){0, 0, 320, 480}; From bd02ef89b66bf46adce514fb7447fb5854a195ae Mon Sep 17 00:00:00 2001 From: Tim Oliver Date: Tue, 28 Jul 2026 00:39:49 +0900 Subject: [PATCH 5/5] Note the rotation and zoom geometry fixes in the changelog --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f80d5320..35df2759 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ x.y.z Release Notes (yyyy-MM-dd) ## Fixed - The Swift `CropViewController` framework failing to build under Xcode 26's module verifier. +- Setting `angle` to ±360 or beyond hung the app in an infinite loop, and direction changes such as 90° to -90° were silently ignored. +- The `gridOverlayHidden` property setter had no effect, and its animated variant ignored its animation flag. +- Content offset math when rotating 90° with a previously edited crop state, which could shift the view away from the focal region and leave blank space inside the crop box. +- The reported crop rectangle could overhang the image bounds by a point, exporting a black hairline column on opaque images. +- Zero-sized images and aspect ratios with a zero component no longer produce broken layout geometry. +- `rotateImageNinetyDegreesAnimated:clockwise:completion:` never called its completion handler when `animated` was NO, or when the call was dropped because a rotation was already playing. The latter could leave the toolbar's rotation buttons permanently disabled. +- The crop view was briefly assigned the frame for the opposite layout at the start of a device rotation, so the rotation animation interpolated from the wrong geometry (iOS 25 and below). 3.1.2 Release Notes (2026-04-07)