From f9b27b7768c2b23ebb3ace50c7aad5fc2f3b4fc6 Mon Sep 17 00:00:00 2001 From: Bryan Keller Date: Sat, 30 Mar 2024 15:15:33 -0700 Subject: [PATCH] Pixel aligned CGFloat equality fix (#128) --- .../Types/ScreenPixelAlignment.swift | 10 ++++--- MagazineLayout/Public/MagazineLayout.swift | 27 +++++++++---------- Tests/ScreenPixelAlignmentTests.swift | 16 ++++------- 3 files changed, 24 insertions(+), 29 deletions(-) diff --git a/MagazineLayout/LayoutCore/Types/ScreenPixelAlignment.swift b/MagazineLayout/LayoutCore/Types/ScreenPixelAlignment.swift index 95ecd16..7bc634e 100644 --- a/MagazineLayout/LayoutCore/Types/ScreenPixelAlignment.swift +++ b/MagazineLayout/LayoutCore/Types/ScreenPixelAlignment.swift @@ -22,10 +22,12 @@ extension CGFloat { (self * scale).rounded() / scale } - /// Tests `self` for approximate equality using the threshold value. For example, 1.48 equals 1.52 if the threshold is 0.05. - /// `threshold` will be treated as a positive value by taking its absolute value. - func isEqual(to rhs: CGFloat, threshold: CGFloat) -> Bool { - abs(self - rhs) <= abs(threshold) + /// Tests `self` for approximate equality, first rounding the operands to be pixel-aligned for a screen with the given + /// `screenScale`. For example, 1.48 equals 1.52 if the `screenScale` is `2`. + func isEqual(to rhs: CGFloat, screenScale: CGFloat) -> Bool { + let lhs = alignedToPixel(forScreenWithScale: screenScale) + let rhs = rhs.alignedToPixel(forScreenWithScale: screenScale) + return lhs == rhs } } diff --git a/MagazineLayout/Public/MagazineLayout.swift b/MagazineLayout/Public/MagazineLayout.swift index c113147..bc6c25b 100755 --- a/MagazineLayout/Public/MagazineLayout.swift +++ b/MagazineLayout/Public/MagazineLayout.swift @@ -584,22 +584,21 @@ public final class MagazineLayout: UICollectionViewLayout { } override public func shouldInvalidateLayout(forBoundsChange newBounds: CGRect) -> Bool { - // When using the topToBottom layout direction, we only want to invalidate the layout when the - // widths differ. When using the bottomToTop layout direction, we want to invalidate on any - // size change due to the requirement of needing to preserve scroll position from the bottom + let isSameWidth = currentCollectionView.bounds.size.width.isEqual( + to: newBounds.size.width, + screenScale: scale) let shouldInvalidateDueToSize: Bool switch verticalLayoutDirection { case .topToBottom: - shouldInvalidateDueToSize = !currentCollectionView.bounds.size.width.isEqual( - to: newBounds.size.width, - threshold: 1 / scale) + shouldInvalidateDueToSize = !isSameWidth case .bottomToTop: - shouldInvalidateDueToSize = !(currentCollectionView.bounds.size.width.isEqual( - to: newBounds.size.width, - threshold: 1 / scale) && - currentCollectionView.bounds.size.height.isEqual( + // When using the topToBottom layout direction, we only want to invalidate the layout when the + // widths differ. When using the bottomToTop layout direction, we want to invalidate on any + // size change due to the requirement of needing to preserve scroll position from the bottom + let isSameHeight = currentCollectionView.bounds.size.height.isEqual( to: newBounds.size.height, - threshold: 1 / scale)) + screenScale: scale) + shouldInvalidateDueToSize = !isSameWidth || !isSameHeight } return shouldInvalidateDueToSize || hasPinnedHeaderOrFooter @@ -638,7 +637,7 @@ public final class MagazineLayout: UICollectionViewLayout { let isSameHeight = preferredAttributes.size.height.isEqual( to: originalAttributes.size.height, - threshold: 1 / scale) + screenScale: scale) let hasNewPreferredHeight = !isSameHeight switch (preferredAttributes.representedElementCategory, preferredAttributes.representedElementKind) { @@ -655,7 +654,7 @@ public final class MagazineLayout: UICollectionViewLayout { at: preferredAttributes.indexPath) let isSameHeight = preferredAttributes.size.height.isEqual( to: currentPreferredHeight ?? -.greatestFiniteMagnitude, - threshold: 1 / scale) + screenScale: scale) return hasNewPreferredHeight && !isSameHeight case nil: return false @@ -789,7 +788,7 @@ public final class MagazineLayout: UICollectionViewLayout { // because the collection view's width can change without a `contentSizeAdjustment` occurring. let isSameWidth = collectionView?.bounds.size.width.isEqual( to: cachedCollectionViewWidth ?? -.greatestFiniteMagnitude, - threshold: 1 / scale) + screenScale: scale) ?? false if !isSameWidth { prepareActions.formUnion([.updateLayoutMetrics, .cachePreviousWidth]) diff --git a/Tests/ScreenPixelAlignmentTests.swift b/Tests/ScreenPixelAlignmentTests.swift index 349a6c3..6c11974 100644 --- a/Tests/ScreenPixelAlignmentTests.swift +++ b/Tests/ScreenPixelAlignmentTests.swift @@ -62,17 +62,11 @@ final class ScreenPixelAlignmentTests: XCTestCase { // MARK: Approximate equality tests func testApproximateEquality() { - XCTAssert(CGFloat(1.48).isEqual(to: 1.52, threshold: 0.05)) - XCTAssert(!CGFloat(1.48).isEqual(to: 1.53, threshold: 0.05)) - - XCTAssert(CGFloat(1).isEqual(to: 10, threshold: 9)) - XCTAssert(!CGFloat(1).isEqual(to: 11, threshold: 9)) - - XCTAssert(CGFloat(1).isEqual(to: 10, threshold: 9)) - XCTAssert(!CGFloat(1).isEqual(to: 11, threshold: 9)) - - XCTAssert(CGFloat(1.333).isEqual(to: 1.666, threshold: 1 / 3)) - XCTAssert(!CGFloat(1.332).isEqual(to: 1.666, threshold: 1 / 3)) + XCTAssert(CGFloat(1.48).isEqual(to: 1.52, screenScale: 2)) + XCTAssert(!CGFloat(1).isEqual(to: 10, screenScale: 9)) + XCTAssert(!CGFloat(1).isEqual(to: 10, screenScale: 9)) + XCTAssert(!CGFloat(1).isEqual(to: 9, screenScale: 9)) + XCTAssert(!CGFloat(1.333).isEqual(to: 1.666, screenScale: 3)) } }