feat(ooxml): pptx slide size + tables, xlsx merged cells + value types, docx table merges

src/odr/internal/common/table_cursor.cpp

@@ -1,5 +1,6 @@
 #include <odr/internal/common/table_cursor.hpp>
 
+#include <algorithm>
 #include <list>
 
 namespace odr::internal {
@@ -30,14 +31,17 @@ void TableCursor::add_cell(const std::uint32_t colspan,
                            const std::uint32_t repeat) {
   const std::uint32_t next_column = m_column + colspan * repeat;
 
-  // handle rowspan
+  // handle rowspan; keep each row's ranges sorted by start so
+  // `handle_rowspan_` can skip them front-to-back
   auto it = std::begin(m_sparse);
   for (std::uint32_t i = 1; i < rowspan; ++i) {
     if (std::next(it) == std::end(m_sparse)) {
       m_sparse.emplace_back();
     }
     ++it;
-    it->emplace_back(Range{m_column, next_column});
+    const auto pos = std::ranges::find_if(
+        *it, [&](const Range &range) { return range.start > m_column; });
+    it->insert(pos, Range{m_column, next_column});
   }
 
   m_column = next_column;
@@ -55,8 +59,8 @@ std::uint32_t TableCursor::row() const noexcept { return m_row; }
 void TableCursor::handle_rowspan_() noexcept {
   auto &s = m_sparse.front();
   auto it = std::begin(s);
-  while (it != std::end(s) && m_column == it->start) {
-    m_column = it->end;
+  while (it != std::end(s) && it->start <= m_column) {
+    m_column = std::max(m_column, it->end);
     ++it;
   }
   s.erase(std::begin(s), it);

src/odr/internal/html/document_element.cpp

@@ -145,6 +145,10 @@ void html::translate_sheet(const Sheet &sheet, const WritingState &state) {
       const SheetCell cell = sheet.cell(column_index, row_index);
 
       if (cell.is_covered()) {
+        // normally unreachable: the cursor skips positions covered by an
+        // anchor's span; advance one column so inconsistent spans cannot
+        // starve the loop
+        cursor.add_cell();
         continue;
       }
 

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

@@ -6,7 +6,7 @@ OOXML mechanics (registry/adapter pattern, OPC relationships, encryption) in
 
 **Scope.** Read `ppt/presentation.xml` and each slide's shape tree into the
 abstract model so the generic renderer lays out positioned frames. Paragraphs,
-runs, tables (see caveat below), inline text styling.
+runs, tables, inline text styling.
 
 ## Design decisions
 
@@ -15,8 +15,11 @@ relationship target of `presentation.xml` into an `rId → xml` map (masters,
 layouts, theme included — only slides are actually walked). Slide **order = the
 document order of `p:sldId` in `p:sldIdLst`** (not filename/rId order); each
 slide's `r:id` looks up its part, and parsing descends `p:cSld/p:spTree`.
-Dispatch table: `p:sp`→**frame** (shapes are frames), `p:txBody`→group,
-`a:p`→paragraph, `a:r`→span, `a:t`→text, `a:tbl`→table.
+Dispatch table: `p:sp`→**frame** (shapes are frames), `p:graphicFrame`→frame
+(descends `a:graphic/a:graphicData`), `p:txBody`→group, `a:p`→paragraph,
+`a:r`→span, `a:t`→text, `a:tbl`→table (columns from `a:tblGrid/a:gridCol` via
+`append_column`, rows/cells from `a:tr`/`a:tc`; spans from
+`gridSpan`/`rowSpan`, covered cells from `hMerge`/`vMerge`).
 
 **Styles are resolved inline — there is no `StyleRegistry`.** Free functions in
 the document read `a:rPr` / `a:pPr` attributes directly (font, size in
@@ -26,8 +29,10 @@ hundredth-points, bold/italic/underline/strike/shadow/colour/highlight; align,
 computed on-demand from the XML with no cached or master/default-style
 contribution.
 
-**Frame positioning is EMU-based.** `p:spPr/a:xfrm/a:off` + `a:ext` give
-`x/y/width/height` in EMUs; anchor type is always `at_page`.
+**Frame positioning is EMU-based.** `p:spPr/a:xfrm/a:off` + `a:ext` (`p:xfrm`
+for `p:graphicFrame`) give `x/y/width/height` in EMUs; anchor type is always
+`at_page`. Slide size comes from `p:presentation/p:sldSz` (ECMA-376 default
+10in × 7.5in when absent).
 
 ## Module layout
 
@@ -43,23 +48,15 @@ contribution.
 
 Coverage is in [`README.md`](README.md). Foundational gaps, roughly by value:
 
-1. **Slide geometry is hardcoded.** `slide_page_layout` returns a fixed
-   11.02in × 8.27in; `slide_master_page` and `slide_name` return empty. Real
-   slide size (`p:presentation/p:sldSz`), master/layout inheritance, and slide
-   names are all TODO.
-2. **Tables are broken.** `a:tbl` is created with the plain builder, not
-   `create_table_element`, so no `m_tables` entry exists and `table_first_column`
-   throws; `table_dimensions` iterates ODF names (`table:table-column/-row`,
-   copy-paste from ODF) → 0×0. Despite the README checkbox, tables need real
-   wiring (`create_table_element`/`append_column` + `a:gridCol`/`a:tr`/`a:tc`
-   parsers).
-3. **Images not modelled** — no `p:pic`/`a:blip` parser entry; `image_href`
+1. **No master/layout inheritance.** `slide_master_page` returns empty; master
+   and layout parts are loaded into the relationship map but never consulted
+   (no inherited placeholders, backgrounds, or styles).
+2. **Images not modelled** — no `p:pic`/`a:blip` parser entry; `image_href`
    reads ODF-style `xlink:href` (wrong for pptx `r:embed`).
-4. **`is_text_node` copy-paste bug.** It matches `w:t`/`w:tab` (wordprocessing)
-   instead of `a:t`/`a:tab`, so consecutive `a:t` runs are never coalesced —
-   each becomes its own text Element (harmless, but an artifact to fix).
-5. **Read-only, inconsistently.** `Document::is_editable`/`save` say no, yet the
-   adapter's `element_is_editable` returns true and `text_set_content` is fully
-   implemented — the machinery for text editing exists but is not exposed. Wiring
-   edit + save (mirroring docx) is a natural next step.
-6. **Listings, comments/annotations** not modelled.
+3. **Table cell styles unresolved.** Tables are wired (grid, spans, covered
+   cells, column widths/row heights), but `a:tcPr` (fills, borders, margins)
+   is not translated.
+4. **Read-only.** `text_set_content` machinery exists but is dormant
+   (`element_is_editable` → false); wiring edit + save (mirroring docx) is a
+   natural next step.
+5. **Listings, comments/annotations** not modelled.

src/odr/internal/ooxml/presentation/README.md

@@ -19,7 +19,8 @@ Roughly ordered by importance.
 - [x] open
 - [x] slides
   - [x] shapes (`p:sp`), text bodies
-  - [x] slide master page / page layout
+  - [x] slide size (`p:sldSz`) and slide names
+  - [ ] slide master / layout inheritance
 - [x] text extraction
 - [ ] edit
 - [ ] save
@@ -28,7 +29,8 @@ Roughly ordered by importance.
 
 - [x] paragraphs, runs / spans
 - [x] line breaks
-- [x] tables (grid columns, rows, cells)
+- [x] tables (`p:graphicFrame` / `a:tbl`: grid columns incl. widths, rows,
+      cells, merged cells)
 - [ ] images
 - [ ] listings
 - [ ] annotations / comments
@@ -46,8 +48,8 @@ Roughly ordered by importance.
 - [x] paragraph
   - [x] alignment
   - [x] indentation / left & right margins
-- [x] tables
-- [x] page layout (via master page)
+- [x] tables (column widths, row heights; no `a:tcPr` cell styles)
+- [x] page layout (slide size)
 - [ ] graphic / drawing styles
 
 ## References

src/odr/internal/ooxml/presentation/ooxml_presentation_document.cpp

@@ -8,12 +8,13 @@
 #include <odr/internal/abstract/filesystem.hpp>
 #include <odr/internal/common/path.hpp>
 #include <odr/internal/common/style.hpp>
-#include <odr/internal/common/table_cursor.hpp>
 #include <odr/internal/ooxml/ooxml_util.hpp>
 #include <odr/internal/ooxml/presentation/ooxml_presentation_parser.hpp>
 #include <odr/internal/util/document_util.hpp>
 #include <odr/internal/util/xml_util.hpp>
 
+#include <iterator>
+
 namespace odr::internal::ooxml::presentation {
 
 namespace {
@@ -32,13 +33,30 @@ Document::Document(std::shared_ptr<abstract::ReadableFilesystem> files)
         util::xml::parse(*m_files, AbsPath("/ppt").join(RelPath(target)));
   }
 
+  // ECMA-376 default slide size when p:sldSz is absent.
+  m_slide_layout.width = Measure("10 in");
+  m_slide_layout.height = Measure("7.5 in");
+  if (const pugi::xml_node slide_size =
+          m_document_xml.document_element().child("p:sldSz")) {
+    if (const std::optional<Measure> width =
+            read_emus_attribute(slide_size.attribute("cx"))) {
+      m_slide_layout.width = width;
+    }
+    if (const std::optional<Measure> height =
+            read_emus_attribute(slide_size.attribute("cy"))) {
+      m_slide_layout.height = height;
+    }
+  }
+
   const ParseContext parse_context(m_slides_xml);
   m_root_element = parse_tree(m_element_registry, parse_context,
                               m_document_xml.document_element());
 
   m_element_adapter = create_element_adapter(*this, m_element_registry);
 }
 
+const PageLayout &Document::slide_layout() const { return m_slide_layout; }
+
 const ElementRegistry &Document::element_registry() const {
   return m_element_registry;
 }
@@ -257,21 +275,15 @@ class ElementAdapter final : public abstract::ElementAdapter,
 
   [[nodiscard]] PageLayout slide_page_layout(
       [[maybe_unused]] const ElementIdentifier element_id) const override {
-    // TODO
-    return {
-        .width = Measure("11.02 in"),
-        .height = Measure("8.27 in"),
-        .print_orientation = {},
-        .margin = {},
-    };
+    return m_document->slide_layout();
   }
   [[nodiscard]] ElementIdentifier slide_master_page(
       [[maybe_unused]] const ElementIdentifier element_id) const override {
     return {}; // TODO
   }
-  [[nodiscard]] std::string slide_name(
-      [[maybe_unused]] const ElementIdentifier element_id) const override {
-    return {}; // TODO
+  [[nodiscard]] std::string
+  slide_name(const ElementIdentifier element_id) const override {
+    return get_node(element_id).child("p:cSld").attribute("name").value();
   }
 
   [[nodiscard]] TextStyle
@@ -392,25 +404,10 @@ class ElementAdapter final : public abstract::ElementAdapter,
     const pugi::xml_node node = get_node(element_id);
 
     TableDimensions result;
-    TableCursor cursor;
-
-    for (auto column : node.children("table:table-column")) {
-      const auto columns_repeated =
-          column.attribute("table:number-columns-repeated").as_uint(1);
-      cursor.add_column(columns_repeated);
-    }
-
-    result.columns = cursor.column();
-    cursor = {};
-
-    for (auto row : node.children("table:table-row")) {
-      const auto rows_repeated =
-          row.attribute("table:number-rows-repeated").as_uint(1);
-      cursor.add_row(rows_repeated);
-    }
-
-    result.rows = cursor.row();
-
+    result.columns = static_cast<std::uint32_t>(
+        std::ranges::distance(node.child("a:tblGrid").children("a:gridCol")));
+    result.rows = static_cast<std::uint32_t>(
+        std::ranges::distance(node.children("a:tr")));
     return result;
   }
   [[nodiscard]] ElementIdentifier
@@ -428,21 +425,35 @@ class ElementAdapter final : public abstract::ElementAdapter,
 
   [[nodiscard]] TableColumnStyle
   table_column_style(const ElementIdentifier element_id) const override {
-    return get_partial_style(element_id).table_column_style;
+    TableColumnStyle result;
+    if (const std::optional<Measure> width =
+            read_emus_attribute(get_node(element_id).attribute("w"))) {
+      result.width = width;
+    }
+    return result;
   }
 
   [[nodiscard]] TableRowStyle
   table_row_style(const ElementIdentifier element_id) const override {
-    return get_partial_style(element_id).table_row_style;
+    TableRowStyle result;
+    if (const std::optional<Measure> height =
+            read_emus_attribute(get_node(element_id).attribute("h"))) {
+      result.height = height;
+    }
+    return result;
   }
 
-  [[nodiscard]] bool table_cell_is_covered(
-      [[maybe_unused]] const ElementIdentifier element_id) const override {
-    return false;
+  [[nodiscard]] bool
+  table_cell_is_covered(const ElementIdentifier element_id) const override {
+    const pugi::xml_node node = get_node(element_id);
+    return node.attribute("hMerge").as_bool() ||
+           node.attribute("vMerge").as_bool();
   }
-  [[nodiscard]] TableDimensions table_cell_span(
-      [[maybe_unused]] const ElementIdentifier element_id) const override {
-    return {1, 1}; // TODO
+  [[nodiscard]] TableDimensions
+  table_cell_span(const ElementIdentifier element_id) const override {
+    const pugi::xml_node node = get_node(element_id);
+    return {node.attribute("rowSpan").as_uint(1),
+            node.attribute("gridSpan").as_uint(1)};
   }
   [[nodiscard]] ValueType table_cell_value_type(
       [[maybe_unused]] const ElementIdentifier element_id) const override {
@@ -459,48 +470,32 @@ class ElementAdapter final : public abstract::ElementAdapter,
   }
   [[nodiscard]] std::optional<std::string>
   frame_x(const ElementIdentifier element_id) const override {
-    if (const std::optional<Measure> x =
-            read_emus_attribute(get_node(element_id)
-                                    .child("p:spPr")
-                                    .child("a:xfrm")
-                                    .child("a:off")
-                                    .attribute("x"))) {
+    if (const std::optional<Measure> x = read_emus_attribute(
+            get_frame_xfrm(element_id).child("a:off").attribute("x"))) {
       return x->to_string();
     }
     return {};
   }
   [[nodiscard]] std::optional<std::string>
   frame_y(const ElementIdentifier element_id) const override {
-    if (const std::optional<Measure> y =
-            read_emus_attribute(get_node(element_id)
-                                    .child("p:spPr")
-                                    .child("a:xfrm")
-                                    .child("a:off")
-                                    .attribute("y"))) {
+    if (const std::optional<Measure> y = read_emus_attribute(
+            get_frame_xfrm(element_id).child("a:off").attribute("y"))) {
       return y->to_string();
     }
     return {};
   }
   [[nodiscard]] std::optional<std::string>
   frame_width(const ElementIdentifier element_id) const override {
-    if (const std::optional<Measure> cx =
-            read_emus_attribute(get_node(element_id)
-                                    .child("p:spPr")
-                                    .child("a:xfrm")
-                                    .child("a:ext")
-                                    .attribute("cx"))) {
+    if (const std::optional<Measure> cx = read_emus_attribute(
+            get_frame_xfrm(element_id).child("a:ext").attribute("cx"))) {
       return cx->to_string();
     }
     return {};
   }
   [[nodiscard]] std::optional<std::string>
   frame_height(const ElementIdentifier element_id) const override {
-    if (const std::optional<Measure> cy =
-            read_emus_attribute(get_node(element_id)
-                                    .child("p:spPr")
-                                    .child("a:xfrm")
-                                    .child("a:ext")
-                                    .attribute("cy"))) {
+    if (const std::optional<Measure> cy = read_emus_attribute(
+            get_frame_xfrm(element_id).child("a:ext").attribute("cy"))) {
       return cy->to_string();
     }
     return {};
@@ -542,6 +537,17 @@ class ElementAdapter final : public abstract::ElementAdapter,
     return m_registry->element_at(element_id).node;
   }
 
+  /// `p:sp` carries its transform in `p:spPr/a:xfrm`, `p:graphicFrame` in
+  /// `p:xfrm`.
+  [[nodiscard]] pugi::xml_node
+  get_frame_xfrm(const ElementIdentifier element_id) const {
+    const pugi::xml_node node = get_node(element_id);
+    if (const pugi::xml_node xfrm = node.child("p:spPr").child("a:xfrm")) {
+      return xfrm;
+    }
+    return node.child("p:xfrm");
+  }
+
   [[nodiscard]] static std::string get_text(const pugi::xml_node node) {
     const std::string name = node.name();
 

src/odr/internal/ooxml/presentation/ooxml_presentation_document.hpp

@@ -1,5 +1,7 @@
 #pragma once
 
+#include <odr/style.hpp>
+
 #include <odr/internal/common/document.hpp>
 #include <odr/internal/ooxml/presentation/ooxml_presentation_element_registry.hpp>
 
@@ -14,6 +16,7 @@ class Document final : public internal::Document {
   explicit Document(std::shared_ptr<abstract::ReadableFilesystem> files);
 
   [[nodiscard]] const ElementRegistry &element_registry() const;
+  [[nodiscard]] const PageLayout &slide_layout() const;
 
   [[nodiscard]] bool is_editable() const noexcept override;
   [[nodiscard]] bool is_savable(bool encrypted) const noexcept override;
@@ -24,6 +27,7 @@ class Document final : public internal::Document {
 private:
   pugi::xml_document m_document_xml;
   std::unordered_map<std::string, pugi::xml_document> m_slides_xml;
+  PageLayout m_slide_layout;
 
   ElementRegistry m_element_registry;
 };