Skip to content

Commit 3ee1ad2

Browse files
bryankellerbryankeller
authored
Reduce index path lookups (#152)
* Reduce index path lookups * Update tests
1 parent 013e301 commit 3ee1ad2

File tree

4 files changed

+60
-29
lines changed

4 files changed

+60
-29
lines changed

‎MagazineLayout/LayoutCore/LayoutState.swift‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ struct LayoutState {
124124
let distanceFromTop = firstVisibleItemLocationFramePair.frame.minY - top
125125
return .topItem(
126126
id: firstVisibleItemID,
127+
elementLocation: firstVisibleItemLocationFramePair.elementLocation,
127128
distanceFromTop: distanceFromTop.alignedToPixel(forScreenWithScale: scale))
128129
}
129130
case .bottomToTop:
@@ -133,31 +134,48 @@ struct LayoutState {
133134
let distanceFromBottom = lastVisibleItemLocationFramePair.frame.maxY - bottom
134135
return .bottomItem(
135136
id: lastVisibleItemID,
137+
elementLocation: lastVisibleItemLocationFramePair.elementLocation,
136138
distanceFromBottom: distanceFromBottom.alignedToPixel(forScreenWithScale: scale))
137139
case .atBottom:
138140
return .bottom
139141
}
140142
}
141143
}
142144

143-
func yOffset(for targetContentOffsetAnchor: TargetContentOffsetAnchor) -> CGFloat {
145+
func yOffset(
146+
for targetContentOffsetAnchor: TargetContentOffsetAnchor,
147+
isPerformingBatchUpdates: Bool)
148+
-> CGFloat
149+
{
144150
switch targetContentOffsetAnchor {
145151
case .top:
146152
return minContentOffset.y
147153

148154
case .bottom:
149155
return maxContentOffset.y
150156

151-
case .topItem(let id, let distanceFromTop):
152-
guard let indexPath = modelState.indexPathForItemModel(withID: id) else { return bounds.minY }
153-
let itemFrame = modelState.frameForItem(at: ElementLocation(indexPath: indexPath))
157+
case .topItem(let id, let _elementLocation, let distanceFromTop):
158+
let elementLocation =
159+
if isPerformingBatchUpdates {
160+
modelState.indexPathForItemModel(withID: id).map(ElementLocation.init(indexPath:))
161+
} else {
162+
_elementLocation
163+
}
164+
guard let elementLocation else { return bounds.minY }
165+
let itemFrame = modelState.frameForItem(at: elementLocation)
154166
let proposedYOffset = itemFrame.minY - contentInset.top - distanceFromTop
155167
// Clamp between minYOffset...maxYOffset
156168
return min(max(proposedYOffset, minContentOffset.y), maxContentOffset.y)
157169

158-
case .bottomItem(let id, let distanceFromBottom):
159-
guard let indexPath = modelState.indexPathForItemModel(withID: id) else { return bounds.minY }
160-
let itemFrame = modelState.frameForItem(at: ElementLocation(indexPath: indexPath))
170+
case .bottomItem(let id, let _elementLocation, let distanceFromBottom):
171+
let elementLocation =
172+
if isPerformingBatchUpdates {
173+
modelState.indexPathForItemModel(withID: id).map(ElementLocation.init(indexPath:))
174+
} else {
175+
_elementLocation
176+
}
177+
guard let elementLocation else { return bounds.minY }
178+
let itemFrame = modelState.frameForItem(at: elementLocation)
161179
let proposedYOffset = itemFrame.maxY - bounds.height + contentInset.bottom - distanceFromBottom
162180
// Clamp between minYOffset...maxYOffset
163181
return min(max(proposedYOffset, minContentOffset.y), maxContentOffset.y)

‎MagazineLayout/LayoutCore/Types/TargetContentOffsetAnchor.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,6 @@ import UIKit
2121
enum TargetContentOffsetAnchor: Equatable {
2222
case top
2323
case bottom
24-
case topItem(id: UInt64, distanceFromTop: CGFloat)
25-
case bottomItem(id: UInt64, distanceFromBottom: CGFloat)
24+
case topItem(id: UInt64, elementLocation: ElementLocation, distanceFromTop: CGFloat)
25+
case bottomItem(id: UInt64, elementLocation: ElementLocation, distanceFromBottom: CGFloat)
2626
}

‎MagazineLayout/Public/MagazineLayout.swift‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,9 @@ public final class MagazineLayout: UICollectionViewLayout {
227227

228228
if let layoutStateBeforeCollectionViewUpdates{
229229
let targetContentOffsetAnchor = layoutStateBeforeCollectionViewUpdates.targetContentOffsetAnchor
230-
let targetYOffset = layoutState.yOffset(for: targetContentOffsetAnchor)
230+
let targetYOffset = layoutState.yOffset(
231+
for: targetContentOffsetAnchor,
232+
isPerformingBatchUpdates: true)
231233
let context = MagazineLayoutInvalidationContext()
232234
context.invalidateLayoutMetrics = false
233235
context.contentOffsetAdjustment.y = targetYOffset - layoutState.bounds.minY
@@ -676,7 +678,9 @@ public final class MagazineLayout: UICollectionViewLayout {
676678
layoutStateBeforeAnimatedBoundsChange ??
677679
self.layoutState
678680
).targetContentOffsetAnchor
679-
let targetYOffsetBefore = layoutState.yOffset(for: targetContentOffsetAnchor)
681+
let targetYOffsetBefore = layoutState.yOffset(
682+
for: targetContentOffsetAnchor,
683+
isPerformingBatchUpdates: layoutStateBeforeCollectionViewUpdates != nil)
680684

681685
modelState.updateItemHeight(
682686
toPreferredHeight: preferredAttributes.size.height,
@@ -690,7 +694,9 @@ public final class MagazineLayout: UICollectionViewLayout {
690694
context.contentOffsetAdjustment.y = layoutState.maxContentOffset.y - layoutState.bounds.minY
691695

692696
case .topItem, .bottomItem:
693-
let targetYOffsetAfter = layoutState.yOffset(for: targetContentOffsetAnchor)
697+
let targetYOffsetAfter = layoutState.yOffset(
698+
for: targetContentOffsetAnchor,
699+
isPerformingBatchUpdates: layoutStateBeforeCollectionViewUpdates != nil)
694700
context.contentOffsetAdjustment.y = targetYOffsetAfter - targetYOffsetBefore
695701
}
696702

@@ -817,7 +823,9 @@ public final class MagazineLayout: UICollectionViewLayout {
817823
return super.targetContentOffset(forProposedContentOffset: proposedContentOffset)
818824
}
819825

820-
let yOffset = layoutState.yOffset(for: layoutStateBefore.targetContentOffsetAnchor)
826+
let yOffset = layoutState.yOffset(
827+
for: layoutStateBefore.targetContentOffsetAnchor,
828+
isPerformingBatchUpdates: layoutStateBeforeCollectionViewUpdates != nil)
821829

822830
targetContentOffsetCompensatingYOffsetForAppearingItem = proposedContentOffset.y - yOffset
823831

‎Tests/LayoutStateTargetContentOffsetTests.swift‎

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,9 @@ final class LayoutStateTargetContentOffsetTests: XCTestCase {
4040
contentInset: UIEdgeInsets(top: 50, left: 0, bottom: 30, right: 0),
4141
scale: 1,
4242
verticalLayoutDirection: .topToBottom)
43-
let id = layoutState.modelState.idForItemModel(at: IndexPath(item: 6, section: 0))!
44-
XCTAssert(layoutState.targetContentOffsetAnchor == .topItem(id: id, distanceFromTop: -25))
43+
let indexPath = IndexPath(item: 6, section: 0)
44+
let id = layoutState.modelState.idForItemModel(at: indexPath)!
45+
XCTAssert(layoutState.targetContentOffsetAnchor == .topItem(id: id, elementLocation: ElementLocation(indexPath: indexPath), distanceFromTop: -25))
4546
}
4647

4748
func testAnchor_TopToBottom_ScrolledToBottom() throws {
@@ -61,8 +62,9 @@ final class LayoutStateTargetContentOffsetTests: XCTestCase {
6162
contentInset: measurementLayoutState.contentInset,
6263
scale: measurementLayoutState.scale,
6364
verticalLayoutDirection: measurementLayoutState.verticalLayoutDirection)
64-
let id = layoutState.modelState.idForItemModel(at: IndexPath(item: 9, section: 0))!
65-
XCTAssert(layoutState.targetContentOffsetAnchor == .topItem(id: id, distanceFromTop: 25))
65+
let indexPath = IndexPath(item: 9, section: 0)
66+
let id = layoutState.modelState.idForItemModel(at: indexPath)!
67+
XCTAssert(layoutState.targetContentOffsetAnchor == .topItem(id: id, elementLocation: ElementLocation(indexPath: indexPath), distanceFromTop: 25))
6668
}
6769

6870
func testAnchor_TopToBottom_NoFullyVisibleCells_UsesFallback() throws {
@@ -78,8 +80,9 @@ final class LayoutStateTargetContentOffsetTests: XCTestCase {
7880

7981
// Since no items are fully visible, the fallback should use the first partially visible item
8082
// instead of returning .top or .bottom
81-
let id = layoutState.modelState.idForItemModel(at: IndexPath(item: 0, section: 0))!
82-
XCTAssert(layoutState.targetContentOffsetAnchor == .topItem(id: id, distanceFromTop: -300))
83+
let indexPath = IndexPath(item: 0, section: 0)
84+
let id = layoutState.modelState.idForItemModel(at: indexPath)!
85+
XCTAssert(layoutState.targetContentOffsetAnchor == .topItem(id: id, elementLocation: ElementLocation(indexPath: indexPath), distanceFromTop: -300))
8386
}
8487

8588
// MARK: Bottom-to-Top Anchor Tests
@@ -92,8 +95,9 @@ final class LayoutStateTargetContentOffsetTests: XCTestCase {
9295
contentInset: UIEdgeInsets(top: 50, left: 0, bottom: 30, right: 0),
9396
scale: 1,
9497
verticalLayoutDirection: .bottomToTop)
95-
let id = layoutState.modelState.idForItemModel(at: IndexPath(item: 3, section: 0))!
96-
XCTAssert(layoutState.targetContentOffsetAnchor == .bottomItem(id: id, distanceFromBottom: -90))
98+
let indexPath = IndexPath(item: 3, section: 0)
99+
let id = layoutState.modelState.idForItemModel(at: indexPath)!
100+
XCTAssert(layoutState.targetContentOffsetAnchor == .bottomItem(id: id, elementLocation: ElementLocation(indexPath: indexPath), distanceFromBottom: -90))
97101
}
98102

99103
func testAnchor_BottomToTop_ScrolledToMiddle() throws {
@@ -104,8 +108,9 @@ final class LayoutStateTargetContentOffsetTests: XCTestCase {
104108
contentInset: UIEdgeInsets(top: 50, left: 0, bottom: 30, right: 0),
105109
scale: 1,
106110
verticalLayoutDirection: .bottomToTop)
107-
let id = layoutState.modelState.idForItemModel(at: IndexPath(item: 10, section: 0))!
108-
XCTAssert(layoutState.targetContentOffsetAnchor == .bottomItem(id: id, distanceFromBottom: -10))
111+
let indexPath = IndexPath(item: 10, section: 0)
112+
let id = layoutState.modelState.idForItemModel(at: indexPath)!
113+
XCTAssert(layoutState.targetContentOffsetAnchor == .bottomItem(id: id, elementLocation: ElementLocation(indexPath: indexPath), distanceFromBottom: -10))
109114
}
110115

111116
func testAnchor_BottomToTop_ScrolledToBottom() throws {
@@ -139,7 +144,7 @@ final class LayoutStateTargetContentOffsetTests: XCTestCase {
139144
scale: 1,
140145
verticalLayoutDirection: .topToBottom)
141146
let targetContentOffsetAnchor = layoutState.targetContentOffsetAnchor
142-
XCTAssert(layoutState.yOffset(for: targetContentOffsetAnchor) == -50)
147+
XCTAssert(layoutState.yOffset(for: targetContentOffsetAnchor, isPerformingBatchUpdates: false) == -50)
143148
}
144149

145150
func testOffset_TopToBottom_ScrolledToMiddle() {
@@ -151,7 +156,7 @@ final class LayoutStateTargetContentOffsetTests: XCTestCase {
151156
scale: 1,
152157
verticalLayoutDirection: .topToBottom)
153158
let targetContentOffsetAnchor = layoutState.targetContentOffsetAnchor
154-
XCTAssert(layoutState.yOffset(for: targetContentOffsetAnchor) == 500)
159+
XCTAssert(layoutState.yOffset(for: targetContentOffsetAnchor, isPerformingBatchUpdates: false) == 500)
155160
}
156161

157162
func testOffset_TopToBottom_ScrolledToBottom() {
@@ -172,7 +177,7 @@ final class LayoutStateTargetContentOffsetTests: XCTestCase {
172177
scale: measurementLayoutState.scale,
173178
verticalLayoutDirection: measurementLayoutState.verticalLayoutDirection)
174179
let targetContentOffsetAnchor = layoutState.targetContentOffsetAnchor
175-
XCTAssert(layoutState.yOffset(for: targetContentOffsetAnchor) == 690)
180+
XCTAssert(layoutState.yOffset(for: targetContentOffsetAnchor, isPerformingBatchUpdates: false) == 690)
176181
}
177182

178183
// MARK: Bottom-to-Top Target Content Offset Tests
@@ -186,7 +191,7 @@ final class LayoutStateTargetContentOffsetTests: XCTestCase {
186191
scale: 1,
187192
verticalLayoutDirection: .bottomToTop)
188193
let targetContentOffsetAnchor = layoutState.targetContentOffsetAnchor
189-
XCTAssert(layoutState.yOffset(for: targetContentOffsetAnchor) == -50)
194+
XCTAssert(layoutState.yOffset(for: targetContentOffsetAnchor, isPerformingBatchUpdates: false) == -50)
190195
}
191196

192197
func testOffset_BottomToTop_ScrolledToMiddle() {
@@ -198,7 +203,7 @@ final class LayoutStateTargetContentOffsetTests: XCTestCase {
198203
scale: 1,
199204
verticalLayoutDirection: .bottomToTop)
200205
let targetContentOffsetAnchor = layoutState.targetContentOffsetAnchor
201-
XCTAssert(layoutState.yOffset(for: targetContentOffsetAnchor) == 500)
206+
XCTAssert(layoutState.yOffset(for: targetContentOffsetAnchor, isPerformingBatchUpdates: false) == 500)
202207
}
203208

204209
func testOffset_BottomToTop_ScrolledToBottom() {
@@ -219,7 +224,7 @@ final class LayoutStateTargetContentOffsetTests: XCTestCase {
219224
scale: measurementLayoutState.scale,
220225
verticalLayoutDirection: measurementLayoutState.verticalLayoutDirection)
221226
let targetContentOffsetAnchor = layoutState.targetContentOffsetAnchor
222-
XCTAssert(layoutState.yOffset(for: targetContentOffsetAnchor) == 690)
227+
XCTAssert(layoutState.yOffset(for: targetContentOffsetAnchor, isPerformingBatchUpdates: false) == 690)
223228
}
224229

225230
// MARK: Private

0 commit comments

Comments
 (0)