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..96a00dbec 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 {@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 ef63b4e1d..ece92fa13 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,15 @@ 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 + // 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], 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..6a9daaa5d 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,44 @@ void horizontalBarsGrowRightFromTheLeftEdge() { assertThat(barA.y()).isGreaterThan(barB.y()); } + @Test + void stackedBarsAnchorAtZeroEvenWithAnExplicitPositiveAxisMin() { + // 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(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. 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).isCloseTo(95.0, within(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)); + } +}