Revert "Bk/performance improvements (#272)" (#273)

This reverts commit 343cd82.
airbnb · Sep 27, 2023 · a569fbc · a569fbc
1 parent 343cd82
commit a569fbc
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 64 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -33,7 +33,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Changed `ItemView` to determine user interaction capabilities from its content view's `hitTest` / `pointInside` functions
 - Updated content-change animations so that the same scroll offset is maintained throughout the animation
 - Changed the Swift version needed to use HorizonCalendar to 5.8
-- Optimized `layoutSubviews` to avoid doing unnecessary work updating views in some cases
 
 ## [v1.16.0](https://github.com/airbnb/HorizonCalendar/compare/v1.15.0...v1.16.0) - 2023-01-30
 

diff --git a/Sources/Internal/VisibleItemsProvider.swift b/Sources/Internal/VisibleItemsProvider.swift
@@ -70,10 +70,7 @@ final class VisibleItemsProvider {
     -> LayoutItem
   {
     let layoutItem = LayoutItem(itemType: .monthHeader(month), frame: .zero)
-    var context = VisibleItemsContext(
-      centermostLayoutItem: layoutItem,
-      firstLayoutItem: layoutItem,
-      lastLayoutItem: layoutItem)
+    var context = VisibleItemsContext(centermostLayoutItem: layoutItem, firstLayoutItem: layoutItem)
     let monthHeaderHeight = monthHeaderHeight(for: month, context: &context)
 
     let monthOrigin: CGPoint
@@ -115,10 +112,7 @@ final class VisibleItemsProvider {
     -> LayoutItem
   {
     let layoutItem = LayoutItem(itemType: .day(day), frame: .zero)
-    var context = VisibleItemsContext(
-      centermostLayoutItem: layoutItem,
-      firstLayoutItem: layoutItem,
-      lastLayoutItem: layoutItem)
+    var context = VisibleItemsContext(centermostLayoutItem: layoutItem, firstLayoutItem: layoutItem)
     let month = day.month
     let monthHeaderHeight = monthHeaderHeight(for: month, context: &context)
 
@@ -170,7 +164,6 @@ final class VisibleItemsProvider {
     var context = VisibleItemsContext(
       centermostLayoutItem: previouslyVisibleLayoutItem,
       firstLayoutItem: previouslyVisibleLayoutItem,
-      lastLayoutItem: previouslyVisibleLayoutItem,
       calendarItemModelCache: .init(
         minimumCapacity: previousCalendarItemModelCache?.capacity ?? 100))
 
@@ -290,7 +283,6 @@ final class VisibleItemsProvider {
       visibleItems: context.visibleItems,
       centermostLayoutItem: context.centermostLayoutItem,
       firstLayoutItem: context.firstLayoutItem,
-      lastLayoutItem: context.lastLayoutItem,
       visibleDayRange: visibleDayRange,
       visibleMonthRange: visibleMonthRange,
       framesForVisibleMonths: context.framesForVisibleMonths,
@@ -348,8 +340,7 @@ final class VisibleItemsProvider {
     var lastHandledLayoutItemEnumeratingForwards = previouslyVisibleLayoutItem
     var context = VisibleItemsContext(
       centermostLayoutItem: previouslyVisibleLayoutItem,
-      firstLayoutItem: previouslyVisibleLayoutItem,
-      lastLayoutItem: previouslyVisibleLayoutItem)
+      firstLayoutItem: previouslyVisibleLayoutItem)
 
     layoutItemTypeEnumerator.enumerateItemTypes(
       startingAt: previouslyVisibleLayoutItem.itemType,
@@ -426,22 +417,6 @@ final class VisibleItemsProvider {
     return itemOrigin < otherItemOrigin ? item : otherItem
   }
 
-  // Returns the layout item closest to the bottom/trailing edge of `bounds`.
-  private func lastLayoutItem(comparing item: LayoutItem, to otherItem: LayoutItem) -> LayoutItem {
-    let itemOrigin: CGFloat
-    let otherItemOrigin: CGFloat
-    switch content.monthsLayout {
-    case .vertical:
-      itemOrigin = item.frame.maxY
-      otherItemOrigin = otherItem.frame.maxY
-    case .horizontal:
-      itemOrigin = item.frame.maxX
-      otherItemOrigin = otherItem.frame.maxX
-    }
-
-    return itemOrigin > otherItemOrigin ? item : otherItem
-  }
-
   private func boundsForAnimatedUpdatePass(atOffset offset: CGPoint) -> CGRect {
     // Use a larger bounds (3x the viewport size) if we're in an animated update pass, reducing the
     // likelihood of an item popping in / out.
@@ -891,9 +866,6 @@ final class VisibleItemsProvider {
         context.firstLayoutItem = firstLayoutItem(
           comparing: layoutItem,
           to: context.firstLayoutItem)
-        context.lastLayoutItem = lastLayoutItem(
-          comparing: layoutItem,
-          to: context.lastLayoutItem)
       }
     } else {
       shouldStop = true
@@ -1262,7 +1234,6 @@ final class VisibleItemsProvider {
 private struct VisibleItemsContext {
   var centermostLayoutItem: LayoutItem
   var firstLayoutItem: LayoutItem
-  var lastLayoutItem: LayoutItem
   var firstVisibleDay: Day?
   var lastVisibleDay: Day?
   var firstVisibleMonth: Month?
@@ -1287,7 +1258,6 @@ struct VisibleItemsDetails {
   let visibleItems: Set<VisibleItem>
   let centermostLayoutItem: LayoutItem
   let firstLayoutItem: LayoutItem?
-  let lastLayoutItem: LayoutItem?
   let visibleDayRange: DayRange?
   let visibleMonthRange: MonthRange?
   let framesForVisibleMonths: [Month: CGRect]
@@ -1301,11 +1271,6 @@ struct VisibleItemsDetails {
     maxMonthHeight + heightOfPinnedContent
   }
 
-  var layoutRegion: ClosedRange<LayoutItem.ItemType>? {
-    guard let firstLayoutItem, let lastLayoutItem else { return nil }
-    return firstLayoutItem.itemType...lastLayoutItem.itemType
-  }
-
 }
 
 // MARK: - _DayRangeLayoutContext

diff --git a/Sources/Public/CalendarView.swift b/Sources/Public/CalendarView.swift
@@ -258,10 +258,6 @@ public final class CalendarView: UIView {
       invalidateIntrinsicContentSize()
     }
 
-    // Clear this so that we don't return early in our next layout pass even though our layout
-    // region might not have changed.
-    layoutRegion = nil
-
     self.content = content
     setNeedsLayout()
 
@@ -500,7 +496,6 @@ public final class CalendarView: UIView {
   private var anchorLayoutItem: LayoutItem?
   private var _visibleItemsProvider: VisibleItemsProvider?
   private var visibleItemsDetails: VisibleItemsDetails?
-  private var layoutRegion: ClosedRange<LayoutItem.ItemType>?
   private var visibleViewsForVisibleItems = [VisibleItem: ItemView]()
 
   private var isAnimatedUpdatePass = false
@@ -739,14 +734,11 @@ public final class CalendarView: UIView {
       isAnimatedUpdatePass: isAnimatedUpdatePass)
     self.anchorLayoutItem = currentVisibleItemsDetails.centermostLayoutItem
 
-    // If our first / last layout item hasn't changed, then we haven't scrolled enough to trigger
-    // an update of visible views. This short-circuit greatly improves scroll performance.
-    if currentVisibleItemsDetails.layoutRegion != layoutRegion {
-      updateVisibleViews(withVisibleItems: currentVisibleItemsDetails.visibleItems)
-    }
+    updateVisibleViews(
+      withVisibleItems: currentVisibleItemsDetails.visibleItems,
+      previouslyVisibleItems: visibleItemsDetails?.visibleItems ?? [])
 
     visibleItemsDetails = currentVisibleItemsDetails
-    layoutRegion = currentVisibleItemsDetails.layoutRegion
 
     let minimumScrollOffset = visibleItemsDetails?.contentStartBoundary.map {
       ($0 - firstLayoutMarginValue).alignedToPixel(forScreenWithScale: scale)
@@ -768,7 +760,10 @@ public final class CalendarView: UIView {
     }
   }
 
-  private func updateVisibleViews(withVisibleItems visibleItems: Set<VisibleItem>) {
+  private func updateVisibleViews(
+    withVisibleItems visibleItems: Set<VisibleItem>,
+    previouslyVisibleItems _: Set<VisibleItem>)
+  {
     var viewsToHideForVisibleItems = visibleViewsForVisibleItems
     visibleViewsForVisibleItems.removeAll(keepingCapacity: true)
 
@@ -1163,7 +1158,10 @@ extension CalendarView {
       guard cachedAccessibilityElements == nil else {
         return cachedAccessibilityElements
       }
-      guard let visibleItemsDetails, let visibleMonthRange else {
+      guard
+        let visibleItemsDetails,
+        let visibleMonthRange
+      else {
         return nil
       }
 

diff --git a/Sources/Public/CalendarViewContent.swift b/Sources/Public/CalendarViewContent.swift
@@ -188,7 +188,12 @@ public final class CalendarViewContent {
     -> CalendarViewContent
   {
     self.monthHeaderItemProvider = { [defaultMonthHeaderItemProvider] month in
-      monthHeaderItemProvider(month) ?? defaultMonthHeaderItemProvider(month)
+      guard let itemModel = monthHeaderItemProvider(month) else {
+        // If the caller returned nil, fall back to the default item provider
+        return defaultMonthHeaderItemProvider(month)
+      }
+
+      return itemModel
     }
 
     return self
@@ -218,7 +223,12 @@ public final class CalendarViewContent {
     -> CalendarViewContent
   {
     self.dayOfWeekItemProvider = { [defaultDayOfWeekItemProvider] month, weekdayIndex in
-      dayOfWeekItemProvider(month, weekdayIndex) ?? defaultDayOfWeekItemProvider(month, weekdayIndex)
+      guard let itemModel = dayOfWeekItemProvider(month, weekdayIndex) else {
+        // If the caller returned nil, fall back to the default item provider
+        return defaultDayOfWeekItemProvider(month, weekdayIndex)
+      }
+
+      return itemModel
     }
 
     return self
@@ -244,7 +254,12 @@ public final class CalendarViewContent {
     -> CalendarViewContent
   {
     self.dayItemProvider = { [defaultDayItemProvider] day in
-      dayItemProvider(day) ?? defaultDayItemProvider(day)
+      guard let itemModel = dayItemProvider(day) else {
+        // If the caller returned nil, fall back to the default item provider
+        return defaultDayItemProvider(day)
+      }
+
+      return itemModel
     }
 
     return self
@@ -392,7 +407,10 @@ public final class CalendarViewContent {
 
   /// The default `monthHeaderItemProvider` if no provider has been configured,
   /// or if the existing provider returns nil.
-  private lazy var defaultMonthHeaderItemProvider: (Month) -> AnyCalendarItemModel = { [unowned self] month in
+  private lazy var defaultMonthHeaderItemProvider: (Month) -> AnyCalendarItemModel = { [
+    calendar,
+    monthHeaderDateFormatter
+  ] month in
     let firstDateInMonth = calendar.firstDate(of: month)
     let monthText = monthHeaderDateFormatter.string(from: firstDateInMonth)
     let itemModel = MonthHeaderView.calendarItemModel(
@@ -403,17 +421,18 @@ public final class CalendarViewContent {
 
   /// The default `dayHeaderItemProvider` if no provider has been configured,
   /// or if the existing provider returns nil.
-  private lazy var defaultDayOfWeekItemProvider: (Month?, Int) -> AnyCalendarItemModel = { [unowned self] _, weekdayIndex in
-    let dayOfWeekText = monthHeaderDateFormatter.veryShortStandaloneWeekdaySymbols[weekdayIndex]
-    let itemModel = DayOfWeekView.calendarItemModel(
-      invariantViewProperties: .base,
-      content: .init(dayOfWeekText: dayOfWeekText, accessibilityLabel: dayOfWeekText))
-    return itemModel
-  }
+  private lazy var defaultDayOfWeekItemProvider: (Month?, Int)
+    -> AnyCalendarItemModel = { [monthHeaderDateFormatter] _, weekdayIndex in
+      let dayOfWeekText = monthHeaderDateFormatter.veryShortStandaloneWeekdaySymbols[weekdayIndex]
+      let itemModel = DayOfWeekView.calendarItemModel(
+        invariantViewProperties: .base,
+        content: .init(dayOfWeekText: dayOfWeekText, accessibilityLabel: dayOfWeekText))
+      return itemModel
+    }
 
   /// The default `dayItemProvider` if no provider has been configured,
   /// or if the existing provider returns nil.
-  private lazy var defaultDayItemProvider: (Day) -> AnyCalendarItemModel = { [unowned self] day in
+  private lazy var defaultDayItemProvider: (Day) -> AnyCalendarItemModel = { [calendar, dayDateFormatter] day in
     let date = calendar.startDate(of: day)
     let itemModel = DayView.calendarItemModel(
       invariantViewProperties: .baseNonInteractive,