fix: preserve ODF chart data positions

CHANGELOG.md

@@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- ODF charts keep missing points aligned with their categories, honor repeated
+  rows, and retain implicit series. Expanded chart tables are bounded.
+
 - ODF number formats clamp digit counts to 4,096 and the minimum decimals to
   the decimals, as LibreOffice does, before they expand placeholders.
 

src/odr/internal/odf/AGENTS.md

@@ -206,6 +206,9 @@ Unreadable geometry is dropped rather than emitting invalid SVG.
 Number-format digit counts are clamped to 4,096 per field before expanding
 placeholders; a negative count keeps the default.
 
+Chart tables expand at most 1,048,576 cells, including gaps in ragged rows.
+Larger tables fall back to the embedded replacement image.
+
 ## Open work
 
 1. **Save never re-encrypts.** A document decrypted from a password-protected

src/odr/internal/odf/odf_chart.cpp

@@ -126,8 +126,7 @@ class ChartWriter {
     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()) {
+    if (!read_data(plot_area) || m_series.empty()) {
       return {};
     }
 
@@ -196,38 +195,60 @@ class ChartWriter {
 
   /// 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) {
+  bool 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;
+      return false;
     }
 
     const std::size_t leading = std::max<std::size_t>(
         1, count_cells(table.child("table:table-header-columns")
                            .child("table:table-column")));
+    if (leading > max_cells) {
+      return false;
+    }
 
     std::vector<std::string> labels;
-    for (const pugi::xml_node header :
-         table.child("table:table-header-rows").children("table:table-row")) {
-      labels = read_row_text(header);
-      break;
+    if (const pugi::xml_node header =
+            table.child("table:table-header-rows").child("table:table-row")) {
+      const auto cells = read_row(header);
+      if (!cells) {
+        return false;
+      }
+      labels = *cells | std::views::transform(read_text) |
+               std::ranges::to<std::vector<std::string>>();
     }
 
     std::vector<std::vector<std::optional<double>>> columns;
     for (const pugi::xml_node row :
          table.child("table:table-rows").children("table:table-row")) {
-      const std::vector<pugi::xml_node> cells = read_row(row);
-      if (cells.size() <= leading) {
-        continue;
+      const auto cells = read_row(row);
+      const std::uint32_t repeated =
+          row.attribute("table:number-rows-repeated").as_uint(1);
+      if (!cells || repeated == 0 ||
+          repeated > max_cells - m_categories.size()) {
+        return false;
       }
-      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]));
+      const std::size_t rows = m_categories.size() + repeated;
+      const std::size_t width =
+          std::max(columns.size(),
+                   cells->size() > leading ? cells->size() - leading : 0);
+      if (width > max_cells / rows) {
+        return false;
+      }
+      columns.resize(width);
+      for (std::size_t i = 0; i < width; ++i) {
+        columns[i].resize(m_categories.size());
+        columns[i].insert(columns[i].end(), repeated,
+                          leading + i < cells->size()
+                              ? read_value((*cells)[leading + i])
+                              : std::nullopt);
       }
+      m_categories.insert(m_categories.end(), repeated,
+                          leading <= cells->size()
+                              ? read_text((*cells)[leading - 1])
+                              : std::string());
     }
 
     std::size_t index = 0;
@@ -245,26 +266,38 @@ class ChartWriter {
       ++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))});
+    if (m_series.empty()) {
+      for (; index < columns.size(); ++index) {
+        m_series.push_back({.label = leading + index < labels.size()
+                                         ? labels[leading + index]
+                                         : std::string(),
+                            .values = std::move(columns[index]),
+                            .colour = std::string(colour_at(index))});
+      }
     }
 
     trim();
     read_range();
+    return true;
   }
 
+  static constexpr std::size_t max_cells = 1048576;
+
   [[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);
+      const std::size_t repeated =
+          node.attribute("table:number-columns-repeated").as_uint(1);
+      if (repeated > max_cells - result) {
+        return max_cells + 1;
+      }
+      result += repeated;
     }
     return result;
   }
 
-  [[nodiscard]] static std::vector<pugi::xml_node>
+  [[nodiscard]] static std::optional<std::vector<pugi::xml_node>>
   read_row(const pugi::xml_node row) {
     std::vector<pugi::xml_node> result;
     for (const pugi::xml_node cell : row.children()) {
@@ -274,19 +307,14 @@ class ChartWriter {
       }
       const auto repeated =
           cell.attribute("table:number-columns-repeated").as_uint(1);
-      for (unsigned i = 0; i < repeated; ++i) {
-        result.push_back(cell);
+      if (repeated == 0 || repeated > max_cells - result.size()) {
+        return std::nullopt;
       }
+      result.insert(result.end(), repeated, cell);
     }
     return result;
   }
 
-  [[nodiscard]] static std::vector<std::string>
-  read_row_text(const pugi::xml_node row) {
-    return read_row(row) | std::views::transform(read_text) |
-           std::ranges::to<std::vector<std::string>>();
-  }
-
   /// A missing data point is written `office:value="NaN"`.
   [[nodiscard]] static std::optional<double>
   read_value(const pugi::xml_node cell) {

test/src/internal/odf/odf_chart_test.cpp

@@ -156,3 +156,57 @@ TEST(OdfChart, a_part_with_no_chart_renders_nothing) {
                         R"(svg:height="9cm" chart:class="chart:bar"/>)"))
           .has_value());
 }
+
+TEST(OdfChart, ragged_repeated_rows_keep_all_series_on_their_categories) {
+  for (const bool implicit : {false, true}) {
+    pugi::xml_document document;
+    ASSERT_TRUE(document.load_string(bar_chart("chart:scatter").c_str()));
+    const pugi::xml_node chart = document.document_element()
+                                     .child("office:body")
+                                     .child("office:chart")
+                                     .child("chart:chart");
+    if (implicit) {
+      chart.child("chart:plot-area").remove_children();
+    }
+    pugi::xml_node first =
+        chart.child("table:table").child("table:table-rows").first_child();
+    first.remove_child(first.last_child());
+    first.append_attribute("table:number-rows-repeated").set_value(2);
+
+    const auto svg = render_chart(document.document_element());
+    ASSERT_TRUE(svg);
+    pugi::xml_document output;
+    ASSERT_TRUE(output.load_string(svg->c_str()));
+    const auto points = output.select_nodes("/svg/circle");
+    ASSERT_EQ(points.size(), 3u);
+    EXPECT_NEAR(points[0].node().attribute("cx").as_double(), 3333.33, 0.01);
+    EXPECT_NEAR(points[1].node().attribute("cx").as_double(), 8000, 0.01);
+    EXPECT_NEAR(points[2].node().attribute("cx").as_double(), 12666.7, 0.1);
+    EXPECT_EQ(count(*svg, "<path"), 2u);
+  }
+}
+
+TEST(OdfChart, repeated_tables_are_bounded_before_expansion) {
+  for (const char *count : {"0", "1048577", "4294967295"}) {
+    for (const bool rows : {false, true}) {
+      pugi::xml_document document;
+      ASSERT_TRUE(document.load_string(bar_chart().c_str()));
+      pugi::xml_node row =
+          document.select_node("//table:table-rows/table:table-row").node();
+      (rows ? row : row.first_child())
+          .append_attribute(rows ? "table:number-rows-repeated"
+                                 : "table:number-columns-repeated")
+          .set_value(count);
+      EXPECT_FALSE(render_chart(document.document_element()));
+    }
+  }
+  pugi::xml_document document;
+  ASSERT_TRUE(document.load_string(bar_chart().c_str()));
+  pugi::xml_node row =
+      document.select_node("//table:table-rows/table:table-row").node();
+  row.append_attribute("table:number-rows-repeated").set_value(1025);
+  row.last_child()
+      .append_attribute("table:number-columns-repeated")
+      .set_value(1025);
+  EXPECT_FALSE(render_chart(document.document_element()));
+}