fix(charts): anchor stacked bars at zero and guard paintForSeries#191
Merged
Conversation
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).
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Two chart-engine issues:
value / range. An explicitvalueAxis().min()moved the baseline off zero without rescaling the segments — a positive min made the stack overshoot its total (past the plot top), a negative min anchored it below zero (top undershoots).ChartStyle.paintForSeriesleaks a bareIndexOutOfBoundsExceptionon a negative index (the palette modulo gives a negative remainder).What
ChartLayoutSupport.computeFramepins the stacked floor to exactly zero (if (stacked && domain[0] != 0.0) domain[0] = 0.0), regardless of the requested min's sign. Grouped bars still honour an explicit minimum. Single-source — covers vertical and horizontal stacked via the sharedFrame.BarGrouping.STACKEDJavadoc documents the always-zero baseline.ChartStyle.paintForSeriesrejects a negative index withIllegalArgumentException("series index must be >= 0: …").Tests
ChartLayoutResolverTest: stacked + explicit positive min AND negative min — both assert the stack fills exactly to the value-total at the plot top (positive min overshot to ~135, negative undershot to ~75 before the fix).ChartStyleTest(new): negative index rejected; valid index cycles the fallback palette.Scope note
The plan's
NumberFormatSpecDecimalFormat-caching item is not here: a bounded compile-path micro-opt (charts resolve once per layout, not a per-row hot path), ~11 call sites across 3 files for negligible gain. Deferred as an optional separate PR.Lane: canonical charts (correctness; no perf-characteristic change, so no benchmark gate).