diff --git a/damus/Components/ImageCarousel.swift b/damus/Components/ImageCarousel.swift index 49c71993..1f9809dd 100644 --- a/damus/Components/ImageCarousel.swift +++ b/damus/Components/ImageCarousel.swift @@ -7,6 +7,7 @@ import SwiftUI import Kingfisher +import Combine // TODO: all this ShareSheet complexity can be replaced with ShareLink once we update to iOS 16 struct ShareSheet: UIViewControllerRepresentable { @@ -95,64 +96,203 @@ enum ImageShape { } } +/// The `CarouselModel` helps `ImageCarousel` with some state management logic, keeping track of media sizes, and the ideal display size +/// +/// This model is necessary because the state management logic required to keep track of media sizes for each one of the carousel items, +/// and the ideal display size at each moment is not a trivial task. +/// +/// The rules for the media fill are as follows: +/// 1. The media item should generally have a width that completely fills the width of its parent view +/// 2. The height of the carousel should be adjusted accordingly +/// 3. The only exception to rules 1 and 2 is when the total height would be 20% larger than the height of the device +/// 4. If none of the above can be computed (e.g. due to missing information), default to a reasonable height, where the media item will fit into. +/// +/// ## Usage notes +/// +/// The view is has the following state management responsibilities: +/// 1. Watching the size of the images (via the `.observe_image_size` modifier) +/// 2. Notifying this class of geometry reader changes, by setting `geo_size` +/// +/// ## Implementation notes +/// +/// This class is organized in a way to reduce stateful behavior and the transiency bugs it can cause. +/// +/// This is accomplished through the following pattern: +/// 1. The `current_item_fill` is a published property so that any updates instantly re-render the view +/// 2. However, `current_item_fill` has a mathematical dependency on other members of this class +/// 3. Therefore, the members on which the fill property depends on all have `didSet` observers that will cause the `current_item_fill` to be recalculated and published. +/// +/// This pattern helps ensure that the state is always consistent and that the view is always up-to-date. +/// +/// This class is marked as `@MainActor` since most of its properties are published and should be accessed from the main thread to avoid inconsistent SwiftUI state during renders +@MainActor class CarouselModel: ObservableObject { - var current_url: URL? - var fillHeight: CGFloat - var maxHeight: CGFloat - var firstImageHeight: CGFloat? + // MARK: Immutable object attributes + // These are some attributes that are not expected to change throughout the lifecycle of this object + // These should not be modified after initialization to avoid state inconsistency + + /// The state of the app + let damus_state: DamusState + /// All urls in the carousel + let urls: [MediaUrl] + /// The default fill height for the carousel, if we cannot calculate a more appropriate height + /// **Usage note:** Default to this when `current_item_fill` is nil + let default_fill_height: CGFloat + /// The maximum height for any carousel item + let max_height: CGFloat + + + // MARK: Miscellaneous + + /// Holds items that allows us to cancel video size observers during de-initialization + private var all_cancellables: [AnyCancellable] = [] + + + // MARK: State management properties + /// Properties relevant to state management. + /// These should be made into computed/functional properties when possible to avoid stateful behavior + /// When that is not possible (e.g. when dealing with an observed published property), establish its mathematical dependencies, + /// and use `didSet` observers to ensure that the state is always re-computed when necessary. - @Published var open_sheet: Bool - @Published var selectedIndex: Int - @Published var video_size: CGSize? - @Published var image_fill: ImageFill? + /// Stores information about the size of each media item in `urls`. + /// **Usage note:** The view is responsible for setting the size of image urls + var media_size_information: [URL: CGSize] { + didSet { + guard let current_url else { return } + // Upon updating information, update the carousel fill size if the size for the current url has changed + if oldValue[current_url] != media_size_information[current_url] { + self.refresh_current_item_fill() + } + } + } + /// Stores information about the geometry reader + /// **Usage note:** The view is responsible for setting this value + var geo_size: CGSize? { + didSet { self.refresh_current_item_fill() } + } + /// The index of the currently selected item + /// **Usage note:** The view is responsible for setting this value + @Published var selectedIndex: Int { + didSet { self.refresh_current_item_fill() } + } + /// The current fill for the media item. + /// **Usage note:** This property is read-only and should not be set directly. Update `selectedIndex` to update the current item being viewed. + var current_url: URL? { + return urls[safe: selectedIndex]?.url + } + /// Holds the ideal fill dimensions for the current item. + /// **Usage note:** This property is automatically updated when other properties are set, and should not be set directly + /// **Implementation note:** This property is mathematically dependent on geo_size, media_size_information, and `selectedIndex`, + /// and is automatically updated upon changes to these properties. + @Published private(set) var current_item_fill: ImageFill? + + + // MARK: Initialization and de-initialization - init(image_fill: ImageFill?) { - self.current_url = nil - self.fillHeight = 350 - self.maxHeight = UIScreen.main.bounds.height * 1.2 // 1.2 - self.firstImageHeight = nil - self.open_sheet = false + /// Initializes the `CarouselModel` with the given `DamusState` and `MediaUrl` array + init(damus_state: DamusState, urls: [MediaUrl]) { + // Immutable object attributes + self.damus_state = damus_state + self.urls = urls + self.default_fill_height = 350 + self.max_height = UIScreen.main.bounds.height * 1.2 // 1.2 + + // State management properties self.selectedIndex = 0 - self.video_size = nil - self.image_fill = image_fill + self.current_item_fill = nil + self.geo_size = nil + self.media_size_information = [:] + + // Setup the rest of the state management logic + self.observe_video_sizes() + Task { + self.refresh_current_item_fill() + } + } + + /// This private function observes the video sizes for all videos + private func observe_video_sizes() { + for media_url in urls { + switch media_url { + case .video(let url): + let video_player = damus_state.video.get_player(for: url) + if let video_size = video_player.video_size { + self.media_size_information[url] = video_size // Set the initial size if available + } + let observer_cancellable = video_player.$video_size.sink(receiveValue: { new_size in + self.media_size_information[url] = new_size // Update the size when it changes + }) + all_cancellables.append(observer_cancellable) // Store the cancellable to cancel it later + case .image(_): + break; // Observing an image size needs to be done on the view directly, through the `.observe_image_size` modifier + } + } + } + + deinit { + for cancellable_item in all_cancellables { + cancellable_item.cancel() + } + } + + // MARK: State management and logic + + /// This function refreshes the current item fill based on the current state of the model + /// **Usage note:** This is private, do not call this directly from outside the class. + /// **Implementation note:** This should be called using `didSet` observers on properties that affect the fill + private func refresh_current_item_fill() { + if let current_url, + let item_size = self.media_size_information[current_url], + let geo_size { + self.current_item_fill = ImageFill.calculate_image_fill( + geo_size: geo_size, + img_size: item_size, + maxHeight: self.max_height, + fillHeight: self.default_fill_height + ) + } + else { + self.current_item_fill = nil // Not enough information to compute the proper fill. Default to nil + } } } // MARK: - Image Carousel + +/// A carousel that displays images and videos +/// +/// ## Implementation notes +/// +/// - State management logic is mostly handled by `CarouselModel`, as it is complex, and becomes difficult to manage in a view +/// @MainActor struct ImageCarousel: View { - var urls: [MediaUrl] - + /// The event id of the note that this carousel is displaying let evid: NoteId - - let state: DamusState + /// The model that holds information and state of this carousel + /// This is observed to update the view when the model changes @ObservedObject var model: CarouselModel let content: ((_ dismiss: @escaping (() -> Void)) -> Content)? init(state: DamusState, evid: NoteId, urls: [MediaUrl]) { - self.urls = urls self.evid = evid - self.state = state - let media_model = state.events.get_cache_data(evid).media_metadata_model - self._model = ObservedObject(initialValue: CarouselModel(image_fill: media_model.fill)) + self._model = ObservedObject(initialValue: CarouselModel(damus_state: state, urls: urls)) self.content = nil } init(state: DamusState, evid: NoteId, urls: [MediaUrl], @ViewBuilder content: @escaping (_ dismiss: @escaping (() -> Void)) -> Content) { - self.urls = urls self.evid = evid - self.state = state - let media_model = state.events.get_cache_data(evid).media_metadata_model - self._model = ObservedObject(initialValue: CarouselModel(image_fill: media_model.fill)) + self._model = ObservedObject(initialValue: CarouselModel(damus_state: state, urls: urls)) self.content = content } var filling: Bool { - model.image_fill?.filling == true + model.current_item_fill?.filling == true } var height: CGFloat { - model.firstImageHeight ?? model.image_fill?.height ?? model.fillHeight + // Use the calculated fill height if available, otherwise use the default fill height + model.current_item_fill?.height ?? model.default_fill_height } func Placeholder(url: URL, geo_size: CGSize, num_urls: Int) -> some View { @@ -160,7 +300,7 @@ struct ImageCarousel: View { if num_urls > 1 { // jb55: quick hack since carousel with multiple images looks horrible with blurhash background Color.clear - } else if let meta = state.events.lookup_img_metadata(url: url), + } else if let meta = model.damus_state.events.lookup_img_metadata(url: url), case .processed(let blurhash) = meta.state { Image(uiImage: blurhash) .resizable() @@ -177,24 +317,17 @@ struct ImageCarousel: View { case .image(let url): Img(geo: geo, url: url, index: index) .onTapGesture { - model.open_sheet = true + present(full_screen_item: .full_screen_carousel(urls: model.urls, selectedIndex: $model.selectedIndex)) } case .video(let url): - DamusVideoPlayerView(url: url, coordinator: state.video, style: .preview(on_tap: { model.open_sheet = true })) - .onChange(of: model.video_size) { size in - guard let size else { return } - - let fill = ImageFill.calculate_image_fill(geo_size: geo.size, img_size: size, maxHeight: model.maxHeight, fillHeight: model.fillHeight) - - print("video_size changed \(size)") - if self.model.image_fill == nil { - print("video_size firstImageHeight \(fill.height)") - self.model.firstImageHeight = fill.height - state.events.get_cache_data(evid).media_metadata_model.fill = fill - } - - self.model.image_fill = fill - } + let video_model = model.damus_state.video.get_player(for: url) + DamusVideoPlayerView( + model: video_model, + coordinator: model.damus_state.video, + style: .preview(on_tap: { + present(full_screen_item: .full_screen_carousel(urls: model.urls, selectedIndex: $model.selectedIndex)) + }) + ) } } } @@ -203,31 +336,18 @@ struct ImageCarousel: View { KFAnimatedImage(url) .callbackQueue(.dispatch(.global(qos:.background))) .backgroundDecode(true) - .imageContext(.note, disable_animation: state.settings.disable_animation) + .imageContext(.note, disable_animation: model.damus_state.settings.disable_animation) .image_fade(duration: 0.25) .cancelOnDisappear(true) .configure { view in view.framePreloadCount = 3 } - .imageFill(for: geo.size, max: model.maxHeight, fill: model.fillHeight) { fill in - state.events.get_cache_data(evid).media_metadata_model.fill = fill - // blur hash can be discarded when we have the url - // NOTE: this is the wrong place for this... we need to remove - // it when the image is loaded in memory. This may happen - // earlier than this (by the preloader, etc) - DispatchQueue.main.asyncAfter(deadline: .now() + 1.0) { - state.events.lookup_img_metadata(url: url)?.state = .not_needed - } - self.model.image_fill = fill - if index == 0 { - self.model.firstImageHeight = fill.height - //maxHeight = firstImageHeight ?? maxHeight - } else { - //maxHeight = firstImageHeight ?? fill.height - } - } + .observe_image_size(size_changed: { size in + // Observe the image size to update the model when the size changes, so we can calculate the fill + model.media_size_information[url] = size + }) .background { - Placeholder(url: url, geo_size: geo.size, num_urls: urls.count) + Placeholder(url: url, geo_size: geo.size, num_urls: model.urls.count) } .aspectRatio(contentMode: filling ? .fill : .fit) .kfClickable() @@ -242,25 +362,19 @@ struct ImageCarousel: View { var Medias: some View { TabView(selection: $model.selectedIndex) { - ForEach(urls.indices, id: \.self) { index in + ForEach(model.urls.indices, id: \.self) { index in GeometryReader { geo in - Media(geo: geo, url: urls[index], index: index) + Media(geo: geo, url: model.urls[index], index: index) + .onChange(of: geo.size, perform: { new_size in + model.geo_size = new_size + }) + .onAppear { + model.geo_size = geo.size + } } } } .tabViewStyle(PageTabViewStyle(indexDisplayMode: .never)) - .fullScreenCover(isPresented: $model.open_sheet) { - if let content { - FullScreenCarouselView(video_coordinator: state.video, urls: urls, settings: state.settings, selectedIndex: $model.selectedIndex) { - content({ // Dismiss closure - model.open_sheet = false - }) - } - } - else { - FullScreenCarouselView(video_coordinator: state.video, urls: urls, settings: state.settings, selectedIndex: $model.selectedIndex) - } - } .frame(height: height) .onChange(of: model.selectedIndex) { value in model.selectedIndex = value @@ -278,8 +392,8 @@ struct ImageCarousel: View { } - if urls.count > 1 { - PageControlView(currentPage: $model.selectedIndex, numberOfPages: urls.count) + if model.urls.count > 1 { + PageControlView(currentPage: $model.selectedIndex, numberOfPages: model.urls.count) .frame(maxWidth: 0, maxHeight: 0) .padding(.top, 5) } @@ -287,27 +401,6 @@ struct ImageCarousel: View { } } -// MARK: - Image Modifier -extension KFOptionSetter { - /// Sets a block to get image size - /// - /// - Parameter block: The block which is used to read the image object. - /// - Returns: `Self` value after read size - public func imageFill(for size: CGSize, max: CGFloat, fill: CGFloat, block: @escaping (ImageFill) throws -> Void) -> Self { - let modifier = AnyImageModifier { image -> KFCrossPlatformImage in - let img_size = image.size - let geo_size = size - let fill = ImageFill.calculate_image_fill(geo_size: geo_size, img_size: img_size, maxHeight: max, fillHeight: fill) - DispatchQueue.main.async { [block, fill] in - try? block(fill) - } - return image - } - options.imageModifier = modifier - return self - } -} - public struct ImageFill { let filling: Bool? @@ -344,4 +437,3 @@ struct ImageCarousel_Previews: PreviewProvider { .environmentObject(OrientationTracker()) } } - diff --git a/damus/Util/Extensions/KFOptionSetter+.swift b/damus/Util/Extensions/KFOptionSetter+.swift index f1dea588..5b47a80b 100644 --- a/damus/Util/Extensions/KFOptionSetter+.swift +++ b/damus/Util/Extensions/KFOptionSetter+.swift @@ -58,6 +58,22 @@ extension KFOptionSetter { return self } + + /// This allows you to observe the size of the image, and get a callback when the size changes + /// This is useful for when you need to layout views based on the size of the image + /// - Parameter size_changed: A callback that will be called when the size of the image changes + /// - Returns: The same KFOptionSetter instance + func observe_image_size(size_changed: @escaping (CGSize) -> Void) -> Self { + let modifier = AnyImageModifier { image -> KFCrossPlatformImage in + let image_size = image.size + DispatchQueue.main.async { [size_changed, image_size] in + size_changed(image_size) + } + return image + } + options.imageModifier = modifier + return self + } } let MAX_FILE_SIZE = 20_971_520 // 20MiB