fix(svm): draw through a graphics state stack, and stop the fill killing the stroke - #784
Merged
Conversation
andiwand
added a commit
to opendocument-app/OpenDocument.test.output
that referenced
this pull request
Aug 30, 2026
andiwand
force-pushed
the
feat/svm-svg-writer
branch
from
August 30, 2026 08:37
a471320 to
b56b2bb
Compare
andiwand
force-pushed
the
feat/svm-svg-writer
branch
from
August 30, 2026 08:58
b56b2bb to
bd30986
Compare
andiwand
force-pushed
the
fix/svm-graphics-state
branch
from
August 30, 2026 09:09
05c2061 to
7a68fa8
Compare
…ing the stroke `PUSH` and `POP` were skipped like any unimplemented action, so the state stack did not exist: a colour, font or map mode set inside a push leaked out of it and contaminated everything after. That is the most common thing we got wrong - of 1125 metafiles harvested from the odt/ods fixtures, 1124 push, 22317 times in all. The flags matter too: 20919 of those pushes save five of the fourteen state groups, so restoring everything on a pop would be wrong far more often than restoring nothing. Four more, all in what we already drew: - **The fill killed the stroke.** A rectangle or polygon wrote the line style and then the fill style, and the fill style ended in `stroke:none`, so a filled shape never had an outline. The pen outlines what the brush fills. - **A poly-polygon is one shape.** It was one `<polygon>` per sub-polygon, so a donut, a ring and the counter of an "o" came out solid. Now one `<path>` with `fill-rule:evenodd`, which is what vcl fills with - LibreOffice's own svg export puts that rule on its root element. - **A line's `LineInfo` was read and dropped.** Width, dash pattern and join now reach the stroke; only a default `LineInfo` keeps the hairline. - **The font size did not scale.** Coordinates went through the map mode and `font-size` did not, so text kept its size while the drawing around it shrank. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmCr22NW6wPQKiQidk96bq
The 23 private and 6 public files this branch changes, regenerated.
No restatement and no narrative: the seven test doc comments each said what their own test name says, and the source comments that told a story are cut to the point they carry. The one fact worth keeping - that half the pushes in the corpus save a subset of `PushFlags`, which is why a `POP` must restore only what its `PUSH` named - moves to `AGENTS.md`, where rationale belongs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XQpLmJpJ87qbKoG8B7kbLY
`write_path({std::move(points)}, ...)` moved nothing: a braced-init-list is a
`std::initializer_list`, whose elements are `const`, so each call copied the
whole point vector. A span carries the one polygon where it already is, and
the poly-polygon case is unchanged.
Also records why a stroke width and a dash go through the x scale alone, which
reads like an oversight and is not: `svgwriter.cxx` maps them with
`ImplMap(sal_Int32)`, which builds a square and takes its `Width()`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XQpLmJpJ87qbKoG8B7kbLY
andiwand
force-pushed
the
fix/svm-graphics-state
branch
from
August 30, 2026 09:16
7a68fa8 to
488de91
Compare
This was referenced Aug 30, 2026
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.
🤖 Generated with Claude Code
Stage 2 of #772, stacked on #779 — review that one first; this branch's
base is
feat/svm-svg-writer. It fixes the defects in what we already draw,before any new action is added.
The graphics state stack (#772 defect 2)
PUSHandPOPfell through todefault:like any unimplemented action, sothere was no state stack: a colour, font or map mode set inside a push leaked
out of it and contaminated the rest of the drawing.
This is the most common thing we get wrong. Harvesting every embedded metafile
from the odt/ods fixtures — 1125 of them — gives:
PUSH/POPTEXTALIGNSTRETCHTEXTISECTRECTCLIPREGIONThe flags matter as much as the stack: 20919 of those 22317 pushes save five of
the fourteen state groups (
LINECOLOR|FILLCOLOR|FONT|TEXTCOLOR|MAPMODE), only1140 save everything. Restoring the whole state on a pop would therefore be
wrong more often than restoring nothing, so
PushFlagsis modelled and a popputs back exactly what its push asked for.
Four more, all in what we already drew
then the fill style, and the fill style ended in
stroke:none— so a filledshape never had an outline. In vcl the pen outlines what the brush fills.
<polygon>persub-polygon, so donuts, rings and letter counters came out solid. Now one
<path>withfill-rule:evenodd— which is what vcl fills with, and whatLibreOffice's own svg export puts on its root element.
LineInfowas read and dropped (Improve SVM to SVG conversion #772 defect 8). Width, dash pattern andjoin now reach the stroke; a default
LineInfostill gets the hairline.map mode and
font-sizedid not, so text kept its size while the drawingshrank around it.
svgwriter.cxx'sGetPathStringis the model for the path data — which alsoputs the bézier stage within reach, since its
Csegments are the samefunction.
What is not here
The map mode's unit (#772 defect 6) is deferred to its own stage, and
svm/PLAN.mdsays why:MetaMapModeAction::ExecutecallsSetMapMode, whichreplaces the map mode except when the new unit is
MapRelative, where scalesmultiply and origins add. We do neither. It is 4 actions in 1125 files and
wants
vcl/source/outdev/map.cxxread properly rather than guessed at.Verification
7 new tests (15 in the svm suite now), each built from inline bytes: a shape
draws both colours, an unset colour draws nothing, a pop restores what its push
saved, a pop keeps what its push did not save, a poly-polygon is one path
with the fill rule, a polyline takes its
LineInfo, and the font size scales.Rendering: A/B'd the four svm fixtures and the changed pages of the corpus in
headless Chrome. The charts and the 1032 formulas of
odt/Vektoranalysis Zusammenfassung.odtcome out equivalent;test.svm's dataseries is now drawn at the width the file asks for instead of as a hairline,
matching what LibreOffice renders.
This changes the emitted html for 23 files, so the reference output has to be
regenerated and the pin in
test/data.cmakeadvanced before CI's compare stepcan pass. All 23 are documents that carry an svm; the public reference set is
untouched.