From a45f25ca84f0b18e6957b10a918f3ebe618b5707 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 30 Aug 2026 10:05:17 +0200 Subject: [PATCH 1/4] feat(svm): draw the text run a metafile names, with the font it names Two thirds of every action in the corpus is text, and we drew almost none of what the file says about it. **A text action names a run of its string** - `(index, length)` - and the string it carries is the whole paragraph. We drew the string. A document whose text comes in runs, which is every bidirectional one and every formula, had the full sentence stamped at each run's position, overprinting itself into a smear. The run is taken in utf-16 units, before the conversion to utf-8, because that is what the offsets count. **The font attributes of #95**: italic (3068 of the 7962 fonts in the corpus, all of them formula variables we drew upright), bold, underline, strikeout, and the orientation, which rotates the run about its own start. **`TEXTALIGN`**, which says whether the draw point is the top, the baseline or the bottom of the run - vcl has no horizontal text alignment. Its default is `ALIGN_TOP` and svg's is the alphabetic baseline, so it has to be written out. `svgwriter.cxx` shifts the point by the font's ascent, having the metrics; `dominant-baseline` leaves that to the browser, which has them too. **The measurements the file took**: a `TEXTARRAY`'s dx array places every character where it was measured, and a `STRETCHTEXT`'s width becomes `textLength`, so a formula still fits its box when the viewer's font is not the author's. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CmCr22NW6wPQKiQidk96bq --- CHANGELOG.md | 10 ++ src/odr/internal/svm/PLAN.md | 27 ++++- src/odr/internal/svm/svm_format.cpp | 42 +++++++- src/odr/internal/svm/svm_format.hpp | 44 ++++++++ src/odr/internal/svm/svm_to_svg.cpp | 138 +++++++++++++++++++++++-- test/src/internal/svm/svm_test.cpp | 151 ++++++++++++++++++++++++++-- 6 files changed, 387 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 004d9461c..b82c95276 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,16 @@ 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 text action in a StarView metafile draws the run of its string that it + names, not the whole string. A document whose text is drawn in runs - any + bidirectional one, and every formula - used to have the full sentence stamped + at every run's position, overprinting itself into an unreadable smear. + +- StarView metafile text keeps its font: italic, bold, underline, strikeout, + rotation, the alignment that decides which edge the draw point names, the + per-character positions the file measured, and the width a stretched run has + to fill (#95). + - 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. diff --git a/src/odr/internal/svm/PLAN.md b/src/odr/internal/svm/PLAN.md index e8ab5764b..51e82b531 100644 --- a/src/odr/internal/svm/PLAN.md +++ b/src/odr/internal/svm/PLAN.md @@ -22,10 +22,11 @@ Each stage is one pull request, stacked on the one before it. 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. + the run a text action names, and the #95 font attributes (bold, italic, + underline, strikeout, rotation). `TEXTRECT` is still open - it occurs + nowhere in the corpus - and so is 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. @@ -73,6 +74,24 @@ 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. +## Text is where the corpus lives + +Two thirds of every action in the corpus is text, and three things about it +are worth writing down: + +- **`TextAlign` is vertical only.** It says whether the draw point is the top, + the baseline or the bottom of the run; vcl has no horizontal text alignment, + a run always starts at the point. Its default is `ALIGN_TOP`, which is not + what an svg `` does, so it has to be written out. `svgwriter.cxx` + shifts the point by the font's ascent because it has the metrics; we name + `dominant-baseline` and let the browser do it. +- **A text action names a run**, `(index, length)`, of the string it carries - + and the string is the whole paragraph. Drawing the string rather than the run + overprints the sentence at every run's position. +- **The dx array and the stretch width are the file's own measurements**, and + they are what keeps a formula together when the viewer's font is not the + author's. They map onto an `x` list and `textLength` respectively. + ## Shortcuts worth taking - **Bitmaps are `.bmp` files already.** `SvmReader` reads them with diff --git a/src/odr/internal/svm/svm_format.cpp b/src/odr/internal/svm/svm_format.cpp index 54353e8a9..3e99face4 100644 --- a/src/odr/internal/svm/svm_format.cpp +++ b/src/odr/internal/svm/svm_format.cpp @@ -22,6 +22,17 @@ std::string read_bytes(std::istream &in, const std::uint64_t size) { } } +/// `DrawText(…, index, len)`: a text action names the run of its string that +/// it draws, and a length past the end means the rest of it. +template +String select_run(const String &text, const std::uint16_t offset, + const std::uint16_t length) { + if (offset >= text.size()) { + return {}; + } + return text.substr(offset, length); +} + } // namespace std::string svm::read_ascii_string(std::istream &in, @@ -56,6 +67,16 @@ std::string svm::read_uint16_prefixed_utf16_string(std::istream &in) { return read_utf16_string(in, length); } +std::u16string svm::read_uint16_prefixed_u16string(std::istream &in) { + std::uint16_t length; + read_primitive(in, length); + const std::string bytes = + read_bytes(in, static_cast(length) * 2); + std::u16string result(length, u' '); + std::memcpy(result.data(), bytes.data(), bytes.size()); + return result; +} + std::string svm::read_string_with_encoding(std::istream &in, const TextEncoding encoding) { if (encoding == RTL_TEXTENCODING_UCS2) { @@ -426,7 +447,10 @@ svm::TextAction svm::read_text_action(std::istream &in, const VersionLength &vl, read_primitive(in, result.length); if (vl.version >= 2) { - result.text = read_uint16_prefixed_utf16_string(in); + result.text = util::string::u16string_to_string(select_run( + read_uint16_prefixed_u16string(in), result.offset, result.length)); + } else { + result.text = select_run(result.text, result.offset, result.length); } return result; @@ -452,7 +476,10 @@ svm::TextArrayAction svm::read_text_array_action(std::istream &in, } if (vl.version >= 2) { - result.text = read_uint16_prefixed_utf16_string(in); + result.text = util::string::u16string_to_string(select_run( + read_uint16_prefixed_u16string(in), result.offset, result.length)); + } else { + result.text = select_run(result.text, result.offset, result.length); } return result; @@ -470,7 +497,10 @@ svm::read_stretch_text_action(std::istream &in, const VersionLength &vl, read_primitive(in, result.length); if (vl.version >= 2) { - result.text = read_uint16_prefixed_utf16_string(in); + result.text = util::string::u16string_to_string(select_run( + read_uint16_prefixed_u16string(in), result.offset, result.length)); + } else { + result.text = select_run(result.text, result.offset, result.length); } return result; @@ -492,6 +522,12 @@ svm::read_text_rectangle_action(std::istream &in, const VersionLength &vl, return result; } +std::uint16_t svm::read_text_align_action(std::istream &in) { + std::uint16_t result; + read_primitive(in, result); + 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; diff --git a/src/odr/internal/svm/svm_format.hpp b/src/odr/internal/svm/svm_format.hpp index 1b1e5bfd4..6676dd2c3 100644 --- a/src/odr/internal/svm/svm_format.hpp +++ b/src/odr/internal/svm/svm_format.hpp @@ -18,6 +18,47 @@ enum TextEncoding { RTL_TEXTENCODING_UCS2 = 0xFFFF, }; +/// `TextAlign`: which edge of the text the draw point names. Vertical only - +/// vcl has no horizontal text alignment, a run always starts at the point. +enum MetaTextAlign { + ALIGN_TOP = 0, + ALIGN_BASELINE = 1, + ALIGN_BOTTOM = 2, +}; + +/// `FontWeight`. +enum MetaFontWeight { + WEIGHT_DONTKNOW = 0, + WEIGHT_THIN = 1, + WEIGHT_ULTRALIGHT = 2, + WEIGHT_LIGHT = 3, + WEIGHT_SEMILIGHT = 4, + WEIGHT_NORMAL = 5, + WEIGHT_MEDIUM = 6, + WEIGHT_SEMIBOLD = 7, + WEIGHT_BOLD = 8, + WEIGHT_ULTRABOLD = 9, + WEIGHT_BLACK = 10, +}; + +/// `FontItalic`. +enum MetaFontItalic { + ITALIC_NONE = 0, + ITALIC_OBLIQUE = 1, + ITALIC_NORMAL = 2, + ITALIC_DONTKNOW = 3, +}; + +/// `FontLineStyle`, the underline; anything but `NONE` underlines. +enum MetaFontLineStyle { + LINESTYLE_NONE = 0, +}; + +/// `FontStrikeout`; anything but `NONE` strikes through. +enum MetaFontStrikeout { + STRIKEOUT_NONE = 0, +}; + /// `LineStyle`, what a `LineInfo` draws with. enum MetaLineStyle { LINE_NONE = 0, @@ -250,6 +291,7 @@ std::string read_utf16_string(std::istream &in, std::uint32_t length); std::string read_uint16_prefixed_ascii_string(std::istream &in); std::string read_uint32_prefixed_utf16_string(std::istream &in); std::string read_uint16_prefixed_utf16_string(std::istream &in); +std::u16string read_uint16_prefixed_u16string(std::istream &in); std::string read_string_with_encoding(std::istream &in, TextEncoding encoding); VersionLength read_version_length(std::istream &in); @@ -281,5 +323,7 @@ TextRectangleAction read_text_rectangle_action(std::istream &in, 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); +/// The `TextAlign` of a `TEXTALIGN`. +std::uint16_t read_text_align_action(std::istream &in); } // namespace odr::internal::svm diff --git a/src/odr/internal/svm/svm_to_svg.cpp b/src/odr/internal/svm/svm_to_svg.cpp index c99c258cf..f286c7349 100644 --- a/src/odr/internal/svm/svm_to_svg.cpp +++ b/src/odr/internal/svm/svm_to_svg.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -39,6 +40,8 @@ struct GraphicsState final { std::uint32_t text_fill_rgb{}; bool text_fill_rgb_set{}; std::uint32_t over_line_rgb{}; + /// vcl's default, and what a file that never says otherwise draws with. + std::uint16_t text_align{ALIGN_TOP}; }; struct SavedState final { @@ -166,13 +169,71 @@ void write_fill_style(svg::SvgWriter &out, const Context &context) { out.write_style("fill-rule", "evenodd"); } +/// `SVGAttributeWriter::SetFontAttr`'s mapping of `FontWeight`. +std::uint16_t get_font_weight(const std::uint16_t weight) { + switch (weight) { + case WEIGHT_THIN: + return 100; + case WEIGHT_ULTRALIGHT: + return 200; + case WEIGHT_LIGHT: + return 300; + case WEIGHT_MEDIUM: + return 500; + case WEIGHT_SEMIBOLD: + return 600; + case WEIGHT_BOLD: + return 700; + case WEIGHT_ULTRABOLD: + return 800; + case WEIGHT_BLACK: + return 900; + default: + return 400; + } +} + +/// Which edge of the text the draw point names. The browser knows the font's +/// metrics and we do not, so the baseline is named rather than computed - +/// `svgwriter.cxx` shifts the point by the ascent instead, having them. +std::string_view get_dominant_baseline(const std::uint16_t text_align) { + switch (text_align) { + case ALIGN_TOP: + return "text-before-edge"; + case ALIGN_BOTTOM: + return "text-after-edge"; + default: + return "alphabetic"; + } +} + void write_text_style(svg::SvgWriter &out, const Context &context) { const GraphicsState &state = context.state; + const Font &font = state.font; + write_color_style(out, "fill", state.text_rgb, true); out.write_style("stroke", "none"); - out.write_style("font-family", state.font.family_name); + out.write_style("font-family", font.family_name); out.write_style("font-size", - std::abs(transform_height(state.font.size.y, context))); + std::abs(transform_height(font.size.y, context))); + out.write_style("dominant-baseline", get_dominant_baseline(state.text_align)); + + if (font.italic == ITALIC_OBLIQUE) { + out.write_style("font-style", "oblique"); + } else if (font.italic == ITALIC_NORMAL) { + out.write_style("font-style", "italic"); + } + if (const std::uint16_t weight = get_font_weight(font.weight); + weight != 400) { + out.write_style("font-weight", std::to_string(weight)); + } + if (font.underline != LINESTYLE_NONE && font.strikeout != STRIKEOUT_NONE) { + out.write_style("text-decoration", "underline line-through"); + } else if (font.underline != LINESTYLE_NONE) { + out.write_style("text-decoration", "underline"); + } else if (font.strikeout != STRIKEOUT_NONE) { + out.write_style("text-decoration", "line-through"); + } } void write_shape_style(svg::SvgWriter &out, const Context &context, @@ -248,13 +309,70 @@ void write_path(const std::span> polygons, out.write_element_end(); } +/// The number of characters svg will place, which is what an `x` list has to +/// match one for one. +std::size_t count_characters(const std::string_view text) { + return std::ranges::count_if(text, [](const char c) { + return (static_cast(c) & 0xc0) != 0x80; + }); +} + +/// The dx array holds the advance from the run's start to the end of each +/// character, so character *i* starts where character *i-1* ended. +std::string get_x_list_string(const IntPair &point, + const std::vector &dx_array, + const Context &context) { + std::string result = svg::format_number(transform_x(point.x, context)); + for (const std::uint32_t dx : + dx_array | std::views::take(dx_array.size() - 1)) { + result += " "; + result += svg::format_number( + transform_x(point.x + static_cast(dx), context)); + } + return result; +} + +/// @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. void write_text(const IntPair &point, const std::string &text, - const Context &context) { + const std::vector &dx_array, + const std::uint32_t width, const Context &context) { svg::SvgWriter &out = *context.out; + const Font &font = context.state.font; out.write_element_begin("text"); - out.write_attribute("x", transform_x(point.x, context)); + + if (!dx_array.empty() && dx_array.size() == count_characters(text)) { + out.write_attribute("x", get_x_list_string(point, dx_array, context)); + } else { + if (!dx_array.empty()) { + ODR_DEBUG(*context.logger, "dropping a dx array of " + << dx_array.size() << " for " + << count_characters(text) + << " characters"); + } + out.write_attribute("x", transform_x(point.x, context)); + } out.write_attribute("y", transform_y(point.y, context)); + + if (width > 0) { + // the run is drawn to fill this advance, however wide the font we get is + out.write_attribute( + "textLength", + transform_width(static_cast(width), context)); + out.write_attribute("lengthAdjust", "spacingAndGlyphs"); + } + + // the orientation turns the text about its own start, in tenths of a degree + // counter-clockwise, where svg turns clockwise + if (font.orientation != 0) { + out.write_attribute( + "transform", + "rotate(" + svg::format_number(font.orientation * -0.1) + " " + + svg::format_number(transform_x(point.x, context)) + " " + + svg::format_number(transform_y(point.y, context)) + ")"); + } + write_text_style(out, context); out.write_text(text); out.write_element_end(); @@ -300,6 +418,9 @@ void pop_state(Context &context) { state.text_fill_rgb = saved.state.text_fill_rgb; state.text_fill_rgb_set = saved.state.text_fill_rgb_set; } + if (saved.flags & PUSH_TEXTALIGN) { + state.text_align = saved.state.text_align; + } if (saved.flags & PUSH_OVERLINECOLOR) { state.over_line_rgb = saved.state.over_line_rgb; } @@ -358,20 +479,23 @@ void translate_action(const ActionHeader &action_header, std::istream &in, const auto [polygons] = read_poly_polygon_action(in, action_header.vl); write_path(polygons, true, nullptr, context); } break; + case META_TEXTALIGN_ACTION: + state.text_align = read_text_align_action(in); + break; case META_TEXT_ACTION: { const TextAction action = read_text_action(in, action_header.vl, state.encoding); - write_text(action.point, action.text, context); + write_text(action.point, action.text, {}, 0, context); } break; case META_TEXTARRAY_ACTION: { const TextArrayAction action = read_text_array_action(in, action_header.vl, state.encoding); - write_text(action.point, action.text, context); + write_text(action.point, action.text, action.dx_array, 0, context); } break; case META_STRETCHTEXT_ACTION: { const StretchTextAction action = read_stretch_text_action(in, action_header.vl, state.encoding); - write_text(action.point, action.text, context); + write_text(action.point, action.text, {}, action.width, context); } break; default: ODR_DEBUG(*context.logger, diff --git a/test/src/internal/svm/svm_test.cpp b/test/src/internal/svm/svm_test.cpp index 6eb6438ba..963331662 100644 --- a/test/src/internal/svm/svm_test.cpp +++ b/test/src/internal/svm/svm_test.cpp @@ -61,21 +61,46 @@ class SvmBuilder final { } /// The font the text actions below draw with, at @p size. - SvmBuilder &font(const std::string &family, const std::int32_t size) { - action(svm::META_FONT_ACTION) + SvmBuilder &font(const std::string &family, const std::int32_t size, + const std::uint16_t weight = 0, + const std::uint16_t underline = 0, + const std::uint16_t strikeout = 0, + const std::uint16_t italic = 0, + const std::uint16_t orientation = 0) { + return action(svm::META_FONT_ACTION) .begin() .ascii_string(family) .ascii_string("") .point(0, size) - .u16(11); // charset: ascii - for (int i = 0; i < 9; ++i) { - u16(0); // family, pitch, weight, underline, strikeout, italic, - // language, width, orientation - } - for (int i = 0; i < 4; ++i) { - u8(0); // wordline, outline, shadow, kerning - } - return end().end(); + .u16(11) // charset: ascii + .u16(0) // family + .u16(0) // pitch + .u16(weight) + .u16(underline) + .u16(strikeout) + .u16(italic) + .u16(0) // language + .u16(0) // width + .u16(orientation) + .u8(0) // wordline + .u8(0) // outline + .u8(0) // shadow + .u8(0) // kerning + .end() + .end(); + } + + /// A `TEXT` action of @p text at @p point, drawing the run @p offset / + /// @p length names. + SvmBuilder &text(const std::int32_t x, const std::int32_t y, + const std::string &value, const std::uint16_t offset = 0, + const std::uint16_t length = 0xffff) { + return action(svm::META_TEXT_ACTION) + .point(x, y) + .ascii_string(value) + .u16(offset) + .u16(length) + .end(); } /// A pascal string, as `read_uint16_prefixed_ascii_string` reads it. @@ -391,3 +416,107 @@ TEST(SvmToSvg, the_font_size_is_scaled) { EXPECT_NE(std::string::npos, svg.find("font-size:10")); } + +/// vcl aligns text vertically at the point - there is no horizontal alignment +/// - and the browser knows the font metrics that turn that into a baseline. +TEST(SvmToSvg, text_align) { + const std::string svg = translate(SvmBuilder() + .action(svm::META_TEXTALIGN_ACTION) + .u16(svm::ALIGN_BOTTOM) + .end() + .text(0, 0, "x") + .file()); + + EXPECT_NE(std::string::npos, svg.find("dominant-baseline:text-after-edge")); +} + +/// vcl's default, and what a file that never sets one draws with. +TEST(SvmToSvg, text_align_defaults_to_the_top) { + const std::string svg = translate(SvmBuilder().text(0, 0, "x").file()); + + EXPECT_NE(std::string::npos, svg.find("dominant-baseline:text-before-edge")); +} + +/// #95: half the text in the corpus is italic, all of it a formula variable. +TEST(SvmToSvg, font_attributes) { + const std::string svg = translate( + SvmBuilder() + .font("Arial", 10, svm::WEIGHT_BOLD, 1, 1, svm::ITALIC_NORMAL) + .text(0, 0, "x") + .file()); + + EXPECT_NE(std::string::npos, svg.find("font-style:italic")); + EXPECT_NE(std::string::npos, svg.find("font-weight:700")); + EXPECT_NE(std::string::npos, + svg.find("text-decoration:underline line-through")); +} + +/// The orientation turns the text about its own start, counter-clockwise in +/// tenths of a degree. +TEST(SvmToSvg, font_orientation_rotates_the_text) { + const std::string svg = translate( + SvmBuilder().font("Arial", 10, 0, 0, 0, 0, 900).text(5, 7, "x").file()); + + EXPECT_NE(std::string::npos, svg.find("transform=\"rotate(-90 5 7)\"")); +} + +/// The dx array measures every character of the run, so svg can place them +/// one by one instead of trusting whatever font the viewer has. +TEST(SvmToSvg, text_array_places_every_character) { + const std::string svg = translate(SvmBuilder() + .action(svm::META_TEXTARRAY_ACTION) + .point(0, 0) + .ascii_string("abc") + .u16(0) + .u16(3) + .u32(3) // dx array + .u32(10) + .u32(20) + .u32(30) + .end() + .file()); + + EXPECT_NE(std::string::npos, svg.find("x=\"0 10 20\"")); +} + +/// A dx array that does not measure this text is not a placement. +TEST(SvmToSvg, a_text_array_that_does_not_match_is_dropped) { + const std::string svg = translate(SvmBuilder() + .action(svm::META_TEXTARRAY_ACTION) + .point(4, 0) + .ascii_string("abc") + .u16(0) + .u16(3) + .u32(1) // dx array + .u32(10) + .end() + .file()); + + EXPECT_NE(std::string::npos, svg.find("x=\"4\"")); +} + +/// A stretch text names the advance the run has to fill, which is what keeps +/// a formula together when the viewer has a different font. +TEST(SvmToSvg, stretch_text_fills_the_width_it_names) { + const std::string svg = translate(SvmBuilder() + .action(svm::META_STRETCHTEXT_ACTION) + .point(0, 0) + .ascii_string("abc") + .u32(120) // width + .u16(0) + .u16(3) + .end() + .file()); + + EXPECT_NE(std::string::npos, svg.find("textLength=\"120\"")); + EXPECT_NE(std::string::npos, svg.find("lengthAdjust=\"spacingAndGlyphs\"")); +} + +/// A text action draws the run its offset and length name, not the whole +/// string it carries. +TEST(SvmToSvg, text_draws_the_run_it_names) { + const std::string svg = + translate(SvmBuilder().text(0, 0, "abcdef", 2, 3).file()); + + EXPECT_NE(std::string::npos, svg.find(">cde")); +} From 0f5d43bbb0d24d54a699866248c570e3db243a6b Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 30 Aug 2026 10:17:29 +0200 Subject: [PATCH 2/4] test: advance the reference output to the text run and its font The 21 private and 6 public files this branch changes, regenerated. --- test/data.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/data.cmake b/test/data.cmake index ac55b605b..d332ccfab 100644 --- a/test/data.cmake +++ b/test/data.cmake @@ -17,9 +17,9 @@ odr_test_data( odr_test_data( PATH "reference-output/odr-public" URL "https://github.com/opendocument-app/OpenDocument.test.output.git" - REVISION "7f139cdd6494523da598ab3625081ae0584f2e7a") + REVISION "29c6270283527ff91c3ca125f67bd4260aa2acae") odr_test_data( PATH "reference-output/odr-private" URL "https://github.com/opendocument-app/OpenDocument.test-private.output.git" - REVISION "9847681458e5400f3e1e668b0b6f4bfc9074e46e") + REVISION "8acbfdb5abb092e97bfe492281831c96f7dcc303") From 0f787ddbfd46db5cfc76fed3d49d6f0c22d46617 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 30 Aug 2026 11:33:39 +0200 Subject: [PATCH 3/4] docs(svm): trim the comments to the house standard The eight test doc comments each restated their own test name, and the facts they carried were already stated where the code is - `MetaTextAlign`, the `text_align` member, `get_x_list_string`, the `textLength` and `rotate` sites. The one that was not, that half the text in the corpus is italic and all of it a formula variable, moves to `PLAN.md`, where it justifies the stage order. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01XQpLmJpJ87qbKoG8B7kbLY --- src/odr/internal/svm/PLAN.md | 3 +++ src/odr/internal/svm/svm_to_svg.cpp | 9 ++++----- test/src/internal/svm/svm_test.cpp | 13 ------------- 3 files changed, 7 insertions(+), 18 deletions(-) diff --git a/src/odr/internal/svm/PLAN.md b/src/odr/internal/svm/PLAN.md index 51e82b531..07b9c5f02 100644 --- a/src/odr/internal/svm/PLAN.md +++ b/src/odr/internal/svm/PLAN.md @@ -54,6 +54,9 @@ metafiles harvested from the `odt`/`ods` fixtures: | `LINE` | 47 | 2 | | `BMPEXSCALE` | 1 | 1 | +Half the text in the corpus is italic, all of it a formula variable, which is +what puts the #95 font attributes in stage 3 rather than later. + `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 diff --git a/src/odr/internal/svm/svm_to_svg.cpp b/src/odr/internal/svm/svm_to_svg.cpp index f286c7349..4cfb14f8d 100644 --- a/src/odr/internal/svm/svm_to_svg.cpp +++ b/src/odr/internal/svm/svm_to_svg.cpp @@ -193,9 +193,9 @@ std::uint16_t get_font_weight(const std::uint16_t weight) { } } -/// Which edge of the text the draw point names. The browser knows the font's -/// metrics and we do not, so the baseline is named rather than computed - -/// `svgwriter.cxx` shifts the point by the ascent instead, having them. +/// Which edge of the text the draw point names. Named rather than computed: +/// `svgwriter.cxx` shifts the point by the ascent, having the font metrics +/// that we do not. std::string_view get_dominant_baseline(const std::uint16_t text_align) { switch (text_align) { case ALIGN_TOP: @@ -309,8 +309,7 @@ void write_path(const std::span> polygons, out.write_element_end(); } -/// The number of characters svg will place, which is what an `x` list has to -/// match one for one. +/// What an `x` list has to match one for one. std::size_t count_characters(const std::string_view text) { return std::ranges::count_if(text, [](const char c) { return (static_cast(c) & 0xc0) != 0x80; diff --git a/test/src/internal/svm/svm_test.cpp b/test/src/internal/svm/svm_test.cpp index 963331662..c5a17df5d 100644 --- a/test/src/internal/svm/svm_test.cpp +++ b/test/src/internal/svm/svm_test.cpp @@ -417,8 +417,6 @@ TEST(SvmToSvg, the_font_size_is_scaled) { EXPECT_NE(std::string::npos, svg.find("font-size:10")); } -/// vcl aligns text vertically at the point - there is no horizontal alignment -/// - and the browser knows the font metrics that turn that into a baseline. TEST(SvmToSvg, text_align) { const std::string svg = translate(SvmBuilder() .action(svm::META_TEXTALIGN_ACTION) @@ -430,14 +428,12 @@ TEST(SvmToSvg, text_align) { EXPECT_NE(std::string::npos, svg.find("dominant-baseline:text-after-edge")); } -/// vcl's default, and what a file that never sets one draws with. TEST(SvmToSvg, text_align_defaults_to_the_top) { const std::string svg = translate(SvmBuilder().text(0, 0, "x").file()); EXPECT_NE(std::string::npos, svg.find("dominant-baseline:text-before-edge")); } -/// #95: half the text in the corpus is italic, all of it a formula variable. TEST(SvmToSvg, font_attributes) { const std::string svg = translate( SvmBuilder() @@ -451,8 +447,6 @@ TEST(SvmToSvg, font_attributes) { svg.find("text-decoration:underline line-through")); } -/// The orientation turns the text about its own start, counter-clockwise in -/// tenths of a degree. TEST(SvmToSvg, font_orientation_rotates_the_text) { const std::string svg = translate( SvmBuilder().font("Arial", 10, 0, 0, 0, 0, 900).text(5, 7, "x").file()); @@ -460,8 +454,6 @@ TEST(SvmToSvg, font_orientation_rotates_the_text) { EXPECT_NE(std::string::npos, svg.find("transform=\"rotate(-90 5 7)\"")); } -/// The dx array measures every character of the run, so svg can place them -/// one by one instead of trusting whatever font the viewer has. TEST(SvmToSvg, text_array_places_every_character) { const std::string svg = translate(SvmBuilder() .action(svm::META_TEXTARRAY_ACTION) @@ -479,7 +471,6 @@ TEST(SvmToSvg, text_array_places_every_character) { EXPECT_NE(std::string::npos, svg.find("x=\"0 10 20\"")); } -/// A dx array that does not measure this text is not a placement. TEST(SvmToSvg, a_text_array_that_does_not_match_is_dropped) { const std::string svg = translate(SvmBuilder() .action(svm::META_TEXTARRAY_ACTION) @@ -495,8 +486,6 @@ TEST(SvmToSvg, a_text_array_that_does_not_match_is_dropped) { EXPECT_NE(std::string::npos, svg.find("x=\"4\"")); } -/// A stretch text names the advance the run has to fill, which is what keeps -/// a formula together when the viewer has a different font. TEST(SvmToSvg, stretch_text_fills_the_width_it_names) { const std::string svg = translate(SvmBuilder() .action(svm::META_STRETCHTEXT_ACTION) @@ -512,8 +501,6 @@ TEST(SvmToSvg, stretch_text_fills_the_width_it_names) { EXPECT_NE(std::string::npos, svg.find("lengthAdjust=\"spacingAndGlyphs\"")); } -/// A text action draws the run its offset and length name, not the whole -/// string it carries. TEST(SvmToSvg, text_draws_the_run_it_names) { const std::string svg = translate(SvmBuilder().text(0, 0, "abcdef", 2, 3).file()); From c112a6caec9855a98666123febc771fdc8973af2 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 30 Aug 2026 11:37:04 +0200 Subject: [PATCH 4/4] fix(svm): slice a `UCS2` text run in the units its offsets count A version-1 text action's `offset` and `length` count utf-16 code units, but `read_string_with_encoding` has already decoded a `UCS2` string to utf-8 by the time they are read. Slicing that by byte drew the wrong run and could cut a character in half, and invalid utf-8 costs the whole image exactly as an unescaped `&` did. `select_run_with_encoding` slices where the offsets point. Also: an empty dx array no longer reaches `size() - 1` in `get_x_list_string`, and `read_utf16_string` is the new `read_u16string` converted, rather than a second copy of it. The changelog entries under `Unreleased` are cut to a claim and its consequence; several ran to four lines of it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01XQpLmJpJ87qbKoG8B7kbLY --- CHANGELOG.md | 34 ++++++++++------------- src/odr/internal/svm/svm_format.cpp | 43 ++++++++++++++++++++--------- src/odr/internal/svm/svm_to_svg.cpp | 4 +++ test/src/internal/svm/svm_test.cpp | 40 +++++++++++++++++++++------ 4 files changed, 79 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b82c95276..fee4e65ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,33 +19,27 @@ 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 text action in a StarView metafile draws the run of its string that it - names, not the whole string. A document whose text is drawn in runs - any - bidirectional one, and every formula - used to have the full sentence stamped - at every run's position, overprinting itself into an unreadable smear. +- A text action in a StarView metafile draws the run it names, not the whole + string. Text drawn in runs used to overprint itself into a smear. - StarView metafile text keeps its font: italic, bold, underline, strikeout, - rotation, the alignment that decides which edge the draw point names, the - per-character positions the file measured, and the width a stretched run has - to fill (#95). + rotation, alignment, per-character positions, and the width a stretched run + fills (#95). -- 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 StarView metafile draws through a graphics state stack: a colour, font or + map mode set inside a `PUSH` no longer leaks past its `POP`. -- 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. +- A filled StarView shape keeps its outline, a poly-polygon cuts its holes, a + line takes its `LineInfo`, 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. +- Text in a StarView metafile is escaped into the svg it renders as. An `&` or + `<` in a label used to cost the whole image. -- A StarView metafile translation logs what it drops: the actions it does not - implement, and a translation failure it used to fall back from silently. +- A StarView metafile translation logs what it drops: unimplemented actions, + and a failure it used to fall back from silently. -- An html attribute value drops the control characters xml forbids, rather - than carrying them through. One escaper writes both html and svg now. +- An html attribute value drops the control characters xml forbids. One + escaper writes both html and svg now. ## v6.12.0 - 2026-08-30 diff --git a/src/odr/internal/svm/svm_format.cpp b/src/odr/internal/svm/svm_format.cpp index 3e99face4..b0d9390d1 100644 --- a/src/odr/internal/svm/svm_format.cpp +++ b/src/odr/internal/svm/svm_format.cpp @@ -33,6 +33,28 @@ String select_run(const String &text, const std::uint16_t offset, return text.substr(offset, length); } +/// @ref select_run in the units the run is measured in - utf-16 code units for +/// `UCS2`, bytes otherwise. @p text has already been decoded to utf-8, where a +/// `UCS2` offset addresses nothing and a `substr` splits a character. +std::string select_run_with_encoding(const std::string &text, + const svm::TextEncoding encoding, + const std::uint16_t offset, + const std::uint16_t length) { + if (encoding != svm::RTL_TEXTENCODING_UCS2) { + return select_run(text, offset, length); + } + return util::string::u16string_to_string( + select_run(util::string::string_to_u16string(text), offset, length)); +} + +std::u16string read_u16string(std::istream &in, const std::uint32_t length) { + const std::string bytes = + read_bytes(in, static_cast(length) * 2); + std::u16string result(length, u' '); + std::memcpy(result.data(), bytes.data(), bytes.size()); + return result; +} + } // namespace std::string svm::read_ascii_string(std::istream &in, @@ -42,11 +64,7 @@ std::string svm::read_ascii_string(std::istream &in, std::string svm::read_utf16_string(std::istream &in, const std::uint32_t length) { - const std::string bytes = - read_bytes(in, static_cast(length) * 2); - std::u16string result_u16(length, u' '); - std::memcpy(result_u16.data(), bytes.data(), bytes.size()); - return util::string::u16string_to_string(result_u16); + return util::string::u16string_to_string(read_u16string(in, length)); } std::string svm::read_uint16_prefixed_ascii_string(std::istream &in) { @@ -70,11 +88,7 @@ std::string svm::read_uint16_prefixed_utf16_string(std::istream &in) { std::u16string svm::read_uint16_prefixed_u16string(std::istream &in) { std::uint16_t length; read_primitive(in, length); - const std::string bytes = - read_bytes(in, static_cast(length) * 2); - std::u16string result(length, u' '); - std::memcpy(result.data(), bytes.data(), bytes.size()); - return result; + return read_u16string(in, length); } std::string svm::read_string_with_encoding(std::istream &in, @@ -450,7 +464,8 @@ svm::TextAction svm::read_text_action(std::istream &in, const VersionLength &vl, result.text = util::string::u16string_to_string(select_run( read_uint16_prefixed_u16string(in), result.offset, result.length)); } else { - result.text = select_run(result.text, result.offset, result.length); + result.text = select_run_with_encoding(result.text, encoding, result.offset, + result.length); } return result; @@ -479,7 +494,8 @@ svm::TextArrayAction svm::read_text_array_action(std::istream &in, result.text = util::string::u16string_to_string(select_run( read_uint16_prefixed_u16string(in), result.offset, result.length)); } else { - result.text = select_run(result.text, result.offset, result.length); + result.text = select_run_with_encoding(result.text, encoding, result.offset, + result.length); } return result; @@ -500,7 +516,8 @@ svm::read_stretch_text_action(std::istream &in, const VersionLength &vl, result.text = util::string::u16string_to_string(select_run( read_uint16_prefixed_u16string(in), result.offset, result.length)); } else { - result.text = select_run(result.text, result.offset, result.length); + result.text = select_run_with_encoding(result.text, encoding, result.offset, + result.length); } return result; diff --git a/src/odr/internal/svm/svm_to_svg.cpp b/src/odr/internal/svm/svm_to_svg.cpp index 4cfb14f8d..4d82eb217 100644 --- a/src/odr/internal/svm/svm_to_svg.cpp +++ b/src/odr/internal/svm/svm_to_svg.cpp @@ -322,6 +322,10 @@ std::string get_x_list_string(const IntPair &point, const std::vector &dx_array, const Context &context) { std::string result = svg::format_number(transform_x(point.x, context)); + if (dx_array.empty()) { + return result; + } + for (const std::uint32_t dx : dx_array | std::views::take(dx_array.size() - 1)) { result += " "; diff --git a/test/src/internal/svm/svm_test.cpp b/test/src/internal/svm/svm_test.cpp index c5a17df5d..46a6e1666 100644 --- a/test/src/internal/svm/svm_test.cpp +++ b/test/src/internal/svm/svm_test.cpp @@ -61,20 +61,19 @@ class SvmBuilder final { } /// The font the text actions below draw with, at @p size. - SvmBuilder &font(const std::string &family, const std::int32_t size, - const std::uint16_t weight = 0, - const std::uint16_t underline = 0, - const std::uint16_t strikeout = 0, - const std::uint16_t italic = 0, - const std::uint16_t orientation = 0) { + SvmBuilder & + font(const std::string &family, const std::int32_t size, + const std::uint16_t weight = 0, const std::uint16_t underline = 0, + const std::uint16_t strikeout = 0, const std::uint16_t italic = 0, + const std::uint16_t orientation = 0, const std::uint16_t charset = 11) { return action(svm::META_FONT_ACTION) .begin() .ascii_string(family) .ascii_string("") .point(0, size) - .u16(11) // charset: ascii - .u16(0) // family - .u16(0) // pitch + .u16(charset) + .u16(0) // family + .u16(0) // pitch .u16(weight) .u16(underline) .u16(strikeout) @@ -90,6 +89,14 @@ class SvmBuilder final { .end(); } + SvmBuilder &ucs2_string(const std::u16string &value) { + u32(static_cast(value.size())); + for (const char16_t c : value) { + u16(static_cast(c)); + } + return *this; + } + /// A `TEXT` action of @p text at @p point, drawing the run @p offset / /// @p length names. SvmBuilder &text(const std::int32_t x, const std::int32_t y, @@ -501,6 +508,21 @@ TEST(SvmToSvg, stretch_text_fills_the_width_it_names) { EXPECT_NE(std::string::npos, svg.find("lengthAdjust=\"spacingAndGlyphs\"")); } +TEST(SvmToSvg, a_version_1_text_action_slices_a_ucs2_run_by_character) { + const std::string svg = + translate(SvmBuilder() + .font("f", 10, 0, 0, 0, 0, 0, svm::RTL_TEXTENCODING_UCS2) + .action(svm::META_TEXT_ACTION) + .point(0, 0) + .ucs2_string(u"\u00e4bc") + .u16(1) // offset + .u16(2) // length + .end() + .file()); + + EXPECT_NE(std::string::npos, svg.find(">bc")); +} + TEST(SvmToSvg, text_draws_the_run_it_names) { const std::string svg = translate(SvmBuilder().text(0, 0, "abcdef", 2, 3).file());