From 10eaec373eab8bd2e96df81c85e8604fce19347e Mon Sep 17 00:00:00 2001 From: DemchaAV Date: Sun, 14 Jun 2026 14:48:34 +0100 Subject: [PATCH 1/2] fix(charts): anchor stacked bars at zero and guard paintForSeries Stacked bars sum their segments up from a fixed floor, with each segment's height measured from zero (value / range). An explicit positive valueAxis().min() lifted the baseline without shrinking the segments, so the stack overshot its total and ran past the plot top. Pin the stacked floor to zero in ChartLayoutSupport.computeFrame, independent of the requested min; grouped bars still honour an explicit minimum. Document the zero baseline on BarGrouping.STACKED. ChartStyle.paintForSeries now rejects a negative index with a value-naming IllegalArgumentException instead of leaking a bare IndexOutOfBoundsException from the palette modulo. Tests: a stacked + positive-min case asserting the stack stays within the plot, and paintForSeries negative-index + cycling cases. Existing chart snapshots are unchanged (the floor guard fires only for stacked + positive min, which no existing chart uses). Full suite green (1377). --- CHANGELOG.md | 9 ++++ .../compose/document/chart/BarGrouping.java | 2 + .../document/chart/ChartLayoutSupport.java | 8 ++++ .../compose/document/chart/ChartStyle.java | 3 ++ .../chart/ChartLayoutResolverTest.java | 28 ++++++++++++ .../document/chart/ChartStyleTest.java | 43 +++++++++++++++++++ 6 files changed, 93 insertions(+) create mode 100644 src/test/java/com/demcha/compose/document/chart/ChartStyleTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 6efbb6ad9..30394d0da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -228,6 +228,15 @@ Entries land here as they merge. ### Bug fixes +- **Stacked bars anchor at zero even with an explicit positive axis minimum.** + A stacked bar chart with `valueAxis().min(positive)` lifted the baseline + while segment heights stayed measured from zero, so the stack overshot its + total and ran past the plot top. The stacked floor is now pinned to zero + (parts summing to a whole), independent of the requested minimum. Grouped + bars still honour an explicit minimum. +- **`ChartStyle.paintForSeries` rejects a negative series index** with a + value-naming `IllegalArgumentException` instead of leaking a bare + `IndexOutOfBoundsException` from the palette modulo. - **SVG path reader no longer hangs on malformed `d` data.** A `Z`/`z` close command (which consumes no operands) followed by a stray non-command token — e.g. `"M0 0 Z5"` — made the scanner loop forever, diff --git a/src/main/java/com/demcha/compose/document/chart/BarGrouping.java b/src/main/java/com/demcha/compose/document/chart/BarGrouping.java index c7130b103..581ae7bb0 100644 --- a/src/main/java/com/demcha/compose/document/chart/BarGrouping.java +++ b/src/main/java/com/demcha/compose/document/chart/BarGrouping.java @@ -13,6 +13,8 @@ public enum BarGrouping { GROUPED, /** * Series stack on top of each other; the axis scales to stacked sums. + * The stacked baseline is always zero (parts summing to a whole), so an + * explicit positive {@code valueAxis().min()} does not lift it. */ STACKED } diff --git a/src/main/java/com/demcha/compose/document/chart/ChartLayoutSupport.java b/src/main/java/com/demcha/compose/document/chart/ChartLayoutSupport.java index ef63b4e1d..9cd202f86 100644 --- a/src/main/java/com/demcha/compose/document/chart/ChartLayoutSupport.java +++ b/src/main/java/com/demcha/compose/document/chart/ChartLayoutSupport.java @@ -92,6 +92,14 @@ static Frame computeFrame(ChartData data, AxisSpec axis, LegendReserve reserve, if (axis.max() != null) { domain[1] = axis.max(); } + // Stacked bars sum their segments up from a fixed floor, and each + // segment's height is measured from zero (value / range). An explicit + // positive axis.min() would lift the baseline without shrinking the + // segments, so the stack would overshoot its true total — pin the + // stacked floor to zero regardless of the requested min. + if (stacked && domain[0] > 0.0) { + domain[0] = 0.0; + } NiceScale scale = NiceScale.compute(domain[0], domain[1], axis.baselineAtZero() && axis.min() == null, ChartDefaults.TARGET_TICKS); diff --git a/src/main/java/com/demcha/compose/document/chart/ChartStyle.java b/src/main/java/com/demcha/compose/document/chart/ChartStyle.java index 7f6bc5314..485892de9 100644 --- a/src/main/java/com/demcha/compose/document/chart/ChartStyle.java +++ b/src/main/java/com/demcha/compose/document/chart/ChartStyle.java @@ -169,6 +169,9 @@ public ChartStyle mergedUnder(ChartStyle top) { * @return the paint to fill / stroke this series with */ public DocumentPaint paintForSeries(int index, List fallbackPalette) { + if (index < 0) { + throw new IllegalArgumentException("series index must be >= 0: " + index); + } DocumentPaint override = seriesPaintOverrides.get(index); if (override != null) { return override; diff --git a/src/test/java/com/demcha/compose/document/chart/ChartLayoutResolverTest.java b/src/test/java/com/demcha/compose/document/chart/ChartLayoutResolverTest.java index 575efbfba..61ff471ec 100644 --- a/src/test/java/com/demcha/compose/document/chart/ChartLayoutResolverTest.java +++ b/src/test/java/com/demcha/compose/document/chart/ChartLayoutResolverTest.java @@ -181,6 +181,34 @@ void horizontalBarsGrowRightFromTheLeftEdge() { assertThat(barA.y()).isGreaterThan(barB.y()); } + @Test + void stackedBarsAnchorAtZeroEvenWithAnExplicitPositiveAxisMin() { + // A stacked bar sums its parts up from zero. An explicit positive + // axis.min() must not lift the baseline: doing so left segment heights + // measured from zero while the stack started above it, so the stack + // overshot the plot. Pin the stacked floor to zero. + ChartData data = ChartData.builder().categories("A") + .series("S1", 10.0).series("S2", 20.0).build(); // total 30 + ChartSpec.Bar bar = ChartSpec.bar().data(data) + .grouping(BarGrouping.STACKED) + .valueAxis(AxisSpec.builder().min(10.0).max(30.0).build()) + .build(); + + List out = ChartLayoutResolver.resolve( + bar, baseStyle(), ChartDefaults.DEFAULT_THEME, 200.0, 100.0, METRICS); + + // Same frame as the grouped test: plotBottomY = 14, plotTopY = 95. + // The bottom segment sits on the zero baseline and the whole stack stays + // inside the plot (with the floor wrongly at 10 it reached ~135 > 95). + double baseY = byName(out, "bar_c0_s0").y(); + double stackTop = out.stream() + .filter(p -> p.node().name().startsWith("bar_c0_")) + .mapToDouble(p -> p.y() + p.height()) + .max().orElseThrow(); + assertThat(baseY).isCloseTo(14.0, within(1e-6)); + assertThat(stackTop).isLessThanOrEqualTo(95.0 + 1e-6); + } + @Test void stackedBarsLabelTheCategoryTotal() { ChartData data = ChartData.builder().categories("A") diff --git a/src/test/java/com/demcha/compose/document/chart/ChartStyleTest.java b/src/test/java/com/demcha/compose/document/chart/ChartStyleTest.java new file mode 100644 index 000000000..96f2a614f --- /dev/null +++ b/src/test/java/com/demcha/compose/document/chart/ChartStyleTest.java @@ -0,0 +1,43 @@ +package com.demcha.compose.document.chart; + +import com.demcha.compose.document.style.DocumentColor; +import com.demcha.compose.document.style.DocumentPaint; +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Contract of {@link ChartStyle#paintForSeries(int, List)}: a negative index is + * rejected with a value-naming message (rather than a bare + * {@code IndexOutOfBoundsException} from the modulo), and a valid index cycles + * through the palette. + */ +class ChartStyleTest { + + private static final List PALETTE = List.of( + DocumentPaint.solid(DocumentColor.rgb(10, 20, 30)), + DocumentPaint.solid(DocumentColor.rgb(40, 50, 60))); + + @Test + void paintForSeriesRejectsNegativeIndex() { + ChartStyle style = ChartDefaults.DEFAULT_THEME.toChartStyle(); + + assertThatThrownBy(() -> style.paintForSeries(-1, PALETTE)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("series index must be >= 0"); + } + + @Test + void paintForSeriesCyclesThroughTheFallbackPalette() { + // A style with no palette of its own falls back to the supplied palette, + // cycling by modulo so it never runs out of colours. + ChartStyle style = ChartStyle.inherit(); + + assertThat(style.paintForSeries(0, PALETTE)).isEqualTo(PALETTE.get(0)); + assertThat(style.paintForSeries(1, PALETTE)).isEqualTo(PALETTE.get(1)); + assertThat(style.paintForSeries(2, PALETTE)).isEqualTo(PALETTE.get(0)); + } +} From d80a1fec0b7883044328ec682f24bd7df4bc04fc Mon Sep 17 00:00:00 2001 From: DemchaAV Date: Sun, 14 Jun 2026 15:02:39 +0100 Subject: [PATCH 2/2] fix(charts): pin the stacked floor to zero for a negative axis.min too The stacked zero-floor pin only fired for a positive valueAxis().min(); a negative explicit min still anchored the stack below zero, so its top undershot the total. Pin the floor to exactly zero for any non-zero min, and align the BarGrouping.STACKED Javadoc. Add a negative-min regression case and tighten both stacked assertions to the exact plot-top fill. --- .../compose/document/chart/BarGrouping.java | 2 +- .../document/chart/ChartLayoutSupport.java | 9 +++--- .../chart/ChartLayoutResolverTest.java | 28 +++++++++++++------ 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/demcha/compose/document/chart/BarGrouping.java b/src/main/java/com/demcha/compose/document/chart/BarGrouping.java index 581ae7bb0..96a00dbec 100644 --- a/src/main/java/com/demcha/compose/document/chart/BarGrouping.java +++ b/src/main/java/com/demcha/compose/document/chart/BarGrouping.java @@ -14,7 +14,7 @@ public enum BarGrouping { /** * Series stack on top of each other; the axis scales to stacked sums. * The stacked baseline is always zero (parts summing to a whole), so an - * explicit positive {@code valueAxis().min()} does not lift it. + * explicit {@code valueAxis().min()} of either sign does not move it. */ STACKED } diff --git a/src/main/java/com/demcha/compose/document/chart/ChartLayoutSupport.java b/src/main/java/com/demcha/compose/document/chart/ChartLayoutSupport.java index 9cd202f86..ece92fa13 100644 --- a/src/main/java/com/demcha/compose/document/chart/ChartLayoutSupport.java +++ b/src/main/java/com/demcha/compose/document/chart/ChartLayoutSupport.java @@ -94,10 +94,11 @@ static Frame computeFrame(ChartData data, AxisSpec axis, LegendReserve reserve, } // Stacked bars sum their segments up from a fixed floor, and each // segment's height is measured from zero (value / range). An explicit - // positive axis.min() would lift the baseline without shrinking the - // segments, so the stack would overshoot its true total — pin the - // stacked floor to zero regardless of the requested min. - if (stacked && domain[0] > 0.0) { + // axis.min() of either sign would move the baseline off zero without + // rescaling the segments — a positive min makes the stack overshoot its + // total, a negative min anchors it below zero — so pin the stacked floor + // to exactly zero regardless of the requested min. + if (stacked && domain[0] != 0.0) { domain[0] = 0.0; } NiceScale scale = NiceScale.compute(domain[0], domain[1], diff --git a/src/test/java/com/demcha/compose/document/chart/ChartLayoutResolverTest.java b/src/test/java/com/demcha/compose/document/chart/ChartLayoutResolverTest.java index 61ff471ec..6a9daaa5d 100644 --- a/src/test/java/com/demcha/compose/document/chart/ChartLayoutResolverTest.java +++ b/src/test/java/com/demcha/compose/document/chart/ChartLayoutResolverTest.java @@ -183,30 +183,40 @@ void horizontalBarsGrowRightFromTheLeftEdge() { @Test void stackedBarsAnchorAtZeroEvenWithAnExplicitPositiveAxisMin() { - // A stacked bar sums its parts up from zero. An explicit positive - // axis.min() must not lift the baseline: doing so left segment heights - // measured from zero while the stack started above it, so the stack - // overshot the plot. Pin the stacked floor to zero. + // total 30, axis.min(10) — a positive floor used to lift the baseline + // while segments stayed measured from zero, so the stack overshot + // (~135 > 95). The floor is pinned to zero, so the stack fills exactly + // to the value-30 level at the plot top. + assertStackFillsToTheTotal(10.0); + } + + @Test + void stackedBarsAnchorAtZeroEvenWithAnExplicitNegativeAxisMin() { + // total 30, axis.min(-10) — a negative floor used to anchor the stack + // below zero (top reached only ~75 < 95). Same zero-floor pin fixes it. + assertStackFillsToTheTotal(-10.0); + } + + private static void assertStackFillsToTheTotal(double axisMin) { ChartData data = ChartData.builder().categories("A") .series("S1", 10.0).series("S2", 20.0).build(); // total 30 ChartSpec.Bar bar = ChartSpec.bar().data(data) .grouping(BarGrouping.STACKED) - .valueAxis(AxisSpec.builder().min(10.0).max(30.0).build()) + .valueAxis(AxisSpec.builder().min(axisMin).max(30.0).build()) .build(); List out = ChartLayoutResolver.resolve( bar, baseStyle(), ChartDefaults.DEFAULT_THEME, 200.0, 100.0, METRICS); - // Same frame as the grouped test: plotBottomY = 14, plotTopY = 95. - // The bottom segment sits on the zero baseline and the whole stack stays - // inside the plot (with the floor wrongly at 10 it reached ~135 > 95). + // Same frame as the grouped test: plotBottomY = 14, plotTopY = 95. With + // the floor at zero and max == total, the stack fills the whole plot. double baseY = byName(out, "bar_c0_s0").y(); double stackTop = out.stream() .filter(p -> p.node().name().startsWith("bar_c0_")) .mapToDouble(p -> p.y() + p.height()) .max().orElseThrow(); assertThat(baseY).isCloseTo(14.0, within(1e-6)); - assertThat(stackTop).isLessThanOrEqualTo(95.0 + 1e-6); + assertThat(stackTop).isCloseTo(95.0, within(1e-6)); } @Test