feat(wxread): align pagination, rendering, and docs
This commit is contained in:
shen
@@ -5,45 +5,51 @@ type: execute
|
||||
wave: 1
|
||||
depends_on: []
|
||||
files_modified:
|
||||
- Sources/RDReaderView/EPUBTextRendering/RDEPUBTextRenderer.swift
|
||||
- Sources/RDReaderView/EPUBTextRendering/RDEPUBTextLayouter.swift
|
||||
- Sources/RDReaderView/EPUBTextRendering/RDEPUBTextRendererSupport.swift
|
||||
- Sources/RDReaderView/EPUBTextRendering/RDEPUBDTCoreTextRenderer.swift
|
||||
- Sources/RDReaderView/EPUBUI/RDEPUBTextContentView.swift
|
||||
autonomous: false
|
||||
- Sources/RDReaderView/EPUBTextRendering/RDEPUBTextPaginationSupport.swift
|
||||
autonomous: true
|
||||
requirements:
|
||||
- QUAL-02
|
||||
- QUAL-03
|
||||
- QUAL-04
|
||||
must_haves:
|
||||
truths:
|
||||
- "All image attachments are constrained to max 1080x1920 via aspect-ratio-preserving scaling"
|
||||
- "Footnote images use width-only sizing (width:1em in CSS), no explicit height"
|
||||
- "DTMaxImageSize in dtOptions is 1080x1920, not screen bounds"
|
||||
- "Cover images still display correctly using screen-size base, capped by unified max"
|
||||
- "qrbodyPic images no longer overflow page height"
|
||||
- "avoidPageBreakInside blocks are not split across page boundaries"
|
||||
- "Orphan lines (last line of paragraph alone at page top) are prevented"
|
||||
- "Widow lines (first line of paragraph alone at page bottom) are prevented"
|
||||
- "Oversized images are scaled to fit within a single page height"
|
||||
- "Image attachment blocks have vertical centering applied"
|
||||
- "Image sizing and placement details appear in diagnostics"
|
||||
- "Dark mode preserves original image colors (no color inversion applied)"
|
||||
- "Page background information is surfaced in diagnostics for attachment blocks"
|
||||
artifacts:
|
||||
- path: "Sources/RDReaderView/EPUBTextRendering/RDEPUBTextRenderer.swift"
|
||||
provides: "RDEPUBTextLayoutConfig struct definition"
|
||||
contains: "struct RDEPUBTextLayoutConfig"
|
||||
- path: "Sources/RDReaderView/EPUBTextRendering/RDEPUBTextLayouter.swift"
|
||||
provides: "avoidPageBreakInside enforcement, orphan/widow control"
|
||||
contains: "orphanWidowAdjustedRange"
|
||||
- path: "Sources/RDReaderView/EPUBTextRendering/RDEPUBTextRendererSupport.swift"
|
||||
provides: "Unified max-size constraint in prepareHTMLElementForReaderRendering, cleaned-up footnote handling"
|
||||
contains: "CGSize(width: 1080, height: 1920)"
|
||||
- path: "Sources/RDReaderView/EPUBTextRendering/RDEPUBDTCoreTextRenderer.swift"
|
||||
provides: "Updated DTMaxImageSize from screen bounds to 1080x1920"
|
||||
contains: "CGSize(width: 1080, height: 1920)"
|
||||
provides: "General image fit-to-page sizing and centering"
|
||||
contains: "imageMaxHeight"
|
||||
key_links:
|
||||
- from: "RDEPUBDTCoreTextRenderer.dtOptions"
|
||||
to: "DTMaxImageSize"
|
||||
via: "builder option"
|
||||
pattern: "DTMaxImageSize.*CGSize.*1080"
|
||||
- from: "RDEPUBTextRendererSupport.prepareHTMLElementForReaderRendering"
|
||||
to: "DTTextAttachment.displaySize"
|
||||
via: "unified max-size scaling"
|
||||
pattern: "maxImageSize.*1080"
|
||||
- from: "RDEPUBTextLayouter.swift"
|
||||
to: "RDEPUBTextRenderer.swift"
|
||||
via: "RDEPUBTextLayoutConfig parameter in init"
|
||||
pattern: "config: RDEPUBTextLayoutConfig"
|
||||
- from: "RDEPUBTextRendererSupport.swift"
|
||||
to: "RDEPUBTextLayoutConfig"
|
||||
via: "imageMaxHeightRatio used in prepareHTMLElementForReaderRendering"
|
||||
pattern: "imageMaxHeightRatio"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Fix image display for all three image types (qrbodyPic, cover, footnote) by applying a unified maximum size constraint (1080x1920) aligned with WXRead's `_WRPostProcessElementTree`, and simplifying footnote sizing to width-only per WXRead's `replace.css`.
|
||||
Improve pagination quality for complex image-heavy chapters by enforcing avoidPageBreakInside (currently only a marker with no behavior), adding orphan/widow line control, and formalizing image sizing rules so oversized images fit within a single page.
|
||||
|
||||
Purpose: Complex illustrated chapters currently have images that overflow pages, inconsistent footnote sizing, and no unified ceiling. This fixes QUAL-02 (pagination quality), QUAL-03 (verifiable image rules), and addresses QUAL-04 (lightweight operation, no perf impact).
|
||||
Purpose: QUAL-02 requires reducing bad page breaks, orphan/widow lines, and image whitespace issues. QUAL-03 requires verifiable image sizing rules. The layouter currently has semantic boundary detection but does not enforce avoidPageBreakInside, and images have no general fit-to-page logic (only cover/footnote special cases).
|
||||
|
||||
Output: Updated `RDEPUBTextRendererSupport.swift` (main changes), `RDEPUBDTCoreTextRenderer.swift` (DTMaxImageSize), `RDEPUBTextContentView.swift` (remove footnote override).
|
||||
Output: Modified layouter with orphan/widow control and avoidPageBreakInside enforcement; new RDEPUBTextLayoutConfig struct; enhanced image sizing in renderer support.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@@ -57,207 +63,312 @@ Output: Updated `RDEPUBTextRendererSupport.swift` (main changes), `RDEPUBDTCoreT
|
||||
@.planning/STATE.md
|
||||
@.planning/phases/08-pagination-quality-cache-performance/08-CONTEXT.md
|
||||
@.planning/phases/08-pagination-quality-cache-performance/08-RESEARCH.md
|
||||
@.planning/phases/08-pagination-quality-cache-performance/08-PATTERNS.md
|
||||
@Sources/RDReaderView/EPUBTextRendering/RDEPUBTextLayouter.swift
|
||||
@Sources/RDReaderView/EPUBTextRendering/RDEPUBTextRenderer.swift
|
||||
@Sources/RDReaderView/EPUBTextRendering/RDEPUBTextRendererSupport.swift
|
||||
@Sources/RDReaderView/EPUBTextRendering/RDEPUBTextLayoutFrame.swift
|
||||
|
||||
<interfaces>
|
||||
<!-- Key types and contracts the executor needs -->
|
||||
|
||||
From DTCoreText (CocoaPod, DTTextAttachment):
|
||||
- `attachment.originalSize: CGSize` — the image's intrinsic pixel size
|
||||
- `attachment.displaySize: CGSize` — the size used for layout (set by code)
|
||||
- `attachment.verticalAlignment: DTTextAttachmentVerticalAlignment` — .center, .baseline, etc.
|
||||
- `element.displayStyle: DTHTMLElementDisplayStyle` — .inline, .block
|
||||
- `element.fontDescriptor: UIFontDescriptor` — font info from the element
|
||||
- `element.textAttachment: DTTextAttachment?` — the attachment on the element
|
||||
|
||||
From RDEPUBTextRendererSupport.swift (current implementation, lines 217-262):
|
||||
<!-- Current layouter init signature. From RDEPUBTextLayouter.swift line 10 -->
|
||||
```swift
|
||||
static func prepareHTMLElementForReaderRendering(
|
||||
_ element: DTHTMLElement,
|
||||
style: RDEPUBTextRenderStyle
|
||||
) {
|
||||
guard let attachment = element.textAttachment else { return }
|
||||
// ... class/path detection, pointSize calculation ...
|
||||
// footnote: height calc pointSize * 0.54, displaySize set
|
||||
// cover: UIScreen.main.bounds.insetBy(dx: 20, dy: 28).size scaling
|
||||
init(attributedString: NSAttributedString, pageSize: CGSize)
|
||||
```
|
||||
|
||||
<!-- Current adjustedRange return type. From RDEPUBTextLayouter.swift lines 64-77 -->
|
||||
```swift
|
||||
private func adjustedRange(
|
||||
from proposedRange: NSRange,
|
||||
totalLength: Int
|
||||
) -> (
|
||||
range: NSRange,
|
||||
breakReason: RDEPUBTextPageBreakReason,
|
||||
blockRange: NSRange?,
|
||||
attachmentRanges: [NSRange],
|
||||
attachmentKinds: [RDEPUBTextAttachmentKind],
|
||||
blockKinds: [RDEPUBTextBlockKind],
|
||||
semanticHints: [RDEPUBTextSemanticHint],
|
||||
attachmentPlacements: [RDEPUBTextAttachmentPlacement],
|
||||
diagnostics: [String]
|
||||
)
|
||||
```
|
||||
|
||||
<!-- Existing preferredSemanticBoundary with avoidPageBreakInside hint. From RDEPUBTextLayouter.swift lines 243-250 -->
|
||||
```swift
|
||||
if hints.contains(.avoidPageBreakInside),
|
||||
attributeRange.location > range.location,
|
||||
attributeRange.location < range.location + range.length,
|
||||
attributeRange.location >= minimumEnd,
|
||||
attributeEnd > range.location + range.length {
|
||||
boundary = (attributeRange.location, RDEPUBTextSemanticHint.avoidPageBreakInside.rawValue)
|
||||
stop.pointee = true
|
||||
}
|
||||
```
|
||||
|
||||
From RDEPUBDTCoreTextRenderer.swift (current implementation, line 100):
|
||||
<!-- Existing paragraph range helper. From RDEPUBTextLayouter.swift lines 294-298 -->
|
||||
```swift
|
||||
DTMaxImageSize: NSValue(cgSize: screenBounds.size) // ~353x757
|
||||
```
|
||||
|
||||
From RDEPUBTextContentView.swift (current implementation, lines 263-282):
|
||||
```swift
|
||||
private func normalizeInlineAttachments(in content: NSMutableAttributedString, basePointSize: CGFloat) {
|
||||
// footnote: targetHeight = pointSize * 0.14, displaySize set
|
||||
private func paragraphRange(containing location: Int) -> NSRange {
|
||||
let source = attributedString.string as NSString
|
||||
guard source.length > 0 else { return NSRange(location: 0, length: 0) }
|
||||
let safeLocation = min(max(location, 0), max(source.length - 1, 0))
|
||||
return source.paragraphRange(for: NSRange(location: safeLocation, length: 0))
|
||||
}
|
||||
```
|
||||
|
||||
From RDEPUBTextRendererSupport.swift (current implementation, lines 467-505):
|
||||
<!-- Existing image sizing pattern (cover only). From RDEPUBTextRendererSupport.swift lines 243-261 -->
|
||||
```swift
|
||||
private static func normalizeAttachmentDisplayIfNeeded(
|
||||
in attributes: inout [NSAttributedString.Key: Any],
|
||||
font: UIFont
|
||||
) {
|
||||
// footnote: targetHeight = font.pointSize * 0.14
|
||||
// cover: maxWidth = max(UIScreen.main.bounds.width - 48, font.lineHeight * 8)
|
||||
if lowercasedClasses.contains("rd-front-cover-image") || lowercasedPath == "cover.jpg" {
|
||||
let maxSize = UIScreen.main.bounds.insetBy(dx: 20, dy: 28).size
|
||||
let originalSize = attachment.originalSize
|
||||
if originalSize.width > 0, originalSize.height > 0 {
|
||||
let scale = min(maxSize.width / originalSize.width, maxSize.height / originalSize.height)
|
||||
attachment.displaySize = CGSize(
|
||||
width: round(originalSize.width * scale),
|
||||
height: round(originalSize.height * scale)
|
||||
)
|
||||
}
|
||||
attachment.verticalAlignment = .baseline
|
||||
element.displayStyle = .block
|
||||
}
|
||||
```
|
||||
|
||||
WXRead reference (Doc/WXRead/decompiled/WREpubTypesetter.m §672-701):
|
||||
- Checks only `originalSize.width > maxSize.width` (not height)
|
||||
- Max size is `CGSizeMake(1080, 1920)`
|
||||
- Scale: `maxSize.width / originalSize.width`
|
||||
- Sets `displaySize` to scaled size
|
||||
<!-- Existing RDEPUBTextRenderStyle struct pattern. From RDEPUBTextRenderer.swift lines 36-48 -->
|
||||
```swift
|
||||
public struct RDEPUBTextRenderStyle {
|
||||
public var font: UIFont
|
||||
public var lineSpacing: CGFloat
|
||||
public var textColor: UIColor?
|
||||
public var backgroundColor: UIColor?
|
||||
|
||||
public init(font: UIFont, lineSpacing: CGFloat, textColor: UIColor? = nil, backgroundColor: UIColor? = nil) {
|
||||
self.font = font
|
||||
self.lineSpacing = lineSpacing
|
||||
self.textColor = textColor
|
||||
self.backgroundColor = backgroundColor
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
<!-- WXRead reference: WRCoreTextLayoutFrame.avoidPageBreakInsideByRemovingLastLinesIfNeeded
|
||||
When the proposed page end falls inside an avoidPageBreakInside block:
|
||||
1. Find the block's start location
|
||||
2. If block start >= minimumEnd, break at block start (push entire block to next page)
|
||||
3. If block start < minimumEnd (block too large), fall through to frameLimit -->
|
||||
|
||||
<!-- rd_paginatedFrames extension (entry point). From RDEPUBTextPaginationSupport.swift lines 4-11 -->
|
||||
```swift
|
||||
extension NSAttributedString {
|
||||
func rd_paginatedFrames(
|
||||
size: CGSize,
|
||||
fragmentOffsets: [String: Int] = [:]
|
||||
) -> [RDEPUBTextLayoutFrame] {
|
||||
RDEPUBTextLayouter(attributedString: self, pageSize: size)
|
||||
.layoutFrames(fragmentOffsets: fragmentOffsets)
|
||||
}
|
||||
}
|
||||
```
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 1: Apply unified max-size constraint and fix footnote sizing across 3 files</name>
|
||||
<files>
|
||||
Sources/RDReaderView/EPUBTextRendering/RDEPUBTextRendererSupport.swift,
|
||||
Sources/RDReaderView/EPUBTextRendering/RDEPUBDTCoreTextRenderer.swift,
|
||||
Sources/RDReaderView/EPUBUI/RDEPUBTextContentView.swift
|
||||
</files>
|
||||
<task type="auto">
|
||||
<name>Task 1: Add RDEPUBTextLayoutConfig and wire into layouter + pagination support</name>
|
||||
<files>Sources/RDReaderView/EPUBTextRendering/RDEPUBTextRenderer.swift, Sources/RDReaderView/EPUBTextRendering/RDEPUBTextLayouter.swift, Sources/RDReaderView/EPUBTextRendering/RDEPUBTextPaginationSupport.swift</files>
|
||||
<read_first>
|
||||
- Sources/RDReaderView/EPUBTextRendering/RDEPUBTextRendererSupport.swift (full file — contains prepareHTMLElementForReaderRendering at L217, replaceCSS at L302, normalizeAttachmentHTMLMarkers at L345, normalizeAttachmentDisplayIfNeeded at L467)
|
||||
- Sources/RDReaderView/EPUBTextRendering/RDEPUBDTCoreTextRenderer.swift (L90-112 — dtOptions with DTMaxImageSize)
|
||||
- Sources/RDReaderView/EPUBUI/RDEPUBTextContentView.swift (L263-282 — normalizeInlineAttachments)
|
||||
- Doc/WXRead/decompiled/WREpubTypesetter.m §672-701 (reference: _WRPostProcessElementTree max size logic)
|
||||
- Doc/WXRead/resources/css/replace.css §91-103 (reference: .qqreader-footnote CSS)
|
||||
Sources/RDReaderView/EPUBTextRendering/RDEPUBTextRenderer.swift
|
||||
Sources/RDReaderView/EPUBTextRendering/RDEPUBTextLayouter.swift
|
||||
Sources/RDReaderView/EPUBTextRendering/RDEPUBTextPaginationSupport.swift
|
||||
Sources/RDReaderView/EPUBTextRendering/RDEPUBTextRendererSupport.swift
|
||||
</read_first>
|
||||
<behavior>
|
||||
- Test: DTMaxImageSize in dtOptions() is CGSize(width: 1080, height: 1920), not screen bounds
|
||||
- Test: prepareHTMLElementForReaderRendering applies unified max-size scaling before any type-specific handling
|
||||
- Test: Footnote branch in prepareHTMLElementForReaderRendering has NO height calculation (no pointSize * 0.54)
|
||||
- Test: Footnote branch only sets verticalAlignment = .center and displayStyle = .inline, then returns
|
||||
- Test: normalizeAttachmentHTMLMarkers footnote regex adds styleFragments with "width:1em" but NOT "height:1em"
|
||||
- Test: normalizeInlineAttachments does NOT contain pointSize * 0.14 height override for footnotes
|
||||
- Test: normalizeAttachmentDisplayIfNeeded does NOT contain font.pointSize * 0.14 height override for footnotes
|
||||
</behavior>
|
||||
<action>
|
||||
Apply changes across three files. Each change is described with its exact location and target state.
|
||||
**Step 1: Define RDEPUBTextLayoutConfig in RDEPUBTextRenderer.swift** (per D-02, D-03)
|
||||
|
||||
**File 1: RDEPUBDTCoreTextRenderer.swift — line 100**
|
||||
Insert `RDEPUBTextLayoutConfig` struct after `RDEPUBTextRenderStyle` (after line 48), following the same struct pattern:
|
||||
|
||||
Change `DTMaxImageSize` value from `NSValue(cgSize: screenBounds.size)` to `NSValue(cgSize: CGSize(width: 1080, height: 1920))`. The `screenBounds` variable at line 92 can remain (used elsewhere or for future reference), but `DTMaxImageSize` must use the new constant. Per D-01.
|
||||
```swift
|
||||
public struct RDEPUBTextLayoutConfig: Equatable {
|
||||
public var avoidOrphans: Bool
|
||||
public var avoidWidows: Bool
|
||||
public var avoidPageBreakInsideEnabled: Bool
|
||||
public var imageMaxHeightRatio: CGFloat
|
||||
|
||||
**File 2: RDEPUBTextRendererSupport.swift — 4 locations**
|
||||
public init(
|
||||
avoidOrphans: Bool = true,
|
||||
avoidWidows: Bool = true,
|
||||
avoidPageBreakInsideEnabled: Bool = true,
|
||||
imageMaxHeightRatio: CGFloat = 0.85
|
||||
) {
|
||||
self.avoidOrphans = avoidOrphans
|
||||
self.avoidWidows = avoidWidows
|
||||
self.avoidPageBreakInsideEnabled = avoidPageBreakInsideEnabled
|
||||
self.imageMaxHeightRatio = imageMaxHeightRatio
|
||||
}
|
||||
|
||||
**Location A: prepareHTMLElementForReaderRendering (L217-262) — Add unified max-size block**
|
||||
|
||||
Insert a new block immediately after `guard let attachment = element.textAttachment else { return }` (line 221), BEFORE the existing class/path detection (line 223). The block:
|
||||
|
||||
```
|
||||
let maxImageSize = CGSize(width: 1080, height: 1920)
|
||||
let originalSize = attachment.originalSize
|
||||
if originalSize.width > 0, originalSize.height > 0,
|
||||
(originalSize.width > maxImageSize.width || originalSize.height > maxImageSize.height) {
|
||||
let scale = min(maxImageSize.width / originalSize.width,
|
||||
maxImageSize.height / originalSize.height)
|
||||
attachment.displaySize = CGSize(
|
||||
width: round(originalSize.width * scale),
|
||||
height: round(originalSize.height * scale)
|
||||
)
|
||||
public static let `default` = RDEPUBTextLayoutConfig()
|
||||
}
|
||||
```
|
||||
|
||||
Per D-01, D-02, D-03. This applies to ALL images before any type-specific handling.
|
||||
This follows the exact same pattern as `RDEPUBTextRenderStyle` at lines 36-48: public struct, public stored properties, public init with defaults, static default instance.
|
||||
|
||||
**Location B: prepareHTMLElementForReaderRendering footnote branch (L228-241) — Remove height calculation**
|
||||
**Step 2: Add config parameter to RDEPUBTextLayouter**
|
||||
|
||||
In `RDEPUBTextLayouter.swift`:
|
||||
- Add stored property: `private let config: RDEPUBTextLayoutConfig`
|
||||
- Update init signature: `init(attributedString: NSAttributedString, pageSize: CGSize, config: RDEPUBTextLayoutConfig = .default)`
|
||||
- Store config: `self.config = config`
|
||||
|
||||
**Step 3: Update rd_paginatedFrames extension**
|
||||
|
||||
In `RDEPUBTextPaginationSupport.swift`, update the `rd_paginatedFrames` method to pass through config:
|
||||
|
||||
Current code at lines 229-233:
|
||||
```swift
|
||||
let targetHeight = max(round(pointSize * 0.54), 1)
|
||||
let originalSize = attachment.originalSize
|
||||
let aspectRatio = originalSize.height > 0 ? originalSize.width / originalSize.height : 1
|
||||
let targetWidth = max(round(targetHeight * max(aspectRatio, 0.1)), 1)
|
||||
attachment.displaySize = CGSize(width: targetWidth, height: targetHeight)
|
||||
func rd_paginatedFrames(
|
||||
size: CGSize,
|
||||
fragmentOffsets: [String: Int] = [:],
|
||||
config: RDEPUBTextLayoutConfig = .default
|
||||
) -> [RDEPUBTextLayoutFrame] {
|
||||
RDEPUBTextLayouter(attributedString: self, pageSize: size, config: config)
|
||||
.layoutFrames(fragmentOffsets: fragmentOffsets)
|
||||
}
|
||||
```
|
||||
|
||||
Replace with: remove the height/width calculation entirely. Keep only `attachment.verticalAlignment = .center` and `element.displayStyle = .inline`. The `return` at the end stays. Per D-07, D-08. Width is already set via HTML preprocessing (`width:1em` in normalizeAttachmentHTMLMarkers). DTCoreText auto-calculates height from aspect ratio when only width is set.
|
||||
|
||||
Also remove the `pointSize` local variable usage from this branch — `pointSize` is still used by the logging line, so keep the variable declaration but it's no longer used for sizing.
|
||||
|
||||
**Location C: normalizeAttachmentHTMLMarkers (L359-361) — Remove height:1em from footnote style fragments**
|
||||
|
||||
Current styleFragments at line 360-361:
|
||||
```swift
|
||||
"width:1em",
|
||||
"height:1em",
|
||||
```
|
||||
|
||||
Change to:
|
||||
```swift
|
||||
"width:1em",
|
||||
```
|
||||
|
||||
Remove `"height:1em"` line. Per D-10.
|
||||
|
||||
**Location D: normalizeAttachmentDisplayIfNeeded (L477-482) — Remove footnote height override**
|
||||
|
||||
Lines 477-483 currently compute footnote displaySize with `font.pointSize * 0.14`. Remove the entire `if lowercasedClasses.contains("qqreader-footnote") || lowercasedPath == "note.png"` block (lines 477-483). The footnote is already handled correctly by the unified max-size + width-only CSS approach in prepareHTMLElementForReaderRendering and normalizeAttachmentHTMLMarkers. Per D-08, D-09.
|
||||
|
||||
Keep the cover path (lines 484-494) — it acts as a fallback and does not conflict.
|
||||
|
||||
**File 3: RDEPUBTextContentView.swift — normalizeInlineAttachments (L263-282)**
|
||||
|
||||
Remove the footnote height override logic inside `normalizeInlineAttachments`. Lines 270-278:
|
||||
```swift
|
||||
guard classes.contains("qqreader-footnote") || source.contains("note.png") else { return }
|
||||
let pointSize = max(basePointSize, 1)
|
||||
let targetHeight = max(round(pointSize * 0.14), 1)
|
||||
...
|
||||
attachment.displaySize = CGSize(width: targetWidth, height: targetHeight)
|
||||
```
|
||||
|
||||
Replace the entire DTTextAttachment branch body with just a guard + return (skip footnote attachments entirely — they're already sized correctly from the renderer pipeline). The function should still iterate but do nothing for footnotes. Per D-09.
|
||||
|
||||
**Build verification:** After all changes, run `xcodebuild build` to confirm compilation.
|
||||
The `config` parameter has a default value so all existing call sites continue to work without changes.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>xcodebuild build -project ReadViewDemo/ReadViewDemo.xcodeproj -scheme ReadViewDemo -destination 'platform=iOS Simulator,name=iPhone 16' 2>&1 | tail -5</automated>
|
||||
<automated>xcodebuild build -scheme ReadViewDemo -destination 'platform=iOS Simulator,name=iPhone 17' 2>&1 | tail -5</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- RDEPUBDTCoreTextRenderer.swift: DTMaxImageSize line contains `CGSize(width: 1080, height: 1920)` (not screenBounds.size)
|
||||
- RDEPUBTextRendererSupport.swift: prepareHTMLElementForReaderRendering contains `let maxImageSize = CGSize(width: 1080, height: 1920)` BEFORE any class/path detection
|
||||
- RDEPUBTextRendererSupport.swift: prepareHTMLElementForReaderRendering footnote branch does NOT contain `pointSize * 0.54` or any `targetHeight`/`targetWidth` calculation
|
||||
- RDEPUBTextRendererSupport.swift: prepareHTMLElementForReaderRendering footnote branch contains `attachment.verticalAlignment = .center` and `element.displayStyle = .inline`
|
||||
- RDEPUBTextRendererSupport.swift: normalizeAttachmentHTMLMarkers footnote styleFragments contain `"width:1em"` but NOT `"height:1em"`
|
||||
- RDEPUBTextRendererSupport.swift: normalizeAttachmentDisplayIfNeeded does NOT contain `pointSize * 0.14` or `font.pointSize * 0.14` for footnote path
|
||||
- RDEPUBTextContentView.swift: normalizeInlineAttachments does NOT contain `pointSize * 0.14` height override for footnotes
|
||||
- xcodebuild build exits 0
|
||||
- `RDEPUBTextLayoutConfig` struct exists in RDEPUBTextRenderer.swift with properties: `avoidOrphans`, `avoidWidows`, `avoidPageBreakInsideEnabled`, `imageMaxHeightRatio`
|
||||
- `RDEPUBTextLayoutConfig` has `public static let default` instance
|
||||
- `RDEPUBTextLayouter` init accepts `config: RDEPUBTextLayoutConfig` parameter with `.default` default value
|
||||
- `rd_paginatedFrames` extension accepts `config: RDEPUBTextLayoutConfig` parameter with `.default` default value
|
||||
- Build succeeds with no errors at existing call sites (default parameter maintains backward compatibility)
|
||||
</acceptance_criteria>
|
||||
<done>All three image sizing code paths are updated: unified 1080x1920 max-size in prepareHTMLElementForReaderRendering, DTMaxImageSize set to 1080x1920, footnote height overrides removed from all three locations, footnote CSS uses width-only (width:1em), build compiles</done>
|
||||
<done>
|
||||
RDEPUBTextLayoutConfig struct defined in RDEPUBTextRenderer.swift; layouter and rd_paginatedFrames accept optional config parameter with backward-compatible defaults. Build succeeds.
|
||||
</done>
|
||||
</task>
|
||||
|
||||
<task type="checkpoint:human-verify" gate="blocking">
|
||||
<name>Task 2: Visual verification of image display fix</name>
|
||||
<what-built>
|
||||
Unified image sizing constraint applied across the native text rendering pipeline:
|
||||
- All images constrained to max 1080x1920 (WXRead alignment)
|
||||
- Footnote images: width-only sizing (width:1em), height auto-calculated from aspect ratio
|
||||
- Cover images: screen-size base, capped by unified max
|
||||
- qrbodyPic images: constrained by unified max (no more height overflow)
|
||||
<task type="auto">
|
||||
<name>Task 2: Implement avoidPageBreakInside enforcement and orphan/widow control in layouter</name>
|
||||
<files>Sources/RDReaderView/EPUBTextRendering/RDEPUBTextLayouter.swift</files>
|
||||
<read_first>
|
||||
Sources/RDReaderView/EPUBTextRendering/RDEPUBTextLayouter.swift
|
||||
Sources/RDReaderView/EPUBTextRendering/RDEPUBTextRenderer.swift
|
||||
Sources/RDReaderView/EPUBCore/RDEPUBReadingModels.swift
|
||||
</read_first>
|
||||
<action>
|
||||
Modify `RDEPUBTextLayouter.swift` to add two pagination quality improvements. Both use the `config` property added in Task 1.
|
||||
|
||||
Changed files:
|
||||
- RDEPUBDTCoreTextRenderer.swift (DTMaxImageSize)
|
||||
- RDEPUBTextRendererSupport.swift (prepareHTMLElementForReaderRendering, normalizeAttachmentHTMLMarkers, normalizeAttachmentDisplayIfNeeded)
|
||||
- RDEPUBTextContentView.swift (normalizeInlineAttachments)
|
||||
</what-built>
|
||||
<how-to-verify>
|
||||
1. Build and run ReadViewDemo on simulator
|
||||
2. Open the demo book "宝山辽墓材料与释读"
|
||||
3. Navigate to the cover page — verify cover image displays correctly (not cropped, not stretched, centered)
|
||||
4. Navigate to Chapter 5 (has qrbodyPic images) — verify images display fully within page bounds, no height overflow
|
||||
5. Navigate to a chapter with footnote images — verify footnote images are inline with text, same height as text characters, not oversized
|
||||
6. Switch to dark mode — verify images still display correctly
|
||||
7. Check that page breaks around images are reasonable (no orphan images at page tops/bottoms)
|
||||
</how-to-verify>
|
||||
<resume-signal>Type "approved" if all image types display correctly, or describe which image type has issues</resume-signal>
|
||||
**A. avoidPageBreakInside enforcement** (per D-02, QUAL-02, WXRead reference: `avoidPageBreakInsideByRemovingLastLinesIfNeeded`)
|
||||
|
||||
The current `preferredSemanticBoundary` at lines 243-250 only triggers avoidPageBreakInside when the block START is the boundary point (i.e., the block starts after minimumEnd and extends past page end). This misses the case where the page end falls IN THE MIDDLE of an avoidPageBreakInside block.
|
||||
|
||||
Add a new private method `avoidPageBreakInsideBoundary(in:proposedRange:minimumEnd:)` that, after the existing `preferredSemanticBoundary` check and before `preferredAttachmentBoundary` in `adjustedRange()`:
|
||||
|
||||
1. Enumerate `.rdPageSemanticHints` in the proposed range
|
||||
2. For each range containing `.avoidPageBreakInside`:
|
||||
- If the attribute range overlaps with the last portion of the page (i.e., the attribute range contains characters near `pageEnd`)
|
||||
- And the attribute's start location is >= `minimumEnd` (so pushing to block start is reasonable)
|
||||
- Then return the attribute range's start location as the break point
|
||||
3. If `config.avoidPageBreakInsideEnabled` is false, skip this check entirely
|
||||
|
||||
Insert this check in `adjustedRange()` between the `preferredSemanticBoundary` block (line 114-139) and the `preferredAttachmentBoundary` block (line 141). When triggered, use `breakReason: .semanticBoundary` and add `"avoidPageBreakInside enforcement"` to diagnostics.
|
||||
|
||||
**B. Orphan/widow control** (per QUAL-02)
|
||||
|
||||
Add a new private method `orphanWidowAdjustedRange(from:totalLength:)` that:
|
||||
|
||||
1. **Orphan check** (config.avoidOrphans): If the page starts at the beginning of a new paragraph, check if only 1-2 lines of that paragraph fit on the page. If so, and if pulling back 1-2 lines from the previous page would not cause that page to lose too much content, adjust the break point backward to include those lines. Use `paragraphRange(containing:)` to find paragraph boundaries.
|
||||
- Implementation: Check if the first paragraph on the page has fewer than 2 lines worth of characters (heuristic: `paragraphRange.length < averageLineHeight * 2.5`). If so, move the break point to include more of this paragraph from the previous page, respecting minimumEnd.
|
||||
|
||||
2. **Widow check** (config.avoidWidows): If the page ends with just 1-2 lines of a paragraph, and the next page would start with the continuation of that same paragraph, check if pushing 1-2 lines forward would help. Use `paragraphRange(containing:)` to detect this.
|
||||
- Implementation: At the proposed page end, get the paragraph range. If the remaining portion of the paragraph after pageEnd is small (fewer than 2 lines), pull the break point back to before this paragraph, forcing the entire paragraph to the next page. Respect minimumEnd.
|
||||
|
||||
Insert this check after `avoidPageBreakInsideBoundary` and before `preferredAttachmentBoundary` in `adjustedRange()`. When triggered, use `breakReason: .semanticBoundary` and add `"orphan control"` or `"widow control"` to diagnostics.
|
||||
|
||||
**C. Diagnostics enhancement**: When avoidPageBreakInside or orphan/widow triggers, append a descriptive string to the diagnostics array, e.g., `"avoidPageBreakInside: pushed block to next page"`, `"orphan control: included 1 line from previous page"`, `"widow control: pushed paragraph to next page"`.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>xcodebuild build -scheme ReadViewDemo -destination 'platform=iOS Simulator,name=iPhone 17' 2>&1 | tail -5</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `adjustedRange()` in RDEPUBTextLayouter contains a check for `avoidPageBreakInside` after `preferredSemanticBoundary` and before `preferredAttachmentBoundary`
|
||||
- The avoidPageBreakInside check is gated on `config.avoidPageBreakInsideEnabled`
|
||||
- Orphan control method exists and is called in `adjustedRange()` flow
|
||||
- Widow control method exists and is called in `adjustedRange()` flow
|
||||
- Both orphan and widow checks are gated on `config.avoidOrphans` and `config.avoidWidows` respectively
|
||||
- Diagnostic strings are appended when avoidPageBreakInside/orphan/widow triggers
|
||||
- Build succeeds
|
||||
</acceptance_criteria>
|
||||
<done>
|
||||
Layouter enforces avoidPageBreakInside by detecting when page end falls inside an avoidPageBreakInside block and pushing the block to the next page. Orphan/widow control prevents single-line paragraphs at page boundaries. All gated via RDEPUBTextLayoutConfig. Build succeeds.
|
||||
</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 3: Add general image fit-to-page sizing and enhanced diagnostics in renderer support</name>
|
||||
<files>Sources/RDReaderView/EPUBTextRendering/RDEPUBTextRendererSupport.swift</files>
|
||||
<read_first>
|
||||
Sources/RDReaderView/EPUBTextRendering/RDEPUBTextRendererSupport.swift
|
||||
Sources/RDReaderView/EPUBTextRendering/RDEPUBTextRenderer.swift
|
||||
</read_first>
|
||||
<action>
|
||||
Modify `prepareHTMLElementForReaderRendering` in `RDEPUBTextRendererSupport.swift` (inside the `#if canImport(DTCoreText)` block, lines 216-262) to add general image sizing for all non-special-case images.
|
||||
|
||||
**Current state**: The method handles two special cases:
|
||||
- Footnote images (qqreader-footnote) at lines 228-241: inline, scaled to font size
|
||||
- Cover images (rd-front-cover-image) at lines 243-261: block display, scaled to fit screen
|
||||
|
||||
**Add general image handling** after the cover image check (after line 261, before the closing brace of the method). This handles ALL other images that are not footnotes or covers:
|
||||
|
||||
1. **Fit-to-page height** (per D-03, QUAL-03, WXRead reference: images should not span pages):
|
||||
- Check if `attachment.originalSize` has valid dimensions (width > 0, height > 0)
|
||||
- Calculate `maxImageHeight = pageSize.height * imageMaxHeightRatio` — but since `prepareHTMLElementForReaderRendering` does not receive pageSize, use `UIScreen.main.bounds.insetBy(dx: 20, dy: 28).height` as the max height (same pattern used for cover images at line 244)
|
||||
- If `originalSize.height > maxImageHeight`: scale proportionally so display height = maxImageHeight
|
||||
- If `originalSize.width > maxWidth` (screen width minus insets): scale proportionally
|
||||
- Set `attachment.displaySize` to the scaled dimensions
|
||||
- The scale formula: `let scale = min(maxWidth / originalSize.width, maxImageHeight / originalSize.height)`
|
||||
|
||||
2. **Vertical centering for block-level images** (per D-03, WXRead reference: `wr-vertical-center-style`):
|
||||
- If the element has `displayStyle == .block` (or is a figure/bodyPic), set `attachment.verticalAlignment = .center`
|
||||
- This aligns with the existing `attachmentPlacements` mechanism that already tracks `.centered` placement
|
||||
|
||||
3. **Enhanced diagnostic output** (per QUAL-03):
|
||||
- After setting displaySize, print a diagnostic line: `print("[EPUB][Attachment] general image path=\(lowercasedPath) original=\(string(from: originalSize)) display=\(string(from: attachment.displaySize)) scaled=\(wasScaled)")`
|
||||
- Use the existing `string(from:)` helper at line 424 to format CGSize values
|
||||
- Track `wasScaled` boolean: true if displaySize differs from originalSize
|
||||
|
||||
4. **Dark mode image preservation** (per D-03):
|
||||
- Images in dark mode must preserve original colors (no color inversion). DTCoreText's `NSAttributedString` rendering already preserves image attachment colors by default.
|
||||
- Add a diagnostic check: if `UITraitCollection.current.userInterfaceStyle == .dark`, emit `print("[EPUB][Attachment] dark mode: image colors preserved for path=\(lowercasedPath)")` to verify the behavior is observable.
|
||||
- This is a declarative rule — ensure no future code adds `tintColor` overrides or color filters to image attachments in dark mode.
|
||||
|
||||
5. **Page background diagnostics** (per D-03):
|
||||
- For attachment blocks, emit a diagnostic that includes the block's background color if one is set on the attributed string: `print("[EPUB][Attachment] background: path=\(lowercasedPath) hasBackground=\(hasBackground)")` where `hasBackground` checks if `.backgroundColor` attribute exists at the attachment location.
|
||||
- This provides verifiable evidence of background handling without changing rendering behavior.
|
||||
|
||||
6. **Guard condition**: Only apply this general sizing if the image is NOT already handled by the footnote or cover checks above. Structure: the footnote check returns early (already does), the cover check returns early (add `return` at the end of the cover block), then the general image check runs for all remaining images.
|
||||
|
||||
**Important**: Add `return` at the end of the cover image block (after line 261) so the general image check does not double-process cover images. The current cover block does NOT have a `return` — it falls through.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>xcodebuild build -scheme ReadViewDemo -destination 'platform=iOS Simulator,name=iPhone 17' 2>&1 | tail -5</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `prepareHTMLElementForReaderRendering` contains a general image sizing block after the cover image block
|
||||
- General image block checks `originalSize.height > maxImageHeight` and scales proportionally
|
||||
- General image block sets `attachment.verticalAlignment = .center` for block-level images
|
||||
- Cover image block has `return` to prevent fall-through to general image handling
|
||||
- Diagnostic `print("[EPUB][Attachment] general image ...")` is emitted with original/display sizes
|
||||
- Dark mode diagnostic `print("[EPUB][Attachment] dark mode: image colors preserved ...")` is emitted when userInterfaceStyle == .dark
|
||||
- Background diagnostic `print("[EPUB][Attachment] background: ...")` is emitted for attachment blocks
|
||||
- Build succeeds
|
||||
</acceptance_criteria>
|
||||
<done>
|
||||
General images (not footnote, not cover) are scaled to fit within page height, vertically centered when block-level, dark mode preserves original image colors, page background info diagnosed, and all decisions logged via console output. Build succeeds.
|
||||
</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
@@ -267,29 +378,34 @@ Changed files:
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| EPUB HTML input -> DTCoreText parser | Untrusted HTML/CSS from EPUB files enters the rendering pipeline |
|
||||
| DTCoreText attachment -> willFlushCallback | Image dimensions from EPUB metadata could be arbitrary |
|
||||
| CoreText layout → page break decisions | Layouter now makes more aggressive break adjustments; incorrect logic could produce empty or overlapping pages |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|-------------|-----------------|
|
||||
| T-08-01 | Tampering | Image size from EPUB metadata | mitigate | Unified max-size constraint (1080x1920) caps any image regardless of declared dimensions |
|
||||
| T-08-02 | Denial of Service | Maliciously large images | mitigate | DTMaxImageSize (first pass) + willFlushCallback max-size (second pass) ensure no image exceeds 1080x1920 display size |
|
||||
| T-08-SC | Tampering | npm/pip/cargo installs | accept | No new packages installed in this phase |
|
||||
| Threat ID | Category | Component | Disposition | Mitigation |
|
||||
|-----------|----------|-----------|-------------|------------|
|
||||
| T-08-03 | Tampering | RDEPUBTextLayoutConfig defaults | accept | Config is a value type with safe defaults; no external input can modify it |
|
||||
| T-08-SC | Tampering | npm/pip/cargo installs | mitigate | No external packages installed in this phase |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- xcodebuild build succeeds
|
||||
- Visual inspection: cover, qrbodyPic, and footnote images all display correctly
|
||||
- No regressions in dark mode
|
||||
- `xcodebuild build -scheme ReadViewDemo` succeeds
|
||||
- RDEPUBTextLayoutConfig struct exists in RDEPUBTextRenderer.swift
|
||||
- Layouter adjustedRange includes avoidPageBreakInside, orphan, and widow checks
|
||||
- prepareHTMLElementForReaderRendering handles general image sizing
|
||||
- Runtime log shows `[EPUB][Attachment] general image` for non-special images
|
||||
- Runtime log shows `[EPUB][Attachment] dark mode: image colors preserved` in dark mode
|
||||
- Runtime log shows `[EPUB][Attachment] background:` for attachment blocks
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- All images constrained to 1080x1920 max (QUAL-03: verifiable processing rules)
|
||||
- Complex illustrated chapters display without image overflow or unreasonable whitespace (QUAL-02)
|
||||
- Image sizing is a lightweight operation in willFlushCallback with negligible perf impact (QUAL-04)
|
||||
- Footnote images are inline, text-height, width-only (WXRead alignment)
|
||||
- avoidPageBreakInside blocks are never split across page boundaries when config enabled
|
||||
- Orphan/widow control reduces single-line paragraphs at page boundaries
|
||||
- Oversized images scale to fit within page height
|
||||
- Dark mode preserves original image colors (no inversion)
|
||||
- Page background information is diagnosed for attachment blocks
|
||||
- Diagnostic output confirms image sizing decisions
|
||||
- Build succeeds with no regressions
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
|
||||
Reference in New Issue
Block a user