From eec0439cea8c9b08dd0e0d4ea9e32e083b0a4260 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 30 Aug 2026 11:14:32 +0200 Subject: [PATCH] feat(svm): draw the primitives, an arc as an arc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `LINE`, `ELLIPSE`, `ROUNDRECT`, `ARC`, `PIE`, `CHORD`, `POINT` and `PIXEL` were all skipped. The lines matter most: they are what rules a table drawn as a metafile, and two documents in the fixtures had their tables come out as floating numbers with no grid at all. `svgwriter.cxx` turns an arc into a polygon of up to 256 points because it has nothing better; svg has `A`, so the arc goes out as an arc. The geometry is `ImplPolygon`'s: centre and radii from the rectangle, each ray's ellipse parameter `atan2(radius_x·sin θ, radius_y·cos θ)` over the angle θ to the point, and vcl sweeps the mathematically positive way round - counter-clockwise on screen, which is svg's sweep flag 0. A start ray equal to the end one is the whole ellipse rather than nothing, and svg draws nothing for an arc that ends where it starts, so every arc is written as its two halves. Checked against LibreOffice on a metafile written for the purpose: for the same action it puts the arc's ends at (441,544) and (100,650), and so do we; the pie closes through the centre and the chord straight back, as it does. A `POINT` and a `PIXEL` are one device pixel, which no length in the drawing can be. Both go out as a dot of the hairline's own width - a degenerate path with a round cap - rather than the zero-length `` `svgwriter.cxx` writes, which draws nothing at all. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CmCr22NW6wPQKiQidk96bq --- CHANGELOG.md | 5 + src/odr/internal/svm/PLAN.md | 3 +- src/odr/internal/svm/svm_format.cpp | 43 +++++++ src/odr/internal/svm/svm_format.hpp | 29 +++++ src/odr/internal/svm/svm_to_svg.cpp | 173 +++++++++++++++++++++++++++- test/data.cmake | 2 +- test/src/internal/svm/svm_test.cpp | 108 +++++++++++++++++ 7 files changed, 359 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be579919b..988c57bef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,11 @@ 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 its remaining primitives: `LINE`, `ELLIPSE`, + `ROUNDRECT`, `ARC`, `PIE`, `CHORD`, `POINT` and `PIXEL`. The lines are what + rules a table drawn as a metafile, which used to come out as floating + numbers. + - A StarView metafile clips what it says it clips: `CLIPREGION` and the two that intersect one into it. A drawing that ran outside the clip used to be shown in full. diff --git a/src/odr/internal/svm/PLAN.md b/src/odr/internal/svm/PLAN.md index 5d9a8694a..534dcff8f 100644 --- a/src/odr/internal/svm/PLAN.md +++ b/src/odr/internal/svm/PLAN.md @@ -32,8 +32,7 @@ Each stage is one pull request, stacked on the one before it. 5. **Bitmaps** (#194) - done for the `BMP` and `BMPEX` families; `MASK`, which stencils one colour through a bitmap, and `ZCOMPRESS`, which needs inflating first, are still open. See the shortcut below. -6. **Primitives.** `PIXEL`, `POINT`, `LINE`, `ROUNDRECT`, `ELLIPSE`, `ARC`, - `PIE`, `CHORD` — one `svgwriter.cxx` case each. +6. **Primitives** - done. 7. **Fills and transparency.** `GRADIENT`, `GRADIENTEX`, `HATCH`, `WALLPAPER`, `TRANSPARENT`, `FLOATTRANSPARENT`. 8. **The map mode's unit** (#772 defect 6), see below. diff --git a/src/odr/internal/svm/svm_format.cpp b/src/odr/internal/svm/svm_format.cpp index fe47e087c..a0bd70c12 100644 --- a/src/odr/internal/svm/svm_format.cpp +++ b/src/odr/internal/svm/svm_format.cpp @@ -515,6 +515,49 @@ svm::Font svm::read_font(std::istream &in) { return result; } +svm::PixelAction svm::read_pixel_action(std::istream &in) { + PixelAction result; + + result.point = read_int_pair(in); + read_primitive(in, result.color); + + return result; +} + +svm::LineAction svm::read_line_action(std::istream &in, + const VersionLength &vl) { + LineAction result; + + result.start = read_int_pair(in); + result.end = read_int_pair(in); + + if (vl.version >= 2) { + result.line_info = read_line_info(in); + } + + return result; +} + +svm::RoundRectangleAction svm::read_round_rectangle_action(std::istream &in) { + RoundRectangleAction result; + + result.rectangle = read_rectangle(in); + read_primitive(in, result.horizontal_round); + read_primitive(in, result.vertical_round); + + return result; +} + +svm::ArcAction svm::read_arc_action(std::istream &in) { + ArcAction result; + + result.rectangle = read_rectangle(in); + result.start = read_int_pair(in); + result.end = read_int_pair(in); + + return result; +} + svm::PolyLineAction svm::read_poly_line_action(std::istream &in, const VersionLength &vl) { PolyLineAction result; diff --git a/src/odr/internal/svm/svm_format.hpp b/src/odr/internal/svm/svm_format.hpp index c60f199a3..9096c789b 100644 --- a/src/odr/internal/svm/svm_format.hpp +++ b/src/odr/internal/svm/svm_format.hpp @@ -227,6 +227,31 @@ struct ActionHeader final { VersionLength vl; }; +struct PixelAction final { + IntPair point; + std::uint32_t color{}; +}; + +struct LineAction final { + IntPair start; + IntPair end; + LineInfo line_info; +}; + +struct RoundRectangleAction final { + Rectangle rectangle; + /// The corner ellipse's radii. + std::uint32_t horizontal_round{}; + std::uint32_t vertical_round{}; +}; + +/// `ARC`, `PIE` and `CHORD`: the ellipse, and the two rays that cut it. +struct ArcAction final { + Rectangle rectangle; + IntPair start; + IntPair end; +}; + struct PolyLineAction final { std::vector points; LineInfo line_info; @@ -345,6 +370,10 @@ ActionHeader read_action_header(std::istream &in); MapMode read_map_mode(std::istream &in); LineInfo read_line_info(std::istream &in); Font read_font(std::istream &in); +PixelAction read_pixel_action(std::istream &in); +LineAction read_line_action(std::istream &in, const VersionLength &vl); +RoundRectangleAction read_round_rectangle_action(std::istream &in); +ArcAction read_arc_action(std::istream &in); PolyLineAction read_poly_line_action(std::istream &in, const VersionLength &vl); PolygonAction read_polygon_action(std::istream &in, const VersionLength &vl); PolyPolygonAction read_poly_polygon_action(std::istream &in, diff --git a/src/odr/internal/svm/svm_to_svg.cpp b/src/odr/internal/svm/svm_to_svg.cpp index 83ca315dc..e94c3f5ef 100644 --- a/src/odr/internal/svm/svm_to_svg.cpp +++ b/src/odr/internal/svm/svm_to_svg.cpp @@ -10,6 +10,7 @@ #include #include +#include #include #include #include @@ -259,7 +260,9 @@ void write_shape_style(svg::SvgWriter &out, const Context &context, } } -void write_rectangle(const Rectangle &rect, const Context &context) { +void write_rectangle(const Rectangle &rect, const Context &context, + const std::uint32_t horizontal_round = 0, + const std::uint32_t vertical_round = 0) { svg::SvgWriter &out = *context.out; out.write_element_begin("rect"); @@ -269,6 +272,16 @@ void write_rectangle(const Rectangle &rect, const Context &context) { transform_x(rect.left, context)); out.write_attribute("height", transform_y(rect.bottom, context) - transform_y(rect.top, context)); + if (horizontal_round != 0) { + out.write_attribute( + "rx", + transform_width(static_cast(horizontal_round), context)); + } + if (vertical_round != 0) { + out.write_attribute( + "ry", + transform_height(static_cast(vertical_round), context)); + } write_shape_style(out, context, true); out.write_element_end(); } @@ -552,6 +565,125 @@ void write_bitmap(const BitmapAction &action, Context &context) { out.write_element_end(); } +void write_ellipse(const Rectangle &rect, const Context &context) { + svg::SvgWriter &out = *context.out; + + const double left = transform_x(rect.left, context); + const double top = transform_y(rect.top, context); + const double right = transform_x(rect.right, context); + const double bottom = transform_y(rect.bottom, context); + + out.write_element_begin("ellipse"); + out.write_attribute("cx", (left + right) / 2); + out.write_attribute("cy", (top + bottom) / 2); + out.write_attribute("rx", std::abs(right - left) / 2); + out.write_attribute("ry", std::abs(bottom - top) / 2); + write_shape_style(out, context, true); + out.write_element_end(); +} + +/// What an arc draws between its two rays. +enum class ArcKind { + arc, ///< the curve alone + pie, ///< closed through the centre + chord, ///< closed straight from end to start +}; + +ArcKind get_arc_kind(const std::uint16_t action_type) { + switch (action_type) { + case META_PIE_ACTION: + return ArcKind::pie; + case META_CHORD_ACTION: + return ArcKind::chord; + default: + return ArcKind::arc; + } +} + +/// `ImplGetParameter`: the ellipse parameter of the ray through @p point, in +/// vcl's y-up angles. +double get_arc_parameter(const IntPair &point, const double center_x, + const double center_y, const double radius_x, + const double radius_y) { + const double angle = std::atan2(center_y - point.y, point.x - center_x); + return std::atan2(radius_x * std::sin(angle), radius_y * std::cos(angle)); +} + +/// vcl sweeps counter-clockwise on screen, which is svg's sweep flag 0. +void write_arc(const ArcAction &action, const ArcKind kind, + const Context &context) { + svg::SvgWriter &out = *context.out; + const Rectangle &rect = action.rectangle; + + const double center_x = (rect.left + rect.right) / 2.0; + const double center_y = (rect.top + rect.bottom) / 2.0; + const double radius_x = std::abs(rect.right - rect.left) / 2.0; + const double radius_y = std::abs(rect.bottom - rect.top) / 2.0; + if (radius_x == 0 || radius_y == 0) { + return; + } + + const double start = + get_arc_parameter(action.start, center_x, center_y, radius_x, radius_y); + const double end = + get_arc_parameter(action.end, center_x, center_y, radius_x, radius_y); + // the same ray twice is the whole ellipse, not nothing + double sweep = end - start; + if (sweep <= 0) { + sweep += 2 * std::numbers::pi; + } + + const auto point = [&](const double x, const double y) { + return svg::format_number(transform_x( + static_cast(std::lround(x)), context)) + + "," + + svg::format_number( + transform_y(static_cast(std::lround(y)), context)); + }; + const auto at = [&](const double parameter) { + return point(center_x + radius_x * std::cos(parameter), + center_y - radius_y * std::sin(parameter)); + }; + const std::string radii = + " A " + + svg::format_number(transform_width( + static_cast(std::lround(radius_x)), context)) + + "," + + svg::format_number(transform_height( + static_cast(std::lround(radius_y)), context)) + + " 0 0 0 "; + + std::string path = "M "; + if (kind == ArcKind::pie) { + path += point(center_x, center_y) + " L "; + } + // svg draws nothing where an arc ends where it began, so a full ellipse is + // written as its two halves + path += at(start) + radii + at(start + sweep / 2) + radii + at(end); + if (kind != ArcKind::arc) { + path += " Z"; + } + + out.write_element_begin("path"); + out.write_attribute("d", path); + // an arc is drawn, not filled; a pie and a chord are shapes + write_shape_style(out, context, kind != ArcKind::arc); + out.write_element_end(); +} + +/// A dot of the stroke's own width - a hairline is one device pixel. +void write_point(const IntPair &point, const Context &context) { + svg::SvgWriter &out = *context.out; + + out.write_element_begin("path"); + out.write_attribute( + "d", "M " + svg::format_number(transform_x(point.x, context)) + "," + + svg::format_number(transform_y(point.y, context)) + " Z"); + out.write_style("stroke-linecap", "round"); + write_shape_style(out, context, false); + out.write_element_end(); +} + void write_text(const IntPair &point, const std::string &text, const std::vector &dx_array, const std::uint32_t width, const Context &context) { @@ -683,6 +815,45 @@ void translate_action(const ActionHeader &action_header, std::istream &in, case META_MAPMODE_ACTION: state.map_mode = read_map_mode(in); break; + case META_PIXEL_ACTION: { + const PixelAction action = read_pixel_action(in); + ensure_clip(context); + // the action carries its own colour, and nothing else draws with it + const std::uint32_t line_rgb = std::exchange(state.line_rgb, action.color); + const bool line_rgb_set = std::exchange(state.line_rgb_set, true); + write_point(action.point, context); + state.line_rgb = line_rgb; + state.line_rgb_set = line_rgb_set; + } break; + case META_POINT_ACTION: { + const IntPair action = read_int_pair(in); + ensure_clip(context); + write_point(action, context); + } break; + case META_LINE_ACTION: { + const LineAction action = read_line_action(in, action_header.vl); + const std::vector points{action.start, action.end}; + ensure_clip(context); + write_path({&points, 1}, false, &action.line_info, context); + } break; + case META_ROUNDRECT_ACTION: { + const RoundRectangleAction action = read_round_rectangle_action(in); + ensure_clip(context); + write_rectangle(action.rectangle, context, action.horizontal_round, + action.vertical_round); + } break; + case META_ELLIPSE_ACTION: { + const Rectangle action = read_rectangle(in); + ensure_clip(context); + write_ellipse(action, context); + } break; + case META_ARC_ACTION: + case META_PIE_ACTION: + case META_CHORD_ACTION: { + const ArcAction action = read_arc_action(in); + ensure_clip(context); + write_arc(action, get_arc_kind(action_header.type), context); + } break; case META_RECT_ACTION: { const Rectangle action = read_rectangle(in); ensure_clip(context); diff --git a/test/data.cmake b/test/data.cmake index d02c71ab5..fda352b4e 100644 --- a/test/data.cmake +++ b/test/data.cmake @@ -22,4 +22,4 @@ odr_test_data( odr_test_data( PATH "reference-output/odr-private" URL "https://github.com/opendocument-app/OpenDocument.test-private.output.git" - REVISION "3ef61b792470da12cfd76e74afa1e0fad0812617") + REVISION "931589fe01472f0f02eae8423c713afa4db90bd8") diff --git a/test/src/internal/svm/svm_test.cpp b/test/src/internal/svm/svm_test.cpp index 762078986..9d5108d23 100644 --- a/test/src/internal/svm/svm_test.cpp +++ b/test/src/internal/svm/svm_test.cpp @@ -721,3 +721,111 @@ TEST(SvmToSvg, the_same_clip_twice_is_one_group) { EXPECT_EQ(1, count_of(svg, "` does not. A `PIXEL` brings its own colour and leaves +/// the state's alone. +TEST(SvmToSvg, a_point_and_a_pixel_are_dots) { + const std::string svg = translate(SvmBuilder() + .action(svm::META_LINECOLOR_ACTION) + .u32(0x0000ff) + .u8(1) + .end() + .action(svm::META_POINT_ACTION) + .point(1, 2) + .end() + .action(svm::META_PIXEL_ACTION) + .point(3, 4) + .u32(0x00ff00) + .end() + .action(svm::META_POINT_ACTION) + .point(5, 6) + .end() + .file()); + + EXPECT_EQ(3, count_of(svg, "stroke-linecap:round")); + EXPECT_NE(std::string::npos, + svg.find("d=\"M 1,2 Z\" style=\"stroke-linecap:round;" + "stroke:rgb(0,0,255)")); + EXPECT_NE(std::string::npos, + svg.find("d=\"M 3,4 Z\" style=\"stroke-linecap:round;" + "stroke:rgb(0,255,0)")); + EXPECT_NE(std::string::npos, + svg.find("d=\"M 5,6 Z\" style=\"stroke-linecap:round;" + "stroke:rgb(0,0,255)")); +}