From d85fc12f578af42329f09b1ee711659b7015e800 Mon Sep 17 00:00:00 2001 From: DemchaAV Date: Thu, 25 Jun 2026 12:03:53 +0100 Subject: [PATCH] fix(api): honor negative bottom margin, reject negative page margin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A negative bottom margin on a composite was silently dropped by the vertical flow — its bottom-edge advance early-returned on a non-positive amount — while a negative page margin was accepted and silently overflowed the sheet. Composites now close their bottom edge through closeBottomSpace, which lets a negative bottom margin pull the following sibling up, symmetric with a negative top margin (which already offsets via placementTopY). The top-of-node reservation and the row/leaf paths are untouched, so existing layouts are byte-identical. DocumentSession now rejects a negative page margin with IllegalArgumentException pointing to bleed(); node margins still allow negatives for overlap tricks. Tests: NegativeMarginTest covers the page margin rejected via the builder and via the session setter, zero/positive accepted, and a negative bottom margin shifting the following section up by the margin. Full suite green, no visual baselines changed. --- CHANGELOG.md | 9 +++ .../compose/document/api/DocumentSession.java | 28 +++++++- .../document/layout/LayoutCompiler.java | 21 +++++- .../document/api/NegativeMarginTest.java | 72 +++++++++++++++++++ 4 files changed, 126 insertions(+), 4 deletions(-) create mode 100644 src/test/java/com/demcha/compose/document/api/NegativeMarginTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 9cf580bb8..708c39893 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,15 @@ PDF `GoTo` actions. External links are unchanged. ### Public API +- **Negative-margin handling** (`@since 1.9.0`). A negative **page** margin + (`DocumentSession.margin(...)` or the builder's `margin(...)`) is now rejected + with an `IllegalArgumentException` — it would make the content area larger than + the sheet, silently overflowing it; use a node's `bleed(...)` to reach the page + edge instead. Separately, a negative **node** bottom margin now pulls the + following content up — symmetric with a negative top margin, which was already + honoured (the vertical flow previously dropped it). Existing documents are + unaffected, since neither shape was usable before. + - **`DocumentSession.pageIndex()` + `PageIndex` / `PageReference`** (`@since 1.9.0`). Resolves every declared `anchor(...)` to its final page in a single, backend-neutral pass over the laid-out document — `pageNumberOf("intro")` for a diff --git a/src/main/java/com/demcha/compose/document/api/DocumentSession.java b/src/main/java/com/demcha/compose/document/api/DocumentSession.java index 171b44789..df8ad6d10 100644 --- a/src/main/java/com/demcha/compose/document/api/DocumentSession.java +++ b/src/main/java/com/demcha/compose/document/api/DocumentSession.java @@ -100,7 +100,7 @@ public DocumentSession(Path defaultOutputFile, boolean guideLines) { this.defaultOutputFile = defaultOutputFile; this.pageSize = Objects.requireNonNull(pageSize, "pageSize"); - this.margin = margin == null ? DocumentInsets.zero() : margin; + this.margin = margin == null ? DocumentInsets.zero() : requireNonNegativePageMargin(margin); this.canvas = LayoutCanvas.from(pageSize.width(), pageSize.height(), toEngineMargin(this.margin)); this.markdown = markdown; this.debug = DocumentDebugOptions.none().withGuides(guideLines); @@ -297,16 +297,38 @@ public DocumentSession pageSize(double width, double height) { * * @param margin new canvas margin, or {@code null} to reset to zero * @return this session - * @throws IllegalStateException if this session has already been closed + * @throws IllegalStateException if this session has already been closed + * @throws IllegalArgumentException if any margin component is negative — a + * negative page margin overflows the sheet; + * use a node's {@code bleed(...)} instead */ public DocumentSession margin(DocumentInsets margin) { ensureOpen(); - this.margin = margin == null ? DocumentInsets.zero() : margin; + this.margin = margin == null ? DocumentInsets.zero() : requireNonNegativePageMargin(margin); this.canvas = LayoutCanvas.from(pageSize.width(), pageSize.height(), toEngineMargin(this.margin)); invalidate(); return this; } + /** + * Rejects a negative page margin. Unlike a node margin (where a negative + * value is a valid overlap / inset trick), a negative page margin makes the + * content area larger than the page, so content silently overflows the sheet. + * To draw content past the page edge, use a node's {@code bleed(...)} instead. + * + * @param margin the requested page margin + * @return {@code margin} when every component is non-negative + * @throws IllegalArgumentException if any component is negative + */ + private static DocumentInsets requireNonNegativePageMargin(DocumentInsets margin) { + if (margin.top() < 0 || margin.right() < 0 || margin.bottom() < 0 || margin.left() < 0) { + throw new IllegalArgumentException( + "page margin must be non-negative: " + margin + + " — use a node's bleed(...) to extend content past the page edge"); + } + return margin; + } + /** * Enables or disables markdown parsing for semantic paragraph blocks. * diff --git a/src/main/java/com/demcha/compose/document/layout/LayoutCompiler.java b/src/main/java/com/demcha/compose/document/layout/LayoutCompiler.java index d817cbdd4..20e326be8 100644 --- a/src/main/java/com/demcha/compose/document/layout/LayoutCompiler.java +++ b/src/main/java/com/demcha/compose/document/layout/LayoutCompiler.java @@ -390,7 +390,7 @@ private void compileComposite(PreparedNode prepared, } } - advanceSpace(padding.bottom() + margin.bottom(), state); + closeBottomSpace(padding.bottom() + margin.bottom(), state); int endPage = state.pageIndex; double endPageBottomY = state.pageTop() - state.usedHeight + margin.bottom(); @@ -1527,6 +1527,25 @@ private void advanceSpace(double amount, CompilerState state) { state.usedHeight = Math.min(state.canvas.innerHeight(), state.usedHeight + amount); } + /** + * Closes out a composite's bottom edge. A positive bottom inset advances the + * flow as usual; a NEGATIVE one (a negative bottom margin) pulls the following + * sibling up — symmetric with a negative top margin, which already offsets via + * {@code placementTopY}. The plain {@link #advanceSpace} drops a non-positive + * amount, so the closing edge needs this dedicated path. The top-of-node + * reservation deliberately stays on {@link #advanceSpace} so a negative top + * margin keeps its existing flow behaviour; only the closing edge gains the + * pull-up. The cursor never drops below the page top. + */ + private void closeBottomSpace(double amount, CompilerState state) { + if (amount >= EPS) { + advanceSpace(amount, state); + } else if (amount <= -EPS) { + state.touchPage(); + state.usedHeight = Math.max(0.0, state.usedHeight + amount); + } + } + private String pathFor(DocumentNode node, String parentPath, int childIndex) { String base = semanticName(node); if (base == null || base.isBlank()) { diff --git a/src/test/java/com/demcha/compose/document/api/NegativeMarginTest.java b/src/test/java/com/demcha/compose/document/api/NegativeMarginTest.java new file mode 100644 index 000000000..abd015e64 --- /dev/null +++ b/src/test/java/com/demcha/compose/document/api/NegativeMarginTest.java @@ -0,0 +1,72 @@ +package com.demcha.compose.document.api; + +import com.demcha.compose.GraphCompose; +import com.demcha.compose.document.layout.PlacedNode; +import com.demcha.compose.document.style.DocumentInsets; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.Assertions.within; + +/** + * A negative page margin is rejected (it would make the content area + * larger than the sheet), while a negative node bottom margin now pulls + * following content up — symmetric with a negative top margin, which was already + * honoured. Previously the vertical flow silently dropped it. + */ +class NegativeMarginTest { + + @Test + void negativePageMarginIsRejectedViaTheBuilder() { + assertThatIllegalArgumentException() + .isThrownBy(() -> GraphCompose.document().pageSize(300, 300).margin(-2, 0, 0, 0).create()) + .withMessageContaining("bleed"); + } + + @Test + void negativePageMarginIsRejectedViaTheSessionSetter() { + try (DocumentSession session = GraphCompose.document().pageSize(300, 300).create()) { + assertThatIllegalArgumentException() + .isThrownBy(() -> session.margin(new DocumentInsets(0, 0, -5, 0))) + .withMessageContaining("non-negative"); + } + } + + @Test + void zeroAndPositivePageMarginAreAccepted() { + GraphCompose.document().pageSize(300, 300).margin(0, 0, 0, 0).create().close(); + GraphCompose.document().pageSize(300, 300).margin(DocumentInsets.of(24)).create().close(); + } + + @Test + void negativeBottomNodeMarginPullsFollowingContentUp() { + double baseline = followingNodeY(0); + double pulled = followingNodeY(-20); + + // y grows upward, so "pulled up" means a larger y; the shift equals the margin. + assertThat(pulled).isGreaterThan(baseline); + assertThat(pulled - baseline).isCloseTo(20.0, within(0.5)); + } + + /** Lays out [filler, section A (bottom margin = m), section B] and returns B's placed y. */ + private double followingNodeY(double aBottomMargin) { + try (DocumentSession session = GraphCompose.document() + .pageSize(240, 400) + .margin(DocumentInsets.of(20)) + .create()) { + session.pageFlow(page -> { + page.addParagraph("filler so the cursor is well below the page top"); + page.addSection(s -> s.name("A") + .margin(new DocumentInsets(0, 0, aBottomMargin, 0)) + .addParagraph("A")); + page.addSection(s -> s.name("B").addParagraph("B")); + }); + PlacedNode b = session.layoutGraph().nodes().stream() + .filter(n -> "B".equals(n.semanticName())) + .findFirst() + .orElseThrow(() -> new AssertionError("section B not found in layout graph")); + return b.placementY(); + } + } +}