Issue #577: wrap horizontal legends instead of overflowing the image #1007
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| package org.knowm.xchart.standalone.issues; | ||
|
|
||
| import org.knowm.xchart.SwingWrapper; | ||
| import org.knowm.xchart.XYChart; | ||
| import org.knowm.xchart.XYChartBuilder; | ||
| import org.knowm.xchart.style.Styler; | ||
|
|
||
| /** | ||
| * Reproducer for https://github.com/knowm/XChart/issues/577 | ||
| * | ||
| * <p>"When displaying horizontal legends, and too many series, the legends are extending beyond the | ||
| * chart, and beyond the image, and are truncated. This happens often when the series names are | ||
| * long." (reported with a horizontal, OutsideS legend) | ||
| * | ||
| * <p>Expected: legends should go to the next line when the current line is too long. | ||
| * | ||
| * <p>Before the fix, all legend entries were laid out on a single row, so the centered OutsideS | ||
| * legend spilled past both image edges and got truncated. Entries now wrap onto additional rows, | ||
| * keeping the whole legend within the image width. | ||
| */ | ||
| public class TestForIssue577 { | ||
|
|
||
| public static void main(String[] args) { | ||
|
|
||
| new SwingWrapper<>(getChart()).displayChart(); | ||
| } | ||
|
|
||
| /** Constructs and returns the chart without launching a window (headless-safe). */ | ||
| public static XYChart getChart() { | ||
|
|
||
| // Create chart | ||
| XYChart chart = | ||
| new XYChartBuilder() | ||
| .width(900) | ||
| .height(500) | ||
| .title("Horizontal Legend Wrapping Demo") | ||
| .xAxisTitle("X") | ||
| .yAxisTitle("Y") | ||
| .build(); | ||
|
|
||
| // General Styler settings: a horizontal legend below the plot | ||
| chart.getStyler().setLegendPosition(Styler.LegendPosition.OutsideS); | ||
| chart.getStyler().setLegendLayout(Styler.LegendLayout.Horizontal); | ||
| chart.getStyler().setLegendVisible(true); | ||
|
|
||
| // Many series with long names so a single legend row would overflow the image width | ||
| String[] seriesNames = { | ||
| "Temperature Sensor North", | ||
| "Temperature Sensor South", | ||
| "Humidity Sensor East Wing", | ||
| "Pressure Gauge Basement", | ||
| "Wind Speed Rooftop Array", | ||
| "Solar Irradiance Panel 12", | ||
| "CO2 Concentration Lobby", | ||
| "Particulate Matter PM2.5" | ||
| }; | ||
| for (int i = 0; i < seriesNames.length; i++) { | ||
| chart.addSeries( | ||
| seriesNames[i], new double[] {0, 1, 2}, new double[] {i, i + 1, i}); | ||
| } | ||
|
|
||
| return chart; | ||
| } | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -140,6 +140,17 @@ public void paint(Graphics2D g) { | |
| break; | ||
| } | ||
|
|
||
| // An OutsideS legend is centered on the plot, whose center sits right of the image center (the | ||
| // left y-axis consumes horizontal space). A wide (wrapped) horizontal legend can therefore | ||
| // still run off the right image edge even though every row fits within | ||
| // getHorizontalLegendMaxRowWidth(). Clamp the box so it always stays fully within the image | ||
| // (issue #577). For a legend narrower than the image this only nudges it left when it would | ||
| // otherwise be cut off; a normal centered legend is left untouched. | ||
| if (chart.getStyler().getLegendPosition() == Styler.LegendPosition.OutsideS) { | ||
| xOffset = Math.min(xOffset, chart.getWidth() - bounds.getWidth() - LEGEND_MARGIN); | ||
| xOffset = Math.max(xOffset, LEGEND_MARGIN); | ||
| } | ||
|
|
||
| // draw legend box background and border | ||
| Shape rect = new Rectangle2D.Double(xOffset, yOffset, bounds.getWidth(), height); | ||
| g.setColor(chart.getStyler().getLegendBackgroundColor()); | ||
|
|
@@ -230,11 +241,20 @@ private Rectangle2D getBoundsHintHorizontal() { | |
| // 0). | ||
| } | ||
|
|
||
| // All rows in a wrapping horizontal legend share the same (tallest) row height so entries line | ||
| // up regardless of series order (issue #892). | ||
| double rowHeight = computeHorizontalRowHeight(); | ||
|
|
||
| // Entries flow left-to-right and wrap to a new row once the current row would exceed the | ||
| // available width, so a legend with many (or long-named) series no longer spills past the image | ||
| // edge (issue #577). getBoundsHintHorizontal() and the subclass doPaint() methods walk the same | ||
| // series in the same order using the same per-entry advance width, so they wrap at identical | ||
| // points and the reported box matches what is painted. | ||
| double maxRowWidth = getHorizontalLegendMaxRowWidth(); | ||
|
|
||
| double currentRowWidth = 0; | ||
| double widestRow = 0; | ||
| int rowCount = 1; | ||
|
|
||
| Map<String, S> map = chart.getSeriesMap(); | ||
| for (S series : map.values()) { | ||
|
|
@@ -246,41 +266,117 @@ private Rectangle2D getBoundsHintHorizontal() { | |
| continue; | ||
| } | ||
|
|
||
| double entryAdvanceWidth = getHorizontalLegendEntryAdvanceWidth(series); | ||
|
|
||
| // Wrap to the next row when this entry would overflow the current one (but never wrap an | ||
| // empty row, so a single over-wide entry still gets its own row). | ||
| if (currentRowWidth > 0 && currentRowWidth + entryAdvanceWidth > maxRowWidth) { | ||
| widestRow = Math.max(widestRow, currentRowWidth); | ||
| rowCount++; | ||
| currentRowWidth = 0; | ||
| } | ||
| currentRowWidth += entryAdvanceWidth; | ||
| } | ||
| widestRow = Math.max(widestRow, currentRowWidth); | ||
|
|
||
| // Legend Box. For a single row this reduces to the previous formula | ||
| // (widestRow + padding wide, rowHeight + 2*padding tall). | ||
| double width = widestRow + chart.getStyler().getLegendPadding(); | ||
| double height = | ||
| rowCount * rowHeight | ||
| + (rowCount - 1) * chart.getStyler().getLegendPadding() | ||
| + chart.getStyler().getLegendPadding() * 2; | ||
|
|
||
| return new Rectangle2D.Double(0, 0, width, height); // 0 indicates not sure yet. | ||
| } | ||
|
|
||
| /** | ||
| * The tallest legend entry across all shown series (text or graphic, whichever is taller). Every | ||
| * row in a horizontal legend uses this so entries share a common baseline (issue #892). | ||
| */ | ||
| double computeHorizontalRowHeight() { | ||
|
|
||
| double rowHeight = 0; | ||
| for (S series : chart.getSeriesMap().values()) { | ||
| if (!series.isShowInLegend() || !series.isEnabled()) { | ||
| continue; | ||
| } | ||
| rowHeight = | ||
| Math.max( | ||
| rowHeight, | ||
| getLegendEntryHeight( | ||
| getSeriesTextBounds(series), (int) getSeriesLegendRenderGraphicHeight(series))); | ||
| } | ||
| return rowHeight; | ||
| } | ||
|
|
||
| /** | ||
| * The horizontal distance a single legend entry consumes, including the trailing padding that | ||
| * separates it from the next entry. Used by both bounds calculation and painting so they wrap at | ||
| * exactly the same points. | ||
| */ | ||
| double getHorizontalLegendEntryAdvanceWidth(S series) { | ||
|
|
||
| return getLegendEntryWidth(getSeriesTextBounds(series), getLegendEntryMarkerWidth(series)) | ||
| + chart.getStyler().getLegendPadding(); | ||
| } | ||
|
|
||
| /** | ||
| * The width of the legend graphic (line/marker or box) preceding an entry's text. Line and | ||
| * Scatter entries reserve the series-line length (their text is painted at that offset, with the | ||
| * marker centered within it); box-style entries reserve {@link #BOX_SIZE}. Subclasses whose | ||
| * graphic isn't sized by render type (e.g. OHLC) override this. | ||
| */ | ||
| int getLegendEntryMarkerWidth(S series) { | ||
|
|
||
| return (series.getLegendRenderType() == LegendRenderType.Line | ||
| || series.getLegendRenderType() == LegendRenderType.Scatter) | ||
| ? chart.getStyler().getLegendSeriesLineLength() | ||
| : BOX_SIZE; | ||
| } | ||
|
|
||
| /** | ||
| * The maximum width one row of a horizontal legend may occupy before wrapping. Keyed off the | ||
| * chart width (minus margin and padding) so the centered OutsideS legend box never extends past | ||
| * the image edge (issue #577). | ||
| */ | ||
| double getHorizontalLegendMaxRowWidth() { | ||
|
Member
Author
There was a problem hiding this comment. Bug (verified): a near-max-width row can still be truncated on the right edge. The wrap threshold is keyed off the chart width, but Reproduced empirically (800px-wide chart, 14 series, sweeping name lengths): The wrap threshold can't simply use the plot width instead — legend bounds are consumed during axis preparation, before the plot width is final (circular dependency). The clean fix is to clamp the computed // after the switch statement, for horizontal layouts
xOffset = Math.min(xOffset, chart.getWidth() - bounds.getWidth() - LEGEND_MARGIN);
xOffset = Math.max(xOffset, LEGEND_MARGIN);This keeps the plot-centered position in the common case and only nudges the box left when it would otherwise be cut off. The regression test should then also assert position (
Member
Author
There was a problem hiding this comment. Fixed in 3a87915. Clamped |
||
|
|
||
| return chart.getWidth() - 2.0 * LEGEND_MARGIN - 2.0 * chart.getStyler().getLegendPadding(); | ||
| } | ||
|
|
||
| /** | ||
| * A left-to-right pen for laying out a wrapping horizontal legend. Subclass painters advance it | ||
| * per entry and read {@link #x}/{@link #y} as the current entry's origin. | ||
| */ | ||
| final class HorizontalCursor { | ||
|
|
||
| final double leftOrigin; | ||
| final double rowHeight; | ||
| private final double maxRowWidth; | ||
| double x; | ||
| double y; | ||
|
|
||
| HorizontalCursor(double startx, double starty) { | ||
| this.leftOrigin = startx; | ||
| this.x = startx; | ||
| this.y = starty; | ||
| this.rowHeight = computeHorizontalRowHeight(); | ||
| this.maxRowWidth = getHorizontalLegendMaxRowWidth(); | ||
| } | ||
|
|
||
| /** Wrap to the next row if placing an entry of the given advance width would overflow. */ | ||
| void maybeWrap(double entryAdvanceWidth) { | ||
| if (x > leftOrigin && (x - leftOrigin) + entryAdvanceWidth > maxRowWidth) { | ||
| x = leftOrigin; | ||
| y += rowHeight + chart.getStyler().getLegendPadding(); | ||
| } | ||
| } | ||
|
|
||
| /** Move past an entry of the given advance width. */ | ||
| void advance(double entryAdvanceWidth) { | ||
| x += entryAdvanceWidth; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
||
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor (pre-existing, carried over):
Scatterentries under-advance bylegendSeriesLineLength - BOX_SIZE(4px at defaults).Legend_Marker.doPaint()draws bothLineandScatterentries' text atstartx + getLegendSeriesLineLength() + padding(the marker is centered atlineLength / 2), but this method returnsBOX_SIZEforScatterbecause it only special-casesLine. So a Scatter entry actually occupieslineLength + padding + textWidthwhile its advance width claimsBOX_SIZE + padding + textWidth.The old single-row code had the same inconsistency (it also only checked
== Line), so this PR faithfully preserves existing spacing. But now that the advance width also decides wrap points, a Scatter-heavy legend can pack a row a few px too tight and let the last entry's text touch the box border.Suggested follow-up (would slightly change existing horizontal spacing for scatter charts, hence not snuck into this PR):
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 3a87915.
getLegendEntryMarkerWidth()now treatsScatterlikeLine(reserves the series-line length), matching whereLegend_Markerpaints the text, so the advance width and the wrap points are consistent for scatter charts.