Skip to content

Issue #892: fix horizontal legend layout with mixed render styles#999

Merged
timmolter merged 2 commits into
developfrom
issue-892-mixed-legend-layout
Jul 9, 2026
Merged

Issue #892: fix horizontal legend layout with mixed render styles#999
timmolter merged 2 commits into
developfrom
issue-892-mixed-legend-layout

Conversation

@timmolter

Copy link
Copy Markdown
Member

Fixes #892.

Problem

With a horizontal legend (LegendLayout.Horizontal, e.g. LegendPosition.OutsideS) mixing render styles such as Bar + Line, the legend markers/boxes and text were misaligned, and the whole entry group sat too low inside the legend box.

Reported by @JamesPaceOnTailwind.

Root causes

Two independent bugs, both only visible when entries in one horizontal row differ in height:

  1. Entries not aligned to each otherLegend_Marker centered each entry against its own graphic height, so a Bar's 20px box and a Line's smaller marker landed on different vertical baselines. In a horizontal layout every entry shares one row, so the fix computes a common row height (the tallest graphic/text across all shown series) and centers the box, line, marker, and text against it uniformly. Vertical layout is unchanged (each entry is its own row).

  2. Group sat too lowLegend_.getBoundsHintHorizontal() computed the legend-box height by assignment inside the series loop, so the container was sized to the last series' height instead of the tallest. With the Line series added last, the box was too short for the 20px Bar box and the content overflowed downward. Now it accumulates the max across all series.

Before / After

Before: the line/marker and its label sit higher than the box and its label, and the group is low in the box.
After: box, line, marker, and both labels share one vertical center, centered in the container.

Testing

  • Added RegressionTestIssue892 — renders the reporter's chart and asserts (1) the Bar box center and Line marker center align, and (2) the entry group is vertically centered in the legend container. Each assertion fails independently if its respective fix is reverted.
  • Added demo TestForIssue892 (the reporter's example, headless-safe getChart()).
  • Full xchart suite: 130 pass. fmt:check clean.

🤖 Generated with Claude Code

A horizontal legend mixing render styles (e.g. Bar + Line) rendered the
markers/boxes and text misaligned, and the whole entry group sat too low
in the legend box.

Two root causes:

1. Legend_Marker centered each entry against its own graphic height, so a
   Bar's 20px box and a Line's smaller marker landed on different vertical
   baselines. In a horizontal layout every entry shares one row, so a common
   row height (max graphic/text height across all shown series) is now used
   to vertically center the box, line, marker, and text uniformly. Vertical
   layout keeps its per-entry height (each entry is its own row).

2. Legend_.getBoundsHintHorizontal computed the legend-box height by
   assignment inside the series loop, so the box was sized to the LAST
   series' height instead of the tallest. With the Line series added last,
   the box was too short for the 20px Bar box and the content overflowed
   downward. Now it accumulates the max across all series.

Added RegressionTestIssue892 (asserts entries align to each other and are
centered in the container; fails on either bug in isolation) and demo
TestForIssue892.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes horizontal legend layout issues when mixing render styles (e.g., Bar + Line) by (1) aligning legend entry graphics/text to a shared row height and (2) sizing the horizontal legend container based on the tallest entry, and adds a regression test + demo reproducer for Issue #892.

Changes:

  • Compute a common row height for horizontal legends so mixed render-style entries align vertically (Legend_Marker).
  • Fix horizontal legend bounds calculation to use the max entry height across all series (Legend_).
  • Add regression coverage and a runnable demo reproducer for Issue #892.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
xchart/src/main/java/org/knowm/xchart/internal/chartpart/Legend_Marker.java Centers legend graphics/text against a shared row height for horizontal legends.
xchart/src/main/java/org/knowm/xchart/internal/chartpart/Legend_.java Fixes horizontal legend height calculation to accumulate the maximum across series.
xchart/src/test/java/org/knowm/xchart/regressiontests/RegressionTestIssue892.java Adds an image-based regression test asserting alignment + centering for the reported scenario.
xchart-demo/src/main/java/org/knowm/xchart/standalone/issues/TestForIssue892.java Adds a headless-safe demo reproducer for Issue #892.

Comment on lines +32 to +36
CategoryChart chart =
new CategoryChartBuilder().width(800).height(600).title("issue 892").build();
chart.getStyler().setLegendPosition(Styler.LegendPosition.OutsideS);
chart.getStyler().setLegendLayout(Styler.LegendLayout.Horizontal);

Comment on lines +51 to +54
// The OutsideS legend sits at the very bottom. Its background matches the (white) plot
// background higher up, so locate the legend container as the bottom-most contiguous block of
// background-colored rows (below it is only the chart-background margin). Then measure the
// entries strictly within its rows, so plot content can never leak into the measurement.
+ chart.getStyler().getLegendSeriesLineLength()
+ chart.getStyler().getLegendPadding();
paintSeriesText(g, seriesTextBounds, axesChartStyler.getMarkerSize(), x, starty);
paintSeriesText(g, seriesTextBounds, (int) rowHeight, x, starty);
Comment on lines +18 to +21
* <p>Expected: in a horizontal legend, the markers/boxes and their text should line up uniformly
* across mixed render styles. Currently a Bar entry centers its text within the 20px legend box
* while a Line entry centers within the (smaller) marker size, so the two entries sit at different
* vertical baselines.
- Legend_Marker: instead of passing the shared row height into
  paintSeriesText() (which altered its per-line centering math for
  multi-line labels and was inconsistent with other Legend_* impls),
  center each entry's natural block within the shared row by shifting a
  per-entry vertical origin (entryStarty). All per-element math is now
  identical to the vertical layout, just drawn from entryStarty.

- RegressionTestIssue892: clarify how the legend container is located
  (default XChartTheme uses a grey chart background, so the white legend
  is the bottom-most white band) and add a guard that fails loudly if the
  detected container is implausibly tall (background detection degenerated
  into the plot area).

- TestForIssue892: reword Javadoc to past tense since the fix ships with it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@timmolter

Copy link
Copy Markdown
Member Author

Thanks @copilot — addressed in 0410740:

  1. paintSeriesText centering math (Legend_Marker) — good catch. Reverted to passing the real graphic size (markerSize / BOX_SIZE) into paintSeriesText, so its per-line/multi-line centering is untouched and consistent with the other Legend_* implementations. Instead of scaling the centering reference, each entry's natural block is now centered in the shared row by shifting a per-entry vertical origin (entryStarty = starty + (commonRowHeight - legendEntryHeight)/2). All per-element math is identical to the vertical layout, just drawn from entryStarty.

  2. bottomMostBand / white background (test) — the concern is valid for AbstractBaseTheme (white chart background), but the default theme used here is XChartTheme, which overrides the chart background to grey while the plot and legend backgrounds stay white. So the legend is unambiguously the bottom-most white band, separated from the white plot area by the grey margin — deterministic, not flaky. I also added a guard that fails loudly if the detected container is implausibly tall, so a future default change can't silently let plot pixels leak in.

  3. Misleading comment (test) — rewritten to describe the actual strategy (grey chart background vs. white legend/plot).

  4. Demo Javadoc tense — reworded "Currently …" to "Before the fix …".

Re-verified: RegressionTestIssue892 passes with both fixes and fails independently if either fix is reverted to develop; full xchart suite 130 pass; fmt:check clean.

@timmolter timmolter merged commit 3d5c6fd into develop Jul 9, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mixed series render style legend layout

2 participants