fix: calculate spreadsheet content within preview limits

CHANGELOG.md

@@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- Spreadsheet previews retain ODS cells at window boundaries, clip repeated
+  and merged cells, and trim XLSX output to populated cells inside the window.
+
 - Filesystem copies preserve their source when copying onto itself and leave
   existing destinations intact when reading or writing fails.
 

src/odr/internal/odf/odf_document.cpp

@@ -679,34 +679,28 @@ class ElementAdapter final : public AdapterBase {
   [[nodiscard]] TableDimensions
   sheet_content(const ElementIdentifier element_id,
                 const std::optional<TableDimensions> range) const override {
-    const pugi::xml_node node = get_node(element_id);
-
+    const TableDimensions limit = range.value_or(
+        TableDimensions(std::numeric_limits<std::uint32_t>::max(),
+                        std::numeric_limits<std::uint32_t>::max()));
     TableDimensions result;
-
-    TableCursor cursor;
-    for_each_table_row(node, [&](const pugi::xml_node row) {
-      const auto rows_repeated =
-          row.attribute("table:number-rows-repeated").as_uint(1);
-      cursor.add_row(rows_repeated);
-
-      for (auto cell : row.children("table:table-cell")) {
-        const auto columns_repeated =
-            cell.attribute("table:number-columns-repeated").as_uint(1);
-        const auto colspan =
-            cell.attribute("table:number-columns-spanned").as_uint(1);
-        const auto rowspan =
-            cell.attribute("table:number-rows-spanned").as_uint(1);
-        cursor.add_cell(colspan, rowspan, columns_repeated);
-
-        const std::uint32_t new_rows = cursor.row();
-        const std::uint32_t new_cols =
-            std::max(result.columns, cursor.column());
-        if (cell.first_child() &&
-            (!range || (new_rows < range->rows && new_cols < range->columns))) {
-          result.rows = new_rows;
-          result.columns = new_cols;
-        }
+    for_each_cell_run(element_id, [&](const ElementRegistry::Sheet::Cell &cell,
+                                      const std::uint32_t row,
+                                      const TableDimensions &repeated) {
+      if (!cell.node.first_child() || row >= limit.rows ||
+          cell.begin >= limit.columns) {
+        return;
       }
+      const std::uint32_t colspan = std::max(
+          1u, cell.node.attribute("table:number-columns-spanned").as_uint(1));
+      const std::uint32_t rowspan = std::max(
+          1u, cell.node.attribute("table:number-rows-spanned").as_uint(1));
+      const auto end_row = static_cast<std::uint32_t>(std::min<std::uint64_t>(
+          std::uint64_t{row} + repeated.rows + rowspan - 1, limit.rows));
+      const auto end_column =
+          static_cast<std::uint32_t>(std::min<std::uint64_t>(
+              std::uint64_t{cell.end} + colspan - 1, limit.columns));
+      result.rows = std::max(result.rows, end_row);
+      result.columns = std::max(result.columns, end_column);
     });
 
     return result;

src/odr/internal/ooxml/spreadsheet/AGENTS.md

@@ -104,10 +104,8 @@ the evaluator cannot compute.
    stale formulas, and saving after an edit invokes it. Unsupported and array
    formulas retain their cached results. Number formats supply displayed
    numbers, dates and times.
-2. `sheet_content` ignores the requested range and returns the full
-   `<dimension>`.
-3. No `cellStyleXfs` inheritance. Borders render as `0.75pt solid` whatever
+2. No `cellStyleXfs` inheritance. Borders render as `0.75pt solid` whatever
    the style. Cell `protection` is read and dropped.
-4. `sheet_set_cell` writes numbers, strings, booleans, dates and times. `text_set_content`
+3. `sheet_set_cell` writes numbers, strings, booleans, dates and times. `text_set_content`
    throws `UnsupportedOperation`. A `<hyperlink>` is not modelled, so
    `link_href` is empty. Comments are not modelled.

src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp

@@ -24,6 +24,7 @@
 #include <cmath>
 #include <cstdint>
 #include <iterator>
+#include <limits>
 #include <optional>
 #include <ostream>
 #include <ranges>
@@ -307,6 +308,13 @@ pugi::xml_node Document::create_styles_() {
 
 namespace {
 
+/// Whether a cell draws something on its own: a border or a visible fill.
+bool draws(const TableCellStyle &style) {
+  const DirectionalStyle<std::string> &border = style.border;
+  return (style.background_color && style.background_color->alpha != 0) ||
+         border.top || border.right || border.bottom || border.left;
+}
+
 using AdapterBase = internal::RegistryElementAdapter<
     ElementRegistry, abstract::SheetAdapter, abstract::SheetCellAdapter,
     abstract::LineBreakAdapter, abstract::ParagraphAdapter,
@@ -333,11 +341,36 @@ class ElementAdapter final : public AdapterBase {
   }
   [[nodiscard]] TableDimensions
   sheet_content(const ElementIdentifier element_id,
-                [[maybe_unused]] const std::optional<TableDimensions> range)
-      const override {
-    // TODO the range is ignored: this answers the whole `<dimension>` rather
-    // than trimming to the populated cells inside it.
-    return sheet_dimensions(element_id);
+                const std::optional<TableDimensions> range) const override {
+    const TableDimensions limit = range.value_or(
+        TableDimensions(std::numeric_limits<std::uint32_t>::max(),
+                        std::numeric_limits<std::uint32_t>::max()));
+    TableDimensions result;
+    for (const auto &[position, cell] :
+         m_registry->sheet_element_at(element_id).cells) {
+      if (position.row >= limit.rows || position.column >= limit.columns) {
+        continue;
+      }
+      // an empty cell still counts where its border or fill draws something
+      if (!cell.node.first_child() &&
+          !draws(sheet_cell_style(element_id, position.column, position.row))) {
+        continue;
+      }
+      const ElementRegistry::SheetCell &element =
+          m_registry->sheet_cell_element_at(cell.element_id);
+      if (element.is_covered) {
+        continue;
+      }
+      const auto end_row = static_cast<std::uint32_t>(std::min<std::uint64_t>(
+          std::uint64_t{position.row} + element.span.rows, limit.rows));
+      const auto end_column =
+          static_cast<std::uint32_t>(std::min<std::uint64_t>(
+              std::uint64_t{position.column} + element.span.columns,
+              limit.columns));
+      result.rows = std::max(result.rows, end_row);
+      result.columns = std::max(result.columns, end_column);
+    }
+    return result;
   }
   [[nodiscard]] ElementIdentifier
   sheet_cell(const ElementIdentifier element_id, const std::uint32_t column,

test/src/internal/odf/odf_sheet_repeat_test.cpp

@@ -159,3 +159,42 @@ TEST(OdfSheetRepeat, a_write_into_a_repeat_does_not_expand_it) {
   EXPECT_EQ(sheet.cell(512, 1023).value().text(), "x");
   EXPECT_LT(document->element_registry().size(), 32);
 }
+
+TEST(OdfSheetRepeat, content_extent_clips_repeated_runs_at_the_window) {
+  const auto held = document_of(flat_sheet(repeated_rows(4, 3)));
+  const Sheet sheet =
+      odr::Document(held).root_element().first_child().as_sheet();
+
+  for (const TableDimensions limit :
+       {TableDimensions(4, 3), TableDimensions(2, 2), TableDimensions(1, 1)}) {
+    const TableDimensions content = sheet.content(limit);
+    EXPECT_EQ(content.rows, limit.rows);
+    EXPECT_EQ(content.columns, limit.columns);
+  }
+  EXPECT_EQ(sheet.content(TableDimensions(0, 3)).columns, 0);
+  EXPECT_EQ(sheet.content(TableDimensions(4, 0)).rows, 0);
+}
+
+TEST(OdfSheetRepeat, content_extent_uses_indexed_positions_and_merged_spans) {
+  const auto held = document_of(flat_sheet(
+      R"(<table:table-row><table:covered-table-cell/>)"
+      R"(<table:table-cell table:number-columns-spanned="2")"
+      R"( table:number-rows-spanned="2"><text:p>x</text:p>)"
+      R"(</table:table-cell><table:covered-table-cell/></table:table-row>)"
+      R"(<table:table-row><table:table-cell/>)"
+      R"(<table:covered-table-cell table:number-columns-repeated="2"/>)"
+      R"(</table:table-row>)"
+      R"(<table:table-row table:number-rows-repeated="8"/>)"
+      R"(<table:table-row><table:table-cell table:number-columns-repeated="9"/>)"
+      R"(<table:table-cell><text:p>outside</text:p></table:table-cell>)"
+      R"(</table:table-row>)"));
+  const Sheet sheet =
+      odr::Document(held).root_element().first_child().as_sheet();
+
+  const TableDimensions content = sheet.content(TableDimensions(5, 5));
+  EXPECT_EQ(content.rows, 2);
+  EXPECT_EQ(content.columns, 3);
+  const TableDimensions blank = sheet.content(TableDimensions(5, 1));
+  EXPECT_EQ(blank.rows, 0);
+  EXPECT_EQ(blank.columns, 0);
+}

test/src/internal/ooxml/ooxml_spreadsheet_value_test.cpp

@@ -1,6 +1,7 @@
 #include <odr/document.hpp>
 #include <odr/document_element.hpp>
 #include <odr/filesystem.hpp>
+#include <odr/table_dimension.hpp>
 
 #include <internal/ooxml/ooxml_spreadsheet_test_util.hpp>
 
@@ -331,3 +332,54 @@ TEST(OoxmlSpreadsheetValue, an_invalid_index_keeps_the_rest_of_the_sheet) {
                       "", "<si><t>zero</t></si>"));
   EXPECT_EQ(first_sheet(document).cell(0, 0).value().text(), "zero");
 }
+
+TEST(OoxmlSpreadsheetValue, content_extent_counts_only_cells_in_the_window) {
+  const Document document =
+      decode(workbook(R"(<row r="2"><c r="B2"><v>1</v></c></row>)"
+                      R"(<row r="4"><c r="D4" s="0"/></row>)"
+                      R"(<row r="10"><c r="J10"><v>2</v></c></row>)",
+                      "", "", "", R"(<dimension ref="A1:Z1000"/>)"));
+  const Sheet sheet = first_sheet(document);
+
+  for (const TableDimensions limit :
+       {TableDimensions(2, 2), TableDimensions(5, 5)}) {
+    const TableDimensions content = sheet.content(limit);
+    EXPECT_EQ(content.rows, 2);
+    EXPECT_EQ(content.columns, 2);
+  }
+  EXPECT_EQ(sheet.content(std::nullopt).rows, 10);
+  EXPECT_EQ(sheet.content(std::nullopt).columns, 10);
+  EXPECT_EQ(sheet.content(TableDimensions(1, 5)).columns, 0);
+  EXPECT_EQ(sheet.content(TableDimensions(5, 1)).rows, 0);
+}
+
+TEST(OoxmlSpreadsheetValue, content_extent_clips_merged_cells_at_the_window) {
+  const Document document =
+      decode(workbook(R"(<row r="2"><c r="B2"><v>1</v></c></row>)",
+                      R"(<mergeCells><mergeCell ref="B2:E4"/></mergeCells>)"));
+  const Sheet sheet = first_sheet(document);
+
+  EXPECT_EQ(sheet.content(std::nullopt).rows, 4);
+  EXPECT_EQ(sheet.content(std::nullopt).columns, 5);
+  const TableDimensions content = sheet.content(TableDimensions(3, 3));
+  EXPECT_EQ(content.rows, 3);
+  EXPECT_EQ(content.columns, 3);
+}
+
+// An empty cell still draws its border, so the preview has to reach it.
+TEST(OoxmlSpreadsheetValue, content_extent_counts_an_empty_cell_with_a_border) {
+  const Document document = decode(workbook(
+      R"(<row r="2"><c r="B2"><v>1</v></c></row>)"
+      R"(<row r="4"><c r="D4" s="1"/></row>)"
+      R"(<row r="6"><c r="F6" s="0"/></row>)",
+      "", "", "", "",
+      R"(<fonts count="1"><font><sz val="11"/></font></fonts>)"
+      R"(<fills count="1"><fill><patternFill patternType="none"/></fill></fills>)"
+      R"(<borders count="2"><border/>)"
+      R"(<border><bottom style="thin"/></border></borders>)"
+      R"(<cellXfs count="2"><xf fontId="0" fillId="0" borderId="0"/>)"
+      R"(<xf fontId="0" fillId="0" borderId="1" applyBorder="1"/></cellXfs>)"));
+  const TableDimensions content = first_sheet(document).content(std::nullopt);
+  EXPECT_EQ(content.rows, 4);
+  EXPECT_EQ(content.columns, 4);
+}