Skip to main content

Merge pull request #22 from alexwlchan/track-asset-identifiers

ID
701b7a7
date
2023-06-16 06:57:15+00:00
author
Alex Chan <alex@alexwlchan.net>
parents
7495512, da50dba
message
Merge pull request #22 from alexwlchan/track-asset-identifiers

More performance improvements
changed files
8 files, 401 additions, 146 deletions

Changed files

BlinkReviewer/Blink.xcodeproj/project.pbxproj (34148) → BlinkReviewer/Blink.xcodeproj/project.pbxproj (34735)

diff --git a/BlinkReviewer/Blink.xcodeproj/project.pbxproj b/BlinkReviewer/Blink.xcodeproj/project.pbxproj
index 1617cc2..4c81314 100644
--- a/BlinkReviewer/Blink.xcodeproj/project.pbxproj
+++ b/BlinkReviewer/Blink.xcodeproj/project.pbxproj
@@ -14,6 +14,7 @@
 		945F17B42A33D726004FC479 /* ReviewStateIcon.swift in Sources */ = {isa = PBXBuildFile; fileRef = 945F17B32A33D726004FC479 /* ReviewStateIcon.swift */; };
 		945F17B62A33D7AA004FC479 /* ReviewStateBorder.swift in Sources */ = {isa = PBXBuildFile; fileRef = 945F17B52A33D7AA004FC479 /* ReviewStateBorder.swift */; };
 		945F17B82A33DAC7004FC479 /* ReviewStateSaturation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 945F17B72A33DAC7004FC479 /* ReviewStateSaturation.swift */; };
+		9492C9702A3C3803002E44EC /* Timer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9492C96F2A3C3803002E44EC /* Timer.swift */; };
 		94A0835E2A33E49E00238964 /* FocusedImage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 94A0835D2A33E49E00238964 /* FocusedImage.swift */; };
 		94A083612A33E98000238964 /* AlbumInfoOverlay.swift in Sources */ = {isa = PBXBuildFile; fileRef = 94A083602A33E98000238964 /* AlbumInfoOverlay.swift */; };
 		94A083632A33F30300238964 /* LoadingIndicatorOverlay.swift in Sources */ = {isa = PBXBuildFile; fileRef = 94A083622A33F30300238964 /* LoadingIndicatorOverlay.swift */; };
@@ -63,6 +64,7 @@
 		945F17B32A33D726004FC479 /* ReviewStateIcon.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReviewStateIcon.swift; sourceTree = "<group>"; };
 		945F17B52A33D7AA004FC479 /* ReviewStateBorder.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReviewStateBorder.swift; sourceTree = "<group>"; };
 		945F17B72A33DAC7004FC479 /* ReviewStateSaturation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReviewStateSaturation.swift; sourceTree = "<group>"; };
+		9492C96F2A3C3803002E44EC /* Timer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Timer.swift; sourceTree = "<group>"; };
 		94A0835D2A33E49E00238964 /* FocusedImage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FocusedImage.swift; sourceTree = "<group>"; };
 		94A083602A33E98000238964 /* AlbumInfoOverlay.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AlbumInfoOverlay.swift; sourceTree = "<group>"; };
 		94A083622A33F30300238964 /* LoadingIndicatorOverlay.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LoadingIndicatorOverlay.swift; sourceTree = "<group>"; };
@@ -138,6 +140,14 @@
 			path = Thumbnails;
 			sourceTree = "<group>";
 		};
+		9492C96E2A3C37FA002E44EC /* Helpers */ = {
+			isa = PBXGroup;
+			children = (
+				9492C96F2A3C3803002E44EC /* Timer.swift */,
+			);
+			path = Helpers;
+			sourceTree = "<group>";
+		};
 		94A0835F2A33E7E900238964 /* FocusedImage */ = {
 			isa = PBXGroup;
 			children = (
@@ -191,6 +201,7 @@
 		94D750EE2A31A796005859E7 /* Blink */ = {
 			isa = PBXGroup;
 			children = (
+				9492C96E2A3C37FA002E44EC /* Helpers */,
 				94D2C8B72A320E6600BEE15B /* Model */,
 				94D751272A31D61D005859E7 /* Photos */,
 				94D7511A2A31A7A6005859E7 /* Views */,
@@ -423,6 +434,7 @@
 				94A083682A33F6E900238964 /* ThumbnailList.swift in Sources */,
 				94D750F02A31A796005859E7 /* BlinkApp.swift in Sources */,
 				940331732A336B5100200C5D /* DeferredRendering.swift in Sources */,
+				9492C9702A3C3803002E44EC /* Timer.swift in Sources */,
 				94A0835E2A33E49E00238964 /* FocusedImage.swift in Sources */,
 				945F17B82A33DAC7004FC479 /* ReviewStateSaturation.swift in Sources */,
 				94D751222A31BD8E005859E7 /* PhotoReviewer.swift in Sources */,
@@ -588,7 +600,7 @@
 				CODE_SIGN_ENTITLEMENTS = Blink/Blink.entitlements;
 				CODE_SIGN_STYLE = Automatic;
 				COMBINE_HIDPI_IMAGES = YES;
-				CURRENT_PROJECT_VERSION = 3;
+				CURRENT_PROJECT_VERSION = 68;
 				DEVELOPMENT_ASSET_PATHS = "\"Blink/Preview Content\"";
 				ENABLE_PREVIEWS = YES;
 				GENERATE_INFOPLIST_FILE = YES;
@@ -615,7 +627,7 @@
 				CODE_SIGN_ENTITLEMENTS = Blink/Blink.entitlements;
 				CODE_SIGN_STYLE = Automatic;
 				COMBINE_HIDPI_IMAGES = YES;
-				CURRENT_PROJECT_VERSION = 3;
+				CURRENT_PROJECT_VERSION = 68;
 				DEVELOPMENT_ASSET_PATHS = "\"Blink/Preview Content\"";
 				ENABLE_PREVIEWS = YES;
 				GENERATE_INFOPLIST_FILE = YES;
@@ -640,7 +652,7 @@
 				ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = YES;
 				BUNDLE_LOADER = "$(TEST_HOST)";
 				CODE_SIGN_STYLE = Automatic;
-				CURRENT_PROJECT_VERSION = 3;
+				CURRENT_PROJECT_VERSION = 68;
 				GENERATE_INFOPLIST_FILE = YES;
 				MACOSX_DEPLOYMENT_TARGET = 13.3;
 				MARKETING_VERSION = 1.0;
@@ -658,7 +670,7 @@
 				ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = YES;
 				BUNDLE_LOADER = "$(TEST_HOST)";
 				CODE_SIGN_STYLE = Automatic;
-				CURRENT_PROJECT_VERSION = 3;
+				CURRENT_PROJECT_VERSION = 68;
 				GENERATE_INFOPLIST_FILE = YES;
 				MACOSX_DEPLOYMENT_TARGET = 13.3;
 				MARKETING_VERSION = 1.0;
@@ -675,7 +687,7 @@
 			buildSettings = {
 				ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = YES;
 				CODE_SIGN_STYLE = Automatic;
-				CURRENT_PROJECT_VERSION = 3;
+				CURRENT_PROJECT_VERSION = 68;
 				GENERATE_INFOPLIST_FILE = YES;
 				MARKETING_VERSION = 1.0;
 				PRODUCT_BUNDLE_IDENTIFIER = net.alexwlchan.BlinkReviewerUITests;
@@ -691,7 +703,7 @@
 			buildSettings = {
 				ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = YES;
 				CODE_SIGN_STYLE = Automatic;
-				CURRENT_PROJECT_VERSION = 3;
+				CURRENT_PROJECT_VERSION = 68;
 				GENERATE_INFOPLIST_FILE = YES;
 				MARKETING_VERSION = 1.0;
 				PRODUCT_BUNDLE_IDENTIFIER = net.alexwlchan.BlinkReviewerUITests;

BlinkReviewer/Blink/Helpers/Timer.swift (0) → BlinkReviewer/Blink/Helpers/Timer.swift (823)

diff --git a/BlinkReviewer/Blink/Helpers/Timer.swift b/BlinkReviewer/Blink/Helpers/Timer.swift
new file mode 100644
index 0000000..840b128
--- /dev/null
+++ b/BlinkReviewer/Blink/Helpers/Timer.swift
@@ -0,0 +1,27 @@
+import Foundation
+
+/// A basic profiler for timing operations.
+///
+/// This is based on some code written by JeremyP on Stack Overflow.
+/// See https://stackoverflow.com/a/24755958/1558022
+/// 
+struct Timer {
+    private let start: DispatchTime
+    private var elapsed: DispatchTime
+    
+    init() {
+        self.start = DispatchTime.now()
+        self.elapsed = start
+    }
+    
+    mutating func printTime(_ label: String) -> Void {
+        let now = DispatchTime.now()
+
+        let totalInterval = Double(now.uptimeNanoseconds - self.start.uptimeNanoseconds) / 1_000_000_000
+        let elapsedInterval = Double(now.uptimeNanoseconds - self.elapsed.uptimeNanoseconds) / 1_000_000_000
+
+        self.elapsed = now
+        
+        print("Time to \(label):\n  \(elapsedInterval) seconds (\(totalInterval) total)")
+    }
+}

BlinkReviewer/Blink/Photos/PhotosLibrary.swift (8979) → BlinkReviewer/Blink/Photos/PhotosLibrary.swift (16753)

diff --git a/BlinkReviewer/Blink/Photos/PhotosLibrary.swift b/BlinkReviewer/Blink/Photos/PhotosLibrary.swift
index 89660a3..9eda73d 100644
--- a/BlinkReviewer/Blink/Photos/PhotosLibrary.swift
+++ b/BlinkReviewer/Blink/Photos/PhotosLibrary.swift
@@ -16,6 +16,26 @@ class PhotosLibrary: NSObject, ObservableObject, PHPhotoLibraryChangeObserver {
     @Published var rejectedAssets: PHFetchResult<PHAsset> = PHFetchResult()
     @Published var needsActionAssets: PHFetchResult<PHAsset> = PHFetchResult()
     
+    // These lists/sets allow us to do some fast lookups for getting the
+    // state of an image, without going back to the Photos database.
+    // Individual database calls are fast; 25,000 if you need to retrieve
+    // all the thumbnails adds noticeable latency.
+    //
+    // 99% of the time, these match the PHFetchResult data; they differ when
+    // somebody has just modified state (e.g. reviewed a photo as "approved").
+    // We can update the internal set as soon as the PHChangeRequest completes,
+    // without waiting to get the update back from the Photos Library.
+    // That might not seem like much, but the latency is enough to feel
+    // noticeable, and tracking our own copy of that state makes the UI
+    // feel much more responsive.
+    @Published var assetIdentifiers: [String] = []
+    
+    private var approvedAssetIdentifiers: Set<String> = Set()
+    private var rejectedAssetIdentifiers: Set<String> = Set()
+    private var needsActionAssetIdentifiers: Set<String> = Set()
+    
+    private var favoriteAssetIdentifiers: Set<String> = Set()
+    
     // We publish the latest changes we detect from the Photos library.
     //
     // Views can subscribe to updates with
@@ -42,89 +62,144 @@ class PhotosLibrary: NSObject, ObservableObject, PHPhotoLibraryChangeObserver {
         getInitialData()
     }
     
+    /// Get the initial batch of data from the Photos Library when the app starts.
+    ///
+    /// This is populating all the cached data structures.
+    ///
+    /// You may see this method called twice, if you're running the app for the first time:
+    ///
+    ///    - When the app initially starts, we don't have permission to read the user's
+    ///      Photos Library.  This method runs pretty quickly, because we skip fetching
+    ///      anything from the database -- it'll appear empty to us.
+    ///
+    ///    - After the user grants permission, we'll call this method a second time, when
+    ///      we can actually get all the data.
+    ///
+    private func getInitialData() {
+        DispatchQueue.main.async {
+            var timer = Timer()
+            
+            let options = PHFetchOptions()
+            options.sortDescriptors = [NSSortDescriptor(key: "creationDate", ascending: false)]
+            
+            self.isPhotoLibraryAuthorized = PHPhotoLibrary.authorizationStatus() == .authorized
+            
+            if (self.isPhotoLibraryAuthorized) {
+                self.assets = PHAsset.fetchAssets(with: PHAssetMediaType.image, options: options)
+
+                self.approvedAssets = PHAsset.fetchAssets(in: self.approved, options: nil)
+                self.rejectedAssets = PHAsset.fetchAssets(in: self.rejected, options: nil)
+                self.needsActionAssets = PHAsset.fetchAssets(in: self.needsAction, options: nil)
+                
+                self.regenerateAssetIdentifiers()
+                
+                self.approvedAssetIdentifiers = getSetOfIdentifiers(fetchResult: self.approvedAssets)
+                self.rejectedAssetIdentifiers = getSetOfIdentifiers(fetchResult: self.rejectedAssets)
+                self.needsActionAssetIdentifiers = getSetOfIdentifiers(fetchResult: self.needsActionAssets)
+            }
+            
+            timer.printTime("get initial Photos data (isPhotoLibraryAuthorized = \(self.isPhotoLibraryAuthorized))")
+        }
+    }
+    
+    /// React to changes from the Photos Library.
+    ///
+    /// The PhotoKit APIs give us a bunch of information about deltas and updates,
+    /// so we don't need to reload all the information from scratch -- we can apply
+    /// partial updates to our local data.
+    ///
+    /// Note: this method is carefully tuned to balance accuracy and speed; we always
+    /// want to have the right data from Photos, but it can add noticeable latency
+    /// to UI updates if it's inefficient.
+    ///
+    /// See https://developer.apple.com/documentation/photokit/phphotolibrarychangeobserver
+    ///
     func photoLibraryDidChange(_ changeInstance: PHChange) {
         // If we've just received permission to read the user's Photos Library, go
         // ahead and populate all the initial data structures.
         if !self.isPhotoLibraryAuthorized && PHPhotoLibrary.authorizationStatus() == .authorized {
             getInitialData()
+            
+            // This is wrapped in an async dispatch to fix a warning from Xcode:
+            //
+            //     Publishing changes from background threads is not allowed; make sure
+            //     to publish values from the main thread (via operators like receive(on:))
+            //     on model updates.
+            //
+            DispatchQueue.main.async {
+                self.isPhotoLibraryAuthorized = PHPhotoLibrary.authorizationStatus() == .authorized
+            }
+            
+            return
         }
         
         DispatchQueue.main.async {
-            let start = DispatchTime.now()
-            var elapsed = start
-
-            func printElapsed(_ label: String) -> Void {
-              let now = DispatchTime.now()
-
-              let totalInterval = Double(now.uptimeNanoseconds - start.uptimeNanoseconds) / 1_000_000_000
-              let elapsedInterval = Double(now.uptimeNanoseconds - elapsed.uptimeNanoseconds) / 1_000_000_000
-
-              elapsed = DispatchTime.now()
-
-              print("Time to \(label):\n  \(elapsedInterval) seconds (\(totalInterval) total)")
-            }
-            
-            let options = PHFetchOptions()
-            options.sortDescriptors = [NSSortDescriptor(key: "creationDate", ascending: false)]
+            var timer = Timer()
             
             if let assetsChangeDetails = changeInstance.changeDetails(for: self.assets) {
                 self.assets = assetsChangeDetails.fetchResultAfterChanges
+                
+                assetsChangeDetails.changedObjects.forEach { asset in
+                    if asset.isFavorite {
+                        self.favoriteAssetIdentifiers.insert(asset.localIdentifier)
+                    } else {
+                        self.favoriteAssetIdentifiers.remove(asset.localIdentifier)
+                    }
+                }
+                
+                if assetsChangeDetails.hasMoves {
+                    self.regenerateAssetIdentifiers()
+                }
+                
                 self.latestChangeDetails = assetsChangeDetails
             }
             
             if let approvedChangeDetails = changeInstance.changeDetails(for: self.approvedAssets) {
                 self.approvedAssets = approvedChangeDetails.fetchResultAfterChanges
+                
+                approvedChangeDetails.insertedObjects.forEach { asset in
+                    self.approvedAssetIdentifiers.insert(asset.localIdentifier)
+                }
+                
+                approvedChangeDetails.removedObjects.forEach { asset in
+                    self.approvedAssetIdentifiers.remove(asset.localIdentifier)
+                }
             }
             
             if let rejectedChangeDetails = changeInstance.changeDetails(for: self.rejectedAssets) {
                 self.rejectedAssets = rejectedChangeDetails.fetchResultAfterChanges
+                
+                rejectedChangeDetails.insertedObjects.forEach { asset in
+                    self.rejectedAssetIdentifiers.insert(asset.localIdentifier)
+                }
+                
+                rejectedChangeDetails.removedObjects.forEach { asset in
+                    self.rejectedAssetIdentifiers.remove(asset.localIdentifier)
+                }
             }
             
             if let needsActionChangeDetails = changeInstance.changeDetails(for: self.needsActionAssets) {
                 self.needsActionAssets = needsActionChangeDetails.fetchResultAfterChanges
+                
+                needsActionChangeDetails.insertedObjects.forEach { asset in
+                    self.needsActionAssetIdentifiers.insert(asset.localIdentifier)
+                }
+                
+                needsActionChangeDetails.removedObjects.forEach { asset in
+                    self.needsActionAssetIdentifiers.remove(asset.localIdentifier)
+                }
             }
             
-            printElapsed("get all photos data (update)")
+            timer.printTime("process change to Photos data")
             
             self.isPhotoLibraryAuthorized = PHPhotoLibrary.authorizationStatus() == .authorized
         }
     }
-
-    private func getInitialData() {
-        DispatchQueue.main.async {
-            let start = DispatchTime.now()
-            var elapsed = start
-
-            func printElapsed(_ label: String) -> Void {
-              let now = DispatchTime.now()
-
-              let totalInterval = Double(now.uptimeNanoseconds - start.uptimeNanoseconds) / 1_000_000_000
-              let elapsedInterval = Double(now.uptimeNanoseconds - elapsed.uptimeNanoseconds) / 1_000_000_000
-
-              elapsed = DispatchTime.now()
-
-              print("Time to \(label):\n  \(elapsedInterval) seconds (\(totalInterval) total)")
-            }
-            
-            let options = PHFetchOptions()
-            options.sortDescriptors = [NSSortDescriptor(key: "creationDate", ascending: false)]
-            
-            self.isPhotoLibraryAuthorized = PHPhotoLibrary.authorizationStatus() == .authorized
-            
-            if (self.isPhotoLibraryAuthorized) {
-                self.assets = PHAsset.fetchAssets(with: PHAssetMediaType.image, options: options)
-
-                self.approvedAssets = PHAsset.fetchAssets(in: self.approved, options: nil)
-                self.rejectedAssets = PHAsset.fetchAssets(in: self.rejected, options: nil)
-                self.needsActionAssets = PHAsset.fetchAssets(in: self.needsAction, options: nil)
-            }
-            
-            printElapsed("get all photos data (new)")
-            
-            
-        }
-    }
     
+    /// Retrieve an asset at a particular position.
+    ///
+    /// Just a convenience wrapper around PHFetchResult.object(at: Int).
+    ///
     func asset(at index: Int) -> PHAsset {
         assets.object(at: index)
     }
@@ -163,6 +238,87 @@ class PhotosLibrary: NSObject, ObservableObject, PHPhotoLibraryChangeObserver {
         state(of: asset(at: index))
     }
     
+    func state(ofLocalIdentifier localIdentifier: String) -> ReviewState? {
+        if self.rejectedAssetIdentifiers.contains(localIdentifier) {
+            return .Rejected
+        }
+        
+        if self.needsActionAssetIdentifiers.contains(localIdentifier) {
+            return .NeedsAction
+        }
+        
+        if self.approvedAssetIdentifiers.contains(localIdentifier) {
+            return .Approved
+        }
+        
+        return nil
+    }
+    
+    /// Set the review state of an asset.
+    ///
+    /// This will record the change in the Photos Library and update any internal
+    /// data structures.
+    /// 
+    func setState(ofAsset asset: PHAsset, to newState: ReviewState) -> Void {
+        let existingState = self.state(of: asset)
+    
+        try! PHPhotoLibrary.shared().performChangesAndWait {
+            // The first condition is a combination of two:
+            //
+            //      -- the photo is already approved and you hit the "approve" hotkey,
+            //      -- so un-approve it
+            //      state == .Approved && e.characters == "1"
+            //
+            //      -- the photo is already approved and you selected a different review
+            //      -- state, so unapprove it
+            //      state == .Approved && e.characters != "1"
+            //
+            // We can optimise it into a single case, but it does make sense!
+            //
+            // Similar logic applies for all three conditions.
+            if existingState == .Approved {
+                asset.remove(fromAlbum: self.approved)
+            } else if newState == .Approved {
+                asset.add(toAlbum: self.approved)
+            }
+
+            if existingState == .Rejected {
+                asset.remove(fromAlbum: self.rejected)
+            } else if newState == .Rejected {
+                asset.add(toAlbum: self.rejected)
+            }
+
+            if existingState == .NeedsAction {
+                asset.remove(fromAlbum: self.needsAction)
+            } else if newState == .NeedsAction {
+                asset.add(toAlbum: self.needsAction)
+            }
+        }
+        
+        if existingState == .Approved {
+            self.approvedAssetIdentifiers.remove(asset.localIdentifier)
+        } else if newState == .Approved {
+            self.approvedAssetIdentifiers.insert(asset.localIdentifier)
+        }
+
+        if existingState == .Rejected {
+            self.rejectedAssetIdentifiers.remove(asset.localIdentifier)
+        } else if newState == .Rejected {
+            self.rejectedAssetIdentifiers.insert(asset.localIdentifier)
+        }
+
+        if existingState == .NeedsAction {
+            self.needsActionAssetIdentifiers.remove(asset.localIdentifier)
+        } else if newState == .NeedsAction {
+            self.needsActionAssetIdentifiers.insert(asset.localIdentifier)
+        }
+    }
+    
+    /// Returns true if this asset is a favorite, false otherwise.
+    func isFavorite(localIdentifier: String) -> Bool {
+        self.favoriteAssetIdentifiers.contains(localIdentifier)
+    }
+    
     // Implements a basic cache for thumbnail images.
     //
     // Thumbnail images are small and easily reused; I've put them here because
@@ -183,7 +339,7 @@ class PhotosLibrary: NSObject, ObservableObject, PHPhotoLibraryChangeObserver {
     //
     // On my M2 MacBook Air, these numbers mean the app peaks at ~250MB of memory,
     // which seems pretty reasonable.
-    private var thumbnailCache = LRUCache<PHAsset, PHAssetImage>(withMaxSize: 100)
+    private var thumbnailCache = LRUCache<PHAsset, PHAssetImage>(withMaxSize: 500)
     
     func getThumbnail(for asset: PHAsset) -> PHAssetImage {
         if thumbnailCache[asset] == nil {
@@ -222,4 +378,30 @@ class PhotosLibrary: NSObject, ObservableObject, PHPhotoLibraryChangeObserver {
 
         return fullSizeImageCache[asset]!
     }
+    
+    private func regenerateAssetIdentifiers() -> Void {
+        var assetIdentifiers: [String] = []
+        var favoriteAssetIdentifiers: Set<String> = Set()
+        
+        self.assets.enumerateObjects { asset, _, _ in
+            assetIdentifiers.append(asset.localIdentifier)
+            
+            if asset.isFavorite {
+                favoriteAssetIdentifiers.insert(asset.localIdentifier)
+            }
+        }
+        
+        self.assetIdentifiers = assetIdentifiers
+        self.favoriteAssetIdentifiers = favoriteAssetIdentifiers
+    }
+}
+
+func getSetOfIdentifiers(fetchResult: PHFetchResult<PHAsset>) -> Set<String> {
+    var result: Set<String> = Set()
+    
+    fetchResult.enumerateObjects { asset, _, _ in
+        result.insert(asset.localIdentifier)
+    }
+    
+    return result
 }

BlinkReviewer/Blink/Views/FocusedImage/FocusedImage.swift (548) → BlinkReviewer/Blink/Views/FocusedImage/FocusedImage.swift (534)

diff --git a/BlinkReviewer/Blink/Views/FocusedImage/FocusedImage.swift b/BlinkReviewer/Blink/Views/FocusedImage/FocusedImage.swift
index 8eb394c..8f877dd 100644
--- a/BlinkReviewer/Blink/Views/FocusedImage/FocusedImage.swift
+++ b/BlinkReviewer/Blink/Views/FocusedImage/FocusedImage.swift
@@ -4,16 +4,17 @@ import Photos
 /// Render the big image that gets shown in the main view.
 struct FocusedImage: View, Identifiable {
     var id: String {
-        focusedAssetImage.asset!.localIdentifier
+        asset.localIdentifier
     }
     
+    var asset: PHAsset
     @ObservedObject var focusedAssetImage: PHAssetImage
     
     var body: some View {
         Image(nsImage: focusedAssetImage.image)
             .resizable()
             .aspectRatio(contentMode: .fit)
-            .albumInfo(for: focusedAssetImage.asset)
+            .albumInfo(for: asset)
             .loadingIndicator(isLoading: focusedAssetImage.isDegraded)
     }
 }

BlinkReviewer/Blink/Views/Helpers/PHAssetHStack.swift (4391) → BlinkReviewer/Blink/Views/Helpers/PHAssetHStack.swift (4355)

diff --git a/BlinkReviewer/Blink/Views/Helpers/PHAssetHStack.swift b/BlinkReviewer/Blink/Views/Helpers/PHAssetHStack.swift
index d80cea0..ccd601a 100644
--- a/BlinkReviewer/Blink/Views/Helpers/PHAssetHStack.swift
+++ b/BlinkReviewer/Blink/Views/Helpers/PHAssetHStack.swift
@@ -1,6 +1,20 @@
 import SwiftUI
 import Photos
 
+struct AssetIdentifiersCollection: RandomAccessCollection, Equatable {
+    typealias Element = (Int, String)
+    typealias Index = Int
+    
+    let assetIdentifiers: [String]
+
+    var startIndex: Int { 0 }
+    var endIndex: Int { assetIdentifiers.count }
+
+    subscript(position: Int) -> Element {
+        (position, assetIdentifiers[position])
+    }
+}
+
 /// Creates an HStack of PHAssets that fills in right-to-left.
 ///
 /// This provides lazy loading to the left-hand side, and assumes you're
@@ -17,21 +31,25 @@ import Photos
 /// LazyHStack to the far right, it loads every element immediately.
 ///
 /// This takes a subview which is used to render the individual entries;
-/// these subviews receive the original PHAsset and the index from the
-/// original PHFetchResult -- you can use this index to retrieve adjacent
-/// items in the FetchResult, if necessary.
+/// these subviews receive the position and identifier of the original asset.
+///
+/// Note: this operates on a list of asset identifiers, but not the assets
+/// themselves -- this is a performance optimisation.  If the user scrolls
+/// deep into the list, SwiftUI will try to render lots of entries, and if
+/// those are PHAsset elements, it'll go back to the Photos database, even
+/// though we don't really need any Photos data in our views.
 ///
 struct PHAssetHStack<Content: View>: View {
-    var subview: (PHAsset, Int) -> Content
-    var collection: PHFetchResultCollection
+    var subview: (String, Int) -> Content
+    var assetIdentifiers: [String]
     
     init(
-        _ fetchResult: PHFetchResult<PHAsset>,
-        @ViewBuilder subview: @escaping (PHAsset, Int) -> Content
+        assetIdentifiers: [String],
+        @ViewBuilder subview: @escaping (String, Int) -> Content
     ) {
         print("--> creating PHAssetHStack")
         self.subview = subview
-        self.collection = PHFetchResultCollection(fetchResult)
+        self.assetIdentifiers = assetIdentifiers
     }
     
     var body: some View {
@@ -55,8 +73,10 @@ struct PHAssetHStack<Content: View>: View {
                 // 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: enumerated is okay
+                ForEach(AssetIdentifiersCollection(assetIdentifiers: self.assetIdentifiers), id: \.1) { index, localIdentifier in
+                    subview(localIdentifier, index)
                 }
                 
                 // Note: these two uses of RTL direction are a way to get the LazyHStack
@@ -78,23 +98,3 @@ struct PHAssetHStack<Content: View>: View {
             .environment(\.layoutDirection, .rightToLeft)
     }
 }
-
-struct PHAssetHStack_Previews: PreviewProvider {
-    static var fetchResult: PHFetchResult<PHAsset> {
-        let options = PHFetchOptions()
-        options.sortDescriptors = [NSSortDescriptor(key: "creationDate", ascending: false)]
-        options.fetchLimit = 25
-        
-        return PHAsset.fetchAssets(with: options)
-    }
-    
-    static var previews: some View {
-        PHAssetHStack(fetchResult) { asset, index in
-            VStack {
-                Text("view index = \(index)")
-                Text("asset ID =\n\(asset.localIdentifier)")
-                Text("fetchResult.object(at: \(index)) =\n\(fetchResult.object(at: index).localIdentifier)")
-            }
-        }
-    }
-}

BlinkReviewer/Blink/Views/PhotoReviewer.swift (14002) → BlinkReviewer/Blink/Views/PhotoReviewer.swift (13094)

diff --git a/BlinkReviewer/Blink/Views/PhotoReviewer.swift b/BlinkReviewer/Blink/Views/PhotoReviewer.swift
index ecc434f..2d35372 100644
--- a/BlinkReviewer/Blink/Views/PhotoReviewer.swift
+++ b/BlinkReviewer/Blink/Views/PhotoReviewer.swift
@@ -45,7 +45,10 @@ struct PhotoReviewer: View {
                         .frame(height: 90)
                         .background(.gray.opacity(0.3))
                     
-                    FocusedImage(focusedAssetImage: photosLibrary.getFullSizedImage(for: focusedAsset))
+                    FocusedImage(
+                        asset: focusedAsset,
+                        focusedAssetImage: photosLibrary.getFullSizedImage(for: focusedAsset)
+                    )
                     
                     Spacer()
                 }
@@ -224,46 +227,30 @@ struct PhotoReviewer: View {
                 return nil
             
             case let e where e.characters == "1" || e.characters == "2" || e.characters == "3":
-                print("time to review!")
-                let state = photosLibrary.state(of: focusedAsset)
+                let newState: ReviewState =
+                    e.characters == "1" ? .Approved :
+                    e.characters == "2" ? .Rejected : .NeedsAction
             
-                let approved = getAlbum(withName: "Approved")
-                let rejected = getAlbum(withName: "Rejected")
-                let needsAction = getAlbum(withName: "Needs Action")
+                photosLibrary.setState(ofAsset: focusedAsset, to: newState)
             
-                try! PHPhotoLibrary.shared().performChangesAndWait {
-                    // The first condition is a combination of two:
-                    //
-                    //      -- the photo is already approved and you hit the "approve" hotkey,
-                    //      -- so un-approve it
-                    //      state == .Approved && e.characters == "1"
-                    //
-                    //      -- the photo is already approved and you selected a different review
-                    //      -- state, so unapprove it
-                    //      state == .Approved && e.characters != "1"
-                    //
-                    // We can optimise it into a single case, but it does make sense!
-                    //
-                    // Similar logic applies for all three conditions.
-                    if state == .Approved {
-                        focusedAsset.remove(fromAlbum: approved)
-                    } else if e.characters == "1" {
-                        focusedAsset.add(toAlbum: approved)
-                    }
-
-                    if state == .Rejected {
-                        focusedAsset.remove(fromAlbum: rejected)
-                    } else if e.characters == "2" {
-                        focusedAsset.add(toAlbum: rejected)
-                    }
-
-                    if state == .NeedsAction {
-                        focusedAsset.remove(fromAlbum: needsAction)
-                    } else if e.characters == "3" {
-                        focusedAsset.add(toAlbum: needsAction)
-                    }
+                if focusedAssetIndex < photosLibrary.assets.count - 1 {
+                    focusedAssetIndex += 1
+                }
+                
+                return nil
+            
+            case let e where e.characters == "2":
+                photosLibrary.setState(ofAsset: focusedAsset, to: .Rejected)
+            
+                if focusedAssetIndex < photosLibrary.assets.count - 1 {
+                    focusedAssetIndex += 1
                 }
+                
+                return nil
             
+            case let e where e.characters == "3":
+                photosLibrary.setState(ofAsset: focusedAsset, to: .NeedsAction)
+
                 if focusedAssetIndex < photosLibrary.assets.count - 1 {
                     focusedAssetIndex += 1
                 }

BlinkReviewer/Blink/Views/Thumbnails/ThumbnailImage.swift (1601) → BlinkReviewer/Blink/Views/Thumbnails/ThumbnailImage.swift (2651)

diff --git a/BlinkReviewer/Blink/Views/Thumbnails/ThumbnailImage.swift b/BlinkReviewer/Blink/Views/Thumbnails/ThumbnailImage.swift
index e968d7e..12a62e9 100644
--- a/BlinkReviewer/Blink/Views/Thumbnails/ThumbnailImage.swift
+++ b/BlinkReviewer/Blink/Views/Thumbnails/ThumbnailImage.swift
@@ -1,6 +1,19 @@
 import SwiftUI
 import Photos
 
+struct ThumbnailImageInner: View {
+    @ObservedObject var assetImage: PHAssetImage
+    var size: CGFloat
+    
+    var body: some View {
+        Image(nsImage: assetImage.image)
+            .resizable()
+            .scaledToFill()
+            .clipped()
+            .frame(width: size, height: size, alignment: .center)
+    }
+}
+
 /// Render a single thumbnail image in the thumbnail picker.
 ///
 /// Thumbnails are square, and they expand to fill the square.  This may
@@ -16,10 +29,27 @@ struct ThumbnailImage: View {
     // But EnvironmentObject values aren't passed down until you call the
     // `body` method, which is too late!  So instead we have the parent
     // view call into PhotosLibrary and pass in the relevant values here.
-    @ObservedObject var assetImage: PHAssetImage
+    //
+    // The `getAssetImage` is a callback so we can load those assets somewhat
+    // lazily; the ThumbnailImageInner is separate so it can subscribe to
+    // updates to the PHAssetImage.
+    @State var assetImage: PHAssetImage? = nil
+    
+    var index: Int
     var state: ReviewState?
     var isFocused: Bool
     var isFavorite: Bool
+    private var getAssetImage: () -> PHAssetImage
+    
+    init(index: Int, state: ReviewState?, isFavorite: Bool, isFocused: Bool, getAssetImage: @escaping () -> PHAssetImage) {
+        self.index = index
+        
+        self.isFavorite = isFavorite
+        self.state = state
+        
+        self.isFocused = isFocused
+        self.getAssetImage = getAssetImage
+    }
     
     private func size() -> CGFloat {
         isFocused ? 70 : 50
@@ -30,15 +60,20 @@ struct ThumbnailImage: View {
     }
     
     var body: some View {
-        Image(nsImage: assetImage.image)
-            .resizable()
-            .scaledToFill()
-            .clipped()
-            .frame(width: size(), height: size(), alignment: .center)
-            .cornerRadius(cornerRadius())
-            .reviewStateColor(isRejected: state == .Rejected)
-            .reviewStateBorder(for: state, with: cornerRadius())
-            .reviewStateIcon(for: state, isFocused)
-            .favoriteHeartIcon(isFavorite, isFocused)
+        if let thisAssetImage = assetImage {
+            ThumbnailImageInner(assetImage: thisAssetImage, size: size())
+                .cornerRadius(cornerRadius())
+                .reviewStateColor(isRejected: state == .Rejected)
+                .reviewStateBorder(for: state, with: cornerRadius())
+                .reviewStateIcon(for: state, isFocused)
+                .favoriteHeartIcon(isFavorite, isFocused)
+                
+        } else {
+            ProgressView()
+                .onAppear {
+                    self.assetImage = getAssetImage()
+                }
+        }
+        
     }
 }

BlinkReviewer/Blink/Views/Thumbnails/ThumbnailList.swift (1075) → BlinkReviewer/Blink/Views/Thumbnails/ThumbnailList.swift (1836)

diff --git a/BlinkReviewer/Blink/Views/Thumbnails/ThumbnailList.swift b/BlinkReviewer/Blink/Views/Thumbnails/ThumbnailList.swift
index efcc95b..f1562ce 100644
--- a/BlinkReviewer/Blink/Views/Thumbnails/ThumbnailList.swift
+++ b/BlinkReviewer/Blink/Views/Thumbnails/ThumbnailList.swift
@@ -13,18 +13,29 @@ struct ThumbnailList: View {
     
     var body: some View {
         ScrollViewReader { proxy in
-            PHAssetHStack(photosLibrary.assets) { asset, index in
+            PHAssetHStack(assetIdentifiers: photosLibrary.assetIdentifiers) { localIdentifier, index in
                 ThumbnailImage(
-                    assetImage: photosLibrary.getThumbnail(for: asset),
-                    state: photosLibrary.state(of: asset),
+                    index: index,
+                    state: photosLibrary.state(ofLocalIdentifier: localIdentifier),
+                    isFavorite: photosLibrary.isFavorite(localIdentifier: localIdentifier),
                     isFocused: index == focusedAssetIndex,
-                    isFavorite: asset.isFavorite
-                ).onTapGesture {
+                    getAssetImage: {
+                        photosLibrary.getThumbnail(for: photosLibrary.asset(at: index))
+                    }
+                )
+                .environmentObject(photosLibrary)
+                .onTapGesture {
                     focusedAssetIndex = index
                 }
             }
-            .onChange(of: focusedAssetIndex, perform: { newIndex in
-                withAnimation {
+            // When the focusedAssetIndex changes, scroll to the new position.
+            //
+            // By default this is an animated scroll, but if the user is scrolling
+            // a long way, we skip the animation and jump straight to it -- if somebody
+            // jumps over several thousand images in one go, it looks better to snap
+            // rather than do a jaggy animation.
+            .onChange(of: focusedAssetIndex, perform: { [oldIndex = focusedAssetIndex] newIndex in
+                withAnimation(abs(newIndex - oldIndex) < 100 ? .default : nil) {
                     proxy.scrollTo(
                         photosLibrary.asset(at: newIndex).localIdentifier,
                         anchor: .center