Skip to main content

Fix the performance issues in PHAssetHStack

ID
f464461
date
2023-06-10 18:31:26+00:00
author
Alex Chan <alex@alexwlchan.net>
parent
f7c0be6
message
Fix the performance issues in PHAssetHStack
changed files
2 files, 35 additions, 13 deletions

Changed files

BlinkReviewer/BlinkReviewer/Views/Helpers/PHAssetHStack.swift (3777) → BlinkReviewer/BlinkReviewer/Views/Helpers/PHAssetHStack.swift (4391)

diff --git a/BlinkReviewer/BlinkReviewer/Views/Helpers/PHAssetHStack.swift b/BlinkReviewer/BlinkReviewer/Views/Helpers/PHAssetHStack.swift
index 043bb27..d80cea0 100644
--- a/BlinkReviewer/BlinkReviewer/Views/Helpers/PHAssetHStack.swift
+++ b/BlinkReviewer/BlinkReviewer/Views/Helpers/PHAssetHStack.swift
@@ -41,13 +41,22 @@ struct PHAssetHStack<Content: View>: View {
                 // array index as the id here, because the app gets way slower if
                 // you use the PHFetchResult index -- it tries to regenerate a bunch of
                 // the thumbnails every time you change position.
-                ForEach(
-                    Array(
-                        zip(self.collection.indices, self.collection)
-                    ),
-                    id: \.1.localIdentifier
-                ) { index, asset in
-                    subview(asset, index)
+                //
+                // Note: an older implementation of this code had
+                //
+                // ```swift
+                //      ForEach(
+                //          Array(zip(self.collection.indices, self.collection)),
+                //          id: \.1.localIdentifier
+                //      )
+                // ```
+                //
+                // For some reason this caused the app to slow to a crawl -- I think it was
+                // creating the entire Array, which is quite expensive.  I switched the
+                // PHFetchResultCollection to vend a struct with both the asset and the
+                // position, but now it does it by random access -- this seems faster.
+                ForEach(self.collection, id: \.asset.localIdentifier) { indexedAsset in
+                    subview(indexedAsset.asset, indexedAsset.position)
                 }
                 
                 // Note: these two uses of RTL direction are a way to get the LazyHStack

BlinkReviewer/BlinkReviewer/Views/Helpers/PHFetchResultCollection.swift (1769) → BlinkReviewer/BlinkReviewer/Views/Helpers/PHFetchResultCollection.swift (2184)

diff --git a/BlinkReviewer/BlinkReviewer/Views/Helpers/PHFetchResultCollection.swift b/BlinkReviewer/BlinkReviewer/Views/Helpers/PHFetchResultCollection.swift
index b1be7a8..a6a0e61 100644
--- a/BlinkReviewer/BlinkReviewer/Views/Helpers/PHFetchResultCollection.swift
+++ b/BlinkReviewer/BlinkReviewer/Views/Helpers/PHFetchResultCollection.swift
@@ -15,12 +15,22 @@ import SwiftUI
 ///         ...
 ///     }
 /// }
+/// ```
+///
+/// This collection vends the IndexedPHAsset struct, which tells us both
+/// what asset we're on and where we are -- this is necessary for performance
+/// reasons.  See the comment above the ForEach in PHAssetHStack.
 ///
 /// This is based on code written by Slava Semeniuk on Stack Overflow:
 /// https://stackoverflow.com/q/62745595/1558022
-///
+
+struct IndexedPHAsset {
+    var position: Int
+    var asset: PHAsset
+}
+
 struct PHFetchResultCollection: RandomAccessCollection, Equatable {
-    typealias Element = PHAsset
+    typealias Element = IndexedPHAsset
     typealias Index = Int
 
     let fetchResult: PHFetchResult<PHAsset>
@@ -32,8 +42,11 @@ struct PHFetchResultCollection: RandomAccessCollection, Equatable {
     var startIndex: Int { 0 }
     var endIndex: Int { fetchResult.count }
 
-    subscript(position: Int) -> PHAsset {
-        fetchResult.object(at: position)
+    subscript(position: Int) -> IndexedPHAsset {
+        IndexedPHAsset(
+            position: position,
+            asset: fetchResult.object(at: position)
+        )
     }
 }
 
@@ -52,8 +65,8 @@ struct PHFetchResultCollection_Previews: PreviewProvider {
         VStack {
             Text("These dates should be in descending order:")
             
-            ForEach(self.resultCollection, id: \.localIdentifier) { asset in
-                Text("\(asset.creationDate?.ISO8601Format() ?? "(unknown)")")
+            ForEach(self.resultCollection, id: \.asset.localIdentifier) { indexedAsset in
+                Text("\(indexedAsset.asset.creationDate?.ISO8601Format() ?? "(unknown)")")
             }
         }
     }