Skip to content

Commit 34d89b4

Browse files
authored
fix(api): honor negative bottom margin, reject negative page margin (#232)
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.
1 parent a36839e commit 34d89b4

4 files changed

Lines changed: 126 additions & 4 deletions

File tree

CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,15 @@ PDF `GoTo` actions. External links are unchanged.
1212

1313
### Public API
1414

15+
- **Negative-margin handling** (`@since 1.9.0`). A negative **page** margin
16+
(`DocumentSession.margin(...)` or the builder's `margin(...)`) is now rejected
17+
with an `IllegalArgumentException` — it would make the content area larger than
18+
the sheet, silently overflowing it; use a node's `bleed(...)` to reach the page
19+
edge instead. Separately, a negative **node** bottom margin now pulls the
20+
following content up — symmetric with a negative top margin, which was already
21+
honoured (the vertical flow previously dropped it). Existing documents are
22+
unaffected, since neither shape was usable before.
23+
1524
- **`DocumentSession.pageIndex()` + `PageIndex` / `PageReference`** (`@since 1.9.0`).
1625
Resolves every declared `anchor(...)` to its final page in a single,
1726
backend-neutral pass over the laid-out document — `pageNumberOf("intro")` for a

src/main/java/com/demcha/compose/document/api/DocumentSession.java

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ public DocumentSession(Path defaultOutputFile,
100100
boolean guideLines) {
101101
this.defaultOutputFile = defaultOutputFile;
102102
this.pageSize = Objects.requireNonNull(pageSize, "pageSize");
103-
this.margin = margin == null ? DocumentInsets.zero() : margin;
103+
this.margin = margin == null ? DocumentInsets.zero() : requireNonNegativePageMargin(margin);
104104
this.canvas = LayoutCanvas.from(pageSize.width(), pageSize.height(), toEngineMargin(this.margin));
105105
this.markdown = markdown;
106106
this.debug = DocumentDebugOptions.none().withGuides(guideLines);
@@ -297,16 +297,38 @@ public DocumentSession pageSize(double width, double height) {
297297
*
298298
* @param margin new canvas margin, or {@code null} to reset to zero
299299
* @return this session
300-
* @throws IllegalStateException if this session has already been closed
300+
* @throws IllegalStateException if this session has already been closed
301+
* @throws IllegalArgumentException if any margin component is negative — a
302+
* negative page margin overflows the sheet;
303+
* use a node's {@code bleed(...)} instead
301304
*/
302305
public DocumentSession margin(DocumentInsets margin) {
303306
ensureOpen();
304-
this.margin = margin == null ? DocumentInsets.zero() : margin;
307+
this.margin = margin == null ? DocumentInsets.zero() : requireNonNegativePageMargin(margin);
305308
this.canvas = LayoutCanvas.from(pageSize.width(), pageSize.height(), toEngineMargin(this.margin));
306309
invalidate();
307310
return this;
308311
}
309312

313+
/**
314+
* Rejects a negative page margin. Unlike a node margin (where a negative
315+
* value is a valid overlap / inset trick), a negative page margin makes the
316+
* content area larger than the page, so content silently overflows the sheet.
317+
* To draw content past the page edge, use a node's {@code bleed(...)} instead.
318+
*
319+
* @param margin the requested page margin
320+
* @return {@code margin} when every component is non-negative
321+
* @throws IllegalArgumentException if any component is negative
322+
*/
323+
private static DocumentInsets requireNonNegativePageMargin(DocumentInsets margin) {
324+
if (margin.top() < 0 || margin.right() < 0 || margin.bottom() < 0 || margin.left() < 0) {
325+
throw new IllegalArgumentException(
326+
"page margin must be non-negative: " + margin
327+
+ " — use a node's bleed(...) to extend content past the page edge");
328+
}
329+
return margin;
330+
}
331+
310332
/**
311333
* Enables or disables markdown parsing for semantic paragraph blocks.
312334
*

src/main/java/com/demcha/compose/document/layout/LayoutCompiler.java

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ private void compileComposite(PreparedNode<DocumentNode> prepared,
390390
}
391391
}
392392

393-
advanceSpace(padding.bottom() + margin.bottom(), state);
393+
closeBottomSpace(padding.bottom() + margin.bottom(), state);
394394
int endPage = state.pageIndex;
395395
double endPageBottomY = state.pageTop() - state.usedHeight + margin.bottom();
396396

@@ -1527,6 +1527,25 @@ private void advanceSpace(double amount, CompilerState state) {
15271527
state.usedHeight = Math.min(state.canvas.innerHeight(), state.usedHeight + amount);
15281528
}
15291529

1530+
/**
1531+
* Closes out a composite's bottom edge. A positive bottom inset advances the
1532+
* flow as usual; a NEGATIVE one (a negative bottom margin) pulls the following
1533+
* sibling up — symmetric with a negative top margin, which already offsets via
1534+
* {@code placementTopY}. The plain {@link #advanceSpace} drops a non-positive
1535+
* amount, so the closing edge needs this dedicated path. The top-of-node
1536+
* reservation deliberately stays on {@link #advanceSpace} so a negative top
1537+
* margin keeps its existing flow behaviour; only the closing edge gains the
1538+
* pull-up. The cursor never drops below the page top.
1539+
*/
1540+
private void closeBottomSpace(double amount, CompilerState state) {
1541+
if (amount >= EPS) {
1542+
advanceSpace(amount, state);
1543+
} else if (amount <= -EPS) {
1544+
state.touchPage();
1545+
state.usedHeight = Math.max(0.0, state.usedHeight + amount);
1546+
}
1547+
}
1548+
15301549
private String pathFor(DocumentNode node, String parentPath, int childIndex) {
15311550
String base = semanticName(node);
15321551
if (base == null || base.isBlank()) {
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
package com.demcha.compose.document.api;
2+
3+
import com.demcha.compose.GraphCompose;
4+
import com.demcha.compose.document.layout.PlacedNode;
5+
import com.demcha.compose.document.style.DocumentInsets;
6+
import org.junit.jupiter.api.Test;
7+
8+
import static org.assertj.core.api.Assertions.assertThat;
9+
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
10+
import static org.assertj.core.api.Assertions.within;
11+
12+
/**
13+
* A negative <em>page</em> margin is rejected (it would make the content area
14+
* larger than the sheet), while a negative <em>node</em> bottom margin now pulls
15+
* following content up — symmetric with a negative top margin, which was already
16+
* honoured. Previously the vertical flow silently dropped it.
17+
*/
18+
class NegativeMarginTest {
19+
20+
@Test
21+
void negativePageMarginIsRejectedViaTheBuilder() {
22+
assertThatIllegalArgumentException()
23+
.isThrownBy(() -> GraphCompose.document().pageSize(300, 300).margin(-2, 0, 0, 0).create())
24+
.withMessageContaining("bleed");
25+
}
26+
27+
@Test
28+
void negativePageMarginIsRejectedViaTheSessionSetter() {
29+
try (DocumentSession session = GraphCompose.document().pageSize(300, 300).create()) {
30+
assertThatIllegalArgumentException()
31+
.isThrownBy(() -> session.margin(new DocumentInsets(0, 0, -5, 0)))
32+
.withMessageContaining("non-negative");
33+
}
34+
}
35+
36+
@Test
37+
void zeroAndPositivePageMarginAreAccepted() {
38+
GraphCompose.document().pageSize(300, 300).margin(0, 0, 0, 0).create().close();
39+
GraphCompose.document().pageSize(300, 300).margin(DocumentInsets.of(24)).create().close();
40+
}
41+
42+
@Test
43+
void negativeBottomNodeMarginPullsFollowingContentUp() {
44+
double baseline = followingNodeY(0);
45+
double pulled = followingNodeY(-20);
46+
47+
// y grows upward, so "pulled up" means a larger y; the shift equals the margin.
48+
assertThat(pulled).isGreaterThan(baseline);
49+
assertThat(pulled - baseline).isCloseTo(20.0, within(0.5));
50+
}
51+
52+
/** Lays out [filler, section A (bottom margin = m), section B] and returns B's placed y. */
53+
private double followingNodeY(double aBottomMargin) {
54+
try (DocumentSession session = GraphCompose.document()
55+
.pageSize(240, 400)
56+
.margin(DocumentInsets.of(20))
57+
.create()) {
58+
session.pageFlow(page -> {
59+
page.addParagraph("filler so the cursor is well below the page top");
60+
page.addSection(s -> s.name("A")
61+
.margin(new DocumentInsets(0, 0, aBottomMargin, 0))
62+
.addParagraph("A"));
63+
page.addSection(s -> s.name("B").addParagraph("B"));
64+
});
65+
PlacedNode b = session.layoutGraph().nodes().stream()
66+
.filter(n -> "B".equals(n.semanticName()))
67+
.findFirst()
68+
.orElseThrow(() -> new AssertionError("section B not found in layout graph"));
69+
return b.placementY();
70+
}
71+
}
72+
}

0 commit comments

Comments
 (0)