fix: XYSeries.getXYSeriesRenderStyle() returns empty until paint() - #988
Merged
timmolter merged 4 commits intoJul 4, 2026
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes XYSeries.getXYSeriesRenderStyle() so it returns an effective render style before the first paint, by lazily falling back to the chart’s current default render style (while preserving explicit per-series overrides).
Changes:
XYSeries.getXYSeriesRenderStyle()now falls back to a lazily-evaluated default render style supplier when no explicit style is set.XYChart.addSeries(...)wires each newXYSeriesto the chart’s default render style via a supplier, andpaint(...)validates an effective style exists without mutating the series’ explicit style.- Added
XYChartTestcoverage for: default style available immediately, explicit override surviving paint, late default changes (including between repaints) affecting unoverridden series.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
xchart/src/main/java/org/knowm/xchart/XYSeries.java |
Adds lazy default render-style resolution via a supplier when no explicit style is set. |
xchart/src/main/java/org/knowm/xchart/XYChart.java |
Sets the default-style supplier on new series; adjusts paint-time behavior to avoid mutating series style. |
xchart/src/test/java/org/knowm/xchart/XYChartTest.java |
Adds regression tests for render-style getter behavior before/after paint and across default-style changes. |
Comment on lines
31
to
+37
| public Optional<XYSeriesRenderStyle> getXYSeriesRenderStyle() { | ||
|
|
||
| if (xySeriesRenderStyle.isPresent()) { | ||
| return xySeriesRenderStyle; | ||
| } | ||
| return Optional.ofNullable(defaultRenderStyleSupplier.get()); | ||
| } |
Comment on lines
39
to
42
| Optional<XYSeriesRenderStyle> getExplicitXYSeriesRenderStyle() { | ||
|
|
||
| return xySeriesRenderStyle; | ||
| } |
Comment on lines
411
to
418
| // Resolve the series render styles if they are not set. Legend and Plot need it. | ||
| for (XYSeries xySeries : seriesMap.values()) { | ||
| if (xySeries.getXYSeriesRenderStyle().isEmpty()) { // wasn't overridden, use default from Style Manager | ||
| xySeries.setXYSeriesRenderStyle(getStyler().getDefaultSeriesRenderStyle()); | ||
| XYSeries.XYSeriesRenderStyle renderStyle = | ||
| xySeries.getXYSeriesRenderStyle().orElse(getStyler().getDefaultSeriesRenderStyle()); | ||
| if (renderStyle == null) { | ||
| throw new IllegalStateException("XY render style not set"); | ||
| } | ||
| } |
…arify paint validation - PlotContent_XY: resolve effective XYSeriesRenderStyle once per series instead of per data point, avoiding repeated supplier/Optional overhead in the render hot loop for large datasets - XYSeries: remove unused getExplicitXYSeriesRenderStyle() - XYChart.paint(): simplify the render-style check to validation-only and clarify that resolution is lazy (nothing is mutated at paint time) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Thanks @timmolter! Nice that getXYSeriesRenderStyle() reports the real style before first paint now. |
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
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.
Summary
XYSeries.getXYSeriesRenderStyle()now returns the effective render style before the chart is painted, instead of returning empty until the firstpaint(). When a series has no explicit style, the getter falls back to the chart's current default style, resolved lazily so later default changes are still reflected.Why this matters
Issue #849 reports that
getXYSeriesRenderStyle()returnsnull/empty until the chart is rendered, so code that inspects a series' render style before painting gets no useful value. The naive fix (snapshotting the default when the series is added) regresses two real usages: settingsetDefaultSeriesRenderStyle(...)after adding a series but before rendering (as in the demo'sTestForIssue181), and changing the default between repaints in live-update scenarios.This resolves the style lazily: an explicitly set per-series style always wins; otherwise the getter and the paint path both read the chart's current default at call time. The series' explicit-style field is never mutated by painting, so a default change after the first paint still applies to unoverridden series.
Testing
mvn -pl xchart -Dtest=XYChartTest test— 11 tests pass, including new coverage for: the getter returning the default before paint, setting the default after adding a series (renders as the configured default), and changing the default between repaints (unoverridden series follow the new default).Fixes #849