From 28dd6146b91bb95e7f15a1fb9ef6f5e00bf0bb4e Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 30 Aug 2026 23:05:49 +0200 Subject: [PATCH 1/3] feat(odf): draw the chart an embedded object holds, not its replacement image MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A libreoffice chart is ``. `draw:object` had no parser, so what drew was the sibling replacement: a flattened metafile with no selectable text, no structure, and nothing at all where the producer wrote none. The chart's own markup — series, axes, and a `table:table` of the plotted values — sat unread in the package. 1122 `draw:object` over 21 files in the test corpus. Closes #179. `odf_chart.cpp` renders `` to svg: bar, line, area, scatter, pie and ring, with the title, legend, axes and the series colours the chart's own styles name. The object reaches the renderer as an image carrying that svg, so the existing image path writes it out with no renderer change at all. Two decisions worth recording. The values come from the part's own `local-table` rather than the cells it names in the host document: that is the snapshot the chart carries, and the only source an embedded chart is guaranteed — it is also what lets a chart in a text document work, where there is no sheet to read. And the layout is `chart:plot-area`'s box and `chartooo:coordinate-region`, so the plot sits where the producer put it rather than somewhere we invent. The `draw:image` beside an object is skipped, or the two would stack; an object holding no chart we can read — a formula, an ole blob — falls back to it, which is what keeps the 1027 math objects in `Vektoranalysis Zusammenfassung.odt` drawing. Open, and recorded in `PLAN.md`: stacked and percentage plots, secondary axes, trend lines, data labels, and the number format an axis names — a date axis shows its serial number today. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_017XABfEapaADjFQCt1vjmDF --- CHANGELOG.md | 5 + CMakeLists.txt | 1 + src/odr/internal/odf/PLAN.md | 35 +- src/odr/internal/odf/README.md | 7 + src/odr/internal/odf/odf_chart.cpp | 619 +++++++++++++++++++++++ src/odr/internal/odf/odf_chart.hpp | 15 + src/odr/internal/odf/odf_document.cpp | 92 +++- src/odr/internal/odf/odf_geometry.cpp | 19 + src/odr/internal/odf/odf_geometry.hpp | 5 + src/odr/internal/odf/odf_parser.cpp | 15 +- test/CMakeLists.txt | 1 + test/src/internal/odf/odf_chart_test.cpp | 158 ++++++ 12 files changed, 957 insertions(+), 15 deletions(-) create mode 100644 src/odr/internal/odf/odf_chart.cpp create mode 100644 src/odr/internal/odf/odf_chart.hpp create mode 100644 test/src/internal/odf/odf_chart_test.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index 4afccd454..81e8fc2e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,11 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- An ODF `draw:object` chart is drawn from the chart part's own markup, not + from the replacement image beside it: bar, line, area, scatter, pie and ring, + with their titles, legends, axes and series colours. An object holding no + chart we can read keeps the replacement. Closes #179. + - An ODF custom shape is drawn as the shape its `draw:enhanced-geometry` describes rather than as its bounding box: `draw:enhanced-path`, `draw:equation`, `draw:modifiers` and the two mirror attributes. Closes #159. diff --git a/CMakeLists.txt b/CMakeLists.txt index 82b0ab3d4..ddf8fc430 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -177,6 +177,7 @@ set(ODR_SOURCE_FILES "src/odr/internal/markdown/markdown_parser.cpp" "src/odr/internal/markdown/markdown_style.cpp" + "src/odr/internal/odf/odf_chart.cpp" "src/odr/internal/odf/odf_crypto.cpp" "src/odr/internal/odf/odf_document.cpp" "src/odr/internal/odf/odf_element_registry.cpp" diff --git a/src/odr/internal/odf/PLAN.md b/src/odr/internal/odf/PLAN.md index 56655c5d8..44256683e 100644 --- a/src/odr/internal/odf/PLAN.md +++ b/src/odr/internal/odf/PLAN.md @@ -4,17 +4,15 @@ Closing #771: everything an ODF shape carries beyond its bounding box — `draw:transform`, the shape elements with no parser, `draw:enhanced-geometry`, and the `draw:object` chart. The module as it stands is in [`AGENTS.md`](AGENTS.md); the feature checklist is [`README.md`](README.md). -Keep this file honest as stages land, and delete it when they all have. +Keep this file honest as stages land, and delete it when nothing is left +under *Today*. ## Today -`parse_any_element_tree` (`odf_parser.cpp`) knows nine drawing tags — -`draw:frame`, `draw:image`, `draw:rect`, `draw:line`, `draw:circle`, -`draw:custom-shape`, `draw:text-box`, `draw:g`, `draw:a`. Everything else falls -through its final `return {null_element_id, …}` and vanishes with its subtree. -`draw:custom-shape` reaches `html::translate_custom_shape` as a positioned -`
` with fill and stroke: the box, never the shape. `draw:transform` is not -read, so a rotated shape draws unrotated. +All five stages have landed. What is left is listed under each of them and in +[`README.md`](README.md): `draw:text-areas` and `draw:handle` on an enhanced +geometry, the arrowheads `draw:marker` names, `dr3d:scene`, and the chart +features below stage 5. ## The corpus @@ -148,12 +146,23 @@ subpath painted differently from the rest. `hasstroke` and `hasfill` are always true — the geometry reader has no style in hand — and no corpus formula reads them. -### 5 — `draw:object` charts +### 5 — `draw:object` charts — landed -`` in the embedded part, rendered from its series, axes and -`table:table` of plotted data, with the SVM replacement kept as the fallback. -Closes #179. Large enough to deserve splitting again if it grows; it shares -nothing with stages 1–4 but the `draw:frame` it hangs off. +`odf_chart.cpp` renders the embedded part's `` to svg, and the +object reaches the renderer as an image carrying it, so the existing image path +writes it out. The `draw:image` beside an object is the replacement the producer +wrote, and is skipped where the object itself draws; an object with no chart we +can read — a formula, an ole blob — still leaves it. Closes #179. + +Decisions: the plotted values come from the chart's own `local-table` rather +than the cells it names in the host document, which is the snapshot the part +carries and the only one an embedded chart is guaranteed; the layout comes from +`chart:plot-area` and `chartooo:coordinate-region`, so it matches what the +producer laid out rather than something we invent. + +Open: stacked and percentage plots, secondary axes, trend lines, data labels, +and the number format an axis names — a date axis shows its serial number +today. ## Not scoped diff --git a/src/odr/internal/odf/README.md b/src/odr/internal/odf/README.md index 0cc7428d1..a22746528 100644 --- a/src/odr/internal/odf/README.md +++ b/src/odr/internal/odf/README.md @@ -55,6 +55,13 @@ Roughly ordered by importance. - [x] images - [x] internal and external references - [x] svm +- [x] embedded objects (`draw:object`) + - [x] charts (`chart:bar`, `line`, `area`, `scatter`, `circle`, `ring`), + drawn from the chart part's own `local-table` #179 + - [ ] stacked and percentage plots, secondary axes, trend lines, data labels + - [ ] the axis number format (a date axis shows its serial number) + - [ ] `draw:object-ole` (an OLE blob, not ODF markup) + - [x] anything else falls back to the `draw:image` replacement - [x] tables - [x] column width, row height, table width - [x] cell vertical alignment, background, padding, borders diff --git a/src/odr/internal/odf/odf_chart.cpp b/src/odr/internal/odf/odf_chart.cpp new file mode 100644 index 000000000..92b99f1ea --- /dev/null +++ b/src/odr/internal/odf/odf_chart.cpp @@ -0,0 +1,619 @@ +#include + +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace odr::internal::odf { + +namespace { + +/// The chart's own coordinates: 1/100 mm, which is what `svg:width` and the +/// plot area's box reduce to. +struct Box final { + double x{0}; + double y{0}; + double width{0}; + double height{0}; +}; + +struct Series final { + std::string label; + std::vector> values; + std::string colour; +}; + +/// What libreoffice paints an unstyled series with, in order. +constexpr std::array default_colours{ + "#004586", "#ff420e", "#ffd320", "#579d1c", "#7e0021", "#83caff", + "#314004", "#aecf00", "#4b1f6f", "#ff950e", "#c5000b", "#0084d1"}; + +/// 1/100 mm per typographic point, the unit chart text is sized in. +constexpr double units_per_point = 2540.0 / 72.0; + +/// The size a legend entry and an axis label are drawn at, and roughly what +/// one of their characters is wide in ems. +constexpr double label_points = 9; +constexpr double character_width = 0.55; + +std::string number(const double value) { + return util::number::to_string_significant(value == 0 ? 0 : value, 6); +} + +double read_length(const pugi::xml_attribute attribute, + const double fallback = 0) { + return read_hundredth_millimetres(attribute).value_or(fallback); +} + +void collect_text(const pugi::xml_node node, std::string &out) { + for (const pugi::xml_node child : node.children()) { + if (child.type() == pugi::node_pcdata) { + out += child.value(); + } else { + collect_text(child, out); + } + } +} + +/// The text of a `text:p` run, which is all a chart title or label is. +std::string read_text(const pugi::xml_node node) { + std::string result; + for (const pugi::xml_node paragraph : node.children("text:p")) { + collect_text(paragraph, result); + } + // A title is written with its trailing newline; one line is all we draw. + while (!result.empty() && (result.back() == '\n' || result.back() == '\r' || + result.back() == ' ')) { + result.pop_back(); + } + return result; +} + +/// A step that lands on 1, 2 or 5 times a power of ten, so the axis reads. +double nice_step(const double rough) { + if (rough <= 0) { + return 1; + } + const double magnitude = std::pow(10, std::floor(std::log10(rough))); + const double normalised = rough / magnitude; + if (normalised <= 1) { + return magnitude; + } + if (normalised <= 2) { + return 2 * magnitude; + } + if (normalised <= 5) { + return 5 * magnitude; + } + return 10 * magnitude; +} + +/// Reads `` and draws it. +class ChartWriter { +public: + explicit ChartWriter(const pugi::xml_node content_root) + : m_chart{content_root.child("office:body") + .child("office:chart") + .child("chart:chart")} { + for (const pugi::xml_node style : + content_root.child("office:automatic-styles") + .children("style:style")) { + m_styles.emplace(style.attribute("style:name").value(), style); + } + } + + [[nodiscard]] std::optional render() { + if (!m_chart) { + return {}; + } + m_size.width = read_length(m_chart.attribute("svg:width")); + m_size.height = read_length(m_chart.attribute("svg:height")); + if (m_size.width <= 0 || m_size.height <= 0) { + return {}; + } + + const pugi::xml_node plot_area = m_chart.child("chart:plot-area"); + read_class(); + read_plot_box(plot_area); + read_data(plot_area); + if (m_series.empty()) { + return {}; + } + + open(); + write_title(); + write_legend(); + if (m_pie) { + write_pie(); + } else { + write_value_axis(); + write_category_axis(); + write_series(); + } + m_out += ""; + return m_out; + } + +private: + pugi::xml_node m_chart; + std::unordered_map m_styles; + + Box m_size; + Box m_plot; + std::string m_class; + bool m_bars{false}; + bool m_area{false}; + bool m_symbols{false}; + bool m_pie{false}; + + std::vector m_categories; + std::vector m_series; + double m_minimum{0}; + double m_maximum{0}; + double m_step{1}; + + std::string m_out; + + [[nodiscard]] pugi::xml_node style_of(const pugi::xml_node node) const { + const auto it = m_styles.find(node.attribute("chart:style-name").value()); + return it == m_styles.end() ? pugi::xml_node() : it->second; + } + + void read_class() { + m_class = m_chart.attribute("chart:class").value(); + m_bars = m_class == "chart:bar"; + m_area = m_class == "chart:area"; + m_pie = m_class == "chart:circle" || m_class == "chart:ring"; + m_symbols = + m_class == "chart:scatter" || style_of(m_chart.child("chart:plot-area")) + .child("style:chart-properties") + .attribute("chart:symbol-type"); + } + + /// `chartooo:coordinate-region` is the region the data is drawn in, which the + /// plot area's own box only bounds. + void read_plot_box(const pugi::xml_node plot_area) { + pugi::xml_node box = plot_area.child("chartooo:coordinate-region"); + if (!box) { + box = plot_area; + } + m_plot.x = read_length(box.attribute("svg:x")); + m_plot.y = read_length(box.attribute("svg:y")); + m_plot.width = read_length(box.attribute("svg:width"), m_size.width); + m_plot.height = read_length(box.attribute("svg:height"), m_size.height); + } + + /// The `local-table` carries the plotted values; its header columns are the + /// categories and its header rows the series labels. + void read_data(const pugi::xml_node plot_area) { + const pugi::xml_node table = m_chart.find_child_by_attribute( + "table:table", "table:name", "local-table"); + if (!table) { + return; + } + + const std::size_t leading = std::max( + 1, count_cells(table.child("table:table-header-columns") + .child("table:table-column"))); + + std::vector labels; + for (const pugi::xml_node header : + table.child("table:table-header-rows").children("table:table-row")) { + labels = read_row_text(header); + break; + } + + std::vector>> columns; + for (const pugi::xml_node row : + table.child("table:table-rows").children("table:table-row")) { + const std::vector cells = read_row(row); + if (cells.size() <= leading) { + continue; + } + m_categories.emplace_back(read_text(cells[leading - 1])); + for (std::size_t i = leading; i < cells.size(); ++i) { + if (columns.size() < i - leading + 1) { + columns.resize(i - leading + 1); + } + columns[i - leading].push_back(read_value(cells[i])); + } + } + + std::size_t index = 0; + for (const pugi::xml_node series : plot_area.children("chart:series")) { + if (index >= columns.size()) { + break; + } + Series result; + result.values = columns[index]; + if (leading + index < labels.size()) { + result.label = labels[leading + index]; + } + result.colour = read_colour(series, index); + m_series.push_back(std::move(result)); + ++index; + } + // A chart with no `chart:series` at all still has its table. + for (; index < columns.size() && m_series.empty(); ++index) { + m_series.push_back({.label = {}, + .values = columns[index], + .colour = std::string(colour_at(index))}); + } + + trim(); + read_range(); + } + + [[nodiscard]] static std::size_t count_cells(const pugi::xml_node column) { + std::size_t result = 0; + for (pugi::xml_node node = column; node; + node = node.next_sibling("table:table-column")) { + result += node.attribute("table:number-columns-repeated").as_uint(1); + } + return result; + } + + [[nodiscard]] static std::vector + read_row(const pugi::xml_node row) { + std::vector result; + for (const pugi::xml_node cell : row.children()) { + if (std::strcmp(cell.name(), "table:table-cell") != 0 && + std::strcmp(cell.name(), "table:covered-table-cell") != 0) { + continue; + } + const auto repeated = + cell.attribute("table:number-columns-repeated").as_uint(1); + for (unsigned i = 0; i < repeated; ++i) { + result.push_back(cell); + } + } + return result; + } + + [[nodiscard]] static std::vector + read_row_text(const pugi::xml_node row) { + std::vector result; + for (const pugi::xml_node cell : read_row(row)) { + result.emplace_back(read_text(cell)); + } + return result; + } + + /// A missing data point is written `office:value="NaN"`. + [[nodiscard]] static std::optional + read_value(const pugi::xml_node cell) { + const pugi::xml_attribute value = cell.attribute("office:value"); + if (!value) { + return {}; + } + const double result = value.as_double(); + return std::isfinite(result) ? std::optional(result) : std::nullopt; + } + + /// The table is written to the chart's full range, so it ends in rows that + /// carry neither a category nor a value. + void trim() { + while (!m_categories.empty()) { + const std::size_t last = m_categories.size() - 1; + if (!m_categories[last].empty()) { + return; + } + for (const Series &series : m_series) { + if (last < series.values.size() && series.values[last].has_value()) { + return; + } + } + m_categories.pop_back(); + for (Series &series : m_series) { + if (last < series.values.size()) { + series.values.pop_back(); + } + } + } + } + + [[nodiscard]] static std::string_view colour_at(const std::size_t index) { + return default_colours[index % default_colours.size()]; + } + + [[nodiscard]] std::string read_colour(const pugi::xml_node series, + const std::size_t index) const { + const pugi::xml_node properties = + style_of(series).child("style:graphic-properties"); + for (const char *name : {"draw:fill-color", "svg:stroke-color"}) { + if (const pugi::xml_attribute colour = properties.attribute(name); + colour && colour.value()[0] == '#') { + return colour.value(); + } + } + return std::string(colour_at(index)); + } + + void read_range() { + bool empty = true; + for (const Series &series : m_series) { + for (const std::optional value : series.values) { + if (!value.has_value()) { + continue; + } + m_minimum = empty ? *value : std::min(m_minimum, *value); + m_maximum = empty ? *value : std::max(m_maximum, *value); + empty = false; + } + } + if (empty) { + m_series.clear(); + return; + } + // A bar or an area is measured from zero, or it lies about its size. + if (m_bars || m_area) { + m_minimum = std::min(m_minimum, 0.0); + m_maximum = std::max(m_maximum, 0.0); + } + if (m_maximum == m_minimum) { + m_maximum = m_minimum + 1; + } + m_step = nice_step((m_maximum - m_minimum) / 5); + m_minimum = std::floor(m_minimum / m_step) * m_step; + m_maximum = std::ceil(m_maximum / m_step) * m_step; + } + + [[nodiscard]] double value_to_y(const double value) const { + return m_plot.y + + m_plot.height * (1 - (value - m_minimum) / (m_maximum - m_minimum)); + } + + void open() { + m_out += R"("; + m_out += ""; + m_out += ""; + } + + void write_text(const double x, const double y, const std::string &text, + const double points, const char *anchor) { + if (text.empty()) { + return; + } + m_out += ""; + m_out += xml::escape_text(text); + m_out += ""; + } + + void write_title() { + write_text(m_size.width / 2, + read_length(m_chart.child("chart:title").attribute("svg:y"), + m_size.height * 0.06) + + 13 * units_per_point, + read_text(m_chart.child("chart:title")), 13, "middle"); + write_text(m_size.width / 2, + read_length(m_chart.child("chart:subtitle").attribute("svg:y"), + m_size.height * 0.12) + + 10 * units_per_point, + read_text(m_chart.child("chart:subtitle")), 10, "middle"); + } + + void write_legend() { + const pugi::xml_node legend = m_chart.child("chart:legend"); + if (!legend) { + return; + } + const double x = + read_length(legend.attribute("svg:x"), m_plot.x + m_plot.width + 200); + double y = read_length(legend.attribute("svg:y"), m_plot.y); + const double size = label_points * units_per_point; + for (const Series &series : m_series) { + m_out += ""; + write_text(x + size * 1.5, y + size * 0.85, series.label, label_points, + "start"); + y += size * 1.6; + } + } + + void write_value_axis() { + const auto ticks = + static_cast(std::lround((m_maximum - m_minimum) / m_step)); + for (int tick = 0; tick <= ticks; ++tick) { + const double value = m_minimum + m_step * tick; + const double y = value_to_y(value); + m_out += ""; + write_text(m_plot.x - 100, y + 3 * units_per_point / 2, + util::number::to_string_significant(value, 6), label_points, + "end"); + } + } + + void write_category_axis() { + if (m_categories.empty()) { + return; + } + // Every nth, n chosen so the widest label still has room beside it. + std::size_t longest = 1; + for (const std::string &category : m_categories) { + longest = std::max(longest, category.size()); + } + const double width = static_cast(longest) * label_points * + units_per_point * character_width; + const auto stride = std::max( + 1, + static_cast(std::ceil( + width * static_cast(m_categories.size()) / m_plot.width))); + for (std::size_t i = 0; i < m_categories.size(); i += stride) { + write_text(category_centre(i), + m_plot.y + m_plot.height + 10 * units_per_point, + m_categories[i], label_points, "middle"); + } + } + + [[nodiscard]] double category_centre(const std::size_t index) const { + return m_plot.x + m_plot.width * (static_cast(index) + 0.5) / + static_cast( + std::max(1, m_categories.size())); + } + + void write_series() { + if (m_bars) { + write_bars(); + return; + } + for (const Series &series : m_series) { + write_line(series); + } + } + + void write_bars() { + const auto count = static_cast(m_series.size()); + const double band = + m_plot.width / + static_cast(std::max(1, m_categories.size())); + const double width = band * 0.8 / count; + const double zero = value_to_y(std::clamp(0.0, m_minimum, m_maximum)); + + for (std::size_t s = 0; s < m_series.size(); ++s) { + for (std::size_t i = 0; i < m_series[s].values.size(); ++i) { + const std::optional value = m_series[s].values[i]; + if (!value.has_value()) { + continue; + } + const double y = value_to_y(*value); + const double x = + category_centre(i) - band * 0.4 + width * static_cast(s); + m_out += ""; + } + } + } + + void write_line(const Series &series) { + std::string points; + for (std::size_t i = 0; i < series.values.size(); ++i) { + const std::optional value = series.values[i]; + if (!value.has_value()) { + continue; + } + points += points.empty() ? "M " : " L "; + points += number(category_centre(i)); + points += ' '; + points += number(value_to_y(*value)); + } + if (points.empty()) { + return; + } + + if (m_area) { + const double zero = value_to_y(std::clamp(0.0, m_minimum, m_maximum)); + m_out += ""; + } + m_out += ""; + + if (!m_symbols) { + return; + } + for (std::size_t i = 0; i < series.values.size(); ++i) { + const std::optional value = series.values[i]; + if (!value.has_value()) { + continue; + } + m_out += ""; + } + } + + /// A pie plots one series, its slices the categories. + void write_pie() { + const Series &series = m_series.front(); + double total = 0; + for (const std::optional value : series.values) { + total += std::max(0.0, value.value_or(0)); + } + if (total <= 0) { + return; + } + + const double cx = m_plot.x + m_plot.width / 2; + const double cy = m_plot.y + m_plot.height / 2; + const double radius = std::min(m_plot.width, m_plot.height) / 2; + const double inner = m_class == "chart:ring" ? radius / 2 : 0; + + double from = -90; + for (std::size_t i = 0; i < series.values.size(); ++i) { + const double value = series.values[i].value_or(0); + if (value <= 0) { + continue; + } + const double to = from + 360 * value / total; + m_out += ""; + from = to; + } + } + + [[nodiscard]] static std::string slice(const double cx, const double cy, + const double radius, + const double inner, const double from, + const double to) { + const auto point = [&](const double degrees, const double r) { + const double radians = degrees * std::numbers::pi / 180; + return number(cx + r * std::cos(radians)) + " " + + number(cy + r * std::sin(radians)); + }; + const char *large = to - from > 180 ? "1" : "0"; + std::string result = "M " + point(from, radius) + " A " + number(radius) + + " " + number(radius) + " 0 " + large + " 1 " + + point(to, radius); + if (inner > 0) { + result += " L " + point(to, inner) + " A " + number(inner) + " " + + number(inner) + " 0 " + large + " 0 " + point(from, inner); + } else { + result += " L " + number(cx) + " " + number(cy); + } + return result + " Z"; + } +}; + +} // namespace + +} // namespace odr::internal::odf + +namespace odr::internal { + +std::optional +odf::render_chart(const pugi::xml_node content_root) { + return odf::ChartWriter(content_root).render(); +} + +} // namespace odr::internal diff --git a/src/odr/internal/odf/odf_chart.hpp b/src/odr/internal/odf/odf_chart.hpp new file mode 100644 index 000000000..9a0e04709 --- /dev/null +++ b/src/odr/internal/odf/odf_chart.hpp @@ -0,0 +1,15 @@ +#pragma once + +#include +#include + +#include + +namespace odr::internal::odf { + +/// The `` an embedded object's `content.xml` holds (12), drawn +/// to svg. Nothing where the part carries no chart we can read. +[[nodiscard]] std::optional +render_chart(pugi::xml_node content_root); + +} // namespace odr::internal::odf diff --git a/src/odr/internal/odf/odf_document.cpp b/src/odr/internal/odf/odf_document.cpp index 82a440d2c..0c157dd8b 100644 --- a/src/odr/internal/odf/odf_document.cpp +++ b/src/odr/internal/odf/odf_document.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -19,8 +20,10 @@ #include #include +#include #include #include +#include namespace odr::internal::odf { @@ -1003,6 +1006,9 @@ class ElementAdapter final : public abstract::ElementAdapter, if (m_document->as_filesystem() == nullptr) { return false; } + if (is_object(element_id)) { + return object_file(element_id).has_value(); + } try { const AbsPath path = Path(image_href(element_id)).make_absolute(); return m_document->as_filesystem()->is_file(path); @@ -1019,6 +1025,9 @@ class ElementAdapter final : public abstract::ElementAdapter, if (m_document->as_filesystem() == nullptr) { return std::nullopt; } + if (is_object(element_id)) { + return object_file(element_id); + } const AbsPath path = Path(image_href(element_id)).make_absolute(); return File(m_document->as_filesystem()->open(path)); } @@ -1029,12 +1038,19 @@ class ElementAdapter final : public abstract::ElementAdapter, if (image_data(element_id)) { return "image" + std::to_string(element_id); } - return get_node(element_id).attribute("xlink:href").value(); + std::string href = get_node(element_id).attribute("xlink:href").value(); + if (is_object(element_id)) { + return replacement_href(element_id).value_or(href + "/chart.svg"); + } + return href; } private: const Document *m_document{nullptr}; ElementRegistry *m_registry{nullptr}; + mutable std::mutex m_charts_mutex; + mutable std::unordered_map> + m_charts; [[nodiscard]] pugi::xml_node get_node(const ElementIdentifier element_id) const { @@ -1049,6 +1065,80 @@ class ElementAdapter final : public abstract::ElementAdapter, return data.text().empty() ? pugi::xml_node() : data; } + [[nodiscard]] bool is_object(const ElementIdentifier element_id) const { + return std::strcmp(get_node(element_id).name(), "draw:object") == 0; + } + + /// The `draw:image` the producer wrote beside the object (10.4.6.2), which + /// is what draws where the object itself cannot be read. + [[nodiscard]] std::optional + replacement_href(const ElementIdentifier element_id) const { + if (chart_svg(element_id).has_value()) { + return {}; + } + const pugi::xml_attribute href = get_node(element_id) + .parent() + .child("draw:image") + .attribute("xlink:href"); + if (!href) { + return {}; + } + return href.value(); + } + + [[nodiscard]] std::optional + object_file(const ElementIdentifier element_id) const { + if (const std::optional svg = chart_svg(element_id)) { + return File(std::make_shared(*svg)); + } + const std::optional replacement = replacement_href(element_id); + if (!replacement.has_value()) { + return std::nullopt; + } + try { + const AbsPath path = Path(*replacement).make_absolute(); + return File(m_document->as_filesystem()->open(path)); + } catch (...) { // NOLINT(bugprone-empty-catch): no replacement either + } + return std::nullopt; + } + + [[nodiscard]] std::optional + object_part(const ElementIdentifier element_id) const { + const char *href = get_node(element_id).attribute("xlink:href").value(); + if (href[0] == '\0' || m_document->as_filesystem() == nullptr) { + return {}; + } + try { + AbsPath path = Path(href).make_absolute().join(RelPath("content.xml")); + if (m_document->as_filesystem()->is_file(path)) { + return path; + } + } catch (...) { // NOLINT(bugprone-empty-catch): no part of its own + } + return {}; + } + + /// The object's own part rendered, or nothing where it holds no chart. Kept, + /// because `image_is_internal`, the resource and the `src` each ask for it. + [[nodiscard]] const std::optional & + chart_svg(const ElementIdentifier element_id) const { + const std::lock_guard lock(m_charts_mutex); + if (const auto it = m_charts.find(element_id); it != m_charts.end()) { + return it->second; + } + std::optional result; + if (const std::optional path = object_part(element_id)) { + try { + const pugi::xml_document content = + xml::parse(*m_document->as_filesystem()->open(*path)); + result = render_chart(content.document_element()); + } catch (...) { // NOLINT(bugprone-empty-catch): no chart we can read + } + } + return m_charts.emplace(element_id, std::move(result)).first->second; + } + [[nodiscard]] static std::string get_text(const pugi::xml_node node) { if (node.type() == pugi::node_pcdata) { return node.value(); diff --git a/src/odr/internal/odf/odf_geometry.cpp b/src/odr/internal/odf/odf_geometry.cpp index 4ab214036..6dadba7fe 100644 --- a/src/odr/internal/odf/odf_geometry.cpp +++ b/src/odr/internal/odf/odf_geometry.cpp @@ -628,6 +628,25 @@ std::optional odf::parse_path_data(const std::string_view data) { return odf::PathParser(data).parse(); } +std::optional +odf::read_hundredth_millimetres(const pugi::xml_attribute attribute) { + if (!attribute) { + return {}; + } + odf::ValueCursor in(attribute.value()); + const std::optional value = in.read_number(); + if (!value.has_value()) { + return {}; + } + in.skip_space(); + const double scale = + odf::centimetres_per(in.take_while(odf::ValueCursor::is_letter)); + if (scale == 0) { + return {}; + } + return *value * scale * 1000; +} + std::optional odf::read_path(const pugi::xml_node node) { const std::string_view name = node.name(); diff --git a/src/odr/internal/odf/odf_geometry.hpp b/src/odr/internal/odf/odf_geometry.hpp index a312e27eb..0fd138512 100644 --- a/src/odr/internal/odf/odf_geometry.hpp +++ b/src/odr/internal/odf/odf_geometry.hpp @@ -22,6 +22,11 @@ read_transform(pugi::xml_node node); [[nodiscard]] std::optional parse_transform(std::string_view value); +/// A length attribute in 1/100 mm, the unit ODF measures a chart and an +/// `svg:d` with no view box in. Nothing where it is absent or not a length. +[[nodiscard]] std::optional +read_hundredth_millimetres(pugi::xml_attribute attribute); + /// The outline @p node draws: `draw:path`, `draw:polygon`, `draw:polyline`, /// `draw:regular-polygon`, `draw:connector`, and a `draw:circle`/`draw:ellipse` /// that `draw:kind` cuts. Nothing for a shape with no geometry we can read. diff --git a/src/odr/internal/odf/odf_parser.cpp b/src/odr/internal/odf/odf_parser.cpp index 20c907182..8a652222e 100644 --- a/src/odr/internal/odf/odf_parser.cpp +++ b/src/odr/internal/odf/odf_parser.cpp @@ -237,6 +237,17 @@ parse_sheet(ElementRegistry ®istry, const pugi::xml_node node) { return {element_id, node.next_sibling()}; } +/// A `draw:image` beside a `draw:object` is the object's replacement +/// (10.4.6.2); the object draws itself, and the two would else stack. +std::tuple +parse_replaceable_image(ElementRegistry ®istry, const pugi::xml_node node) { + if (node.parent().child("draw:object")) { + return {null_element_id, pugi::xml_node()}; + } + return parse_element_tree(registry, ElementType::image, node, + parse_any_element_children); +} + /// `draw:circle` / `draw:ellipse`: a full one is the box, one that /// `draw:kind` (19.212) cuts needs the path its arc traces. std::tuple @@ -333,7 +344,9 @@ parse_any_element_tree(ElementRegistry ®istry, const pugi::xml_node node) { {"table:covered-table-cell", create_default_tree_parser(ElementType::table_cell)}, {"draw:frame", create_default_tree_parser(ElementType::frame)}, - {"draw:image", create_default_tree_parser(ElementType::image)}, + {"draw:image", parse_replaceable_image}, + // An embedded object renders as the image its own part is drawn to. + {"draw:object", create_default_tree_parser(ElementType::image)}, {"draw:rect", create_default_tree_parser(ElementType::rect)}, {"draw:line", create_default_tree_parser(ElementType::line)}, {"draw:circle", parse_elliptical_element}, diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 19235d252..3ebb3af8f 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -73,6 +73,7 @@ add_executable(odr_test "src/internal/rtf/rtf_document_test.cpp" "src/internal/rtf/rtf_tokenizer_test.cpp" + "src/internal/odf/odf_chart_test.cpp" "src/internal/odf/odf_enhanced_geometry_test.cpp" "src/internal/odf/odf_flat_file_test.cpp" "src/internal/odf/odf_geometry_test.cpp" diff --git a/test/src/internal/odf/odf_chart_test.cpp b/test/src/internal/odf/odf_chart_test.cpp new file mode 100644 index 000000000..a0e7366a1 --- /dev/null +++ b/test/src/internal/odf/odf_chart_test.cpp @@ -0,0 +1,158 @@ +#include + +#include + +#include + +#include +#include + +using namespace odr::internal::odf; + +namespace { + +/// An embedded chart part, with @p chart wrapped in the document its +/// `content.xml` is. +std::string chart_part(const std::string &chart) { + return R"()" + R"()" + R"()" + R"()" + R"()" + + chart + R"()"; +} + +/// Two series of two points, with one gap, laid out the way libreoffice writes +/// a `local-table`. +std::string bar_chart(const std::string &klass = "chart:bar", + const std::string &head = "", + const std::string &second_category = "two") { + return chart_part( + R"()" + head + + R"()" + R"()" + R"()" + R"()" + R"()" + R"()" + R"()" + R"(alpha)" + R"(beta)" + R"()" + R"()" + R"()" + R"(one)" + R"(1)" + R"(4)" + R"()" + R"()" + R"()" + + second_category + + R"()" + R"(NaN)" + R"(2)" + R"()" + R"()" + R"()"); +} + +std::optional render(const std::string &xml) { + pugi::xml_document document; + EXPECT_TRUE(document.load_string(xml.c_str())); + return render_chart(document.document_element()); +} + +std::size_t count(const std::string &haystack, const std::string &needle) { + std::size_t result = 0; + for (std::size_t at = haystack.find(needle); at != std::string::npos; + at = haystack.find(needle, at + 1)) { + ++result; + } + return result; +} + +} // namespace + +TEST(OdfChart, a_chart_is_sized_in_hundredths_of_a_millimetre) { + const std::optional svg = render(bar_chart()); + ASSERT_TRUE(svg.has_value()); + EXPECT_NE(std::string::npos, svg->find(R"(viewBox="0 0 16000 9000")")); + EXPECT_NE(std::string::npos, svg->find(R"(width="16cm")")); +} + +TEST(OdfChart, the_first_column_names_the_categories) { + const std::optional svg = render(bar_chart()); + ASSERT_TRUE(svg.has_value()); + EXPECT_NE(std::string::npos, svg->find(">one<")); + EXPECT_NE(std::string::npos, svg->find(">two<")); +} + +TEST(OdfChart, the_header_row_names_the_series_in_the_legend) { + const std::optional svg = render( + bar_chart("chart:bar", R"()")); + ASSERT_TRUE(svg.has_value()); + EXPECT_NE(std::string::npos, svg->find(">alpha<")); + EXPECT_NE(std::string::npos, svg->find(">beta<")); +} + +TEST(OdfChart, a_series_takes_the_colour_its_style_gives_it) { + const std::optional svg = render(bar_chart()); + ASSERT_TRUE(svg.has_value()); + EXPECT_NE(std::string::npos, svg->find("#112233")); + // The second series has no style, so it falls to the palette's second entry. + EXPECT_NE(std::string::npos, svg->find("#ff420e")); +} + +TEST(OdfChart, a_nan_value_is_a_gap_rather_than_a_bar) { + const std::optional svg = render(bar_chart()); + ASSERT_TRUE(svg.has_value()); + // Three bars for four cells, plus the two the background and plot area are. + EXPECT_EQ(5, count(*svg, " svg = render(bar_chart()); + ASSERT_TRUE(svg.has_value()); + EXPECT_NE(std::string::npos, svg->find(">0<")); +} + +TEST(OdfChart, a_line_chart_draws_one_path_per_series) { + const std::optional svg = render(bar_chart("chart:line")); + ASSERT_TRUE(svg.has_value()); + EXPECT_EQ(2, count(*svg, " svg = render(bar_chart("chart:circle")); + ASSERT_TRUE(svg.has_value()); + // One slice per point the first series has, and the gap is not one. + EXPECT_EQ(1, count(*svg, " svg = + render(bar_chart("chart:bar", "", long_category)); + ASSERT_TRUE(svg.has_value()); + EXPECT_NE(std::string::npos, svg->find(">one<")); + EXPECT_EQ(std::string::npos, svg->find(">" + long_category + "<")); +} + +TEST(OdfChart, a_title_is_drawn) { + const std::optional svg = + render(bar_chart("chart:bar", R"(Counts)" + "\n" + R"()")); + ASSERT_TRUE(svg.has_value()); + EXPECT_NE(std::string::npos, svg->find(">Counts<")); +} + +TEST(OdfChart, a_part_with_no_chart_renders_nothing) { + EXPECT_FALSE(render(chart_part("")).has_value()); + EXPECT_FALSE( + render(chart_part(R"()")) + .has_value()); +} From 2810018a90488e182a40003258ae90e9e497c124 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 31 Aug 2026 22:38:55 +0200 Subject: [PATCH 2/3] refactor(odf): name the value cursor for what it is `Scanner` promised tokenising it does not do; the class is a position in a string that advances, which is what its own comment already called it and what `common::TableCursor` is named for over table positions. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UJ6NPShQBp1nX7WuMvty8V --- src/odr/internal/odf/odf_enhanced_geometry.cpp | 10 +++++----- src/odr/internal/odf/odf_geometry.cpp | 16 ++++++++-------- .../{odf_scanner.hpp => odf_value_cursor.hpp} | 8 ++++---- 3 files changed, 17 insertions(+), 17 deletions(-) rename src/odr/internal/odf/{odf_scanner.hpp => odf_value_cursor.hpp} (92%) diff --git a/src/odr/internal/odf/odf_enhanced_geometry.cpp b/src/odr/internal/odf/odf_enhanced_geometry.cpp index ea290787f..751ec050a 100644 --- a/src/odr/internal/odf/odf_enhanced_geometry.cpp +++ b/src/odr/internal/odf/odf_enhanced_geometry.cpp @@ -1,6 +1,6 @@ #include -#include +#include #include #include @@ -17,12 +17,12 @@ namespace { /// Recursive descent over 20.36's grammar: sums of products of unary terms, /// with `$N` modifiers, `?name` equations, named values and functions. -class FormulaParser : private Scanner { +class FormulaParser : private ValueCursor { public: FormulaParser(const std::string_view formula, const EnhancedGeometryContext &context, const EquationResolver &equations) - : Scanner{formula}, m_context{&context}, m_equations{&equations} {} + : ValueCursor{formula}, m_context{&context}, m_equations{&equations} {} [[nodiscard]] std::optional parse() { const std::optional value = expression(); @@ -237,12 +237,12 @@ class FormulaParser : private Scanner { }; /// Reads 19.145's commands and writes the svg `d` they trace. -class EnhancedPathParser : private Scanner { +class EnhancedPathParser : private ValueCursor { public: EnhancedPathParser(const std::string_view path, const EnhancedGeometryContext &context, const EquationResolver &equations) - : Scanner{path}, m_context{&context}, m_equations{&equations} {} + : ValueCursor{path}, m_context{&context}, m_equations{&equations} {} [[nodiscard]] std::optional parse() { while (true) { diff --git a/src/odr/internal/odf/odf_geometry.cpp b/src/odr/internal/odf/odf_geometry.cpp index 6dadba7fe..6e10eafe2 100644 --- a/src/odr/internal/odf/odf_geometry.cpp +++ b/src/odr/internal/odf/odf_geometry.cpp @@ -3,7 +3,7 @@ #include #include -#include +#include #include #include @@ -53,9 +53,9 @@ double centimetres_per(const std::string_view unit) { } /// Composes the operation list, holding the translation in centimetres. -class TransformParser : private Scanner { +class TransformParser : private ValueCursor { public: - using Scanner::Scanner; + using ValueCursor::ValueCursor; [[nodiscard]] std::optional parse() { while (true) { @@ -189,9 +189,9 @@ class TransformParser : private Scanner { /// Reads an svg `d` (19.180) and writes it back out, boxed by every point and /// control point it names. Only the numbers are re-rendered. -class PathParser : private Scanner { +class PathParser : private ValueCursor { public: - using Scanner::Scanner; + using ValueCursor::ValueCursor; [[nodiscard]] std::optional parse() { while (true) { @@ -373,7 +373,7 @@ std::optional read_view_box(const pugi::xml_node node) { if (!attribute) { return {}; } - Scanner in(attribute.value()); + ValueCursor in(attribute.value()); std::array values{}; for (double &value : values) { const std::optional number = in.read_number(); @@ -396,7 +396,7 @@ std::optional read_points(const pugi::xml_node node, if (!attribute) { return {}; } - Scanner in(attribute.value()); + ValueCursor in(attribute.value()); std::string result; while (true) { @@ -461,7 +461,7 @@ std::optional read_regular_polygon(const pugi::xml_node node) { /// Space-separated numbers, as `draw:modifiers` (19.214) writes them. std::vector read_numbers(const pugi::xml_attribute attribute) { - Scanner in(attribute.value()); + ValueCursor in(attribute.value()); std::vector result; while (true) { const std::optional value = in.read_number(); diff --git a/src/odr/internal/odf/odf_scanner.hpp b/src/odr/internal/odf/odf_value_cursor.hpp similarity index 92% rename from src/odr/internal/odf/odf_scanner.hpp rename to src/odr/internal/odf/odf_value_cursor.hpp index 961532c45..79c0c9b91 100644 --- a/src/odr/internal/odf/odf_scanner.hpp +++ b/src/odr/internal/odf/odf_value_cursor.hpp @@ -8,11 +8,11 @@ namespace odr::internal::odf { -/// A cursor over the input every reader here shares. Reads are bounded by what -/// remains, which carries no terminator. -class Scanner { +/// A cursor over one of the small languages an odf attribute is written in. +/// Reads are bounded by what remains, which carries no terminator. +class ValueCursor { public: - explicit Scanner(const std::string_view input) : m_rest{input} {} + explicit ValueCursor(const std::string_view input) : m_rest{input} {} [[nodiscard]] bool empty() const { return m_rest.empty(); } From 811973ab1137b25982b7d4992c4321547fe31d65 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 31 Aug 2026 22:47:53 +0200 Subject: [PATCH 3/3] test: advance the reference output to the drawn charts 19 files. Every change is a line-for-line replacement: the metafile replacement image's data uri swapped for the chart's own svg. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UJ6NPShQBp1nX7WuMvty8V --- test/data.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/data.cmake b/test/data.cmake index 6bfd2f1f2..c8c999755 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 "cb5b29143d4cf88340ccbca106bd8d3ab39429bf") + REVISION "ac2bd3dd210505f4282a300089693a3a3a309a50") odr_test_data( PATH "reference-output/odr-private" URL "https://github.com/opendocument-app/OpenDocument.test-private.output.git" - REVISION "c3ec7fd812443b8d0520259cb0e3b10299a6a8da") + REVISION "949af844a65d091ed21cb6e9d1a3516a739ee710")