fix: add bounds checking and content splitting for multi-page songs (Closes #17) (#20)

This commit was merged in pull request #20.
This commit is contained in:
2026-03-17 14:18:07 +01:00
parent 0fb2771279
commit a69d14033d
3 changed files with 536 additions and 32 deletions

View File

@@ -0,0 +1,109 @@
# Issue #17: Fix page overflow — bounds checking and content splitting
## Summary
The PDF renderer (`PdfBookRenderer.renderSongPage()`) currently renders all song sections on page 0 and leaves page 1 blank for 2-page songs. There is no bounds checking — the `y` coordinate can go below the bottom margin, causing content to render outside the visible page area. This plan adds proper `y`-position tracking against a minimum `yMin` boundary, splits content across pages at section boundaries when a song exceeds one page, and reserves space for bottom metadata/references on the last page.
## AC Verification Checklist
1. The renderer tracks `y` against the bottom margin during song page rendering
2. For 2-page songs, content splits across pages when it exceeds page 0's available space — remaining sections continue on page 1
3. Content that would be rendered below the bottom margin (minus reserved footer space) is moved to the next page
4. If metadata is "bottom" position, sufficient space is reserved at the bottom of the last page
5. No text or images are rendered outside the printable page area
6. Existing tests continue to pass
7. New tests verify content splitting for songs exceeding one page
## Implementation Steps
### Step 1: Add a section height calculation helper in PdfBookRenderer
Add a private method `calculateSectionHeight()` that computes how many PDF points a given `SongSection` will consume when rendered. This mirrors the measurement engine logic but uses the actual PDF `BaseFont` widths (not stubs). This is needed to decide whether a section fits on the current page.
The method signature:
```kotlin
private fun calculateSectionHeight(
section: SongSection,
fontMetrics: PdfFontMetrics,
config: BookConfig,
contentWidth: Float
): Float
```
### Step 2: Add footer space reservation calculation
Add a private method `calculateFooterReservation()` that computes how much vertical space must be reserved at the bottom of the **last** page of a song for:
- Bottom-position metadata (if `metadataPosition == "bottom"`)
- Notes
- Reference book footer
```kotlin
private fun calculateFooterReservation(
song: Song,
fontMetrics: PdfFontMetrics,
config: BookConfig,
contentWidth: Float
): Float
```
### Step 3: Refactor renderSongPage() to split content across pages
The key change: Instead of `val sections = if (pageIndex == 0) song.sections else emptyList()`, determine which sections belong on each page by:
1. Calculate `yMin` = bottom margin in points (plus footer reservation for the last page)
2. For `pageIndex == 0`: Render sections in order. Before rendering each section, check if the section's height fits above `yMin`. If not, stop — remaining sections go to page 1.
3. For `pageIndex == 1`: Render the sections that didn't fit on page 0. The split point is stored via a `splitIndex` that is computed during page 0 rendering.
**Approach:** Since `renderSongPage()` is called separately for page 0 and page 1, we need a way to know the split point on both calls. The cleanest approach is to compute the split index as a function:
```kotlin
private fun computeSplitIndex(
song: Song,
fontMetrics: PdfFontMetrics,
config: BookConfig,
contentWidth: Float,
availableHeight: Float // total space on page 0 (contentTop - yMin)
): Int // index of first section that goes to page 1
```
This method calculates the cumulative height of header + sections. When the cumulative height exceeds `availableHeight`, it returns the section index. If all sections fit, it returns `song.sections.size`.
### Step 4: Update renderSongPage() to use bounds checking during rendering
Even after determining the split, the actual rendering loop should still check `y >= yMin` as a safety net. If a section that was estimated to fit actually overflows (due to measurement inaccuracy), clamp rendering — do not render below `yMin`.
### Step 5: Update footer rendering for multi-page songs
Currently `isLastPage` is hardcoded to `pageIndex == 0`. Change it to correctly identify the last page:
- For 1-page songs: `pageIndex == 0` is the last page
- For 2-page songs: `pageIndex == 1` is the last page
The song's `pageCount` isn't directly available in the renderer, but we can determine it: if `pageIndex == 1`, it's always the last page. If `pageIndex == 0`, it's the last page only if the song fits on one page (i.e., `computeSplitIndex == song.sections.size`).
A simpler approach: pass the total page count as a parameter, or compute whether the song needs 2 pages inside `renderSongPage()`.
**Decision:** Add a `totalPages: Int` parameter to `renderSongPage()`. The caller already knows this from the `PageContent.SongPage` list (consecutive song pages with pageIndex 0 and 1 for the same song).
Actually, the simplest approach: The renderer sees `PageContent.SongPage(song, 0)` and `PageContent.SongPage(song, 1)` in the page list. We can pre-scan the pages list to know if a song has 2 pages. But even simpler: we can compute `computeSplitIndex` to know whether the song needs a second page. If `splitIndex < song.sections.size`, the song has 2 pages.
### Step 6: Move notes and bottom-metadata to the last page
Currently notes and bottom metadata only render on `pageIndex == 0`. Change this to render on the last page (which might be page 1 for 2-page songs). The logic:
- Compute `isLastPage` based on split index
- Render notes, bottom metadata, and reference footer only on the last page
### Step 7: Write tests
Add tests in `PdfBookRendererTest`:
1. `render handles two-page song with content split across pages` — Create a song with many sections that exceed one page, render with pageIndex 0 and 1, verify PDF is valid.
2. `render does not overflow below bottom margin` — Create a very long song, verify rendering completes without error.
3. `render places metadata at bottom of last page for two-page songs` — Use `metadataPosition = "bottom"`, create a 2-page song, verify PDF is valid.
4. `render handles notes on last page of two-page song` — Song with notes that spans 2 pages, verify rendering.
### Step 8: Verify existing tests pass
Run `gradle :renderer-pdf:test` and `gradle :app:test` to ensure no regressions.

View File

@@ -63,7 +63,7 @@ class PdfBookRenderer : BookRenderer {
renderSongPage(
cb, chordLyricRenderer, fontMetrics, config,
pageContent.song, pageContent.pageIndex,
contentTop, leftMargin, contentWidth
contentTop, leftMargin, contentWidth, marginBottom
)
}
is PageContent.FillerImage -> {
@@ -91,6 +91,166 @@ class PdfBookRenderer : BookRenderer {
document.close()
}
/**
* Computes the index of the first section that should be rendered on page 1.
* All sections before this index render on page 0; sections from this index
* onward render on page 1.
*
* If all sections fit on page 0, returns song.sections.size (i.e., nothing on page 1).
*/
private fun computeSplitIndex(
song: Song,
fontMetrics: PdfFontMetrics,
config: BookConfig,
contentWidth: Float,
availableHeightOnPage0: Float
): Int {
var consumed = 0f
// Header: title
consumed += config.fonts.title.size * 1.5f
// Top metadata
val renderMetaAtBottom = config.layout.metadataPosition == "bottom"
if (!renderMetaAtBottom) {
val metaParts = buildMetadataLines(song, config)
if (metaParts.isNotEmpty()) {
consumed += config.fonts.metadata.size * 1.8f * metaParts.size
}
}
// Key/capo
if (song.key != null || song.capo != null) {
consumed += config.fonts.metadata.size * 1.8f
}
consumed += 4f // gap before sections
for ((index, section) in song.sections.withIndex()) {
val sectionHeight = calculateSectionHeight(section, fontMetrics, config, contentWidth)
if (consumed + sectionHeight > availableHeightOnPage0) {
// This section doesn't fit on page 0
return index
}
consumed += sectionHeight
// Add verse spacing
consumed += config.layout.verseSpacing / 0.3528f
}
return song.sections.size
}
/**
* Calculates the height in PDF points that a section will consume when rendered.
*/
private fun calculateSectionHeight(
section: SongSection,
fontMetrics: PdfFontMetrics,
config: BookConfig,
contentWidth: Float
): Float {
var height = 0f
val metaSize = config.fonts.metadata.size
// Section label
if (section.label != null || section.type == SectionType.CHORUS) {
val labelText = section.label ?: when (section.type) {
SectionType.CHORUS -> "Refrain"
SectionType.REPEAT -> "Wiederholung"
else -> null
}
if (labelText != null) {
height += metaSize * 1.5f
}
}
// Empty chorus (reference)
if (section.type == SectionType.CHORUS && section.lines.isEmpty()) {
height += metaSize * 1.8f
return height
}
// Repeat start marker (contributes no extra height - drawn at current y)
// Lines
val chordSize = config.fonts.chords.size
val lyricSize = config.fonts.lyrics.size
val chordLineHeight = chordSize * 1.2f
val lyricLineHeight = lyricSize * 1.2f
val chordLyricGap = config.layout.chordLineSpacing / 0.3528f
for (line in section.lines) {
if (line.imagePath != null) {
// Inline image: 40mm max height + gaps
val maxImageHeight = 40f / 0.3528f
height += maxImageHeight + 6f
} else {
val hasChords = line.segments.any { it.chord != null }
var lineHeight = lyricLineHeight
if (hasChords) {
lineHeight += chordLineHeight + chordLyricGap
}
height += lineHeight + 1f // 1pt gap between lines
}
}
// Repeat end marker
if (section.type == SectionType.REPEAT) {
height += metaSize * 1.5f
}
return height
}
/**
* Calculates the space (in PDF points) that must be reserved at the bottom of
* the last page of a song for notes, bottom-position metadata, and reference footer.
*/
private fun calculateFooterReservation(
song: Song,
fontMetrics: PdfFontMetrics,
config: BookConfig,
contentWidth: Float
): Float {
var reserved = 0f
val metaFont = fontMetrics.getBaseFont(config.fonts.metadata)
val metaSize = config.fonts.metadata.size
// Notes
if (song.notes.isNotEmpty()) {
reserved += 4f // gap before notes
val noteLineHeight = metaSize * 1.5f
for ((idx, note) in song.notes.withIndex()) {
val wrappedLines = wrapText(note, metaFont, metaSize, contentWidth)
reserved += noteLineHeight * wrappedLines.size
if (idx < song.notes.size - 1) {
reserved += noteLineHeight * 0.3f
}
}
}
// Bottom metadata
if (config.layout.metadataPosition == "bottom") {
val metaParts = buildMetadataLines(song, config)
if (metaParts.isNotEmpty()) {
reserved += 4f
for (metaLine in metaParts) {
val wrappedLines = wrapText(metaLine, metaFont, metaSize, contentWidth)
reserved += metaSize * 1.5f * wrappedLines.size
}
}
}
// Reference footer
if (config.referenceBooks.isNotEmpty() && song.references.isNotEmpty()) {
val lineHeight = metaSize * 1.4f
reserved += lineHeight * 2 // two rows (headers + numbers)
reserved += lineHeight * 1.5f // separator line gap
reserved += lineHeight * 0.5f // bottom gap
}
return reserved
}
private fun renderSongPage(
cb: PdfContentByte,
chordLyricRenderer: ChordLyricRenderer,
@@ -100,12 +260,28 @@ class PdfBookRenderer : BookRenderer {
pageIndex: Int, // 0 for first page, 1 for second page of 2-page songs
contentTop: Float,
leftMargin: Float,
contentWidth: Float
contentWidth: Float,
bottomMargin: Float
) {
var y = contentTop
val renderMetaAtBottom = config.layout.metadataPosition == "bottom"
// Calculate the footer reservation for the last page
val footerReservation = calculateFooterReservation(song, fontMetrics, config, contentWidth)
// Compute the split index to determine which sections go on which page.
// Page 0 gets sections 0..<splitIndex, page 1 gets sections splitIndex..<size.
// Footer space is reserved on the last page only.
val availableOnPage0 = contentTop - bottomMargin -
(if (song.sections.size > 0) footerReservation else 0f)
val splitIndex = computeSplitIndex(song, fontMetrics, config, contentWidth, availableOnPage0)
val isTwoPageSong = splitIndex < song.sections.size
val isLastPage = if (isTwoPageSong) pageIndex == 1 else pageIndex == 0
// Bottom boundary for content on this page
val yMin = bottomMargin + (if (isLastPage) footerReservation else 0f)
if (pageIndex == 0) {
// Render title
val titleFont = fontMetrics.getBaseFont(config.fonts.title)
@@ -125,6 +301,7 @@ class PdfBookRenderer : BookRenderer {
val metaFont = fontMetrics.getBaseFont(config.fonts.metadata)
val metaSize = config.fonts.metadata.size
for (metaLine in metaParts) {
if (y - metaSize * 1.8f < yMin) break
cb.beginText()
cb.setFontAndSize(metaFont, metaSize)
cb.setColorFill(Color.GRAY)
@@ -143,24 +320,31 @@ class PdfBookRenderer : BookRenderer {
if (infoParts.isNotEmpty()) {
val metaFont = fontMetrics.getBaseFont(config.fonts.metadata)
val metaSize = config.fonts.metadata.size
cb.beginText()
cb.setFontAndSize(metaFont, metaSize)
cb.setColorFill(Color.GRAY)
cb.setTextMatrix(leftMargin, y - metaSize)
cb.showText(infoParts.joinToString(" | "))
cb.endText()
y -= metaSize * 1.8f
if (y - metaSize * 1.8f >= yMin) {
cb.beginText()
cb.setFontAndSize(metaFont, metaSize)
cb.setColorFill(Color.GRAY)
cb.setTextMatrix(leftMargin, y - metaSize)
cb.showText(infoParts.joinToString(" | "))
cb.endText()
y -= metaSize * 1.8f
}
}
y -= 4f // gap before sections
}
// Determine which sections to render on this page
// For simplicity in this implementation, render all sections on pageIndex 0
// A more sophisticated implementation would split sections across pages
val sections = if (pageIndex == 0) song.sections else emptyList()
val sections = if (pageIndex == 0) {
song.sections.subList(0, splitIndex)
} else {
song.sections.subList(splitIndex, song.sections.size)
}
for (section in sections) {
// Safety check: stop rendering if we've gone below the boundary
if (y < yMin) break
// Section label
if (section.label != null || section.type == SectionType.CHORUS) {
val labelText = section.label ?: when (section.type) {
@@ -171,6 +355,7 @@ class PdfBookRenderer : BookRenderer {
if (labelText != null) {
val metaFont = fontMetrics.getBaseFont(config.fonts.metadata)
val metaSize = config.fonts.metadata.size
if (y - metaSize * 1.5f < yMin) break
cb.beginText()
cb.setFontAndSize(metaFont, metaSize)
cb.setColorFill(Color.DARK_GRAY)
@@ -185,6 +370,7 @@ class PdfBookRenderer : BookRenderer {
if (section.type == SectionType.CHORUS && section.lines.isEmpty()) {
val metaFont = fontMetrics.getBaseFont(config.fonts.metadata)
val metaSize = config.fonts.metadata.size
if (y - metaSize * 1.8f < yMin) break
cb.beginText()
cb.setFontAndSize(metaFont, metaSize)
cb.setColorFill(Color.DARK_GRAY)
@@ -209,6 +395,7 @@ class PdfBookRenderer : BookRenderer {
// Render lines
for (line in section.lines) {
if (y < yMin) break
val imgPath = line.imagePath
if (imgPath != null) {
// Render inline image
@@ -223,21 +410,23 @@ class PdfBookRenderer : BookRenderer {
if (section.type == SectionType.REPEAT) {
val metaFont = fontMetrics.getBaseFont(config.fonts.metadata)
val metaSize = config.fonts.metadata.size
cb.beginText()
cb.setFontAndSize(metaFont, metaSize)
cb.setColorFill(Color.DARK_GRAY)
cb.setTextMatrix(leftMargin, y - metaSize)
cb.showText(":\u2502")
cb.endText()
y -= metaSize * 1.5f
if (y - metaSize * 1.5f >= yMin) {
cb.beginText()
cb.setFontAndSize(metaFont, metaSize)
cb.setColorFill(Color.DARK_GRAY)
cb.setTextMatrix(leftMargin, y - metaSize)
cb.showText(":\u2502")
cb.endText()
y -= metaSize * 1.5f
}
}
// Verse spacing
y -= config.layout.verseSpacing / 0.3528f
}
// Render notes at the bottom (with word-wrap for multi-paragraph notes)
if (pageIndex == 0 && song.notes.isNotEmpty()) {
// Render notes on the last page
if (isLastPage && song.notes.isNotEmpty()) {
y -= 4f
val metaFont = fontMetrics.getBaseFont(config.fonts.metadata)
val metaSize = config.fonts.metadata.size
@@ -261,8 +450,8 @@ class PdfBookRenderer : BookRenderer {
}
}
// Render metadata at bottom of song page (if configured)
if (renderMetaAtBottom && pageIndex == 0) {
// Render metadata at bottom of song page (if configured) - on the last page only
if (renderMetaAtBottom && isLastPage) {
val metaParts = buildMetadataLines(song, config)
if (metaParts.isNotEmpty()) {
y -= 4f
@@ -284,15 +473,12 @@ class PdfBookRenderer : BookRenderer {
}
// Render reference book footer on the last page of the song
if (config.referenceBooks.isNotEmpty() && song.references.isNotEmpty()) {
val isLastPage = (pageIndex == 0) // For now, all content renders on page 0
if (isLastPage) {
renderReferenceFooter(
cb, fontMetrics, config, song,
leftMargin, contentWidth,
config.layout.margins.bottom / 0.3528f
)
}
if (config.referenceBooks.isNotEmpty() && song.references.isNotEmpty() && isLastPage) {
renderReferenceFooter(
cb, fontMetrics, config, song,
leftMargin, contentWidth,
bottomMargin
)
}
}

View File

@@ -417,4 +417,213 @@ class PdfBookRendererTest {
baos.size() shouldBeGreaterThan 0
}
// --- Content splitting tests ---
private fun createLongSong(title: String = "Long Song"): Song {
// Create a song with many sections that will exceed one A5 page
val sections = (1..20).map { i ->
SongSection(
type = SectionType.VERSE,
label = "Verse $i",
lines = (1..4).map {
SongLine(
listOf(
LineSegment(chord = "Am", text = "Some text with chords "),
LineSegment(chord = "G", text = "and more text here")
)
)
}
)
}
return Song(title = title, sections = sections)
}
@Test
fun `render splits content across pages for two-page song`() {
val song = createLongSong()
val layout = LayoutResult(
tocPages = 0,
pages = listOf(
PageContent.SongPage(song, 0),
PageContent.SongPage(song, 1)
),
tocEntries = emptyList()
)
val baos = ByteArrayOutputStream()
renderer.render(layout, BookConfig(), baos)
baos.size() shouldBeGreaterThan 0
val bytes = baos.toByteArray()
val header = String(bytes.sliceArray(0..4))
header shouldBe "%PDF-"
}
@Test
fun `render does not overflow below bottom margin for very long song`() {
// Create an extremely long song
val sections = (1..40).map { i ->
SongSection(
type = SectionType.VERSE,
label = "Verse $i",
lines = (1..6).map {
SongLine(
listOf(
LineSegment(chord = "C", text = "A long line of text that should be rendered properly "),
LineSegment(chord = "G", text = "with chords above each segment")
)
)
}
)
}
val song = Song(title = "Very Long Song", sections = sections)
val layout = LayoutResult(
tocPages = 0,
pages = listOf(
PageContent.SongPage(song, 0),
PageContent.SongPage(song, 1)
),
tocEntries = emptyList()
)
val baos = ByteArrayOutputStream()
renderer.render(layout, BookConfig(), baos)
baos.size() shouldBeGreaterThan 0
}
@Test
fun `render places metadata at bottom of last page for two-page song`() {
val config = BookConfig(
layout = LayoutConfig(metadataPosition = "bottom")
)
val song = createLongSong().copy(
composer = "Bach",
lyricist = "Goethe"
)
val layout = LayoutResult(
tocPages = 0,
pages = listOf(
PageContent.SongPage(song, 0),
PageContent.SongPage(song, 1)
),
tocEntries = emptyList()
)
val baos = ByteArrayOutputStream()
renderer.render(layout, config, baos)
baos.size() shouldBeGreaterThan 0
}
@Test
fun `render places notes on last page of two-page song`() {
val song = createLongSong().copy(
notes = listOf("This is a note that should appear on the last page")
)
val layout = LayoutResult(
tocPages = 0,
pages = listOf(
PageContent.SongPage(song, 0),
PageContent.SongPage(song, 1)
),
tocEntries = emptyList()
)
val baos = ByteArrayOutputStream()
renderer.render(layout, BookConfig(), baos)
baos.size() shouldBeGreaterThan 0
}
@Test
fun `render places reference footer on last page of two-page song`() {
val config = BookConfig(
referenceBooks = listOf(
ReferenceBook(id = "mo", name = "Mundorgel", abbreviation = "MO"),
ReferenceBook(id = "pl", name = "Pfadfinderlied", abbreviation = "PL")
)
)
val song = createLongSong().copy(
references = mapOf("mo" to 42, "pl" to 17)
)
val layout = LayoutResult(
tocPages = 0,
pages = listOf(
PageContent.SongPage(song, 0),
PageContent.SongPage(song, 1)
),
tocEntries = emptyList()
)
val baos = ByteArrayOutputStream()
renderer.render(layout, config, baos)
baos.size() shouldBeGreaterThan 0
}
@Test
fun `render handles short song that fits on one page without splitting`() {
// A simple short song should still work correctly after split logic is added
val song = Song(
title = "Short Song",
sections = listOf(
SongSection(
type = SectionType.VERSE,
lines = listOf(
SongLine(listOf(LineSegment(chord = "Am", text = "One line")))
)
)
)
)
val layout = LayoutResult(
tocPages = 0,
pages = listOf(PageContent.SongPage(song, 0)),
tocEntries = emptyList()
)
val baos = ByteArrayOutputStream()
renderer.render(layout, BookConfig(), baos)
baos.size() shouldBeGreaterThan 0
}
@Test
fun `render two-page song with bottom metadata and references`() {
val config = BookConfig(
layout = LayoutConfig(
metadataPosition = "bottom",
metadataLabels = "german"
),
referenceBooks = listOf(
ReferenceBook(id = "mo", name = "Mundorgel", abbreviation = "MO")
)
)
val song = createLongSong().copy(
composer = "Bach",
lyricist = "Goethe",
notes = listOf("Play softly", "Repeat last verse"),
references = mapOf("mo" to 55)
)
val layout = LayoutResult(
tocPages = 0,
pages = listOf(
PageContent.SongPage(song, 0),
PageContent.SongPage(song, 1)
),
tocEntries = emptyList()
)
val baos = ByteArrayOutputStream()
renderer.render(layout, config, baos)
baos.size() shouldBeGreaterThan 0
}
}