Skip to content

Reduce index path lookups #152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bryankeller merged 2 commits into from
Dec 4, 2025
Merged

Reduce index path lookups #152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
713ab24
Reduce index path lookups
bryankeller Dec 3, 2025
d9d1675
Update tests
bryankeller Dec 4, 2025
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
32 changes: 25 additions & 7 deletions MagazineLayout/LayoutCore/LayoutState.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ struct LayoutState {
let distanceFromTop = firstVisibleItemLocationFramePair.frame.minY - top
return .topItem(
id: firstVisibleItemID,
elementLocation: firstVisibleItemLocationFramePair.elementLocation,
distanceFromTop: distanceFromTop.alignedToPixel(forScreenWithScale: scale))
}
case .bottomToTop:
Expand All @@ -133,31 +134,48 @@ struct LayoutState {
let distanceFromBottom = lastVisibleItemLocationFramePair.frame.maxY - bottom
return .bottomItem(
id: lastVisibleItemID,
elementLocation: lastVisibleItemLocationFramePair.elementLocation,
distanceFromBottom: distanceFromBottom.alignedToPixel(forScreenWithScale: scale))
case .atBottom:
return .bottom
}
}
}

func yOffset(for targetContentOffsetAnchor: TargetContentOffsetAnchor) -> CGFloat {
func yOffset(
for targetContentOffsetAnchor: TargetContentOffsetAnchor,
isPerformingBatchUpdates: Bool)
-> CGFloat
{
switch targetContentOffsetAnchor {
case .top:
return minContentOffset.y

case .bottom:
return maxContentOffset.y

case .topItem(let id, let distanceFromTop):
guard let indexPath = modelState.indexPathForItemModel(withID: id) else { return bounds.minY }
let itemFrame = modelState.frameForItem(at: ElementLocation(indexPath: indexPath))
case .topItem(let id, let _elementLocation, let distanceFromTop):
let elementLocation =
if isPerformingBatchUpdates {
modelState.indexPathForItemModel(withID: id).map(ElementLocation.init(indexPath:))
Copy link
Contributor Author

@bryankeller bryankeller Dec 3, 2025

Choose a reason for hiding this comment

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

this is the O(n) call we can avoid as long as we're not performing batch updates

} else {
_elementLocation
}
guard let elementLocation else { return bounds.minY }
let itemFrame = modelState.frameForItem(at: elementLocation)
let proposedYOffset = itemFrame.minY - contentInset.top - distanceFromTop
// Clamp between minYOffset...maxYOffset
return min(max(proposedYOffset, minContentOffset.y), maxContentOffset.y)

case .bottomItem(let id, let distanceFromBottom):
guard let indexPath = modelState.indexPathForItemModel(withID: id) else { return bounds.minY }
let itemFrame = modelState.frameForItem(at: ElementLocation(indexPath: indexPath))
case .bottomItem(let id, let _elementLocation, let distanceFromBottom):
let elementLocation =
if isPerformingBatchUpdates {
modelState.indexPathForItemModel(withID: id).map(ElementLocation.init(indexPath:))
} else {
_elementLocation
}
guard let elementLocation else { return bounds.minY }
let itemFrame = modelState.frameForItem(at: elementLocation)
let proposedYOffset = itemFrame.maxY - bounds.height + contentInset.bottom - distanceFromBottom
// Clamp between minYOffset...maxYOffset
return min(max(proposedYOffset, minContentOffset.y), maxContentOffset.y)
Expand Down
4 changes: 2 additions & 2 deletions MagazineLayout/LayoutCore/Types/TargetContentOffsetAnchor.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ import UIKit
enum TargetContentOffsetAnchor: Equatable {
case top
case bottom
case topItem(id: UInt64, distanceFromTop: CGFloat)
case bottomItem(id: UInt64, distanceFromBottom: CGFloat)
case topItem(id: UInt64, elementLocation: ElementLocation, distanceFromTop: CGFloat)
case bottomItem(id: UInt64, elementLocation: ElementLocation, distanceFromBottom: CGFloat)
}
16 changes: 12 additions & 4 deletions MagazineLayout/Public/MagazineLayout.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,9 @@ public final class MagazineLayout: UICollectionViewLayout {

if let layoutStateBeforeCollectionViewUpdates{
let targetContentOffsetAnchor = layoutStateBeforeCollectionViewUpdates.targetContentOffsetAnchor
let targetYOffset = layoutState.yOffset(for: targetContentOffsetAnchor)
let targetYOffset = layoutState.yOffset(
for: targetContentOffsetAnchor,
isPerformingBatchUpdates: true)
let context = MagazineLayoutInvalidationContext()
context.invalidateLayoutMetrics = false
context.contentOffsetAdjustment.y = targetYOffset - layoutState.bounds.minY
Expand Down Expand Up @@ -676,7 +678,9 @@ public final class MagazineLayout: UICollectionViewLayout {
layoutStateBeforeAnimatedBoundsChange ??
self.layoutState
).targetContentOffsetAnchor
let targetYOffsetBefore = layoutState.yOffset(for: targetContentOffsetAnchor)
let targetYOffsetBefore = layoutState.yOffset(
for: targetContentOffsetAnchor,
isPerformingBatchUpdates: layoutStateBeforeCollectionViewUpdates != nil)

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

case .topItem, .bottomItem:
let targetYOffsetAfter = layoutState.yOffset(for: targetContentOffsetAnchor)
let targetYOffsetAfter = layoutState.yOffset(
for: targetContentOffsetAnchor,
isPerformingBatchUpdates: layoutStateBeforeCollectionViewUpdates != nil)
context.contentOffsetAdjustment.y = targetYOffsetAfter - targetYOffsetBefore
}

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

let yOffset = layoutState.yOffset(for: layoutStateBefore.targetContentOffsetAnchor)
let yOffset = layoutState.yOffset(
for: layoutStateBefore.targetContentOffsetAnchor,
isPerformingBatchUpdates: layoutStateBeforeCollectionViewUpdates != nil)

targetContentOffsetCompensatingYOffsetForAppearingItem = proposedContentOffset.y - yOffset

Expand Down
37 changes: 21 additions & 16 deletions Tests/LayoutStateTargetContentOffsetTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ final class LayoutStateTargetContentOffsetTests: XCTestCase {
contentInset: UIEdgeInsets(top: 50, left: 0, bottom: 30, right: 0),
scale: 1,
verticalLayoutDirection: .topToBottom)
let id = layoutState.modelState.idForItemModel(at: IndexPath(item: 6, section: 0))!
XCTAssert(layoutState.targetContentOffsetAnchor == .topItem(id: id, distanceFromTop: -25))
let indexPath = IndexPath(item: 6, section: 0)
let id = layoutState.modelState.idForItemModel(at: indexPath)!
XCTAssert(layoutState.targetContentOffsetAnchor == .topItem(id: id, elementLocation: ElementLocation(indexPath: indexPath), distanceFromTop: -25))
}

func testAnchor_TopToBottom_ScrolledToBottom() throws {
Expand All @@ -61,8 +62,9 @@ final class LayoutStateTargetContentOffsetTests: XCTestCase {
contentInset: measurementLayoutState.contentInset,
scale: measurementLayoutState.scale,
verticalLayoutDirection: measurementLayoutState.verticalLayoutDirection)
let id = layoutState.modelState.idForItemModel(at: IndexPath(item: 9, section: 0))!
XCTAssert(layoutState.targetContentOffsetAnchor == .topItem(id: id, distanceFromTop: 25))
let indexPath = IndexPath(item: 9, section: 0)
let id = layoutState.modelState.idForItemModel(at: indexPath)!
XCTAssert(layoutState.targetContentOffsetAnchor == .topItem(id: id, elementLocation: ElementLocation(indexPath: indexPath), distanceFromTop: 25))
}

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

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

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

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

func testAnchor_BottomToTop_ScrolledToBottom() throws {
Expand Down Expand Up @@ -139,7 +144,7 @@ final class LayoutStateTargetContentOffsetTests: XCTestCase {
scale: 1,
verticalLayoutDirection: .topToBottom)
let targetContentOffsetAnchor = layoutState.targetContentOffsetAnchor
XCTAssert(layoutState.yOffset(for: targetContentOffsetAnchor) == -50)
XCTAssert(layoutState.yOffset(for: targetContentOffsetAnchor, isPerformingBatchUpdates: false) == -50)
}

func testOffset_TopToBottom_ScrolledToMiddle() {
Expand All @@ -151,7 +156,7 @@ final class LayoutStateTargetContentOffsetTests: XCTestCase {
scale: 1,
verticalLayoutDirection: .topToBottom)
let targetContentOffsetAnchor = layoutState.targetContentOffsetAnchor
XCTAssert(layoutState.yOffset(for: targetContentOffsetAnchor) == 500)
XCTAssert(layoutState.yOffset(for: targetContentOffsetAnchor, isPerformingBatchUpdates: false) == 500)
}

func testOffset_TopToBottom_ScrolledToBottom() {
Expand All @@ -172,7 +177,7 @@ final class LayoutStateTargetContentOffsetTests: XCTestCase {
scale: measurementLayoutState.scale,
verticalLayoutDirection: measurementLayoutState.verticalLayoutDirection)
let targetContentOffsetAnchor = layoutState.targetContentOffsetAnchor
XCTAssert(layoutState.yOffset(for: targetContentOffsetAnchor) == 690)
XCTAssert(layoutState.yOffset(for: targetContentOffsetAnchor, isPerformingBatchUpdates: false) == 690)
}

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

func testOffset_BottomToTop_ScrolledToMiddle() {
Expand All @@ -198,7 +203,7 @@ final class LayoutStateTargetContentOffsetTests: XCTestCase {
scale: 1,
verticalLayoutDirection: .bottomToTop)
let targetContentOffsetAnchor = layoutState.targetContentOffsetAnchor
XCTAssert(layoutState.yOffset(for: targetContentOffsetAnchor) == 500)
XCTAssert(layoutState.yOffset(for: targetContentOffsetAnchor, isPerformingBatchUpdates: false) == 500)
}

func testOffset_BottomToTop_ScrolledToBottom() {
Expand All @@ -219,7 +224,7 @@ final class LayoutStateTargetContentOffsetTests: XCTestCase {
scale: measurementLayoutState.scale,
verticalLayoutDirection: measurementLayoutState.verticalLayoutDirection)
let targetContentOffsetAnchor = layoutState.targetContentOffsetAnchor
XCTAssert(layoutState.yOffset(for: targetContentOffsetAnchor) == 690)
XCTAssert(layoutState.yOffset(for: targetContentOffsetAnchor, isPerformingBatchUpdates: false) == 690)
}

// MARK: Private
Expand Down