fix(pptx): carry c:txPr run properties into chart text - #359
Conversation
- `chart_text_primitive` draws with the family, slant and character spacing the plot geometry resolved, instead of the theme minor font upright and untracked. A chart that names no typeface inherits the theme minor font, injected at chart scope so every scope keeps inheriting from it, and a `+mj-lt`-style token resolves through the theme the same way a shape run's does. - `a:defRPr/@spc` parses into `ChartTextProperties`, resolves into `PlotFont` as a pixel tracking and spaces the glyphs of one chart text without a trailing gap. - A chart title's first `c:rich` run overrides the `c:txPr` paragraph default it sits under, which is where PowerPoint writes a title's own tracking. - Attaching a source refreshes the text properties of a stored chart part. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
All contributors have signed the CLA — thank you! ✍️ ✅ Posted by the CLA bot. |
Greptile SummaryThis PR parses chart character spacing, carries chart font family and slant into PPTX rendering, refreshes stored chart text properties when a source is attached, and adds a focused chart fixture. Three correctness gaps remain:
Confidence Score: 2/5The PR is not yet safe to merge because common title inheritance and composed/fallback chart-text paths can still render incorrect formatting or legend geometry. Three blocking correctness issues remain: title paragraph defaults can be lost, composed chart output drops tracking, and negative tracking can yield invalid fallback legend widths. Files Needing Attention: crates/ooxml-drawingml/src/chart/parse.rs, crates/ooxml-drawingml/src/chart/geometry.rs, crates/pptx-render/src/lib.rs
|
| Filename | Overview |
|---|---|
| crates/ooxml-drawingml/src/chart/parse.rs | Parses spacing and title run overrides, but omits rich paragraph defaults from a three-level title style cascade. |
| crates/ooxml-drawingml/src/chart/geometry.rs | Propagates tracking through chart styles, but its unmeasured legend-width fallback can return negative and trailing-gap-inflated widths. |
| crates/pptx-render/src/layout.rs | Correctly resolves chart font faces and applies cluster-based tracking without a trailing gap on the normal slide-layout path. |
| crates/pptx-render/src/lib.rs | Carries chart italics into composed output but still lacks a carrier for chart character spacing. |
| crates/pptx-edit/src/deck.rs | Refreshes chart-, title-, legend-, and axis-level text properties during source attachment. |
| crates/pptx-render/tests/chart_text_properties.rs | Adds strong integration coverage for the normal renderer, though not the composed chart path or richer title cascades. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
XML["Chart XML: txPr / rich text"] --> Parse["ChartTextProperties"]
Parse --> Plot["PlotTextStyle / PlotFont"]
Plot --> Geometry["Chart geometry and measurement"]
Geometry --> Slide["SlideRenderer positioned glyphs"]
Geometry --> Composed["Composed chart logical TextRun"]
Slide --> Display["PPTX display list"]
Composed --> Host["Host-side text layout"]
Comments Outside Diff (1)
-
crates/pptx-render/src/lib.rs, line 310-320 (link)The composed-slide chart path emits only an unpositioned
TextRun, which has no letter-spacing field, and leaveslinesempty. As a result, chart text withc:txPr/@spcrenders without tracking on this path, even though chart geometry may already have measured and positioned the legend using that spacing.Knowledge Base Used: PPTX rendering and WebAssembly
Reviews (1): Last reviewed commit: "fix(pptx): carry c:txPr run properties i..." | Re-trigger Greptile
The fallback multiplied the character count by size/2 plus tracking, so a negative spc inverted it, and it counted a gap after the last character. A sink without text measurement — docx, xlsx, composed pptx — then wrapped and placed legend entries off a negative width. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Two threads, two different answers. Negative tracking breaks widths — real, fixed ( Confirmed by reading it: the fallback was Extracted it as This is the same defect class as the one found on #325 in Title defaults are skipped — I do not think this one holds.
I agree that is the correct precedence in principle, and I have not implemented it — but I could |
TL;DR:
Before/After:
The repository's new chart fixture,
crates/pptx-render/tests/fixtures/chart-text-properties.pptx,rendered with Georgia mapped to Liberation Serif and Verdana to Liberation Sans. Its chart declares
+mj-ltat chart scope,i="1" spc="300"on the title withspc="600"on the title's own run, andb="1"plusVerdanaon the category axis:minimal-chartslide 1, a PowerPoint design template, is the corpus repro. Its chart namesOpen Sans, which neither engine has, so what changes there is which substitute is used —LibreOffice, before, after:
Repro file:
minimal-chart, slide 1. Every axis label of its chart declares<a:defRPr sz="1197" b="1" ...><a:latin typeface="Open Sans"/></a:defRPr>and was drawn in thetheme's Calibri. The same shapes are reproduced as a committed fixture,
crates/pptx-render/tests/fixtures/chart-text-properties.pptx, described inchart-text-properties.mdbeside it.Summary:
c:txPr, instead ofthe theme minor font upright.
chart_text_primitivewas resolving+mn-ltitself and passingfalsefor italic, so only weight, size and colour of ac:txPrever reached the screen.the chart-scope
PlotTextStyle, which every other scope already inherits from, rather thanoverriding the resolved font. A
+mj-lt-style token resolves through the theme the same way ashape run's family does. Hosts that pass no default keep the shared crate's own fallback.
a:defRPr/@spcis parsed intoChartTextProperties, resolved intoPlotFontas a pixeltracking, and applied after every cluster of a chart text with no trailing gap, disabling
optional ligatures when it is non-zero — the same rules the shape text path uses. This is what
carries fix(pptx): apply run-level spc as character tracking #325's tracking across the chart boundary.
c:richrun now overrides thec:txPrparagraph default it sits underrather than being ignored whenever a
c:txPrexists. PowerPoint writes a title's own tracking onthat run, so this is the difference between reading
spc="300"and reading thespc="0"besideit.
part, next to the fills and lines that were already merged. No schema bump: a document that never
reattaches its source keeps its stored chart text.
ChartTextProperties,PlotTextStyleandPlotFont, soxlsx-renderanddocx-layoutare unchanged.crates/docx-layout/tests/chart_snapshots.rsandcrates/xlsx-render/tests/snapshots/chart_display_list.jsonare byte-identical and were nottouched.
docx-layoutcarriesspacingPtthrough its own deserializer but describes a font witha CSS shorthand, which has no letter-spacing, so docx charts stay untracked.
Measured against a baseline built from this worktree with the change reverted and the binding
rebuilt, over all 17 chart-bearing slides in the corpus:
minimal-chart/013.46 -> 3.51 (+0.05), four slides of one third-party deck 12.41 -> 12.45(+0.04), one slide +0.01, the other eleven +0.00.
Open Sans, which the harness does not register, so their chart text moves from Carlito (thetheme's Calibri) to the fallback face. Registering any face under
Open Sansand re-rendering thesame two slides with the same engine gives 3.43 for
minimal-chart/01, below the 3.46baseline, and 12.41 for the other, equal to it. Nothing in the corpus regresses once the font
a chart asks for exists.
branch draws it 286px wide on the same centre, and main drew it 184px wide. It is a net zero on
the metric because that title also sits ~18px above where LibreOffice puts it, so more correct ink
lands in the wrong band. That vertical offset is a separate defect.
Test plan:
cargo test -p betteroffice-drawingml --features chart— 132 tests. The chart module isbehind a non-default
chartfeature; a plaincargo testruns none of them.cargo test -p betteroffice-pptx-render -p betteroffice-pptx-parse -p betteroffice-pptx-edit -p betteroffice-docx-layout -p betteroffice-xlsx-render— green, including theschema-migration tests and both shared chart snapshots, neither of which needed rebaselining.
cargo clippy --workspace --all-targets --features chartandcargo fmt --check— clean.cargo checkinsidebindings/python-pptx, which is a separate cargo workspace the workspacebuild does not cover.
0f474aaand pass here:chart_text_takes_its_family_slant_size_and_tracking_from_c_txprandtracking_spaces_the_glyphs_of_a_chart_title_without_a_trailing_gap.minimal-chartslide 1 with a face registered underOpen Sans: the fine diffagainst the LibreOffice reference falls to 3.43 from 3.46.
Closes #358