Skip to content

fix: PDF pie chart legend box overlaps slice label (fixes #1680)#1825

Merged
krystophny merged 1 commit intomainfrom
fix/issue-1680-pie-legend-pdf-overlap
Apr 26, 2026
Merged

fix: PDF pie chart legend box overlaps slice label (fixes #1680)#1825
krystophny merged 1 commit intomainfrom
fix/issue-1680-pie-legend-pdf-overlap

Conversation

@krystophny
Copy link
Copy Markdown
Collaborator

Summary

The 'east' legend location string was unrecognized by legend_set_position and fell through to LEGEND_UPPER_RIGHT, placing the legend inside the plot area where it overlapped pie slice labels in PDF output. The merged text appeared as "OnlineNorth" instead of two separate entries.

Added LEGEND_EAST position constant that places the legend outside the plot area to the right, centered vertically — matching matplotlib semantics.

Changes

  • src/ui/fortplot_legend.f90: new LEGEND_EAST constant (5); 'east' case in legend_set_position; ASCII layout case
  • src/ui/fortplot_legend_layout.f90: LEGEND_EAST handling in calculate_legend_position
  • src/backends/ascii/fortplot_ascii.f90: LEGEND_EAST case in ascii_calculate_legend_position
  • src/backends/ascii/fortplot_ascii_legend.f90: LEGEND_EAST case in calculate_ascii_legend_position
  • test/test_pie_legend_east_pdf.f90: regression test — creates pie chart with 'east' legend, saves PDF, verifies no merged text

Verification

Test passes after fix

$ fpm test --target test_pie_legend_east_pdf
 PASS: pie legend at east position does not merge with slice labels in PDF

CI test suite passes

$ make test-ci
CI essential test suite completed successfully

Artifact verification passes

$ make verify-artifacts
Artifact verification passed.

PDF text extraction confirms fix

Before: legend text and slice label merged into single string in PDF.
After: pdftotext shows "Online" and "North" as separate text entries:

$ pdftotext stateful_sales.pdf -
Regional revenue share
Online
North
15.0%
30.0%
...

…p in PDF (fixes #1680)

Add LEGEND_EAST position constant that places the legend outside the plot
area to the right, centered vertically. The 'east' location string was
previously unrecognized and fell through to LEGEND_UPPER_RIGHT, causing
the legend box to overlap pie slice labels in PDF output.

Files changed:
- src/ui/fortplot_legend.f90: add LEGEND_EAST constant and 'east' case
- src/ui/fortplot_legend_layout.f90: handle LEGEND_EAST positioning
- src/backends/ascii/fortplot_ascii.f90: handle LEGEND_EAST in ASCII
- src/backends/ascii/fortplot_ascii_legend.f90: handle LEGEND_EAST
- test/test_pie_legend_east_pdf.f90: regression test for PDF output
@krystophny
Copy link
Copy Markdown
Collaborator Author

Review: Issue-Resolution Verification

Issue #1680: PDF pie chart legend box overlaps slice label, first entry label truncated ("OnlineNorth")

Requirement Evidence
`"east"` location string unrecognized FIXED: `legend_set_position` handles `"east"` at `fortplot_legend.f90:123-124`
Legend fell through to `LEGEND_UPPER_RIGHT` causing overlap FIXED: `LEGEND_EAST` (5) places legend outside plot area at `fortplot_legend_layout.f90:247-249`
PDF merged text ("OnlineNorth") FIXED: regression test `test_pie_legend_east_pdf.f90` verifies separate `"(North)"` and `"(Online)"` in PDF stream
PNG unaffected Confirmed: fix only affects `"east"` path; default behavior unchanged
ASCII backend coverage FIXED: `fortplot_ascii.f90` and `fortplot_ascii_legend.f90` both handle `LEGEND_EAST`
No scope creep Confirmed: only `LEGEND_EAST` constant + `"east"` case added; trailing whitespace cleaned

Code Review

  • `fortplot_legend.f90`: constant, public export, `select case` — correct
  • `fortplot_legend_layout.f90`: `box%x = data_width + margins(1)` places legend outside plot area; `box%y` centers vertically — correct matplotlib semantics
  • ASCII backends: positioning consistent with `LEGEND_EAST` semantics
  • Test: PDF stream text extraction with explicit merged-text negative check — well-designed regression test
  • Module sizes within limits (`fortplot_legend.f90` at 600 lines < 1000 hard limit)

Deferred (polish, tracked separately)

Verdict: PR fully resolves issue #1680. All requirements addressed with evidence. No blocking issues. Ready for merge after CI completes.

@krystophny krystophny merged commit 2689bb6 into main Apr 26, 2026
5 checks passed
@krystophny krystophny deleted the fix/issue-1680-pie-legend-pdf-overlap branch April 26, 2026 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant