fix(svm): read the map mode's unit, and compose the relative one

CHANGELOG.md

@@ -16,6 +16,11 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- A StarView metafile's map mode is read in full: its unit, so a drawing that
+  switches to twips or points is no longer off by the factor between them, and
+  the relative map mode, which composes with the one before it rather than
+  replacing it.
+
 - A StarView metafile draws its gradients, hatches and transparent shapes:
   `GRADIENT`, `GRADIENTEX`, `HATCH` and `TRANSPARENT`.
 

src/odr/internal/svm/AGENTS.md

@@ -95,3 +95,8 @@ drawing should look like:
 /Applications/LibreOffice.app/Contents/MacOS/soffice --headless \
   --convert-to svg --outdir /tmp test/data/input/odr-public/svm/chart-1.svm
 ```
+
+Its *player* is the oracle, not that export: asked to convert a metafile that
+switches to twips and then to a relative map mode, the export drops an action
+and places another outside the box. Where the two disagree, follow the vcl
+source.

src/odr/internal/svm/PLAN.md

@@ -36,7 +36,7 @@ Each stage is one pull request, stacked on the one before it.
 7. **Fills and transparency** - `GRADIENT`, `GRADIENTEX`, `HATCH` and
    `TRANSPARENT` are done. `WALLPAPER` and `FLOATTRANSPARENT` are not: the
    first has a format of its own, the second nests a whole metafile.
-8. **The map mode's unit** (#772 defect 6), see below.
+8. **The map mode** (#772 defect 6) - done.
 9. **Stretch.** Bézier flags (#772 defect 4), the `EPS` substitute metafile,
    and version-1 (pre-`VCLMTF`) files via `SvmConverter.cxx`.
 
@@ -65,20 +65,6 @@ 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.
-
 ## Clipping is nested groups
 
 An svg `clip-path` names one shape, and `ISECT…` intersects. Two clips

src/odr/internal/svm/svm_format.hpp

@@ -61,6 +61,25 @@ enum MetaFontStrikeout {
   STRIKEOUT_NONE = 0,
 };
 
+/// `MapUnit`, the unit a map mode's coordinates are in. `MAP_RELATIVE` has
+/// none of its own: it composes with the map mode before it.
+enum MetaMapUnit {
+  MAP_100TH_MM = 0,
+  MAP_10TH_MM = 1,
+  MAP_MM = 2,
+  MAP_CM = 3,
+  MAP_1000TH_INCH = 4,
+  MAP_100TH_INCH = 5,
+  MAP_10TH_INCH = 6,
+  MAP_INCH = 7,
+  MAP_POINT = 8,
+  MAP_TWIP = 9,
+  MAP_PIXEL = 10,
+  MAP_SYS_FONT = 11,
+  MAP_APP_FONT = 12,
+  MAP_RELATIVE = 13,
+};
+
 /// `awt::GradientStyle`.
 enum MetaGradientStyle {
   GRADIENT_LINEAR = 0,

src/odr/internal/svm/svm_to_svg.cpp

@@ -26,14 +26,29 @@ namespace {
 /// Beyond this the nesting is a broken file, not a drawing.
 constexpr std::size_t max_push_depth = 1024;
 
+/// Nothing in the file says how big a pixel is; this is what a bitmap with no
+/// size of its own is drawn at, and what `MapUnit::MapPixel` resolves against.
+constexpr double assumed_dpi = 96;
+/// The unit the header of every metafile in the wild uses.
+constexpr double hundredth_mm_per_inch = 2540;
+
 std::string action_name(const ActionHeader &action_header) {
   return std::string(action_type_name(action_header.type)) + "(" +
          std::to_string(action_header.type) + ")";
 }
 
+/// What a coordinate goes through to become a drawing one: the origin it is
+/// offset by, in its own units, and the scale that takes it to the header's.
+struct Mapping final {
+  double origin_x{};
+  double origin_y{};
+  double scale_x{1};
+  double scale_y{1};
+};
+
 /// What an action reads and a `PUSH` saves, grouped by `PushFlags` bit.
 struct GraphicsState final {
-  MapMode map_mode;
+  Mapping mapping;
   Font font;
   TextEncoding encoding{RTL_TEXTENCODING_ASCII_US};
   std::uint32_t line_rgb{};
@@ -71,31 +86,96 @@ struct Context final {
   /// The clip the open groups already apply - a prefix of the state's clip
   /// once @ref ensure_clip has run.
   std::vector<std::string> written_clip;
+
+  /// What the header's unit is worth - the drawing is measured in it.
+  double header_unit_length{1};
 };
 
 double scale(const IntPair fraction) {
   return static_cast<double>(fraction.x) / fraction.y;
 }
 
+/// `ImplMapRes::CalcMapResolution`: one unit of @p unit, in 100th mm. A pixel
+/// and the two font units are device-dependent and a metafile has no device,
+/// so all three resolve at @ref assumed_dpi.
+double get_unit_length(const std::uint16_t unit, const Logger &logger) {
+  switch (unit) {
+  case MAP_100TH_MM:
+    return 1;
+  case MAP_10TH_MM:
+    return 10;
+  case MAP_MM:
+    return 100;
+  case MAP_CM:
+    return 1000;
+  case MAP_1000TH_INCH:
+    return hundredth_mm_per_inch / 1000;
+  case MAP_100TH_INCH:
+    return hundredth_mm_per_inch / 100;
+  case MAP_10TH_INCH:
+    return hundredth_mm_per_inch / 10;
+  case MAP_INCH:
+    return hundredth_mm_per_inch;
+  case MAP_POINT:
+    return hundredth_mm_per_inch / 72;
+  case MAP_TWIP:
+    return hundredth_mm_per_inch / 1440;
+  case MAP_PIXEL:
+    return hundredth_mm_per_inch / assumed_dpi;
+  default:
+    ODR_DEBUG(logger, "map unit " << unit << " has no length of its own, "
+                                  << "taking it for a pixel");
+    return hundredth_mm_per_inch / assumed_dpi;
+  }
+}
+
+/// `OutputDevice::SetMapMode`: a map mode replaces the one before it - except
+/// a relative one, whose scales multiply the current and whose origin offsets
+/// it (`ImplMapRes::CalcMapResolution`).
+Mapping compose_mapping(const Mapping &current, const MapMode &map_mode,
+                        const double header_unit_length, const Logger &logger) {
+  const double relative_x = scale(map_mode.scale_x);
+  const double relative_y = scale(map_mode.scale_y);
+
+  if (map_mode.unit == MAP_RELATIVE) {
+    if (relative_x == 0 || relative_y == 0) {
+      ODR_WARNING(logger, "relative map mode with a zero scale, keeping the "
+                              << "mapping it would have composed with");
+      return current;
+    }
+    return {map_mode.origin.x + current.origin_x / relative_x,
+            map_mode.origin.y + current.origin_y / relative_y,
+            current.scale_x * relative_x, current.scale_y * relative_y};
+  }
+
+  // the drawing is measured in the header's unit, so a map mode in another
+  // one scales by what the two are worth
+  const double unit =
+      get_unit_length(map_mode.unit, logger) / header_unit_length;
+  return {static_cast<double>(map_mode.origin.x),
+          static_cast<double>(map_mode.origin.y), relative_x * unit,
+          relative_y * unit};
+}
+
 double transform_x(const std::int32_t x, const Context &context) {
-  const MapMode &map_mode = context.state.map_mode;
-  return (map_mode.origin.x + x) * scale(map_mode.scale_x);
+  const Mapping &mapping = context.state.mapping;
+  return (mapping.origin_x + x) * mapping.scale_x;
 }
 
 double transform_y(const std::int32_t y, const Context &context) {
-  const MapMode &map_mode = context.state.map_mode;
-  return (map_mode.origin.y + y) * scale(map_mode.scale_y);
+  const Mapping &mapping = context.state.mapping;
+  return (mapping.origin_y + y) * mapping.scale_y;
 }
 
 /// A length carries no origin, only the scale. The x scale even for a stroke
 /// width or a dash, which lie along no axis: `svgwriter.cxx` maps those
 /// through `ImplMap(sal_Int32)`, which takes the `Width()` of a square.
 double transform_width(const std::int32_t width, const Context &context) {
-  return width * scale(context.state.map_mode.scale_x);
+  return width * context.state.mapping.scale_x;
 }
 
 double transform_height(const std::int32_t height, const Context &context) {
-  return height * scale(context.state.map_mode.scale_y);
+  return height * context.state.mapping.scale_y;
 }
 
 std::string get_svg_color_string(const std::uint32_t color) {
@@ -672,12 +752,6 @@ std::string get_x_list_string(const IntPair &point,
 
 /// @p dx_array places the characters one by one where the file measured them;
 /// @p width, from a stretch text, is the advance the whole run has to fill.
-/// Nothing in the file says how big a pixel is; this is what
-/// `MapUnit::MapPixel` would resolve against a device.
-constexpr double assumed_dpi = 96;
-/// The unit the header of every metafile in the wild uses.
-constexpr double hundredth_mm_per_inch = 2540;
-
 /// A metafile's mask is white where the bitmap does not show; an svg mask
 /// keeps what is white.
 void write_inverter(const Context &context) {
@@ -1003,7 +1077,7 @@ void pop_state(Context &context) {
     state.text_rgb = saved.state.text_rgb;
   }
   if (saved.flags & PUSH_MAPMODE) {
-    state.map_mode = saved.state.map_mode;
+    state.mapping = saved.state.mapping;
   }
   if (saved.flags & PUSH_TEXTFILLCOLOR) {
     state.text_fill_rgb = saved.state.text_fill_rgb;
@@ -1053,9 +1127,11 @@ void translate_action(const ActionHeader &action_header, std::istream &in,
     state.font = read_font(in);
     state.encoding = static_cast<TextEncoding>(state.font.charset);
     break;
-  case META_MAPMODE_ACTION:
-    state.map_mode = read_map_mode(in);
-    break;
+  case META_MAPMODE_ACTION: {
+    const MapMode map_mode = read_map_mode(in);
+    state.mapping = compose_mapping(
+        state.mapping, map_mode, context.header_unit_length, *context.logger);
+  } break;
   case META_PIXEL_ACTION: {
     const PixelAction action = read_pixel_action(in);
     ensure_clip(context);
@@ -1218,7 +1294,9 @@ void svm::translate_to_svg(const SvmFile &file, std::ostream &out,
 
   const Header header = read_header(in);
 
-  context.state.map_mode = header.map_mode;
+  context.header_unit_length = get_unit_length(header.map_mode.unit, logger);
+  context.state.mapping = compose_mapping(Mapping(), header.map_mode,
+                                          context.header_unit_length, logger);
 
   writer.write_element_begin("svg");
   writer.write_attribute("xmlns", "http://www.w3.org/2000/svg");

test/src/internal/svm/svm_test.cpp

@@ -110,6 +110,24 @@ class SvmBuilder final {
         .end();
   }
 
+  /// A `MAPMODE` action, @p unit saying what its coordinates are in.
+  SvmBuilder &map_mode_action(const std::uint16_t unit, const std::int32_t x,
+                              const std::int32_t y,
+                              const std::int32_t numerator = 1,
+                              const std::int32_t denominator = 1) {
+    return action(svm::META_MAPMODE_ACTION)
+        .begin()
+        .u16(unit)
+        .point(x, y)
+        .i32(numerator)
+        .i32(denominator)
+        .i32(numerator)
+        .i32(denominator)
+        .u8(0)
+        .end()
+        .end();
+  }
+
   /// A colour *inside* an object, which is not the plain `uint32` an action's
   /// own colour is: a name id, and three 16-bit channels behind the user one.
   SvmBuilder &object_color(const std::uint32_t rgb) {
@@ -985,3 +1003,62 @@ TEST(SvmToSvg, transparent) {
   EXPECT_NE(std::string::npos, svg.find("fill:rgb(0,0,255)"));
   EXPECT_NE(std::string::npos, svg.find("opacity:0.4"));
 }
+
+/// The drawing is measured in the header's unit, so a map mode in another one
+/// scales by what the two are worth: a twip is 2540/1440 of a 100th mm.
+TEST(SvmToSvg, a_map_mode_in_another_unit_scales) {
+  const std::string svg = translate(SvmBuilder()
+                                        .map_mode_action(svm::MAP_TWIP, 0, 0)
+                                        .action(svm::META_RECT_ACTION)
+                                        .rectangle(600, 0, 1100, 500)
+                                        .end()
+                                        .file());
+
+  EXPECT_NE(std::string::npos,
+            svg.find("<rect x=\"1058.33\" y=\"0\" width=\"881.944\""));
+}
+
+/// A relative map mode composes with the one before it rather than replacing
+/// it: its scales multiply and its origin offsets, so the twips above stay.
+TEST(SvmToSvg, a_relative_map_mode_composes) {
+  const std::string svg =
+      translate(SvmBuilder()
+                    .map_mode_action(svm::MAP_TWIP, 0, 0)
+                    .map_mode_action(svm::MAP_RELATIVE, 1200, 600)
+                    .action(svm::META_RECT_ACTION)
+                    .rectangle(0, 0, 500, 500)
+                    .end()
+                    .file());
+
+  EXPECT_NE(std::string::npos, svg.find("<rect x=\"2116.67\" y=\"1058.33\""
+                                        " width=\"881.944\""));
+}
+
+TEST(SvmToSvg, a_relative_map_mode_multiplies_the_scale) {
+  const std::string svg =
+      translate(SvmBuilder()
+                    .map_mode_action(svm::MAP_100TH_MM, 0, 0, 1, 2)
+                    .map_mode_action(svm::MAP_RELATIVE, 0, 0, 1, 5)
+                    .action(svm::META_RECT_ACTION)
+                    .rectangle(0, 0, 100, 100)
+                    .end()
+                    .file());
+
+  EXPECT_NE(std::string::npos, svg.find("width=\"10\" height=\"10\""));
+}
+
+TEST(SvmToSvg, a_pop_restores_the_map_mode) {
+  const std::string svg = translate(SvmBuilder()
+                                        .action(svm::META_PUSH_ACTION)
+                                        .u16(svm::PUSH_MAPMODE)
+                                        .end()
+                                        .map_mode_action(svm::MAP_TWIP, 0, 0)
+                                        .action(svm::META_POP_ACTION)
+                                        .end()
+                                        .action(svm::META_RECT_ACTION)
+                                        .rectangle(0, 0, 100, 100)
+                                        .end()
+                                        .file());
+
+  EXPECT_NE(std::string::npos, svg.find("width=\"100\" height=\"100\""));
+}