From 84238562fad7e0632885e7a1535b2d5a062919ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mickae=CC=88l=20Menu?= Date: Thu, 4 Dec 2025 14:47:36 +0100 Subject: [PATCH 1/6] Implement PDF fit --- Sources/Navigator/PDF/PDFDocumentView.swift | 155 ++++++++++++++++++ .../PDF/PDFNavigatorViewController.swift | 21 ++- .../PDF/Preferences/PDFPreferences.swift | 6 + .../Preferences/PDFPreferencesEditor.swift | 12 ++ .../PDF/Preferences/PDFSettings.swift | 8 + Sources/Navigator/Preferences/Types.swift | 11 +- .../Common/Preferences/UserPreferences.swift | 6 +- 7 files changed, 208 insertions(+), 11 deletions(-) diff --git a/Sources/Navigator/PDF/PDFDocumentView.swift b/Sources/Navigator/PDF/PDFDocumentView.swift index d8de95220..19a1b1fcc 100644 --- a/Sources/Navigator/PDF/PDFDocumentView.swift +++ b/Sources/Navigator/PDF/PDFDocumentView.swift @@ -70,4 +70,159 @@ public final class PDFDocumentView: PDFView { editingActions.buildMenu(with: builder) super.buildMenu(with: builder) } + + var isPaginated: Bool { + isUsingPageViewController || displayMode == .twoUp || displayMode == .singlePage + } + + /// Calculates the appropriate scale factor based on the fit preference. + /// + /// Only used in scroll mode, as the paginated mode doesn't support custom + /// scale factors without visual hiccups when swiping pages. + func scaleFactor(for fit: Fit, contentInset: UIEdgeInsets) -> CGFloat { + // While a custom fit works in scroll mode, the paginated mode has + // critical limitations when zooming larger than the page fit. + // + // - Visual snap: There is no API to pre-set the zoom scale for the next + // page. ⁠PDFView resets the scale per page, causing a visible snap + // when swiping. We don’t see the issue with edge taps. + // - Incorrect anchoring: When zooming larger than the page fit, the + // viewport centers vertically instead of showing the top. The API to + // fix this works in scroll mode but is ignored in paginated mode. + guard !isPaginated else { + return scaleFactorForSizeToFit + } + + switch fit { + case .auto, .width: + // Use PDFKit's default auto-fit behavior + return scaleFactorForSizeToFit + case .page: + return scaleFactorForLargestPage(contentInset: contentInset) + } + } + + /// Calculates the scale factor to fit the largest page or spread (by area) + /// to the viewport. + private func scaleFactorForLargestPage( + contentInset: UIEdgeInsets + ) -> CGFloat { + guard let document = document else { + return 1.0 + } + + let spread = (displayMode == .twoUp || displayMode == .twoUpContinuous) + + // Check cache before expensive calculation + let viewSize = bounds.size + if + let cached = cachedScaleFactorForLargestPage, + cached.document == ObjectIdentifier(document), + cached.viewSize == viewSize, + cached.contentInset == contentInset, + cached.spread == spread, + cached.displaysAsBook == displaysAsBook + { + return cached.scaleFactor + } + + var maxSize: CGSize = .zero + var maxArea: CGFloat = 0 + + if !spread { + // No spreads: find largest individual page + for pageIndex in 0 ..< document.pageCount { + guard let page = document.page(at: pageIndex) else { continue } + let pageSize = page.bounds(for: displayBox).size + let area = pageSize.width * pageSize.height + + if area > maxArea { + maxArea = area + maxSize = pageSize + } + } + } else { + // Spreads enabled: find largest spread + let pageCount = document.pageCount + + if displaysAsBook, pageCount > 0 { + // First page displayed alone - check its size + if let firstPage = document.page(at: 0) { + let firstSize = firstPage.bounds(for: displayBox).size + let firstArea = firstSize.width * firstSize.height + if firstArea > maxArea { + maxArea = firstArea + maxSize = firstSize + } + } + } + + // Check spreads (pairs of pages) + let startIndex = displaysAsBook ? 1 : 0 + for pageIndex in stride(from: startIndex, to: pageCount, by: 2) { + let leftIndex = pageIndex + let rightIndex = pageIndex + 1 + + guard let leftPage = document.page(at: leftIndex) else { continue } + let leftSize = leftPage.bounds(for: displayBox).size + + if rightIndex < pageCount, let rightPage = document.page(at: rightIndex) { + // Two-page spread + let rightSize = rightPage.bounds(for: displayBox).size + let spreadSize = CGSize( + width: leftSize.width + rightSize.width, + height: max(leftSize.height, rightSize.height) + ) + let spreadArea = spreadSize.width * spreadSize.height + + if spreadArea > maxArea { + maxArea = spreadArea + maxSize = spreadSize + } + } else { + // Last page alone (odd page count) + let singleArea = leftSize.width * leftSize.height + if singleArea > maxArea { + maxArea = singleArea + maxSize = leftSize + } + } + } + } + + var scale: CGFloat = 1.0 + if maxSize.width > 0, maxSize.height > 0 { + // Account for content insets + let availableSize = CGSize( + width: viewSize.width - contentInset.left - contentInset.right, + height: viewSize.height - contentInset.top - contentInset.bottom + ) + + let widthScale = availableSize.width / maxSize.width + let heightScale = availableSize.height / maxSize.height + + // Use the smaller scale to ensure both dimensions fit + scale = min(widthScale, heightScale) + } + + cachedScaleFactorForLargestPage = ( + document: ObjectIdentifier(document), + scaleFactor: scale, + viewSize: viewSize, + contentInset: contentInset, + spread: spread, + displaysAsBook: displaysAsBook + ) + return scale + } + + /// Cache for expensive largest page scale calculation. + private var cachedScaleFactorForLargestPage: ( + document: ObjectIdentifier, + scaleFactor: CGFloat, + viewSize: CGSize, + contentInset: UIEdgeInsets, + spread: Bool, + displaysAsBook: Bool + )? } diff --git a/Sources/Navigator/PDF/PDFNavigatorViewController.swift b/Sources/Navigator/PDF/PDFNavigatorViewController.swift index f2207ea48..28787f74c 100644 --- a/Sources/Navigator/PDF/PDFNavigatorViewController.swift +++ b/Sources/Navigator/PDF/PDFNavigatorViewController.swift @@ -201,9 +201,10 @@ open class PDFNavigatorViewController: if let pdfView = pdfView { // Makes sure that the PDF is always properly scaled down when // rotating the screen, if the user didn't zoom in. - let isAtMinScaleFactor = (pdfView.scaleFactor == pdfView.minScaleFactor) + let isAtMinScale = (pdfView.scaleFactor == pdfView.minScaleFactor) + coordinator.animate(alongsideTransition: { _ in - self.updateScaleFactors(zoomToFit: isAtMinScaleFactor) + self.updateScaleFactors(zoomToFit: isAtMinScale) // Reset the PDF view to update the spread if needed. if self.settings.spread == .auto { @@ -489,11 +490,23 @@ open class PDFNavigatorViewController: guard let pdfView = pdfView else { return } - pdfView.minScaleFactor = pdfView.scaleFactorForSizeToFit + + let scaleFactorToFit = pdfView.scaleFactor( + for: settings.fit, + contentInset: delegate?.navigatorContentInset(self) ?? .zero + ) + + if settings.scroll { + // Allow zooming out to 25% in scroll mode. + pdfView.minScaleFactor = 0.25 + } else { + pdfView.minScaleFactor = scaleFactorToFit + } + pdfView.maxScaleFactor = 4.0 if zoomToFit { - pdfView.scaleFactor = pdfView.minScaleFactor + pdfView.scaleFactor = scaleFactorToFit } } diff --git a/Sources/Navigator/PDF/Preferences/PDFPreferences.swift b/Sources/Navigator/PDF/Preferences/PDFPreferences.swift index 8c3212024..5a61ea2e6 100644 --- a/Sources/Navigator/PDF/Preferences/PDFPreferences.swift +++ b/Sources/Navigator/PDF/Preferences/PDFPreferences.swift @@ -14,6 +14,9 @@ public struct PDFPreferences: ConfigurablePreferences { /// Background color behind the document pages. public var backgroundColor: Color? + /// Method for fitting the pages within the viewport. + public var fit: Fit? + /// Indicates if the first page should be displayed in its own spread. public var offsetFirstPage: Bool? @@ -41,6 +44,7 @@ public struct PDFPreferences: ConfigurablePreferences { public init( backgroundColor: Color? = nil, + fit: Fit? = nil, offsetFirstPage: Bool? = nil, pageSpacing: Double? = nil, readingProgression: ReadingProgression? = nil, @@ -51,6 +55,7 @@ public struct PDFPreferences: ConfigurablePreferences { ) { precondition(pageSpacing == nil || pageSpacing! >= 0) self.backgroundColor = backgroundColor + self.fit = fit self.offsetFirstPage = offsetFirstPage self.pageSpacing = pageSpacing self.readingProgression = readingProgression @@ -63,6 +68,7 @@ public struct PDFPreferences: ConfigurablePreferences { public func merging(_ other: PDFPreferences) -> PDFPreferences { PDFPreferences( backgroundColor: other.backgroundColor ?? backgroundColor, + fit: other.fit ?? fit, offsetFirstPage: other.offsetFirstPage ?? offsetFirstPage, pageSpacing: other.pageSpacing ?? pageSpacing, readingProgression: other.readingProgression ?? readingProgression, diff --git a/Sources/Navigator/PDF/Preferences/PDFPreferencesEditor.swift b/Sources/Navigator/PDF/Preferences/PDFPreferencesEditor.swift index 28d1f9e25..71a15de0a 100644 --- a/Sources/Navigator/PDF/Preferences/PDFPreferencesEditor.swift +++ b/Sources/Navigator/PDF/Preferences/PDFPreferencesEditor.swift @@ -32,6 +32,18 @@ public final class PDFPreferencesEditor: StatefulPreferencesEditor = + enumPreference( + preference: \.fit, + setting: \.fit, + defaultEffectiveValue: defaults.fit ?? .auto, + isEffective: { $0.settings.scroll }, + supportedValues: [.auto, .page, .width] + ) + /// Indicates if the first page should be displayed in its own spread. /// /// Only effective when `spread` is not off. diff --git a/Sources/Navigator/PDF/Preferences/PDFSettings.swift b/Sources/Navigator/PDF/Preferences/PDFSettings.swift index 82ac91463..c370f8cd3 100644 --- a/Sources/Navigator/PDF/Preferences/PDFSettings.swift +++ b/Sources/Navigator/PDF/Preferences/PDFSettings.swift @@ -13,6 +13,7 @@ import ReadiumShared /// See `PDFPreferences` public struct PDFSettings: ConfigurableSettings { public let backgroundColor: Color? + public let fit: Fit public let offsetFirstPage: Bool public let pageSpacing: Double public let readingProgression: ReadingProgression @@ -25,6 +26,10 @@ public struct PDFSettings: ConfigurableSettings { backgroundColor = preferences.backgroundColor ?? defaults.backgroundColor + fit = preferences.fit + ?? defaults.fit + ?? .auto + offsetFirstPage = preferences.offsetFirstPage ?? defaults.offsetFirstPage ?? false @@ -64,6 +69,7 @@ public struct PDFSettings: ConfigurableSettings { /// See `PDFPreferences`. public struct PDFDefaults { public var backgroundColor: Color? + public var fit: Fit? public var offsetFirstPage: Bool? public var pageSpacing: Double? public var readingProgression: ReadingProgression? @@ -74,6 +80,7 @@ public struct PDFDefaults { public init( backgroundColor: Color? = nil, + fit: Fit? = nil, offsetFirstPage: Bool? = nil, pageSpacing: Double? = nil, readingProgression: ReadingProgression? = nil, @@ -83,6 +90,7 @@ public struct PDFDefaults { visibleScrollbar: Bool? = nil ) { self.backgroundColor = backgroundColor + self.fit = fit self.offsetFirstPage = offsetFirstPage self.pageSpacing = pageSpacing self.readingProgression = readingProgression diff --git a/Sources/Navigator/Preferences/Types.swift b/Sources/Navigator/Preferences/Types.swift index 18d6e1493..ef8a923e8 100644 --- a/Sources/Navigator/Preferences/Types.swift +++ b/Sources/Navigator/Preferences/Types.swift @@ -58,12 +58,15 @@ extension ReadiumShared.ReadingProgression { } } -/// Method for constraining a resource inside the viewport. +/// Method for fitting the content within the viewport. public enum Fit: String, Codable, Hashable { - case cover - case contain + /// Use the best fitting strategy depending on the current settings and + /// content. + case auto + /// The content is scaled to fit both dimensions within the viewport. + case page + /// The content is scaled to fit the viewport width. case width - case height } /// Reader theme for reflowable documents. diff --git a/TestApp/Sources/Reader/Common/Preferences/UserPreferences.swift b/TestApp/Sources/Reader/Common/Preferences/UserPreferences.swift index 759b30341..214f88e29 100644 --- a/TestApp/Sources/Reader/Common/Preferences/UserPreferences.swift +++ b/TestApp/Sources/Reader/Common/Preferences/UserPreferences.swift @@ -79,6 +79,7 @@ struct UserPreferences< case let editor as PDFPreferencesEditor: fixedLayoutUserPreferences( commit: commit, + fit: editor.fit, offsetFirstPage: editor.offsetFirstPage, pageSpacing: editor.pageSpacing, readingProgression: editor.readingProgression, @@ -272,10 +273,9 @@ struct UserPreferences< commit: commit, formatValue: { v in switch v { - case .cover: return "Cover" - case .contain: return "Contain" + case .auto: return "Auto" + case .page: return "Page" case .width: return "Width" - case .height: return "Height" } } ) From 30ebab2adf706807df6a0f5d7a590ccda4fadb02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mickae=CC=88l=20Menu?= Date: Thu, 4 Dec 2025 15:12:32 +0100 Subject: [PATCH 2/6] Fix reset to zoom factor when rotating the screen --- Sources/Navigator/PDF/PDFDocumentView.swift | 9 +++++++++ .../PDF/PDFNavigatorViewController.swift | 17 ++++++++++++----- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/Sources/Navigator/PDF/PDFDocumentView.swift b/Sources/Navigator/PDF/PDFDocumentView.swift index 19a1b1fcc..e5ac32bc6 100644 --- a/Sources/Navigator/PDF/PDFDocumentView.swift +++ b/Sources/Navigator/PDF/PDFDocumentView.swift @@ -75,6 +75,15 @@ public final class PDFDocumentView: PDFView { isUsingPageViewController || displayMode == .twoUp || displayMode == .singlePage } + /// Returns whether the document is currently zoomed to match the given + /// `fit`. + func isAtScaleFactor(for fit: Fit, contentInset: UIEdgeInsets) -> Bool { + let scaleFactorToFit = scaleFactor(for: fit, contentInset: contentInset) + // 1% tolerance for floating point comparison + let tolerance: CGFloat = 0.01 + return abs(scaleFactor - scaleFactorToFit) < tolerance + } + /// Calculates the appropriate scale factor based on the fit preference. /// /// Only used in scroll mode, as the paginated mode doesn't support custom diff --git a/Sources/Navigator/PDF/PDFNavigatorViewController.swift b/Sources/Navigator/PDF/PDFNavigatorViewController.swift index 28787f74c..95a1dad32 100644 --- a/Sources/Navigator/PDF/PDFNavigatorViewController.swift +++ b/Sources/Navigator/PDF/PDFNavigatorViewController.swift @@ -84,6 +84,10 @@ open class PDFNavigatorViewController: private let publicationEndpoint: HTTPServerEndpoint? private var publicationBaseURL: HTTPURL! + private var contentInset: UIEdgeInsets { + delegate?.navigatorContentInset(self) ?? .zero + } + public init( publication: Publication, initialLocation: Locator?, @@ -199,12 +203,15 @@ open class PDFNavigatorViewController: super.viewWillTransition(to: size, with: coordinator) if let pdfView = pdfView { - // Makes sure that the PDF is always properly scaled down when - // rotating the screen, if the user didn't zoom in. - let isAtMinScale = (pdfView.scaleFactor == pdfView.minScaleFactor) + // Makes sure that the PDF is always properly scaled when rotating + // the screen, if the user didn't set a custom zoom. + let isAtScaleFactor = pdfView.isAtScaleFactor( + for: settings.fit, + contentInset: contentInset + ) coordinator.animate(alongsideTransition: { _ in - self.updateScaleFactors(zoomToFit: isAtMinScale) + self.updateScaleFactors(zoomToFit: isAtScaleFactor) // Reset the PDF view to update the spread if needed. if self.settings.spread == .auto { @@ -493,7 +500,7 @@ open class PDFNavigatorViewController: let scaleFactorToFit = pdfView.scaleFactor( for: settings.fit, - contentInset: delegate?.navigatorContentInset(self) ?? .zero + contentInset: contentInset ) if settings.scroll { From 72d03644369b3cc4b791c9d5a4b50220dfbaf14c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mickae=CC=88l=20Menu?= Date: Thu, 4 Dec 2025 16:30:31 +0100 Subject: [PATCH 3/6] Fix the scale factor in paginated spreads --- Sources/Navigator/PDF/PDFDocumentView.swift | 108 +++++++++++++----- .../PDF/PDFNavigatorViewController.swift | 3 +- 2 files changed, 79 insertions(+), 32 deletions(-) diff --git a/Sources/Navigator/PDF/PDFDocumentView.swift b/Sources/Navigator/PDF/PDFDocumentView.swift index e5ac32bc6..63bf62208 100644 --- a/Sources/Navigator/PDF/PDFDocumentView.swift +++ b/Sources/Navigator/PDF/PDFDocumentView.swift @@ -75,6 +75,10 @@ public final class PDFDocumentView: PDFView { isUsingPageViewController || displayMode == .twoUp || displayMode == .singlePage } + var isSpreadEnabled: Bool { + displayMode == .twoUp || displayMode == .twoUpContinuous + } + /// Returns whether the document is currently zoomed to match the given /// `fit`. func isAtScaleFactor(for fit: Fit, contentInset: UIEdgeInsets) -> Bool { @@ -89,8 +93,9 @@ public final class PDFDocumentView: PDFView { /// Only used in scroll mode, as the paginated mode doesn't support custom /// scale factors without visual hiccups when swiping pages. func scaleFactor(for fit: Fit, contentInset: UIEdgeInsets) -> CGFloat { - // While a custom fit works in scroll mode, the paginated mode has - // critical limitations when zooming larger than the page fit. + // While a `width` fit works in scroll mode, the pagination mode has + // critical limitations when zooming larger than the page fit, so it + // does not support a `width` fit. // // - Visual snap: There is no API to pre-set the zoom scale for the next // page. ⁠PDFView resets the scale per page, causing a visible snap @@ -98,8 +103,12 @@ public final class PDFDocumentView: PDFView { // - Incorrect anchoring: When zooming larger than the page fit, the // viewport centers vertically instead of showing the top. The API to // fix this works in scroll mode but is ignored in paginated mode. - guard !isPaginated else { - return scaleFactorForSizeToFit + // + // So we only support a `page` fit in paginated mode. + if isPaginated { + return scaleFactorForSizeToFitVisiblePages( + contentInset: contentInset + ) } switch fit { @@ -111,6 +120,24 @@ public final class PDFDocumentView: PDFView { } } + /// Calculates the scale factor to fit the visible pages (by area) to the + /// viewport. + private func scaleFactorForSizeToFitVisiblePages( + contentInset: UIEdgeInsets + ) -> CGFloat { + // The native `scaleFactorForSizeToFit` is incorrect when displaying + // paginated spreads, so we need to use a custom implementation. + if !isPaginated || !isSpreadEnabled { + scaleFactorForSizeToFit + } else { + calculateScale( + for: spreadSize(for: visiblePages), + viewSize: bounds.size, + contentInset: contentInset + ) + } + } + /// Calculates the scale factor to fit the largest page or spread (by area) /// to the viewport. private func scaleFactorForLargestPage( @@ -120,8 +147,6 @@ public final class PDFDocumentView: PDFView { return 1.0 } - let spread = (displayMode == .twoUp || displayMode == .twoUpContinuous) - // Check cache before expensive calculation let viewSize = bounds.size if @@ -129,7 +154,7 @@ public final class PDFDocumentView: PDFView { cached.document == ObjectIdentifier(document), cached.viewSize == viewSize, cached.contentInset == contentInset, - cached.spread == spread, + cached.spread == isSpreadEnabled, cached.displaysAsBook == displaysAsBook { return cached.scaleFactor @@ -138,7 +163,7 @@ public final class PDFDocumentView: PDFView { var maxSize: CGSize = .zero var maxArea: CGFloat = 0 - if !spread { + if !isSpreadEnabled { // No spreads: find largest individual page for pageIndex in 0 ..< document.pageCount { guard let page = document.page(at: pageIndex) else { continue } @@ -173,23 +198,19 @@ public final class PDFDocumentView: PDFView { let rightIndex = pageIndex + 1 guard let leftPage = document.page(at: leftIndex) else { continue } - let leftSize = leftPage.bounds(for: displayBox).size if rightIndex < pageCount, let rightPage = document.page(at: rightIndex) { // Two-page spread - let rightSize = rightPage.bounds(for: displayBox).size - let spreadSize = CGSize( - width: leftSize.width + rightSize.width, - height: max(leftSize.height, rightSize.height) - ) - let spreadArea = spreadSize.width * spreadSize.height + let currentSpreadSize = spreadSize(for: [leftPage, rightPage]) + let spreadArea = currentSpreadSize.width * currentSpreadSize.height if spreadArea > maxArea { maxArea = spreadArea - maxSize = spreadSize + maxSize = currentSpreadSize } } else { // Last page alone (odd page count) + let leftSize = leftPage.bounds(for: displayBox).size let singleArea = leftSize.width * leftSize.height if singleArea > maxArea { maxArea = singleArea @@ -199,27 +220,18 @@ public final class PDFDocumentView: PDFView { } } - var scale: CGFloat = 1.0 - if maxSize.width > 0, maxSize.height > 0 { - // Account for content insets - let availableSize = CGSize( - width: viewSize.width - contentInset.left - contentInset.right, - height: viewSize.height - contentInset.top - contentInset.bottom - ) - - let widthScale = availableSize.width / maxSize.width - let heightScale = availableSize.height / maxSize.height - - // Use the smaller scale to ensure both dimensions fit - scale = min(widthScale, heightScale) - } + let scale = calculateScale( + for: maxSize, + viewSize: viewSize, + contentInset: contentInset + ) cachedScaleFactorForLargestPage = ( document: ObjectIdentifier(document), scaleFactor: scale, viewSize: viewSize, contentInset: contentInset, - spread: spread, + spread: isSpreadEnabled, displaysAsBook: displaysAsBook ) return scale @@ -234,4 +246,38 @@ public final class PDFDocumentView: PDFView { spread: Bool, displaysAsBook: Bool )? + + /// Calculates the combined size of pages laid out side-by-side horizontally. + private func spreadSize(for pages: [PDFPage]) -> CGSize { + var size = CGSize.zero + for page in pages { + let pageBounds = page.bounds(for: displayBox) + size.height = max(size.height, pageBounds.height) + size.width += pageBounds.width + } + return size + } + + /// Calculates the scale factor needed to fit the given content size within + /// the available viewport, accounting for content insets. + private func calculateScale( + for contentSize: CGSize, + viewSize: CGSize, + contentInset: UIEdgeInsets + ) -> CGFloat { + guard contentSize.width > 0, contentSize.height > 0 else { + return 1.0 + } + + let availableSize = CGSize( + width: viewSize.width - contentInset.left - contentInset.right, + height: viewSize.height - contentInset.top - contentInset.bottom + ) + + let widthScale = availableSize.width / contentSize.width + let heightScale = availableSize.height / contentSize.height + + // Use the smaller scale to ensure both dimensions fit + return min(widthScale, heightScale) + } } diff --git a/Sources/Navigator/PDF/PDFNavigatorViewController.swift b/Sources/Navigator/PDF/PDFNavigatorViewController.swift index 95a1dad32..cfa1cbf33 100644 --- a/Sources/Navigator/PDF/PDFNavigatorViewController.swift +++ b/Sources/Navigator/PDF/PDFNavigatorViewController.swift @@ -411,7 +411,8 @@ open class PDFNavigatorViewController: @objc private func visiblePagesDidChange() { // In paginated mode, we want to refresh the scale factors to properly - // fit the newly visible pages. + // fit the newly visible pages. This is especially important for + // paginated spreads. if !settings.scroll { updateScaleFactors(zoomToFit: true) } From 4669c01493e3c48d212e0c04f61dd1e0b799ecc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mickae=CC=88l=20Menu?= Date: Thu, 4 Dec 2025 16:48:26 +0100 Subject: [PATCH 4/6] Fix content inset handling --- Sources/Navigator/PDF/PDFDocumentView.swift | 57 ++++++++++++------- .../PDF/PDFNavigatorViewController.swift | 14 +---- 2 files changed, 38 insertions(+), 33 deletions(-) diff --git a/Sources/Navigator/PDF/PDFDocumentView.swift b/Sources/Navigator/PDF/PDFDocumentView.swift index 63bf62208..881608152 100644 --- a/Sources/Navigator/PDF/PDFDocumentView.swift +++ b/Sources/Navigator/PDF/PDFDocumentView.swift @@ -50,11 +50,31 @@ public final class PDFDocumentView: PDFView { } private func updateContentInset() { - let insets = documentViewDelegate?.pdfDocumentViewContentInset(self) ?? window?.safeAreaInsets ?? .zero + let insets = contentInset firstScrollView?.contentInset.top = insets.top firstScrollView?.contentInset.bottom = insets.bottom } + private var contentInset: UIEdgeInsets { + if let contentInset = documentViewDelegate?.pdfDocumentViewContentInset(self) { + return contentInset + } + + // We apply the window's safe area insets (representing the system + // status bar, but ignoring app bars) on iPhones only because in most + // cases we prefer to display the content edge-to-edge. + // iPhones are a special case because they are the only devices with a + // physical notch (or Dynamic Island) which is included in the window's + // safe area insets. Therefore, we must always take it into account to + // avoid hiding the content. + if UIDevice.current.userInterfaceIdiom == .phone { + return window?.safeAreaInsets ?? .zero + } else { + // Edge-to-edge on macOS and iPadOS. + return .zero + } + } + override public func canPerformAction(_ action: Selector, withSender sender: Any?) -> Bool { super.canPerformAction(action, withSender: sender) && editingActions.canPerformAction(action) } @@ -81,8 +101,8 @@ public final class PDFDocumentView: PDFView { /// Returns whether the document is currently zoomed to match the given /// `fit`. - func isAtScaleFactor(for fit: Fit, contentInset: UIEdgeInsets) -> Bool { - let scaleFactorToFit = scaleFactor(for: fit, contentInset: contentInset) + func isAtScaleFactor(for fit: Fit) -> Bool { + let scaleFactorToFit = scaleFactor(for: fit) // 1% tolerance for floating point comparison let tolerance: CGFloat = 0.01 return abs(scaleFactor - scaleFactorToFit) < tolerance @@ -92,7 +112,7 @@ public final class PDFDocumentView: PDFView { /// /// Only used in scroll mode, as the paginated mode doesn't support custom /// scale factors without visual hiccups when swiping pages. - func scaleFactor(for fit: Fit, contentInset: UIEdgeInsets) -> CGFloat { + func scaleFactor(for fit: Fit) -> CGFloat { // While a `width` fit works in scroll mode, the pagination mode has // critical limitations when zooming larger than the page fit, so it // does not support a `width` fit. @@ -106,9 +126,7 @@ public final class PDFDocumentView: PDFView { // // So we only support a `page` fit in paginated mode. if isPaginated { - return scaleFactorForSizeToFitVisiblePages( - contentInset: contentInset - ) + return scaleFactorForSizeToFitVisiblePages } switch fit { @@ -116,15 +134,13 @@ public final class PDFDocumentView: PDFView { // Use PDFKit's default auto-fit behavior return scaleFactorForSizeToFit case .page: - return scaleFactorForLargestPage(contentInset: contentInset) + return scaleFactorForLargestPage } } /// Calculates the scale factor to fit the visible pages (by area) to the /// viewport. - private func scaleFactorForSizeToFitVisiblePages( - contentInset: UIEdgeInsets - ) -> CGFloat { + private var scaleFactorForSizeToFitVisiblePages: CGFloat { // The native `scaleFactorForSizeToFit` is incorrect when displaying // paginated spreads, so we need to use a custom implementation. if !isPaginated || !isSpreadEnabled { @@ -133,27 +149,26 @@ public final class PDFDocumentView: PDFView { calculateScale( for: spreadSize(for: visiblePages), viewSize: bounds.size, - contentInset: contentInset + insets: contentInset ) } } /// Calculates the scale factor to fit the largest page or spread (by area) /// to the viewport. - private func scaleFactorForLargestPage( - contentInset: UIEdgeInsets - ) -> CGFloat { + private var scaleFactorForLargestPage: CGFloat { guard let document = document else { return 1.0 } // Check cache before expensive calculation let viewSize = bounds.size + let insets = contentInset if let cached = cachedScaleFactorForLargestPage, cached.document == ObjectIdentifier(document), cached.viewSize == viewSize, - cached.contentInset == contentInset, + cached.contentInset == insets, cached.spread == isSpreadEnabled, cached.displaysAsBook == displaysAsBook { @@ -223,14 +238,14 @@ public final class PDFDocumentView: PDFView { let scale = calculateScale( for: maxSize, viewSize: viewSize, - contentInset: contentInset + insets: insets ) cachedScaleFactorForLargestPage = ( document: ObjectIdentifier(document), scaleFactor: scale, viewSize: viewSize, - contentInset: contentInset, + contentInset: insets, spread: isSpreadEnabled, displaysAsBook: displaysAsBook ) @@ -263,15 +278,15 @@ public final class PDFDocumentView: PDFView { private func calculateScale( for contentSize: CGSize, viewSize: CGSize, - contentInset: UIEdgeInsets + insets: UIEdgeInsets ) -> CGFloat { guard contentSize.width > 0, contentSize.height > 0 else { return 1.0 } let availableSize = CGSize( - width: viewSize.width - contentInset.left - contentInset.right, - height: viewSize.height - contentInset.top - contentInset.bottom + width: viewSize.width - insets.left - insets.right, + height: viewSize.height - insets.top - insets.bottom ) let widthScale = availableSize.width / contentSize.width diff --git a/Sources/Navigator/PDF/PDFNavigatorViewController.swift b/Sources/Navigator/PDF/PDFNavigatorViewController.swift index cfa1cbf33..527c54999 100644 --- a/Sources/Navigator/PDF/PDFNavigatorViewController.swift +++ b/Sources/Navigator/PDF/PDFNavigatorViewController.swift @@ -84,10 +84,6 @@ open class PDFNavigatorViewController: private let publicationEndpoint: HTTPServerEndpoint? private var publicationBaseURL: HTTPURL! - private var contentInset: UIEdgeInsets { - delegate?.navigatorContentInset(self) ?? .zero - } - public init( publication: Publication, initialLocation: Locator?, @@ -205,10 +201,7 @@ open class PDFNavigatorViewController: if let pdfView = pdfView { // Makes sure that the PDF is always properly scaled when rotating // the screen, if the user didn't set a custom zoom. - let isAtScaleFactor = pdfView.isAtScaleFactor( - for: settings.fit, - contentInset: contentInset - ) + let isAtScaleFactor = pdfView.isAtScaleFactor(for: settings.fit) coordinator.animate(alongsideTransition: { _ in self.updateScaleFactors(zoomToFit: isAtScaleFactor) @@ -499,10 +492,7 @@ open class PDFNavigatorViewController: return } - let scaleFactorToFit = pdfView.scaleFactor( - for: settings.fit, - contentInset: contentInset - ) + let scaleFactorToFit = pdfView.scaleFactor(for: settings.fit) if settings.scroll { // Allow zooming out to 25% in scroll mode. From 093a066b9490a5f59a68103ff97d5f64a0483b0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mickae=CC=88l=20Menu?= Date: Thu, 4 Dec 2025 16:57:10 +0100 Subject: [PATCH 5/6] Update changelog --- CHANGELOG.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 518fc35ad..959d97d76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ All notable changes to this project will be documented in this file. Take a look * This callback is called before executing any navigation action. * Useful for hiding UI elements when the user navigates, or implementing analytics. * Added swipe gesture support for navigating in PDF paginated spread mode. +* Added `fit` preference for PDF documents to control how pages are scaled within the viewport. + * Only effective in scroll mode. Paginated mode always uses page fit due to PDFKit limitations. ### Deprecated @@ -26,6 +28,15 @@ All notable changes to this project will be documented in this file. Take a look * Support for asynchronous callbacks with `onCreatePublication` (contributed by [@smoores-dev](https://github.com/readium/swift-toolkit/pull/673)). +#### Navigator + +* The `Fit` enum has been redesigned to fit the PDF implementation. + * **Breaking change:** Update any code using the old `Fit` enum values. +* The PDF navigator's content inset behavior has changed: + * iPhone: Continues to apply window safe area insets (to account for notch/Dynamic Island). + * iPad/macOS: Now displays edge-to-edge with no automatic safe area insets. + * You can customize this behavior with `VisualNavigatorDelegate.navigatorContentInset(_:)`. + ### Fixed #### Navigator From 62a360320223863926114c377ae6456a4db44369 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Menu?= Date: Thu, 4 Dec 2025 17:08:40 +0100 Subject: [PATCH 6/6] Update Sources/Navigator/PDF/PDFDocumentView.swift Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- Sources/Navigator/PDF/PDFDocumentView.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Navigator/PDF/PDFDocumentView.swift b/Sources/Navigator/PDF/PDFDocumentView.swift index 881608152..11e9cf70e 100644 --- a/Sources/Navigator/PDF/PDFDocumentView.swift +++ b/Sources/Navigator/PDF/PDFDocumentView.swift @@ -118,7 +118,7 @@ public final class PDFDocumentView: PDFView { // does not support a `width` fit. // // - Visual snap: There is no API to pre-set the zoom scale for the next - // page. ⁠PDFView resets the scale per page, causing a visible snap + // page. PDFView resets the scale per page, causing a visible snap // when swiping. We don’t see the issue with edge taps. // - Incorrect anchoring: When zooming larger than the page fit, the // viewport centers vertically instead of showing the top. The API to