Simplify how the FocusedImage is determined
- ID
ca1cb3f- date
2023-06-14 20:48:21+00:00- author
Alex Chan <alex@alexwlchan.net>- parent
c0c4a85- message
Simplify how the FocusedImage is determined Putting an @ObservableObject on the PhotoReviewer View was a mistake; it causes the view to reload whenever it changes, and it was in the same view as the thumbnail list. Moving it down into the FocusedImage view means we're doing less reloading whenever the focused image changes. This has a couple of knock-on effects: * Paging back/forth isn't painfully slow when you're navigated deep into the photo library -- it's still a little laggy, but not quite as bad as it was. * The new focused image updates immediately, rather than waiting for the full-sized version to load over the network. There are some mitigations to ensure this still looks good.- changed files
Changed files
BlinkReviewer/Blink/Photos/PhotosLibrary.swift (7880) → BlinkReviewer/Blink/Photos/PhotosLibrary.swift (8762)
diff --git a/BlinkReviewer/Blink/Photos/PhotosLibrary.swift b/BlinkReviewer/Blink/Photos/PhotosLibrary.swift
index 6ca6917..be05ac5 100644
--- a/BlinkReviewer/Blink/Photos/PhotosLibrary.swift
+++ b/BlinkReviewer/Blink/Photos/PhotosLibrary.swift
@@ -195,4 +195,30 @@ class PhotosLibrary: NSObject, ObservableObject, PHPhotoLibraryChangeObserver {
return newThumbnail
}
+
+ // Implement a similar cache for full-sized images.
+ //
+ // This is to avoid having to rebuild the PHAssetImage every time --
+ // which causes a brief "pop" as it starts by loading the low-res fuzzy image,
+ // then the high-res image pops in a second or so later.
+ //
+ // TODO: Surely it should be possible to make SwiftUI cache views like
+ // this for us?
+ private var fullSizeImageCache = Dictionary<PHAsset, PHAssetImage>()
+
+ func getFullSizedImage(for asset: PHAsset) -> PHAssetImage {
+ if let cachedImage = fullSizeImageCache[asset] {
+ return cachedImage
+ }
+
+ let newImage = PHAssetImage(
+ asset,
+ size: PHImageManagerMaximumSize,
+ deliveryMode: .opportunistic
+ )
+
+ fullSizeImageCache[asset] = newImage
+
+ return newImage
+ }
}
BlinkReviewer/Blink/Views/FocusedImage/FocusedImage.swift (1194) → BlinkReviewer/Blink/Views/FocusedImage/FocusedImage.swift (548)
diff --git a/BlinkReviewer/Blink/Views/FocusedImage/FocusedImage.swift b/BlinkReviewer/Blink/Views/FocusedImage/FocusedImage.swift
index 41c8735..8eb394c 100644
--- a/BlinkReviewer/Blink/Views/FocusedImage/FocusedImage.swift
+++ b/BlinkReviewer/Blink/Views/FocusedImage/FocusedImage.swift
@@ -2,28 +2,18 @@ import SwiftUI
import Photos
/// Render the big image that gets shown in the main view.
-///
-/// It's important to avoid a "flash" of empty space when switching between
-/// images, so this View is only created once and then the parent modifies
-/// the `asset` referred to by `assetImage`. This means the image that was
-/// previously being rendered sticks around until the new image loads in.
-///
-/// If this view was being passed the focused image directly, it'd be
-/// recreated every time the focus changed, and there'd be a temporary flash
-/// of empty space until an image could be loaded in.
-struct FocusedImage: View {
- @ObservedObject var assetImage: PHAssetImage
+struct FocusedImage: View, Identifiable {
+ var id: String {
+ focusedAssetImage.asset!.localIdentifier
+ }
- // We don't use anything from PhotosLibrary directly in this view, but we
- // do want to re-render it when we get a change to PhotosLibrary -- e.g.
- // when an asset is added to an album.
- @EnvironmentObject var photosLibrary: PhotosLibrary
+ @ObservedObject var focusedAssetImage: PHAssetImage
var body: some View {
- Image(nsImage: assetImage.image)
+ Image(nsImage: focusedAssetImage.image)
.resizable()
.aspectRatio(contentMode: .fit)
- .albumInfo(for: assetImage.asset)
- .loadingIndicator(isLoading: assetImage.isDegraded)
+ .albumInfo(for: focusedAssetImage.asset)
+ .loadingIndicator(isLoading: focusedAssetImage.isDegraded)
}
}
BlinkReviewer/Blink/Views/PhotoReviewer.swift (14269) → BlinkReviewer/Blink/Views/PhotoReviewer.swift (13578)
diff --git a/BlinkReviewer/Blink/Views/PhotoReviewer.swift b/BlinkReviewer/Blink/Views/PhotoReviewer.swift
index 1e4fc2b..c73ab04 100644
--- a/BlinkReviewer/Blink/Views/PhotoReviewer.swift
+++ b/BlinkReviewer/Blink/Views/PhotoReviewer.swift
@@ -32,10 +32,6 @@ struct PhotoReviewer: View {
@State var showDebug: Bool = false
@State var showInfo: Bool = false
- // This contains the big image that is currently in focus. See the comments
- // on FocusedImage for why this state is defined outside that view.
- @ObservedObject var focusedAssetImage = PHAssetImage(nil, size: PHImageManagerMaximumSize, deliveryMode: .highQualityFormat)
-
var body: some View {
if !photosLibrary.isPhotoLibraryAuthorized {
Text("Waiting for Photos Library authorization…")
@@ -49,8 +45,7 @@ struct PhotoReviewer: View {
.frame(height: 90)
.background(.gray.opacity(0.3))
- FocusedImage(assetImage: focusedAssetImage)
- .environmentObject(photosLibrary)
+ FocusedImage(focusedAssetImage: photosLibrary.getFullSizedImage(for: focusedAsset))
Spacer()
}
@@ -87,13 +82,6 @@ struct PhotoReviewer: View {
handler: handleKeyDown
)
}
- // These two lines update the big image that fills most of the window.
- // See the comments on FocusedImage for more explanation of why this is
- // managed this way.
- .onAppear { focusedAssetImage.asset = focusedAsset }
- .onChange(of: focusedAssetIndex) { _ in
- focusedAssetImage.asset = focusedAsset
- }
// These two methods are used to preserve position when there are changes
// in the Photos Library, e.g. deleted assets.
//