fix(svm): draw through a graphics state stack, and stop the fill killing the stroke

CHANGELOG.md

@@ -19,6 +19,14 @@ The release run heads these entries with the version and opens a fresh
 - A spreadsheet decodes in less memory: 626 MB peak instead of 914 MB on a
   297 MB `content.xml`. Rendered output is unchanged.
 
+- A StarView metafile draws through a graphics state stack, so a colour, font
+  or map mode set inside a `PUSH` no longer leaks out of it and contaminates
+  the rest of the drawing. Nearly every metafile a document carries uses one.
+
+- A filled shape in a StarView metafile keeps its outline, its poly-polygons
+  cut their holes out, a line takes the width, dashing and join it carries, and
+  the font size scales with the drawing.
+
 - Text in a StarView metafile is escaped into the svg it renders as. An `&`,
   `<` or `>` in a label made the svg malformed, and a malformed svg renders as
   nothing.

src/odr/internal/svm/AGENTS.md

@@ -51,6 +51,9 @@ The format is not specified anywhere. The references, best first:
 - **An unimplemented action is skipped by its length, never guessed at.** The
   loop checks how far the reader got: short means the rest is ignored (logged),
   past the end means the file is malformed and we throw.
+- **A `POP` restores only what its `PUSH` named.** Half the pushes in the
+  corpus save a subset of `PushFlags`, so restoring the whole state would be
+  wrong more often than right.
 - **Everything unhandled is logged.** A metafile we cannot draw looks exactly
   like one we drew correctly — a blank rectangle raises no error anywhere — so
   the log is the only way to tell. Anything reached by `default:` says so.

src/odr/internal/svm/PLAN.md

@@ -19,21 +19,60 @@ Each stage is one pull request, stacked on the one before it.
    #772's defects 1 (escaping, but see stage 3 for the encoding half of it), 9
    (style dispatch) and 10 (silence).
 2. **Fixes to what we already emit.** The graphics state stack (`PUSH`/`POP`),
-   poly-polygon fill rule, the font size and map-mode unit in the transform,
-   `LineInfo`. #772 defects 2, 3, 5, 6, 8.
-3. **Text.** `TEXTALIGN`, the `TEXTARRAY` dx array, `TEXTRECT`, the #95 font
-   attributes (bold, italic, underline, strikeout, family), and decoding a
-   non-`UCS2` string instead of passing its bytes through — until then a
-   latin-1 label emits invalid utf-8, which costs the image exactly as an
-   unescaped `&` did.
-4. **Primitives.** `PIXEL`, `POINT`, `LINE`, `ROUNDRECT`, `ELLIPSE`, `ARC`,
-   `PIE`, `CHORD` — one `svgwriter.cxx` case each.
+   the fill that killed the stroke, the poly-polygon fill rule, the font size
+   in the transform, `LineInfo`. #772 defects 2, 3, 5, 8.
+3. **Text.** `TEXTALIGN`, the `TEXTARRAY` dx array, the `STRETCHTEXT` width,
+   `TEXTRECT`, the #95 font attributes (bold, italic, underline, strikeout,
+   family), and decoding a non-`UCS2` string instead of passing its bytes
+   through — until then a latin-1 label emits invalid utf-8, which costs the
+   image exactly as an unescaped `&` did.
+4. **Clipping.** `CLIPREGION`, `ISECTRECTCLIPREGION`,
+   `ISECTREGIONCLIPREGION`, `MOVECLIPREGION`.
 5. **Bitmaps** (#194). See the shortcut below.
-6. **Fills, clipping, transparency.** `GRADIENT`, `GRADIENTEX`, `HATCH`,
-   `WALLPAPER`, the `CLIPREGION` family, `TRANSPARENT`, `FLOATTRANSPARENT`.
-7. **Stretch.** Bézier flags (#772 defect 4), the `EPS` substitute metafile,
+6. **Primitives.** `PIXEL`, `POINT`, `LINE`, `ROUNDRECT`, `ELLIPSE`, `ARC`,
+   `PIE`, `CHORD` — one `svgwriter.cxx` case each.
+7. **Fills and transparency.** `GRADIENT`, `GRADIENTEX`, `HATCH`,
+   `WALLPAPER`, `TRANSPARENT`, `FLOATTRANSPARENT`.
+8. **The map mode's unit** (#772 defect 6), see below.
+9. **Stretch.** Bézier flags (#772 defect 4), the `EPS` substitute metafile,
    and version-1 (pre-`VCLMTF`) files via `SvmConverter.cxx`.
 
+The order follows what files actually contain, not the action list. Over 1125
+metafiles harvested from the `odt`/`ods` fixtures:
+
+| action | occurrences | files (of 1125) |
+| --- | --- | --- |
+| `PUSH` / `POP` | 22317 each | 1124 |
+| `TEXTALIGN` | 21534 | 1117 |
+| `STRETCHTEXT` | 20335 | 1097 |
+| `ISECTRECTCLIPREGION` | 1124 | 1123 |
+| `RECT` | 845 | 324 |
+| `TEXTARRAY` | 764 | 20 |
+| `POLYLINE` | 477 | 13 |
+| `POLYPOLYGON` | 247 | 14 |
+| `LINE` | 47 | 2 |
+| `BMPEXSCALE` | 1 | 1 |
+
+`ELLIPSE`, `ARC`, `PIE`, `CHORD`, `ROUNDRECT`, `POINT`, `PIXEL`, `GRADIENT`,
+`HATCH`, `TRANSPARENT` and `EPS` do not occur at all, which is why they come
+after clipping rather than before it. The one bitmap is the whole data area of
+`odr-private/svm/Vyplaty.svm` — 1.59 MB of `BMPEXSCALE` in a 1.63 MB file, and
+the reason that chart renders as an empty frame today.
+
+## The map mode
+
+Deferred, and not just an oversight — the units are one part of a bigger
+question. `MetaMapModeAction::Execute` calls `OutputDevice::SetMapMode`, which
+*replaces* the map mode, **except** where the new one's unit is
+`MapUnit::MapRelative` (13): then its scales multiply the current ones and its
+origin offsets the current one. We replace unconditionally, and we ignore the
+unit, so a `MAPMODE` action that switches from 100th mm to twips is off by a
+factor of 1.76.
+
+It is rare — 4 `MAPMODE` actions in 1125 files, one of them relative — and
+getting it right means following `vcl/source/outdev/map.cxx` rather than
+guessing, so it is its own stage.
+
 ## Shortcuts worth taking
 
 - **Bitmaps are `.bmp` files already.** `SvmReader` reads them with

src/odr/internal/svm/svm_format.cpp

@@ -287,6 +287,11 @@ svm::MapMode svm::read_map_mode(std::istream &in) {
   result.scale_y = read_int_pair(in);
   read_primitive(in, result.simple);
 
+  // every coordinate divides by these
+  if (result.scale_x.y == 0 || result.scale_y.y == 0) {
+    throw MalformedSvmFile();
+  }
+
   return result;
 }
 
@@ -487,6 +492,16 @@ svm::read_text_rectangle_action(std::istream &in, const VersionLength &vl,
   return result;
 }
 
+std::uint16_t svm::read_push_action(std::istream &in, const VersionLength &vl) {
+  if (vl.length < sizeof(std::uint16_t)) {
+    return PUSH_ALL;
+  }
+
+  std::uint16_t result;
+  read_primitive(in, result);
+  return result;
+}
+
 svm::TextLineAction svm::read_text_line_action(std::istream &in,
                                                const VersionLength &vl) {
   TextLineAction result;

src/odr/internal/svm/svm_format.hpp

@@ -18,6 +18,33 @@ enum TextEncoding {
   RTL_TEXTENCODING_UCS2 = 0xFFFF,
 };
 
+/// `LineStyle`, what a `LineInfo` draws with.
+enum MetaLineStyle {
+  LINE_NONE = 0,
+  LINE_SOLID = 1,
+  LINE_DASH = 2,
+};
+
+/// `vcl::PushFlags`: what a `PUSH` saves; anything not named survives the
+/// `POP`.
+enum PushFlags : std::uint16_t {
+  PUSH_LINECOLOR = 0x0001,
+  PUSH_FILLCOLOR = 0x0002,
+  PUSH_FONT = 0x0004,
+  PUSH_TEXTCOLOR = 0x0008,
+  PUSH_MAPMODE = 0x0010,
+  PUSH_CLIPREGION = 0x0020,
+  PUSH_RASTEROP = 0x0040,
+  PUSH_TEXTFILLCOLOR = 0x0080,
+  PUSH_TEXTALIGN = 0x0100,
+  PUSH_REFPOINT = 0x0200,
+  PUSH_TEXTLINECOLOR = 0x0400,
+  PUSH_TEXTLAYOUTMODE = 0x0800,
+  PUSH_TEXTLANGUAGE = 0x1000,
+  PUSH_OVERLINECOLOR = 0x2000,
+  PUSH_ALL = 0xffff,
+};
+
 enum MetaActionType {
   META_NULL_ACTION = 0,
   META_PIXEL_ACTION = 100,
@@ -252,5 +279,7 @@ TextRectangleAction read_text_rectangle_action(std::istream &in,
                                                const VersionLength &vl,
                                                TextEncoding encoding);
 TextLineAction read_text_line_action(std::istream &in, const VersionLength &vl);
+/// The `PushFlags` of a `PUSH`. A version that carries none saves everything.
+std::uint16_t read_push_action(std::istream &in, const VersionLength &vl);
 
 } // namespace odr::internal::svm