From 471ef6ce3945c6cd60dff9382df746d4e53a2b72 Mon Sep 17 00:00:00 2001 From: DemchaAV Date: Fri, 10 Jul 2026 14:32:47 +0100 Subject: [PATCH] refactor(engine): move row slot-width resolution into RowSlots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit compileHorizontalRow chose its slot widths inline through a three-way flex / explicit-columns / weights branch, and the weight split lived in a private LayoutCompiler.distributeRowSlotWidths. Both belong with the other row slot-width primitives. Move distributeRowSlotWidths onto RowSlots (whose Javadoc already pointed at it) and add RowSlots.resolveLayout, returning the slot widths plus the flex leading / extra-gap in a small SlotLayout record. compileHorizontalRow now calls resolveLayout and reads the three values off the record — byte-identical placement, and LayoutCompiler drops ~64 lines and two now-unused imports. RowSlotsTest gains direct coverage for the weight distribution. --- .../document/layout/LayoutCompiler.java | 69 +---------- .../compose/document/layout/RowSlots.java | 115 +++++++++++++++++- .../compose/document/layout/RowSlotsTest.java | 35 ++++++ 3 files changed, 154 insertions(+), 65 deletions(-) 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 3f9aa36de..ee1961675 100644 --- a/src/main/java/com/demcha/compose/document/layout/LayoutCompiler.java +++ b/src/main/java/com/demcha/compose/document/layout/LayoutCompiler.java @@ -5,12 +5,10 @@ import com.demcha.compose.document.node.DocumentNode; import com.demcha.compose.document.node.LayerStackNode; import com.demcha.compose.document.node.PageBreakNode; -import com.demcha.compose.document.node.RowArrangement; import com.demcha.compose.document.node.RowNode; import com.demcha.compose.document.node.RowVerticalAlign; import com.demcha.compose.document.style.DocumentBleed; import com.demcha.compose.document.style.DocumentEdge; -import com.demcha.compose.document.style.DocumentRowColumn; import com.demcha.compose.engine.components.style.Margin; import com.demcha.compose.engine.components.style.Padding; import org.slf4j.Logger; @@ -58,38 +56,6 @@ public LayoutCompiler(NodeRegistry registry) { this.registry = Objects.requireNonNull(registry, "registry"); } - private static double[] distributeRowSlotWidths(List children, - List weights, - double gap, - double innerWidth) { - int n = children.size(); - double available = Math.max(0.0, innerWidth - gap * Math.max(0, n - 1)); - double[] slots = new double[n]; - if (weights == null || weights.isEmpty()) { - double share = n > 0 ? available / n : 0.0; - for (int i = 0; i < n; i++) { - slots[i] = share; - } - return slots; - } - RowSlots.validateWeightsMatchChildren(weights, n); - double total = 0.0; - for (double w : weights) { - total += w; - } - if (total <= 0.0) { - double share = n > 0 ? available / n : 0.0; - for (int i = 0; i < n; i++) { - slots[i] = share; - } - return slots; - } - for (int i = 0; i < n; i++) { - slots[i] = available * (weights.get(i) / total); - } - return slots; - } - /** * Compiles semantic roots into placed nodes and renderable fragments. * @@ -466,36 +432,11 @@ private void compileHorizontalRow(PreparedNode prepared, double rowInnerY = placementTopY - padding.top(); if (!children.isEmpty()) { - List columns = node instanceof RowNode row ? row.columns() : List.of(); - double[] slotWidths; - // flexLeading / flexExtraGap stay 0.0 for every non-flex row, so the - // cursor math below is byte-identical to the pre-flex placement. - double flexLeading = 0.0; - double flexExtraGap = 0.0; - if (RowSlots.hasFlexLayout(node)) { - slotWidths = RowSlots.distributeFlex(children, layoutSpec.spacing(), childRegionWidth, prepareContext); - RowArrangement arrangement = node instanceof RowNode flexRow - ? flexRow.arrangement() : RowArrangement.START; - if (arrangement != RowArrangement.START) { - double available = RowSlots.rowAvailableWidth( - childRegionWidth, layoutSpec.spacing(), children.size()); - double used = 0.0; - for (double slot : slotWidths) { - used += slot; - } - double[] justify = RowSlots.flexJustify( - arrangement, Math.max(0.0, available - used), children.size()); - flexLeading = justify[0]; - flexExtraGap = justify[1]; - } - } else if (!columns.isEmpty()) { - double available = RowSlots.rowAvailableWidth(childRegionWidth, layoutSpec.spacing(), children.size()); - double[] intrinsic = RowSlots.intrinsicColumnWidths(children, columns, available, prepareContext); - slotWidths = RowSlots.distributeColumns(columns, intrinsic, layoutSpec.spacing(), childRegionWidth, semanticName); - } else { - slotWidths = distributeRowSlotWidths(children, layoutSpec.weights(), - layoutSpec.spacing(), childRegionWidth); - } + RowSlots.SlotLayout slotLayout = RowSlots.resolveLayout( + node, children, layoutSpec, childRegionWidth, prepareContext, semanticName); + double[] slotWidths = slotLayout.widths(); + double flexLeading = slotLayout.leading(); + double flexExtraGap = slotLayout.extraGap(); RowVerticalAlign verticalAlign = node instanceof RowNode rowNode ? rowNode.verticalAlign() : RowVerticalAlign.TOP; double bandContentHeight = naturalMeasure.height() - padding.vertical(); diff --git a/src/main/java/com/demcha/compose/document/layout/RowSlots.java b/src/main/java/com/demcha/compose/document/layout/RowSlots.java index 935128566..e33960e92 100644 --- a/src/main/java/com/demcha/compose/document/layout/RowSlots.java +++ b/src/main/java/com/demcha/compose/document/layout/RowSlots.java @@ -12,7 +12,7 @@ * Shared row slot-width helpers for the compile and measure phases. * *

Centralises the {@link IllegalArgumentException} contract used by both - * {@link LayoutCompiler#distributeRowSlotWidths(List, List, double, double) compile-phase} + * {@link #distributeRowSlotWidths(List, List, double, double) compile-phase} * and {@link NodeDefinitionSupport#measureRow measure-phase} row distribution, * and owns the explicit-column ({@code fixed / intrinsic / weight}) distribution * so both phases compute the same slot widths from a single source.

@@ -261,4 +261,117 @@ static double[] flexJustify(RowArrangement arrangement, double leftover, int n) } return new double[]{leading, extraGap}; } + + /** + * Distributes the row width across weighted slots. With no weights every + * child receives an equal share of the width left after the inter-child + * gaps; with weights each child receives a share proportional to its weight + * (a non-positive weight total falls back to the even split). + * + * @param children the row children (only the count is read) + * @param weights per-child weights, or {@code null} / empty for an even split + * @param gap gap between children + * @param innerWidth the row's content width + * @return resolved slot width per child + * @throws IllegalArgumentException if a non-empty weight list size does not + * match the child count + */ + static double[] distributeRowSlotWidths(List children, + List weights, + double gap, + double innerWidth) { + int n = children.size(); + double available = Math.max(0.0, innerWidth - gap * Math.max(0, n - 1)); + double[] slots = new double[n]; + if (weights == null || weights.isEmpty()) { + double share = n > 0 ? available / n : 0.0; + for (int i = 0; i < n; i++) { + slots[i] = share; + } + return slots; + } + validateWeightsMatchChildren(weights, n); + double total = 0.0; + for (double w : weights) { + total += w; + } + if (total <= 0.0) { + double share = n > 0 ? available / n : 0.0; + for (int i = 0; i < n; i++) { + slots[i] = share; + } + return slots; + } + for (int i = 0; i < n; i++) { + slots[i] = available * (weights.get(i) / total); + } + return slots; + } + + /** + * Resolves a horizontal row's slot layout from its arrangement mode: flex + * (with optional non-START justification), explicit columns, or plain + * weights. Reads no compiler state and reuses the same primitives the + * measure phase uses, so the compile phase seats children on identical + * widths. + * + * @param node the row node (read for its columns / arrangement) + * @param children the row children + * @param layoutSpec the resolved composite layout spec (spacing / weights) + * @param childRegionWidth the row's inner content width + * @param prepareContext preparation context for intrinsic child measurement + * @param semanticName row name for the over-constrained column error + * @return the slot widths plus the flex leading offset and extra inter-child gap + */ + static SlotLayout resolveLayout(DocumentNode node, + List children, + CompositeLayoutSpec layoutSpec, + double childRegionWidth, + PrepareContext prepareContext, + String semanticName) { + List columns = node instanceof RowNode row ? row.columns() : List.of(); + double[] slotWidths; + // flexLeading / flexExtraGap stay 0.0 for every non-flex row, so the + // caller's cursor math is byte-identical to the pre-flex placement. + double flexLeading = 0.0; + double flexExtraGap = 0.0; + if (hasFlexLayout(node)) { + slotWidths = distributeFlex(children, layoutSpec.spacing(), childRegionWidth, prepareContext); + RowArrangement arrangement = node instanceof RowNode flexRow + ? flexRow.arrangement() : RowArrangement.START; + if (arrangement != RowArrangement.START) { + double available = rowAvailableWidth( + childRegionWidth, layoutSpec.spacing(), children.size()); + double used = 0.0; + for (double slot : slotWidths) { + used += slot; + } + double[] justify = flexJustify( + arrangement, Math.max(0.0, available - used), children.size()); + flexLeading = justify[0]; + flexExtraGap = justify[1]; + } + } else if (!columns.isEmpty()) { + double available = rowAvailableWidth(childRegionWidth, layoutSpec.spacing(), children.size()); + double[] intrinsic = intrinsicColumnWidths(children, columns, available, prepareContext); + slotWidths = distributeColumns(columns, intrinsic, layoutSpec.spacing(), childRegionWidth, semanticName); + } else { + slotWidths = distributeRowSlotWidths(children, layoutSpec.weights(), + layoutSpec.spacing(), childRegionWidth); + } + return new SlotLayout(slotWidths, flexLeading, flexExtraGap); + } + + /** + * Resolved slot geometry for one horizontal row: the per-child slot widths + * plus the flex justification offsets applied to the placement cursor. A + * transient return holder — the {@code widths} array is freshly allocated + * per call and not shared. + * + * @param widths resolved slot width per child + * @param leading x offset before the first child (non-START flex only) + * @param extraGap extra gap inserted between adjacent children (non-START flex only) + */ + record SlotLayout(double[] widths, double leading, double extraGap) { + } } diff --git a/src/test/java/com/demcha/compose/document/layout/RowSlotsTest.java b/src/test/java/com/demcha/compose/document/layout/RowSlotsTest.java index 703238973..3e6c9e60e 100644 --- a/src/test/java/com/demcha/compose/document/layout/RowSlotsTest.java +++ b/src/test/java/com/demcha/compose/document/layout/RowSlotsTest.java @@ -1,7 +1,11 @@ package com.demcha.compose.document.layout; +import com.demcha.compose.document.node.DocumentNode; +import com.demcha.compose.document.node.SpacerNode; +import com.demcha.compose.document.style.DocumentInsets; import org.junit.jupiter.api.Test; +import java.util.Collections; import java.util.List; import static org.assertj.core.api.Assertions.assertThat; @@ -58,4 +62,35 @@ void errorMessageHintsAtTheFix() { assertThat(s).containsAnyOf("Pass", "Provide", "Use"); }); } + + @Test + void distributesEvenlyWhenNoWeights() { + assertThat(RowSlots.distributeRowSlotWidths(children(3), List.of(), 0.0, 30.0)) + .containsExactly(10.0, 10.0, 10.0); + } + + @Test + void distributesProportionallyToWeights() { + assertThat(RowSlots.distributeRowSlotWidths(children(2), List.of(1.0, 3.0), 0.0, 40.0)) + .containsExactly(10.0, 30.0); + } + + @Test + void subtractsInterChildGapsBeforeDistributing() { + // 30 wide, two 5-wide gaps -> 20 available, split three ways. + assertThat(RowSlots.distributeRowSlotWidths(children(3), List.of(), 5.0, 30.0)) + .containsExactly(20.0 / 3, 20.0 / 3, 20.0 / 3); + } + + @Test + void fallsBackToEvenSplitWhenWeightsSumToZero() { + assertThat(RowSlots.distributeRowSlotWidths(children(2), List.of(0.0, 0.0), 0.0, 20.0)) + .containsExactly(10.0, 10.0); + } + + /** A list of {@code n} placeholder children — only the count is read. */ + private static List children(int n) { + DocumentNode spacer = new SpacerNode("s", 0.0, 0.0, DocumentInsets.zero(), DocumentInsets.zero()); + return Collections.nCopies(n, spacer); + } }