From 16962be5cf5b47128da8b87d44a4b79815c88da7 Mon Sep 17 00:00:00 2001 From: Hadrien Mary Date: Sun, 27 Sep 2026 20:06:53 +0200 Subject: [PATCH 1/6] Bind table cell merge, split, fill and margins in rpptx Python The rpptx facade already merges and splits table cells, reports merge origins and spans, and reads and writes cell fills and margins, but the Python Cell exposed only its text. A deck pass that merged a header or shaded a cell had to fall back to python-pptx for that step. Cell now has python-pptx's merge(other_cell), split(), is_merge_origin, is_spanned, span_height and span_width over the native staged table operations. They keep the rectangular grid, so no handle goes stale and the revision does not advance. A cell of another table raises ValueError, and the native refusals of an overlapping merge or of a split on a cell that is not a merge origin raise RpptxError with the table unchanged. Cell.fill is a FillFormat through a new table cell fill target, and margin_left, margin_right, margin_top and margin_bottom read the tcPr margins as Length, or None when absent, with the write rules of the text frame margins. GitHub issue #169. --- crates/rpptx-py/README.md | 1 + crates/rpptx-py/python/rpptx/_rpptx.pyi | 28 +++ crates/rpptx-py/src/dml.rs | 18 +- crates/rpptx-py/src/table.rs | 193 +++++++++++++++++- .../tests/test_documented_examples.py | 105 ++++++++++ crates/rpptx-py/tests/typing_smoke.py | 23 +++ docs/hld/10-bindings-spec.md | 12 ++ 7 files changed, 375 insertions(+), 5 deletions(-) diff --git a/crates/rpptx-py/README.md b/crates/rpptx-py/README.md index 3b240bdc..b9f03298 100644 --- a/crates/rpptx-py/README.md +++ b/crates/rpptx-py/README.md @@ -58,6 +58,7 @@ with open("review.pdf", "wb") as output: - Read speaker-note text and inspect or mutate modern comment threads. - Python collections with negative indexes, slices, iteration, and explicit stale-handle errors after structural changes. +- Table cell merge and split, cell fills, and cell margins. ## Use it when diff --git a/crates/rpptx-py/python/rpptx/_rpptx.pyi b/crates/rpptx-py/python/rpptx/_rpptx.pyi index 333a35b8..b2dea0e6 100644 --- a/crates/rpptx-py/python/rpptx/_rpptx.pyi +++ b/crates/rpptx-py/python/rpptx/_rpptx.pyi @@ -602,3 +602,31 @@ class Cell: def text(self) -> str: ... @text.setter def text(self, value: str) -> None: ... + def merge(self, other_cell: Cell) -> None: ... + def split(self) -> None: ... + @property + def is_merge_origin(self) -> bool: ... + @property + def is_spanned(self) -> bool: ... + @property + def span_height(self) -> int: ... + @property + def span_width(self) -> int: ... + @property + def fill(self) -> FillFormat: ... + @property + def margin_left(self) -> _Length | None: ... + @margin_left.setter + def margin_left(self, value: int | None) -> None: ... + @property + def margin_right(self) -> _Length | None: ... + @margin_right.setter + def margin_right(self, value: int | None) -> None: ... + @property + def margin_top(self) -> _Length | None: ... + @margin_top.setter + def margin_top(self, value: int | None) -> None: ... + @property + def margin_bottom(self) -> _Length | None: ... + @margin_bottom.setter + def margin_bottom(self, value: int | None) -> None: ... diff --git a/crates/rpptx-py/src/dml.rs b/crates/rpptx-py/src/dml.rs index 5fb421ea..b419cb8c 100644 --- a/crates/rpptx-py/src/dml.rs +++ b/crates/rpptx-py/src/dml.rs @@ -1,7 +1,7 @@ //! Fill, line, and colour formats, mirroring python-pptx `pptx.dml`. //! -//! Shape fills, line fills, and slide backgrounds share one fill model, so -//! the three formats read and write through a single target. +//! Shape fills, line fills, slide backgrounds, and table cell fills share one +//! fill model, so the formats read and write through a single target. use oxml_py_support::ContentPath; use pyo3::exceptions::{PyIndexError, PyTypeError, PyValueError}; @@ -9,6 +9,7 @@ use pyo3::prelude::*; use crate::presentation::PyPresentation; use crate::shape::{length, shape_mut_at, shape_ref_at, slide_index}; +use crate::table::{cell_mut_at, cell_ref_at}; use crate::{rpptx_to_pyerr, validate_path}; const MAX_LINE_WIDTH_EMU: i64 = 20_116_800; @@ -26,12 +27,13 @@ pub(crate) enum FillTarget { Shape, Line, Background, + TableCell, } impl FillTarget { fn suffix(self) -> &'static str { match self { - Self::Shape => ".fill", + Self::Shape | Self::TableCell => ".fill", Self::Line => ".line.fill", Self::Background => ".background.fill", } @@ -58,6 +60,10 @@ fn current_fill( .ok_or_else(|| PyIndexError::new_err("slide index out of range"))? .background_fill() .cloned(), + FillTarget::TableCell => cell_ref_at(presentation, path) + .ok_or_else(|| PyIndexError::new_err("cell index out of range"))? + .fill() + .cloned(), }) } @@ -99,6 +105,12 @@ fn write_fill( } slide.set_background(fill) } + FillTarget::TableCell => { + cell_mut_at(presentation, path) + .ok_or_else(|| PyIndexError::new_err("cell index out of range"))? + .set_fill(Some(fill)); + Ok(()) + } }; result.map_err(|error| rpptx_to_pyerr(py, error)) } diff --git a/crates/rpptx-py/src/table.rs b/crates/rpptx-py/src/table.rs index f68b3189..f4bb5b6e 100644 --- a/crates/rpptx-py/src/table.rs +++ b/crates/rpptx-py/src/table.rs @@ -3,10 +3,11 @@ use pyo3::exceptions::{PyIndexError, PyTypeError, PyValueError}; use pyo3::prelude::*; use pyo3::types::{PyAny, PyList, PySlice}; +use crate::dml::{FillTarget, PyFillFormat}; use crate::normalize_index; use crate::presentation::PyPresentation; -use crate::shape::{shape_mut_at, shape_ref_at}; -use crate::validate_path; +use crate::shape::{length, shape_mut_at, shape_ref_at}; +use crate::{rpptx_to_pyerr, validate_path}; pub(crate) fn register(module: &Bound<'_, PyModule>) -> PyResult<()> { module.add_class::()?; @@ -30,6 +31,40 @@ fn cell_index(path: &ContentPath) -> Option { }) } +/// Returns the path segments that name a cell's table shape. +fn table_segments(path: &ContentPath) -> impl Iterator { + path.segs + .iter() + .filter(|segment| !matches!(segment, PathSeg::Row(_) | PathSeg::Cell(_))) +} + +pub(crate) fn cell_ref_at<'a>( + presentation: &'a rpptx::Presentation, + path: &ContentPath, +) -> Option> { + shape_ref_at(presentation, path)? + .table()? + .cell(row_index(path)?, cell_index(path)?) +} + +pub(crate) fn cell_mut_at<'a>( + presentation: &'a mut rpptx::Presentation, + path: &ContentPath, +) -> Option> { + let (row, column) = (row_index(path)?, cell_index(path)?); + shape_mut_at(presentation, path)? + .into_table_mut()? + .into_cell_mut(row, column) +} + +/// Left, right, top, and bottom cell margins, as the facade reports them. +type Margins = ( + Option, + Option, + Option, + Option, +); + #[pyclass(name = "Table")] pub struct PyTable { presentation: Py, @@ -231,6 +266,64 @@ pub struct PyCell { path: ContentPath, } +impl PyCell { + fn read( + &self, + py: Python<'_>, + read: impl FnOnce(rpptx::TableCellRef<'_>) -> T, + ) -> PyResult { + let presentation = self.presentation.borrow(py); + validate_path(py, &presentation, &self.path, "cell", "")?; + cell_ref_at(&presentation.inner, &self.path) + .map(read) + .ok_or_else(|| PyIndexError::new_err("cell index out of range")) + } + + fn edit( + &self, + py: Python<'_>, + edit: impl FnOnce(&mut rpptx::TableCellMut<'_>) -> T, + ) -> PyResult { + let mut presentation = self.presentation.borrow_mut(py); + validate_path(py, &presentation, &self.path, "cell", "")?; + cell_mut_at(&mut presentation.inner, &self.path) + .map(|mut cell| edit(&mut cell)) + .ok_or_else(|| PyIndexError::new_err("cell index out of range")) + } + + fn margin( + &self, + py: Python<'_>, + side: fn(&mut Margins) -> &mut Option, + ) -> PyResult>> { + let mut margins = self.read(py, |cell| cell.margins())?; + length(py, *side(&mut margins)) + } + + /// Writes one margin and keeps the other three, leaving the package + /// unchanged when the value equals the stored one. + fn set_margin( + &self, + py: Python<'_>, + side: fn(&mut Margins) -> &mut Option, + value: Option, + ) -> PyResult<()> { + if value.is_some_and(|value| i32::try_from(value).is_err()) { + return Err(PyValueError::new_err( + "cell margin must fit a 32-bit EMU coordinate", + )); + } + let current = self.read(py, |cell| cell.margins())?; + let mut margins = current; + *side(&mut margins) = value.map(rpptx::Emu); + if margins == current { + return Ok(()); + } + let (left, right, top, bottom) = margins; + self.edit(py, |cell| cell.set_margins(left, right, top, bottom)) + } +} + #[pymethods] impl PyCell { #[getter] @@ -260,4 +353,100 @@ impl PyCell { .map(|mut cell| cell.set_text(value)) .ok_or_else(|| PyIndexError::new_err("cell index out of range")) } + + /// Merges the rectangle between this cell and `other_cell`, moving the + /// text of the spanned cells into the top-left origin, as python-pptx does. + fn merge(&self, py: Python<'_>, other_cell: &Bound<'_, PyAny>) -> PyResult<()> { + let other = other_cell.extract::>()?; + other.read(py, |_| ())?; + if !other.presentation.is(&self.presentation) + || !table_segments(&other.path).eq(table_segments(&self.path)) + { + return Err(PyValueError::new_err("other_cell from different table")); + } + let (Some(row), Some(column)) = (row_index(&other.path), cell_index(&other.path)) else { + return Err(PyIndexError::new_err("cell index is missing")); + }; + self.edit(py, |cell| cell.merge_to(row, column))? + .map_err(|error| rpptx_to_pyerr(py, error)) + } + + /// Splits this merge origin back into its grid cells. + fn split(&self, py: Python<'_>) -> PyResult<()> { + self.edit(py, |cell| cell.split())? + .map_err(|error| rpptx_to_pyerr(py, error)) + } + + #[getter] + fn is_merge_origin(&self, py: Python<'_>) -> PyResult { + self.read(py, |cell| cell.is_merge_origin()) + } + + #[getter] + fn is_spanned(&self, py: Python<'_>) -> PyResult { + self.read(py, |cell| cell.is_spanned()) + } + + #[getter] + fn span_height(&self, py: Python<'_>) -> PyResult { + self.read(py, |cell| cell.span_height()) + } + + #[getter] + fn span_width(&self, py: Python<'_>) -> PyResult { + self.read(py, |cell| cell.span_width()) + } + + #[getter] + fn fill(&self, py: Python<'_>) -> PyResult> { + self.read(py, |_| ())?; + Py::new( + py, + PyFillFormat::new( + self.presentation.clone_ref(py), + self.path.clone(), + FillTarget::TableCell, + ), + ) + } + + #[getter] + fn margin_left(&self, py: Python<'_>) -> PyResult>> { + self.margin(py, |margins| &mut margins.0) + } + + #[setter] + fn set_margin_left(&self, py: Python<'_>, value: Option) -> PyResult<()> { + self.set_margin(py, |margins| &mut margins.0, value) + } + + #[getter] + fn margin_right(&self, py: Python<'_>) -> PyResult>> { + self.margin(py, |margins| &mut margins.1) + } + + #[setter] + fn set_margin_right(&self, py: Python<'_>, value: Option) -> PyResult<()> { + self.set_margin(py, |margins| &mut margins.1, value) + } + + #[getter] + fn margin_top(&self, py: Python<'_>) -> PyResult>> { + self.margin(py, |margins| &mut margins.2) + } + + #[setter] + fn set_margin_top(&self, py: Python<'_>, value: Option) -> PyResult<()> { + self.set_margin(py, |margins| &mut margins.2, value) + } + + #[getter] + fn margin_bottom(&self, py: Python<'_>) -> PyResult>> { + self.margin(py, |margins| &mut margins.3) + } + + #[setter] + fn set_margin_bottom(&self, py: Python<'_>, value: Option) -> PyResult<()> { + self.set_margin(py, |margins| &mut margins.3, value) + } } diff --git a/crates/rpptx-py/tests/test_documented_examples.py b/crates/rpptx-py/tests/test_documented_examples.py index 9baf0f1d..539b36da 100644 --- a/crates/rpptx-py/tests/test_documented_examples.py +++ b/crates/rpptx-py/tests/test_documented_examples.py @@ -1983,6 +1983,111 @@ def test_fill_and_line_formats_write_what_python_pptx_reads(tmp_path): assert oracle[1].fill.type == pptx.enum.dml.MSO_FILL.BACKGROUND +def _cell_records(table, rows, columns): + return [ + (cell.text, cell.is_merge_origin, cell.is_spanned, cell.span_height, cell.span_width) + for cell in (table.cell(row, column) for row in range(rows) for column in range(columns)) + ] + + +def _python_pptx_merged_table(pptx, split): + deck = pptx.Presentation() + slide = deck.slides.add_slide(deck.slide_layouts[6]) + table = slide.shapes.add_table(3, 3, 0, 0, 5_486_400, 2_743_200).table + for row in range(3): + for column in range(3): + table.cell(row, column).text = f"{row}{column}" + table.cell(0, 0).merge(table.cell(1, 1)) + if split: + table.cell(0, 0).split() + return table + + +def test_table_cells_merge_split_fill_and_margins_like_python_pptx(tmp_path): + import rpptx + from rpptx.dml.color import RGBColor + from rpptx.enum.dml import MSO_FILL_TYPE + + prs = rpptx.Presentation() + slide = prs.slides.add_slide(prs.slide_layouts[6]) + slide.shapes.add_table(3, 3, 0, 0, 5_486_400, 2_743_200) + prs.slides[0].shapes.add_table(1, 2, 0, 3_000_000, 100, 100) + table = prs.slides[0].shapes[0].table + for row in range(3): + for column in range(3): + table.cell(row, column).text = f"{row}{column}" + origin, held = table.cell(0, 0), table.cell(2, 2) + origin.merge(table.cell(1, 1)) + assert (origin.is_merge_origin, origin.is_spanned, origin.span_height, origin.span_width) == ( + True, + False, + 2, + 2, + ) + assert (table.cell(1, 0).is_merge_origin, table.cell(1, 0).is_spanned) == (False, True) + assert (held.text, held.is_merge_origin, held.span_width) == ("22", False, 1) + before = prs.to_bytes() + with pytest.raises(rpptx.RpptxError, match="merged cells"): + table.cell(1, 1).merge(table.cell(2, 2)) + with pytest.raises(ValueError, match="other_cell from different table"): + held.merge(prs.slides[0].shapes[1].table.cell(0, 0)) + with pytest.raises(rpptx.RpptxError, match="merge-origin"): + held.split() + assert prs.to_bytes() == before + merged = tmp_path / "merged.pptx" + prs.save(merged) + origin.split() + assert not any(record[1] or record[2] for record in _cell_records(table, 3, 3)) + + cell, other = table.cell(2, 0), table.cell(2, 1) + assert cell.fill.type is None + cell.fill.solid() + cell.fill.fore_color.rgb = RGBColor(0x12, 0x34, 0x56) + other.fill.background() + assert (cell.fill.type, other.fill.type) == (MSO_FILL_TYPE.SOLID, MSO_FILL_TYPE.BACKGROUND) + assert (cell.margin_left, cell.margin_right, cell.margin_top, cell.margin_bottom) == (None,) * 4 + cell.margin_left = rpptx.Inches(0.25) + cell.margin_top = rpptx.Pt(3) + assert (cell.margin_left, cell.margin_top) == (rpptx.Inches(0.25), rpptx.Pt(3)) + assert isinstance(cell.margin_left, rpptx.Length) + before = prs.to_bytes() + cell.margin_right = None + with pytest.raises(ValueError, match="32-bit"): + cell.margin_bottom = 2**31 + assert prs.to_bytes() == before + cell.margin_top = None + assert cell.margin_top is None + held_fill = cell.fill + prs.slides.add_slide(prs.slide_layouts[6]) + with pytest.raises(rpptx.StaleElementError, match=r"table\.cell\(2, 0\)\.fill"): + _ = held_fill.type + split = tmp_path / "split.pptx" + prs.save(split) + + pptx = pytest.importorskip("pptx", reason="python-pptx is the differential oracle") + for path, was_split in ((merged, False), (split, True)): + expected = _cell_records(_python_pptx_merged_table(pptx, was_split), 3, 3) + assert _cell_records(pptx.Presentation(path).slides[0].shapes[0].table, 3, 3) == expected + assert _cell_records(rpptx.Presentation(path).slides[0].shapes[0].table, 3, 3) == expected + oracle = pptx.Presentation(split).slides[0].shapes[0].table + assert oracle.cell(2, 0).fill.fore_color.rgb == pptx.dml.color.RGBColor(0x12, 0x34, 0x56) + assert oracle.cell(2, 1).fill.type == pptx.enum.dml.MSO_FILL.BACKGROUND + assert oracle.cell(2, 0).margin_left == rpptx.Inches(0.25) + assert (oracle.cell(2, 0).margin_right, oracle.cell(2, 0).margin_top) == (91_440, 45_720) + + def build(deck): + written = deck.slides.add_slide(deck.slide_layouts[6]).shapes.add_table(1, 1, 0, 0, 100, 100) + written = written.table.cell(0, 0) + written.margin_bottom = rpptx.Pt(4) + written.fill.solid() + written.fill.fore_color.rgb = pptx.dml.color.RGBColor(0xAB, 0xCD, 0xEF) + + source = _python_pptx_deck(tmp_path / "python-pptx-cells.pptx", build) + read = rpptx.Presentation(source).slides[0].shapes[0].table.cell(0, 0) + assert (read.margin_left, read.margin_bottom) == (None, rpptx.Pt(4)) + assert read.fill.fore_color.rgb == RGBColor(0xAB, 0xCD, 0xEF) + + def test_colour_edits_keep_python_pptx_brightness_transforms(tmp_path): import rpptx from rpptx.dml.color import RGBColor diff --git a/crates/rpptx-py/tests/typing_smoke.py b/crates/rpptx-py/tests/typing_smoke.py index 0de75a65..f693219a 100644 --- a/crates/rpptx-py/tests/typing_smoke.py +++ b/crates/rpptx-py/tests/typing_smoke.py @@ -289,6 +289,29 @@ def exercise_rpptx_types(path: Path) -> None: ) +def exercise_rpptx_table_types(table: Table) -> None: + origin: Cell = table.cell(0, 0) + origin.merge(table.cell(1, 1)) + spans: tuple[bool, bool, int, int] = ( + origin.is_merge_origin, + origin.is_spanned, + origin.span_height, + origin.span_width, + ) + origin.split() + cell_fill: FillFormat = origin.fill + cell_fill.solid() + origin.margin_left = Inches(0.1) + origin.margin_right = None + cell_margins: tuple[Length | None, ...] = ( + origin.margin_left, + origin.margin_right, + origin.margin_top, + origin.margin_bottom, + ) + (spans, cell_margins) + + def exercise_rpptx_text_layout_types(presentation: Presentation) -> None: frames: tuple[TextFrameLayout, ...] = presentation.text_layout() narrower: tuple[TextFrameLayout, ...] = presentation.text_layout(width_factor=0.95) diff --git a/docs/hld/10-bindings-spec.md b/docs/hld/10-bindings-spec.md index a6a75eb7..765fffe5 100644 --- a/docs/hld/10-bindings-spec.md +++ b/docs/hld/10-bindings-spec.md @@ -327,6 +327,18 @@ file-like object, which is rewound first when it can seek. `remove` deletes one shape of a slide with the relationships and parts only it used and advances the revision once. Nested collections stay read-only. +A table `Cell` follows python-pptx for `merge(other_cell)`, `split()`, +`is_merge_origin`, `is_spanned`, `span_height`, and `span_width`. Merge and +split run the native staged table operations, which keep the rectangular grid, +so they do not advance the revision. A cell of another table raises +`ValueError`, and a merge range that overlaps a merge or a split of a cell that +is not a merge origin raises `RpptxError` and leaves the table unchanged. +`Cell.fill` is a live `FillFormat` over the direct cell fill. `margin_left`, +`margin_right`, `margin_top`, and `margin_bottom` read the `a:tcPr` margins as +`Length` and follow the text formatting rules below. An absent margin reads +`None` where python-pptx reports its 91440 and 45720 EMU defaults, and a value +outside the 32-bit coordinate range raises `ValueError`. + `TextFrame.autofit` reports `none`, `normal`, or `shape` when the body carries an explicit choice. `Run.font` reads the run's direct Latin name, size, and sRGB colour, while the From a58b43830a14fab78419a57ca60b9109fe774336 Mon Sep 17 00:00:00 2001 From: Hadrien Mary Date: Sun, 27 Sep 2026 20:13:28 +0200 Subject: [PATCH 2/6] Add table row heights and cell borders to rpptx and its binding A table row's stored height and a cell's lnL, lnR, lnT and lnB border lines were parsed and written by oxml-drawing, but the rpptx facade had no accessor for either, and Python had no rows collection at all. A deck pass could not size a row or draw a cell edge without python-pptx and raw XML edits. TableRef and TableMut now read row_height, and TableMut::set_row_height stages the change like set_column_width and keeps the graphic frame height equal to the sum of the rows. Both setters share one checked extent helper. The new CellBorder enum names an edge, and TableCellRef and TableCellMut read border(edge) while set_border replaces or clears it. The writer already emits the four lines in schema order before the cell fill. In Python, Table.rows is a RowCollection of Row handles whose height reads and writes through the facade. python-pptx has no border API, so Cell.border_left, border_right, border_top and border_bottom are LineFormat views over a cell border target, reusing the width, colour and fill logic of shape lines. Reading a border never creates one, and none of these writes advances the revision. GitHub issue #169. --- crates/rpptx-py/README.md | 3 +- crates/rpptx-py/python/rpptx/_rpptx.pyi | 33 ++- crates/rpptx-py/src/dml.rs | 118 +++++++---- crates/rpptx-py/src/lib.rs | 3 + crates/rpptx-py/src/shape.rs | 6 +- crates/rpptx-py/src/table.rs | 192 +++++++++++++++++- .../tests/test_documented_examples.py | 76 +++++++ crates/rpptx-py/tests/typing_smoke.py | 17 +- crates/rpptx/src/lib.rs | 161 +++++++++++---- crates/rpptx/tests/integration.rs | 167 ++++++++++++++- docs/hld/06-presentationml-model.md | 27 ++- docs/hld/10-bindings-spec.md | 9 +- 12 files changed, 721 insertions(+), 91 deletions(-) diff --git a/crates/rpptx-py/README.md b/crates/rpptx-py/README.md index b9f03298..dc139691 100644 --- a/crates/rpptx-py/README.md +++ b/crates/rpptx-py/README.md @@ -58,7 +58,8 @@ with open("review.pdf", "wb") as output: - Read speaker-note text and inspect or mutate modern comment threads. - Python collections with negative indexes, slices, iteration, and explicit stale-handle errors after structural changes. -- Table cell merge and split, cell fills, and cell margins. +- Table cell merge and split, cell fills, margins, and borders, and row + heights. ## Use it when diff --git a/crates/rpptx-py/python/rpptx/_rpptx.pyi b/crates/rpptx-py/python/rpptx/_rpptx.pyi index b2dea0e6..00775e65 100644 --- a/crates/rpptx-py/python/rpptx/_rpptx.pyi +++ b/crates/rpptx-py/python/rpptx/_rpptx.pyi @@ -26,7 +26,8 @@ __all__ = [ "SlideCollection", "Shape", "ShapeCollection", "PlaceholderCollection", "Image", "AdjustmentCollection", "FillFormat", "LineFormat", "ColorFormat", "TextFrame", "Paragraph", "ParagraphCollection", "Run", "RunCollection", - "Font", "Table", "Column", "ColumnCollection", "Cell", + "Font", "Table", "Column", "ColumnCollection", "Row", "RowCollection", + "Cell", ] @@ -572,6 +573,8 @@ class Table: def __new__(cls, *, _private: _Never) -> Table: ... @property def columns(self) -> ColumnCollection: ... + @property + def rows(self) -> RowCollection: ... def cell(self, row: int, col: int) -> Cell: ... @@ -595,6 +598,26 @@ class Column: def width(self, value: int) -> None: ... +@_final +class RowCollection: + def __new__(cls, *, _private: _Never) -> RowCollection: ... + def __len__(self) -> int: ... + @_overload + def __getitem__(self, key: int, /) -> Row: ... + @_overload + def __getitem__(self, key: slice, /) -> list[Row]: ... + def __iter__(self) -> _Iterator[Row]: ... + + +@_final +class Row: + def __new__(cls, *, _private: _Never) -> Row: ... + @property + def height(self) -> _Length: ... + @height.setter + def height(self, value: int) -> None: ... + + @_final class Cell: def __new__(cls, *, _private: _Never) -> Cell: ... @@ -630,3 +653,11 @@ class Cell: def margin_bottom(self) -> _Length | None: ... @margin_bottom.setter def margin_bottom(self, value: int | None) -> None: ... + @property + def border_left(self) -> LineFormat: ... + @property + def border_right(self) -> LineFormat: ... + @property + def border_top(self) -> LineFormat: ... + @property + def border_bottom(self) -> LineFormat: ... diff --git a/crates/rpptx-py/src/dml.rs b/crates/rpptx-py/src/dml.rs index b419cb8c..ce9b78ee 100644 --- a/crates/rpptx-py/src/dml.rs +++ b/crates/rpptx-py/src/dml.rs @@ -1,7 +1,8 @@ //! Fill, line, and colour formats, mirroring python-pptx `pptx.dml`. //! -//! Shape fills, line fills, slide backgrounds, and table cell fills share one -//! fill model, so the formats read and write through a single target. +//! Shape fills, line fills, slide backgrounds, table cell fills, and cell +//! border fills share one fill model, so the formats read and write through a +//! single target. use oxml_py_support::ContentPath; use pyo3::exceptions::{PyIndexError, PyTypeError, PyValueError}; @@ -22,12 +23,15 @@ pub(crate) fn register(module: &Bound<'_, PyModule>) -> PyResult<()> { } /// The DrawingML fill one format object reads and writes. +/// +/// `Line` and `CellBorder` also name the line a `LineFormat` reads and writes. #[derive(Clone, Copy, Eq, PartialEq)] pub(crate) enum FillTarget { Shape, Line, Background, TableCell, + CellBorder(rpptx::CellBorder), } impl FillTarget { @@ -36,8 +40,58 @@ impl FillTarget { Self::Shape | Self::TableCell => ".fill", Self::Line => ".line.fill", Self::Background => ".background.fill", + Self::CellBorder(rpptx::CellBorder::Left) => ".border_left.fill", + Self::CellBorder(rpptx::CellBorder::Right) => ".border_right.fill", + Self::CellBorder(rpptx::CellBorder::Top) => ".border_top.fill", + Self::CellBorder(rpptx::CellBorder::Bottom) => ".border_bottom.fill", } } + + fn line_suffix(self) -> &'static str { + self.suffix() + .strip_suffix(".fill") + .expect("every fill suffix ends in .fill") + } +} + +/// Reads the shape line or cell border a line target names. +fn current_line( + presentation: &rpptx::Presentation, + path: &ContentPath, + target: FillTarget, +) -> PyResult> { + Ok(match target { + FillTarget::CellBorder(edge) => cell_ref_at(presentation, path) + .ok_or_else(|| PyIndexError::new_err("cell index out of range"))? + .border(edge) + .cloned(), + _ => shape_ref_at(presentation, path) + .ok_or_else(|| PyIndexError::new_err("shape index out of range"))? + .line() + .cloned(), + }) +} + +/// Replaces the shape line or cell border a line target names. +fn write_line( + py: Python<'_>, + presentation: &mut rpptx::Presentation, + path: &ContentPath, + target: FillTarget, + line: rpptx::CT_LineProperties, +) -> PyResult<()> { + match target { + FillTarget::CellBorder(edge) => { + cell_mut_at(presentation, path) + .ok_or_else(|| PyIndexError::new_err("cell index out of range"))? + .set_border(edge, Some(line)); + Ok(()) + } + _ => shape_mut_at(presentation, path) + .ok_or_else(|| PyIndexError::new_err("shape index out of range"))? + .set_line(line) + .map_err(|error| rpptx_to_pyerr(py, error)), + } } fn current_fill( @@ -51,10 +105,9 @@ fn current_fill( .ok_or_else(missing)? .fill() .cloned(), - FillTarget::Line => shape_ref_at(presentation, path) - .ok_or_else(missing)? - .line() - .and_then(|line| line.fill.clone()), + FillTarget::Line | FillTarget::CellBorder(_) => { + current_line(presentation, path, target)?.and_then(|line| line.fill) + } FillTarget::Background => presentation .slide(slide_index(path)?) .ok_or_else(|| PyIndexError::new_err("slide index out of range"))? @@ -79,16 +132,10 @@ fn write_fill( FillTarget::Shape => shape_mut_at(presentation, path) .ok_or_else(missing)? .set_fill(fill), - FillTarget::Line => { - let mut line = shape_ref_at(presentation, path) - .ok_or_else(missing)? - .line() - .cloned() - .unwrap_or_default(); + FillTarget::Line | FillTarget::CellBorder(_) => { + let mut line = current_line(presentation, path, target)?.unwrap_or_default(); line.fill = Some(fill); - shape_mut_at(presentation, path) - .ok_or_else(missing)? - .set_line(line) + return write_line(py, presentation, path, target, line); } FillTarget::Background => { let index = slide_index(path)?; @@ -294,16 +341,28 @@ impl PyColorFormat { } } -/// A live view of the outline of one shape, picture, or connector. +/// A live view of the outline of one shape, picture, or connector, or of one +/// table cell border. #[pyclass(name = "LineFormat")] pub struct PyLineFormat { presentation: Py, path: ContentPath, + target: FillTarget, } impl PyLineFormat { - pub(crate) fn new(presentation: Py, path: ContentPath) -> Self { - Self { presentation, path } + /// Creates a view of a shape line, with `FillTarget::Line`, or of a cell + /// border, with `FillTarget::CellBorder`. + pub(crate) fn new( + presentation: Py, + path: ContentPath, + target: FillTarget, + ) -> Self { + Self { + presentation, + path, + target, + } } fn validate(&self, py: Python<'_>) -> PyResult<()> { @@ -312,7 +371,7 @@ impl PyLineFormat { &self.presentation.borrow(py), &self.path, "line", - ".line", + self.target.line_suffix(), ) } } @@ -328,7 +387,7 @@ impl PyLineFormat { PyColorFormat { presentation: self.presentation.clone_ref(py), path: self.path.clone(), - target: FillTarget::Line, + target: self.target, solidify: true, }, ) @@ -342,7 +401,7 @@ impl PyLineFormat { PyFillFormat::new( self.presentation.clone_ref(py), self.path.clone(), - FillTarget::Line, + self.target, ), ) } @@ -350,9 +409,7 @@ impl PyLineFormat { #[getter] fn width(&self, py: Python<'_>) -> PyResult>> { self.validate(py)?; - let width = shape_ref_at(&self.presentation.borrow(py).inner, &self.path) - .ok_or_else(|| PyIndexError::new_err("shape index out of range"))? - .line() + let width = current_line(&self.presentation.borrow(py).inner, &self.path, self.target)? .and_then(|line| line.width) .unwrap_or(0); length(py, Some(rpptx::Emu(i64::from(width)))) @@ -368,16 +425,9 @@ impl PyLineFormat { ))); } let mut presentation = self.presentation.borrow_mut(py); - let missing = || PyIndexError::new_err("shape index out of range"); - let mut line = shape_ref_at(&presentation.inner, &self.path) - .ok_or_else(missing)? - .line() - .cloned() - .unwrap_or_default(); + let mut line = + current_line(&presentation.inner, &self.path, self.target)?.unwrap_or_default(); line.width = Some(width as u32); - shape_mut_at(&mut presentation.inner, &self.path) - .ok_or_else(missing)? - .set_line(line) - .map_err(|error| rpptx_to_pyerr(py, error)) + write_line(py, &mut presentation.inner, &self.path, self.target, line) } } diff --git a/crates/rpptx-py/src/lib.rs b/crates/rpptx-py/src/lib.rs index 8e2e4ab5..84375d0f 100644 --- a/crates/rpptx-py/src/lib.rs +++ b/crates/rpptx-py/src/lib.rs @@ -64,6 +64,9 @@ pub(crate) fn recovery_hint(path: &ContentPath, suffix: &str) -> String { PathSeg::Run(index) => public_path.push_str(&format!(".runs[{index}]")), } } + if let Some(row) = pending_row { + public_path.push_str(&format!(".table.rows[{row}]")); + } public_path.push_str(suffix); format!("Re-fetch it with {public_path}.") } diff --git a/crates/rpptx-py/src/shape.rs b/crates/rpptx-py/src/shape.rs index 7e1b0563..9d75a634 100644 --- a/crates/rpptx-py/src/shape.rs +++ b/crates/rpptx-py/src/shape.rs @@ -361,7 +361,11 @@ impl PyShape { self.require_shape_properties(py, "line")?; Py::new( py, - PyLineFormat::new(self.presentation.clone_ref(py), self.path.clone()), + PyLineFormat::new( + self.presentation.clone_ref(py), + self.path.clone(), + FillTarget::Line, + ), ) } diff --git a/crates/rpptx-py/src/table.rs b/crates/rpptx-py/src/table.rs index f4bb5b6e..a376e632 100644 --- a/crates/rpptx-py/src/table.rs +++ b/crates/rpptx-py/src/table.rs @@ -3,7 +3,7 @@ use pyo3::exceptions::{PyIndexError, PyTypeError, PyValueError}; use pyo3::prelude::*; use pyo3::types::{PyAny, PyList, PySlice}; -use crate::dml::{FillTarget, PyFillFormat}; +use crate::dml::{FillTarget, PyFillFormat, PyLineFormat}; use crate::normalize_index; use crate::presentation::PyPresentation; use crate::shape::{length, shape_mut_at, shape_ref_at}; @@ -13,6 +13,8 @@ pub(crate) fn register(module: &Bound<'_, PyModule>) -> PyResult<()> { module.add_class::()?; module.add_class::()?; module.add_class::()?; + module.add_class::()?; + module.add_class::()?; module.add_class::()?; Ok(()) } @@ -105,6 +107,18 @@ impl PyTable { ) } + #[getter] + fn rows(&self, py: Python<'_>) -> PyResult> { + self.dimensions(py)?; + Py::new( + py, + PyRowCollection { + presentation: self.presentation.clone_ref(py), + path: self.path.clone(), + }, + ) + } + fn cell(&self, py: Python<'_>, row: isize, col: isize) -> PyResult> { let (rows, columns) = self.dimensions(py)?; let row = normalize_index(row, rows, "row")?; @@ -260,6 +274,146 @@ impl PyColumn { } } +#[pyclass(name = "RowCollection")] +pub struct PyRowCollection { + presentation: Py, + path: ContentPath, +} + +impl PyRowCollection { + fn len(&self, py: Python<'_>) -> PyResult { + validate_path( + py, + &self.presentation.borrow(py), + &self.path, + "row collection", + ".table.rows", + )?; + shape_ref_at(&self.presentation.borrow(py).inner, &self.path) + .and_then(|shape| shape.table()) + .map(|table| table.row_count()) + .ok_or_else(|| PyValueError::new_err("shape has no table")) + } + + fn item(&self, py: Python<'_>, index: usize) -> PyResult> { + let mut segments = self.path.segs.clone(); + segments.push(PathSeg::Row(index)); + let path = self.presentation.borrow(py).revisions.capture(segments); + Py::new( + py, + PyRow { + presentation: self.presentation.clone_ref(py), + path, + }, + ) + } +} + +#[pymethods] +impl PyRowCollection { + fn __len__(&self, py: Python<'_>) -> PyResult { + self.len(py) + } + + fn __getitem__(&self, py: Python<'_>, key: &Bound<'_, PyAny>) -> PyResult> { + let len = self.len(py)?; + if let Ok(index) = key.extract::() { + return Ok(self + .item(py, normalize_index(index, len, "row")?)? + .into_any()); + } + if key.is_instance_of::() { + let (start, stop, step): (isize, isize, isize) = + key.call_method1("indices", (len,))?.extract()?; + let items = PyList::empty(py); + let mut index = start; + while if step > 0 { index < stop } else { index > stop } { + items.append(self.item(py, index as usize)?)?; + index += step; + } + return Ok(items.into_any().unbind()); + } + Err(PyTypeError::new_err( + "row indices must be integers or slices", + )) + } + + fn __iter__(&self, py: Python<'_>) -> PyResult> { + self.len(py)?; + Py::new( + py, + PyRowIterator { + presentation: self.presentation.clone_ref(py), + path: self.path.clone(), + index: 0, + }, + ) + } +} + +#[pyclass] +struct PyRowIterator { + presentation: Py, + path: ContentPath, + index: usize, +} + +#[pymethods] +impl PyRowIterator { + fn __iter__(slf: Py) -> Py { + slf + } + + fn __next__(&mut self, py: Python<'_>) -> PyResult>> { + let collection = PyRowCollection { + presentation: self.presentation.clone_ref(py), + path: self.path.clone(), + }; + if self.index >= collection.len(py)? { + return Ok(None); + } + let index = self.index; + self.index += 1; + collection.item(py, index).map(Some) + } +} + +#[pyclass(name = "Row")] +pub struct PyRow { + presentation: Py, + path: ContentPath, +} + +#[pymethods] +impl PyRow { + /// The stored row height, a minimum that PowerPoint grows to fit text. + #[getter] + fn height(&self, py: Python<'_>) -> PyResult>> { + validate_path(py, &self.presentation.borrow(py), &self.path, "row", "")?; + let row = + row_index(&self.path).ok_or_else(|| PyIndexError::new_err("row index is missing"))?; + let height = shape_ref_at(&self.presentation.borrow(py).inner, &self.path) + .and_then(|shape| shape.table()) + .and_then(|table| table.row_height(row)) + .ok_or_else(|| PyIndexError::new_err("row index out of range"))?; + length(py, Some(height)) + } + + /// Sets the row height and keeps the table frame height equal to the sum + /// of its rows. + #[setter] + fn set_height(&self, py: Python<'_>, height: i64) -> PyResult<()> { + validate_path(py, &self.presentation.borrow(py), &self.path, "row", "")?; + let row = + row_index(&self.path).ok_or_else(|| PyIndexError::new_err("row index is missing"))?; + shape_mut_at(&mut self.presentation.borrow_mut(py).inner, &self.path) + .and_then(rpptx::ShapeMut::into_table_mut) + .ok_or_else(|| PyValueError::new_err("shape has no table"))? + .set_row_height(row, rpptx::Emu(height)) + .map_err(|error| rpptx_to_pyerr(py, error)) + } +} + #[pyclass(name = "Cell")] pub struct PyCell { presentation: Py, @@ -291,6 +445,18 @@ impl PyCell { .ok_or_else(|| PyIndexError::new_err("cell index out of range")) } + fn border(&self, py: Python<'_>, edge: rpptx::CellBorder) -> PyResult> { + self.read(py, |_| ())?; + Py::new( + py, + PyLineFormat::new( + self.presentation.clone_ref(py), + self.path.clone(), + FillTarget::CellBorder(edge), + ), + ) + } + fn margin( &self, py: Python<'_>, @@ -449,4 +615,28 @@ impl PyCell { fn set_margin_bottom(&self, py: Python<'_>, value: Option) -> PyResult<()> { self.set_margin(py, |margins| &mut margins.3, value) } + + /// The `a:lnL` border as a live `LineFormat`. + #[getter] + fn border_left(&self, py: Python<'_>) -> PyResult> { + self.border(py, rpptx::CellBorder::Left) + } + + /// The `a:lnR` border as a live `LineFormat`. + #[getter] + fn border_right(&self, py: Python<'_>) -> PyResult> { + self.border(py, rpptx::CellBorder::Right) + } + + /// The `a:lnT` border as a live `LineFormat`. + #[getter] + fn border_top(&self, py: Python<'_>) -> PyResult> { + self.border(py, rpptx::CellBorder::Top) + } + + /// The `a:lnB` border as a live `LineFormat`. + #[getter] + fn border_bottom(&self, py: Python<'_>) -> PyResult> { + self.border(py, rpptx::CellBorder::Bottom) + } } diff --git a/crates/rpptx-py/tests/test_documented_examples.py b/crates/rpptx-py/tests/test_documented_examples.py index 539b36da..79527b20 100644 --- a/crates/rpptx-py/tests/test_documented_examples.py +++ b/crates/rpptx-py/tests/test_documented_examples.py @@ -2088,6 +2088,82 @@ def build(deck): assert read.fill.fore_color.rgb == RGBColor(0xAB, 0xCD, 0xEF) +def test_table_row_heights_and_cell_borders_write_what_python_pptx_reads(tmp_path): + import rpptx + from rpptx.dml.color import RGBColor + from rpptx.enum.dml import MSO_FILL_TYPE + + prs = rpptx.Presentation() + slide = prs.slides.add_slide(prs.slide_layouts[6]) + slide.shapes.add_table(3, 2, 0, 0, rpptx.Inches(4), rpptx.Inches(3)) + shape = prs.slides[0].shapes[0] + rows = shape.table.rows + assert len(rows) == 3 + assert [row.height for row in rows] == [rpptx.Inches(1)] * 3 + assert isinstance(rows[0].height, rpptx.Length) + held = rows[-1] + rows[1].height = rpptx.Inches(1.5) + assert (held.height, shape.height) == (rpptx.Inches(1), rpptx.Inches(3.5)) + assert [row.height for row in rows[1:]] == [rpptx.Inches(1.5), rpptx.Inches(1)] + before = prs.to_bytes() + with pytest.raises(rpptx.RpptxError, match="row height must be positive"): + rows[0].height = 0 + with pytest.raises(IndexError): + _ = rows[3] + + cell = shape.table.cell(0, 0) + left = cell.border_left + assert (left.width, left.color.rgb, left.fill.type) == (0, None, None) + assert prs.to_bytes() == before + left.width = rpptx.Pt(2) + left.color.rgb = RGBColor(0xFF, 0x00, 0x00) + cell.border_bottom.fill.solid() + cell.border_bottom.fill.fore_color.rgb = RGBColor(0x00, 0x80, 0x00) + cell.fill.solid() + cell.fill.fore_color.rgb = RGBColor(0x11, 0x22, 0x33) + assert (cell.border_left.width, cell.border_left.color.rgb) == ( + rpptx.Pt(2), + RGBColor(0xFF, 0x00, 0x00), + ) + assert (cell.border_top.fill.type, cell.border_bottom.fill.type) == (None, MSO_FILL_TYPE.SOLID) + held_border = cell.border_right + prs.slides.add_slide(prs.slide_layouts[6]) + with pytest.raises(rpptx.StaleElementError, match=r"table\.cell\(0, 0\)\.border_right\."): + _ = held_border.width + with pytest.raises(rpptx.StaleElementError, match=r"table\.rows\[2\]\."): + _ = held.height + output = tmp_path / "rows-borders.pptx" + prs.save(output) + xml = _package_parts(output.read_bytes())["ppt/slides/slide1.xml"].decode() + properties = xml[xml.index("') + ) + + pptx = pytest.importorskip("pptx", reason="python-pptx is the differential oracle") + from pptx.oxml.ns import qn + + oracle = pptx.Presentation(output).slides[0].shapes[0] + assert [row.height for row in oracle.table.rows] == [ + rpptx.Inches(1), + rpptx.Inches(1.5), + rpptx.Inches(1), + ] + assert oracle.height == rpptx.Inches(3.5) + border = oracle.table.cell(0, 0)._tc.tcPr.find(qn("a:lnL")) + assert border.get("w") == str(rpptx.Pt(2)) + assert border.find(qn("a:solidFill"))[0].get("val") == "FF0000" + + def build(deck): + table = deck.slides.add_slide(deck.slide_layouts[6]).shapes.add_table(2, 1, 0, 0, 100, 100) + table.table.rows[0].height = rpptx.Pt(30) + + source = _python_pptx_deck(tmp_path / "python-pptx-rows.pptx", build) + assert rpptx.Presentation(source).slides[0].shapes[0].table.rows[0].height == rpptx.Pt(30) + + def test_colour_edits_keep_python_pptx_brightness_transforms(tmp_path): import rpptx from rpptx.dml.color import RGBColor diff --git a/crates/rpptx-py/tests/typing_smoke.py b/crates/rpptx-py/tests/typing_smoke.py index f693219a..23e4dc94 100644 --- a/crates/rpptx-py/tests/typing_smoke.py +++ b/crates/rpptx-py/tests/typing_smoke.py @@ -34,6 +34,8 @@ Paragraph, ParagraphCollection, PlaceholderCollection, + Row, + RowCollection, Run, RunCollection, Shape, @@ -309,7 +311,18 @@ def exercise_rpptx_table_types(table: Table) -> None: origin.margin_top, origin.margin_bottom, ) - (spans, cell_margins) + rows: RowCollection = table.rows + row: Row = rows[0] + row.height = Inches(1) + row_heights: list[Length] = [current.height for current in rows] + borders: tuple[LineFormat, ...] = ( + origin.border_left, + origin.border_right, + origin.border_top, + origin.border_bottom, + ) + borders[0].width = Pt(1) + (spans, cell_margins, row_heights, rows[:]) def exercise_rpptx_text_layout_types(presentation: Presentation) -> None: @@ -370,6 +383,8 @@ def exercise_rpptx_text_layout_types(presentation: Presentation) -> None: Paragraph() # type: ignore[call-arg] ParagraphCollection() # type: ignore[call-arg] PlaceholderCollection() # type: ignore[call-arg] + Row() # type: ignore[call-arg] + RowCollection() # type: ignore[call-arg] Run() # type: ignore[call-arg] RunCollection() # type: ignore[call-arg] Shape() # type: ignore[call-arg] diff --git a/crates/rpptx/src/lib.rs b/crates/rpptx/src/lib.rs index 362154d5..58836716 100644 --- a/crates/rpptx/src/lib.rs +++ b/crates/rpptx/src/lib.rs @@ -6229,6 +6229,11 @@ impl<'a> TableRef<'a> { self.table.grid.columns.get(column).copied() } + /// Returns one stored row height in EMU. + pub fn row_height(&self, row: usize) -> Option { + self.table.rows.get(row).map(|row| row.height) + } + /// Returns whether first-row table styling is enabled. pub fn first_row(&self) -> bool { self.table @@ -6327,68 +6332,78 @@ impl<'a> TableMut<'a> { /// Changes one grid width and synchronizes the containing frame width. pub fn set_column_width(&mut self, column: usize, width: Emu) -> Result<()> { + const OPERATION: &str = "set column width"; if width.0 <= 0 { return Err(invalid_table_mutation( - "set column width", + OPERATION, "column width must be positive".to_owned(), )); } if column >= self.table.grid.columns.len() { return Err(invalid_table_mutation( - "set column width", + OPERATION, format!("column index {column} is out of range"), )); } - let total_width = self - .table - .grid - .columns - .iter() - .enumerate() - .try_fold(0i64, |total, (index, current)| { - total.checked_add(if index == column { width.0 } else { current.0 }) - }) - .ok_or_else(|| { - invalid_table_mutation( - "set column width", - "table width exceeds the EMU range".to_owned(), - ) - })?; - let total_height = self - .table - .rows - .iter() - .try_fold(0i64, |total, row| total.checked_add(row.height.0)) - .ok_or_else(|| { - invalid_table_mutation( - "set column width", - "table height exceeds the EMU range".to_owned(), - ) - })?; - if total_width <= 0 || total_height <= 0 { - return Err(invalid_table_mutation( - "set column width", - "table width and height must remain positive".to_owned(), - )); - } - let mut staged = self.table.clone(); staged.grid.columns[column] = width; + let (total_width, total_height) = table_extent(&staged, OPERATION)?; staged .to_xml() - .map_err(|error| invalid_table_mutation("set column width", error.to_string()))?; + .map_err(|error| invalid_table_mutation(OPERATION, error.to_string()))?; self.table.grid.columns[column] = width; let height = self .transform .extent - .map_or(Emu(total_height), |extent| extent.cy); + .map_or(total_height, |extent| extent.cy); self.transform.extent = Some(CT_PositiveSize2D { - cx: Emu(total_width), + cx: total_width, cy: height, }); Ok(()) } + /// Returns one stored row height in EMU. + pub fn row_height(&self, row: usize) -> Option { + self.table.rows.get(row).map(|row| row.height) + } + + /// Changes one row height and synchronizes the containing frame height. + /// + /// The stored height is a minimum, as in PowerPoint, which grows a row to + /// fit its text when it lays the table out. + pub fn set_row_height(&mut self, row: usize, height: Emu) -> Result<()> { + const OPERATION: &str = "set row height"; + if height.0 <= 0 { + return Err(invalid_table_mutation( + OPERATION, + "row height must be positive".to_owned(), + )); + } + if row >= self.table.rows.len() { + return Err(invalid_table_mutation( + OPERATION, + format!("row index {row} is out of range"), + )); + } + let mut staged = self.table.clone(); + staged.rows[row].height = height; + let (total_width, total_height) = table_extent(&staged, OPERATION)?; + staged + .to_xml() + .map_err(|error| invalid_table_mutation(OPERATION, error.to_string()))?; + self.table.rows[row].height = height; + let width = self + .transform + .extent + .map_or(total_width, |extent| extent.cx); + self.transform.extent = Some(CT_PositiveSize2D { + cx: width, + cy: total_height, + }); + Ok(()) + } + /// Returns whether first-row table styling is enabled. pub fn first_row(&self) -> bool { self.table @@ -6468,6 +6483,16 @@ impl<'a> TableMut<'a> { } } +/// One edge of a table cell, drawn by the `a:lnL`, `a:lnR`, `a:lnT`, or `a:lnB` +/// line of its `a:tcPr`. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum CellBorder { + Left, + Right, + Top, + Bottom, +} + /// A borrowed explicit table-grid cell. #[derive(Clone, Copy)] pub struct TableCellRef<'a> { @@ -6533,6 +6558,17 @@ impl TableCellRef<'_> { ) }) } + + /// Returns the direct line of one cell edge, when present. + pub fn border(&self, edge: CellBorder) -> Option<&CT_LineProperties> { + let properties = self.cell.properties.as_ref()?; + match edge { + CellBorder::Left => properties.left.as_ref(), + CellBorder::Right => properties.right.as_ref(), + CellBorder::Top => properties.top.as_ref(), + CellBorder::Bottom => properties.bottom.as_ref(), + } + } } /// A behavior-bearing mutable table-grid cell addressed within its table. @@ -6603,6 +6639,29 @@ impl TableCellMut<'_> { ) } + /// Returns the direct line of one cell edge, when present. + pub fn border(&self, edge: CellBorder) -> Option<&CT_LineProperties> { + let properties = self.cell_ref().cell.properties.as_ref()?; + match edge { + CellBorder::Left => properties.left.as_ref(), + CellBorder::Right => properties.right.as_ref(), + CellBorder::Top => properties.top.as_ref(), + CellBorder::Bottom => properties.bottom.as_ref(), + } + } + + /// Replaces or clears the direct line of one cell edge. + pub fn set_border(&mut self, edge: CellBorder, line: Option) { + let properties = self.properties_mut(); + let slot = match edge { + CellBorder::Left => &mut properties.left, + CellBorder::Right => &mut properties.right, + CellBorder::Top => &mut properties.top, + CellBorder::Bottom => &mut properties.bottom, + }; + *slot = line; + } + /// Returns whether this cell is the top-left origin of a merge. pub fn is_merge_origin(&self) -> bool { self.cell_ref().is_merge_origin() @@ -6675,6 +6734,32 @@ fn table_properties_mut(table: &mut CT_Table) -> &mut CT_TableProperties { .get_or_insert_with(CT_TableProperties::default) } +/// Returns the checked sums of a table's column widths and row heights. +fn table_extent(table: &CT_Table, operation: &'static str) -> Result<(Emu, Emu)> { + let total_width = table + .grid + .columns + .iter() + .try_fold(0i64, |total, width| total.checked_add(width.0)) + .ok_or_else(|| { + invalid_table_mutation(operation, "table width exceeds the EMU range".to_owned()) + })?; + let total_height = table + .rows + .iter() + .try_fold(0i64, |total, row| total.checked_add(row.height.0)) + .ok_or_else(|| { + invalid_table_mutation(operation, "table height exceeds the EMU range".to_owned()) + })?; + if total_width <= 0 || total_height <= 0 { + return Err(invalid_table_mutation( + operation, + "table width and height must remain positive".to_owned(), + )); + } + Ok((Emu(total_width), Emu(total_height))) +} + fn rectangular_dimensions(table: &CT_Table) -> std::result::Result<(usize, usize), String> { let rows = table.rows.len(); let columns = table.grid.columns.len(); diff --git a/crates/rpptx/tests/integration.rs b/crates/rpptx/tests/integration.rs index cbf98946..aaebc345 100644 --- a/crates/rpptx/tests/integration.rs +++ b/crates/rpptx/tests/integration.rs @@ -7769,12 +7769,12 @@ use oxml_layout::{MediaId, PageFrame, Paint, PositionedElement, Rect, walk}; use oxml_opc::relationship::rel_types; use oxml_opc::{OpcPackage, content_types}; use rpptx::{ - Angle, CT_LineProperties, CT_TextCharacterProperties, CT_TextParagraphProperties, ChartData, - ChartKind, ConnectorType, EmbeddedContentKind, EmbeddedMediaInput, EmbeddedMutationPolicy, - EmbeddedSignatureState, Emu, Error, Fill, HandoutLayout, MediaDiagnostic, MediaFallbackPolicy, - MediaKind, MediaPlaybackPhase, MediaPlaybackSettings, MediaPoster, MediaSourceInput, - Presentation, PresentationPackageClass, ShapeKind, ShapeRef, TextBullet, TextBulletCharacter, - TextBulletChoice, TextFont, TimelinePosition, + Angle, CT_LineProperties, CT_TextCharacterProperties, CT_TextParagraphProperties, CellBorder, + ChartData, ChartKind, ConnectorType, EmbeddedContentKind, EmbeddedMediaInput, + EmbeddedMutationPolicy, EmbeddedSignatureState, Emu, Error, Fill, HandoutLayout, + MediaDiagnostic, MediaFallbackPolicy, MediaKind, MediaPlaybackPhase, MediaPlaybackSettings, + MediaPoster, MediaSourceInput, Presentation, PresentationPackageClass, ShapeKind, ShapeRef, + TextBullet, TextBulletCharacter, TextBulletChoice, TextFont, TimelinePosition, }; use rpptx_layout::{ FlattenedItem, ResolveCtx, ResolvedContent, ResolvedSlide, ResolvedTextBody, ResolvedTextRun, @@ -13888,6 +13888,161 @@ fn table_mutation_rejects_invalid_ranges_without_partial_changes() { assert_eq!(presentation.to_bytes().unwrap(), before_overlap); } +fn table_border_line(color: &str) -> CT_LineProperties { + CT_LineProperties::from_xml( + format!(r#""#) + .as_bytes(), + ) + .unwrap() +} + +#[test] +fn row_heights_and_cell_borders_round_trip_with_the_frame_height_in_step() { + let mut presentation = Presentation::new().expect("open bundled template"); + presentation.add_slide(0).expect("add slide"); + let fill = Fill::from_xml(br#""#).unwrap(); + { + let mut slide = presentation.slide_mut(0).unwrap(); + let mut shape = slide + .add_table(2, 2, Emu(10), Emu(20), Emu(200), Emu(100)) + .expect("add table"); + let mut table = shape.table_mut().unwrap(); + assert_eq!(table.row_height(1), Some(Emu(50))); + table.set_row_height(1, Emu(80)).unwrap(); + for (row, height) in [(2, Emu(10)), (0, Emu(0)), (0, Emu(i64::MAX))] { + assert!(matches!( + table.set_row_height(row, height), + Err(Error::InvalidTableMutation { .. }) + )); + } + assert_eq!( + ( + table.row_height(0), + table.row_height(1), + table.row_height(2) + ), + (Some(Emu(50)), Some(Emu(80)), None) + ); + let mut cell = table.cell_mut(0, 0).unwrap(); + cell.set_fill(Some(fill.clone())); + cell.set_border(CellBorder::Bottom, Some(table_border_line("00FF00"))); + cell.set_border(CellBorder::Left, Some(table_border_line("FF0000"))); + cell.set_border(CellBorder::Top, Some(table_border_line("0000FF"))); + cell.set_border(CellBorder::Top, None); + assert_eq!( + cell.border(CellBorder::Left), + Some(&table_border_line("FF0000")) + ); + } + + let saved = presentation.to_bytes().unwrap(); + let reopened = Presentation::from_bytes(&saved).unwrap(); + let slide = reopened.slide(0).unwrap(); + let shape = slide.shapes().last().unwrap(); + assert_eq!(shape.size(), Some((Emu(200), Emu(130)))); + let table = shape.table().unwrap(); + assert_eq!(table.row_height(1), Some(Emu(80))); + let cell = table.cell(0, 0).unwrap(); + assert_eq!( + [ + CellBorder::Left, + CellBorder::Right, + CellBorder::Top, + CellBorder::Bottom + ] + .map(|edge| cell.border(edge).cloned()), + [ + Some(table_border_line("FF0000")), + None, + None, + Some(table_border_line("00FF00")) + ] + ); + assert_eq!(cell.fill(), Some(&fill)); + assert_eq!(table.cell(1, 1).unwrap().border(CellBorder::Left), None); + + let package = open_opc(&saved, "row heights and cell borders"); + let xml = + String::from_utf8(package.get_part("/ppt/slides/slide1.xml").unwrap().to_vec()).unwrap(); + assert!(xml.contains(r#""#), "{xml}"); + let properties = &xml[xml.find(""#) + .unwrap(); + assert!(left < bottom && bottom < fill, "{properties}"); +} + +/// Runs `script` under pinned python-pptx 1.0.2 with the saved deck path as +/// its only argument and returns what it prints. +fn python_pptx_1_0_2_reads(deck: &[u8], label: &str, script: &str) -> String { + let path = std::env::temp_dir().join(format!( + "rpptx-{label}-python-oracle-{}.pptx", + std::process::id() + )); + fs::write(&path, deck).unwrap(); + let output = Command::new("uv") + .args([ + "run", + "--with", + "python-pptx==1.0.2", + "python", + "-c", + script, + ]) + .arg(&path) + .output() + .expect("run pinned python-pptx oracle"); + fs::remove_file(&path).unwrap(); + assert!( + output.status.success(), + "python-pptx oracle failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8(output.stdout).unwrap() +} + +#[test] +#[ignore = "requires uv and pinned python-pptx 1.0.2"] +fn row_heights_and_cell_borders_read_back_in_pinned_python_pptx() { + let mut presentation = Presentation::new().expect("open bundled template"); + presentation.add_slide(6).expect("add slide"); + { + let mut slide = presentation.slide_mut(0).unwrap(); + let mut shape = slide + .add_table(2, 2, Emu(0), Emu(0), Emu(914_400), Emu(914_400)) + .unwrap(); + let mut table = shape.table_mut().unwrap(); + table.set_row_height(1, Emu(600_000)).unwrap(); + let mut cell = table.cell_mut(0, 0).unwrap(); + cell.set_border(CellBorder::Left, Some(table_border_line("FF0000"))); + cell.set_border(CellBorder::Bottom, Some(table_border_line("00FF00"))); + } + let records = python_pptx_1_0_2_reads( + &presentation.to_bytes().unwrap(), + "row-heights", + r#" +import sys +import pptx +from pptx import Presentation +from pptx.oxml.ns import qn + +assert pptx.__version__ == "1.0.2", pptx.__version__ +shape = Presentation(sys.argv[1]).slides[0].shapes[-1] +print([row.height for row in shape.table.rows], shape.height) +tcPr = shape.table.cell(0, 0)._tc.tcPr +print([child.tag.split("}")[1] for child in tcPr]) +left = tcPr.find(qn("a:lnL")) +print(left.get("w"), left.find(qn("a:solidFill"))[0].get("val")) +"#, + ); + assert_eq!( + records, + "[457200, 600000] 1057200\n['lnL', 'lnB']\n12700 FF0000\n" + ); +} + #[test] fn table_mutation_preserves_unmodelled_xml_and_schema_order() { let fixture = table_mutation_fixture_bytes(); diff --git a/docs/hld/06-presentationml-model.md b/docs/hld/06-presentationml-model.md index e770428e..946c42fd 100644 --- a/docs/hld/06-presentationml-model.md +++ b/docs/hld/06-presentationml-model.md @@ -374,19 +374,32 @@ text-frame handle. Table graphic frames expose concrete borrowed `TableRef` and `TableMut` handles through `ShapeRef::table` and `ShapeMut::table_mut`. Their cell access is total and returns `Option`. Table handles expose row and column counts, -column widths, and the first-row, last-row, first-column, last-column, -horizontal-banding, and vertical-banding flags. Cell handles expose plain text, -typed text-frame mutation, direct fill, four optional margins, merge-origin and -continuation state, and span height and width. +column widths, row heights, and the first-row, last-row, first-column, +last-column, horizontal-banding, and vertical-banding flags. Cell handles +expose plain text, typed text-frame mutation, direct fill, four optional +margins, the direct line of each edge, merge-origin and continuation state, and +span height and width. + +```rust +pub enum CellBorder { Left, Right, Top, Bottom } + +TableRef::row_height(&self, row: usize) -> Option; +TableMut::set_row_height(&mut self, row: usize, height: Emu) -> Result<()>; +TableCellRef::border(&self, edge: CellBorder) -> Option<&CT_LineProperties>; +TableCellMut::set_border(&mut self, edge: CellBorder, line: Option); +``` Changing a column width uses a checked sum and synchronizes the graphic-frame -width. Merge accepts opposite rectangle corners in either order. It validates +width. Changing a row height does the same for the frame height and requires a +positive height. The stored height is a minimum, which PowerPoint grows to fit +the row's text. A border is the `a:lnL`, `a:lnR`, `a:lnT`, or `a:lnB` line of +`a:tcPr`, written in that order before the cell fill. Merge accepts opposite rectangle corners in either order. It validates the complete rectangle before changing state, rejects overlap with an existing merge, migrates typed paragraphs in row-major order, and writes the DrawingML origin and continuation pattern described in `05-drawingml-model.md`. Split is valid only on a checked merge origin. It restores span one and clears -continuation flags without redistributing content. Fallible width, merge, and -split operations stage and serialize a table clone before committing it, so an +continuation flags without redistributing content. Fallible width, height, +merge, and split operations stage and serialize a table clone before committing it, so an error leaves the table unchanged. `SlideMut` also exposes the direct shape construction surface: diff --git a/docs/hld/10-bindings-spec.md b/docs/hld/10-bindings-spec.md index 765fffe5..61094168 100644 --- a/docs/hld/10-bindings-spec.md +++ b/docs/hld/10-bindings-spec.md @@ -337,7 +337,14 @@ is not a merge origin raises `RpptxError` and leaves the table unchanged. `margin_right`, `margin_top`, and `margin_bottom` read the `a:tcPr` margins as `Length` and follow the text formatting rules below. An absent margin reads `None` where python-pptx reports its 91440 and 45720 EMU defaults, and a value -outside the 32-bit coordinate range raises `ValueError`. +outside the 32-bit coordinate range raises `ValueError`. python-pptx has no +cell border API, so `border_left`, `border_right`, `border_top`, and +`border_bottom` are live `LineFormat` views of `a:lnL`, `a:lnR`, `a:lnT`, and +`a:lnB`, which reading never creates. `Table.rows` is a lazy `RowCollection` +of `Row` handles, like `columns`. `Row.height` reads the stored height as +`Length`, and assigning it keeps the frame height equal to the sum of the rows, +as a column width keeps the frame width. A height that is not positive raises +`RpptxError`. None of these writes advances the revision. `TextFrame.autofit` reports `none`, `normal`, or `shape` when the body carries an explicit choice. From 99a4beb9dea894189f054a9338ae8b0648b03881 Mon Sep 17 00:00:00 2001 From: Hadrien Mary Date: Sun, 27 Sep 2026 20:17:22 +0200 Subject: [PATCH 3/6] Read and write picture crop through the rpptx facade and binding A picture's a:srcRect is modelled as BlipFill.source_rect, written in schema order and honoured by the renderer, but no facade method reached it, so neither Rust nor Python callers could read or change a crop. Only replace_image kept an existing one. ShapeRef::crop returns the four insets as Percent1000, now re-exported from rpptx, with zero for an absent edge and None for other shape kinds. ShapeMut::set_crop writes them on a picture and rejects other kinds. It keeps the stored attribute of an unchanged edge, drops a changed edge of zero, and adds no element for four zero insets on a picture without one, so a no-op write leaves the XML as it was. Python Shape gains python-pptx's crop_left, crop_top, crop_right and crop_bottom floats. A write rounds half to even like ST_Percentage, rejects a value that is not finite or out of range, changes nothing when the value is unchanged, and does not advance the revision. GitHub issue #169. --- crates/rpptx-py/README.md | 1 + crates/rpptx-py/python/rpptx/_rpptx.pyi | 16 ++ crates/rpptx-py/src/shape.rs | 92 ++++++++++ .../tests/test_documented_examples.py | 59 +++++++ crates/rpptx-py/tests/typing_smoke.py | 13 ++ crates/rpptx/src/lib.rs | 70 +++++++- crates/rpptx/tests/integration.rs | 164 +++++++++++++++++- docs/hld/06-presentationml-model.md | 10 +- docs/hld/10-bindings-spec.md | 8 +- 9 files changed, 427 insertions(+), 6 deletions(-) diff --git a/crates/rpptx-py/README.md b/crates/rpptx-py/README.md index dc139691..4dbc8b7d 100644 --- a/crates/rpptx-py/README.md +++ b/crates/rpptx-py/README.md @@ -60,6 +60,7 @@ with open("review.pdf", "wb") as output: stale-handle errors after structural changes. - Table cell merge and split, cell fills, margins, and borders, and row heights. +- Picture crop, read and written as in python-pptx. ## Use it when diff --git a/crates/rpptx-py/python/rpptx/_rpptx.pyi b/crates/rpptx-py/python/rpptx/_rpptx.pyi index 00775e65..fa26589a 100644 --- a/crates/rpptx-py/python/rpptx/_rpptx.pyi +++ b/crates/rpptx-py/python/rpptx/_rpptx.pyi @@ -290,6 +290,22 @@ class Shape: def xml(self) -> bytes: ... @property def image(self) -> Image: ... + @property + def crop_left(self) -> float: ... + @crop_left.setter + def crop_left(self, value: float) -> None: ... + @property + def crop_top(self) -> float: ... + @crop_top.setter + def crop_top(self, value: float) -> None: ... + @property + def crop_right(self) -> float: ... + @crop_right.setter + def crop_right(self, value: float) -> None: ... + @property + def crop_bottom(self) -> float: ... + @crop_bottom.setter + def crop_bottom(self, value: float) -> None: ... def replace_image(self, image_file: _ImageFile) -> None: ... @property def shapes(self) -> ShapeCollection: ... diff --git a/crates/rpptx-py/src/shape.rs b/crates/rpptx-py/src/shape.rs index 9d75a634..2a6b34e2 100644 --- a/crates/rpptx-py/src/shape.rs +++ b/crates/rpptx-py/src/shape.rs @@ -17,6 +17,16 @@ const MIN_COORDINATE: i64 = -27_273_042_329_600; const MAX_COORDINATE: i64 = 27_273_042_316_900; const ANGLE_UNITS_PER_DEGREE: f64 = 60_000.0; const ANGLE_UNITS_PER_TURN: i64 = 21_600_000; +/// `a:srcRect` stores a crop inset in thousandths of a percent. +const CROP_UNITS_PER_FRACTION: f64 = 100_000.0; + +/// Left, top, right, and bottom picture crop insets, as the facade reports them. +type Crop = ( + rpptx::Percent1000, + rpptx::Percent1000, + rpptx::Percent1000, + rpptx::Percent1000, +); pub(crate) fn register(module: &Bound<'_, PyModule>) -> PyResult<()> { module.add_class::()?; @@ -168,6 +178,44 @@ impl PyShape { } } + fn crop(&self, py: Python<'_>) -> PyResult { + self.read(py, |shape| shape.crop())? + .ok_or_else(|| PyValueError::new_err("shape is not a picture")) + } + + fn crop_edge( + &self, + py: Python<'_>, + edge: fn(&mut Crop) -> &mut rpptx::Percent1000, + ) -> PyResult { + let mut crop = self.crop(py)?; + Ok(f64::from(edge(&mut crop).0) / CROP_UNITS_PER_FRACTION) + } + + /// Writes one crop inset as python-pptx does, rounding half to even, and + /// leaves the picture unchanged when the value equals the stored one. + fn set_crop_edge( + &self, + py: Python<'_>, + edge: fn(&mut Crop) -> &mut rpptx::Percent1000, + value: f64, + ) -> PyResult<()> { + let units = (value * CROP_UNITS_PER_FRACTION).round_ties_even(); + if !(f64::from(i32::MIN)..=f64::from(i32::MAX)).contains(&units) { + return Err(PyValueError::new_err(format!( + "crop must be a finite fraction from -21474.83648 to 21474.83647, got {value}" + ))); + } + let current = self.crop(py)?; + let mut crop = current; + *edge(&mut crop) = rpptx::Percent1000(units as i32); + if crop == current { + return Ok(()); + } + let (left, top, right, bottom) = crop; + self.edit(py, |shape| shape.set_crop(left, top, right, bottom)) + } + fn picture_id(&self, py: Python<'_>) -> PyResult<(usize, u32)> { let (kind, id) = self.read(py, |shape| (shape.kind(), shape.non_visual_id()))?; match (kind, id) { @@ -393,6 +441,50 @@ impl PyShape { }) } + /// The share of the image cropped from the picture's left edge. + #[getter] + fn crop_left(&self, py: Python<'_>) -> PyResult { + self.crop_edge(py, |crop| &mut crop.0) + } + + #[setter] + fn set_crop_left(&self, py: Python<'_>, value: f64) -> PyResult<()> { + self.set_crop_edge(py, |crop| &mut crop.0, value) + } + + /// The share of the image cropped from the picture's top edge. + #[getter] + fn crop_top(&self, py: Python<'_>) -> PyResult { + self.crop_edge(py, |crop| &mut crop.1) + } + + #[setter] + fn set_crop_top(&self, py: Python<'_>, value: f64) -> PyResult<()> { + self.set_crop_edge(py, |crop| &mut crop.1, value) + } + + /// The share of the image cropped from the picture's right edge. + #[getter] + fn crop_right(&self, py: Python<'_>) -> PyResult { + self.crop_edge(py, |crop| &mut crop.2) + } + + #[setter] + fn set_crop_right(&self, py: Python<'_>, value: f64) -> PyResult<()> { + self.set_crop_edge(py, |crop| &mut crop.2, value) + } + + /// The share of the image cropped from the picture's bottom edge. + #[getter] + fn crop_bottom(&self, py: Python<'_>) -> PyResult { + self.crop_edge(py, |crop| &mut crop.3) + } + + #[setter] + fn set_crop_bottom(&self, py: Python<'_>, value: f64) -> PyResult<()> { + self.set_crop_edge(py, |crop| &mut crop.3, value) + } + /// Replaces the picture's image, keeping its position, size, and crop. fn replace_image(&self, py: Python<'_>, image_file: &Bound<'_, PyAny>) -> PyResult<()> { let (slide, shape_id) = self.picture_id(py)?; diff --git a/crates/rpptx-py/tests/test_documented_examples.py b/crates/rpptx-py/tests/test_documented_examples.py index 79527b20..f7e975c2 100644 --- a/crates/rpptx-py/tests/test_documented_examples.py +++ b/crates/rpptx-py/tests/test_documented_examples.py @@ -2232,6 +2232,65 @@ def test_pictures_accept_bytes_and_file_objects_and_replace_their_image(tmp_path assert [oracle[index].image.blob for index in range(3)] == [blue, jpeg, jpeg] +def test_picture_crop_matches_python_pptx_in_both_directions(tmp_path): + import rpptx + + header = struct.pack(">IIBBBBB", 2, 1, 8, 2, 0, 0, 0) + red_then_blue = ( + b"\x89PNG\r\n\x1a\n" + + _png_chunk(b"IHDR", header) + + _png_chunk(b"IDAT", zlib.compress(bytes((0, 0xFF, 0, 0, 0, 0, 0xFF)))) + + _png_chunk(b"IEND", b"") + ) + prs = rpptx.Presentation() + slide = prs.slides.add_slide(prs.slide_layouts[6]) + slide.shapes.add_picture(red_then_blue, 0, 0, rpptx.Inches(2), rpptx.Inches(1)) + prs.slides[0].shapes.add_textbox(0, 0, 10, 10) + picture = prs.slides[0].shapes[0] + assert (picture.crop_left, picture.crop_top, picture.crop_right, picture.crop_bottom) == (0.0,) * 4 + before = prs.to_bytes() + picture.crop_right = 0.0 + assert prs.to_bytes() == before + uncropped = prs.render_slide_to_png(0, dpi=36.0) + held = prs.slides[0].shapes[0] + picture.crop_left = 0.5 + picture.crop_bottom = -0.125 + assert (held.crop_left, held.crop_bottom) == (0.5, -0.125) + assert prs.render_slide_to_png(0, dpi=36.0) != uncropped + cropped = prs.to_bytes() + for bad in (float("nan"), float("inf"), 21474.83648 + 1): + with pytest.raises(ValueError, match="crop must be a finite fraction"): + picture.crop_top = bad + textbox = prs.slides[0].shapes[1] + with pytest.raises(ValueError, match="shape is not a picture"): + _ = textbox.crop_left + with pytest.raises(ValueError, match="shape is not a picture"): + textbox.crop_left = 0.1 + assert prs.to_bytes() == cropped + output = tmp_path / "cropped.pptx" + prs.save(output) + + pptx = pytest.importorskip("pptx", reason="python-pptx is the differential oracle") + oracle = pptx.Presentation(output).slides[0].shapes[0] + assert (oracle.crop_left, oracle.crop_top, oracle.crop_right, oracle.crop_bottom) == ( + 0.5, + 0.0, + 0.0, + -0.125, + ) + + def build(deck): + written = deck.slides.add_slide(deck.slide_layouts[6]).shapes.add_picture( + io.BytesIO(_tiny_png()), 0, 0 + ) + written.crop_top = 0.2 + written.crop_right = 1 / 3 + + source = _python_pptx_deck(tmp_path / "python-pptx-crop.pptx", build) + read = rpptx.Presentation(source).slides[0].shapes[0] + assert (read.crop_left, read.crop_top, read.crop_right) == (0.0, 0.2, 0.33333) + + def test_shapes_and_slides_are_removed_and_reordered_with_stale_handles(tmp_path): import rpptx diff --git a/crates/rpptx-py/tests/typing_smoke.py b/crates/rpptx-py/tests/typing_smoke.py index 23e4dc94..2796f637 100644 --- a/crates/rpptx-py/tests/typing_smoke.py +++ b/crates/rpptx-py/tests/typing_smoke.py @@ -325,6 +325,19 @@ def exercise_rpptx_table_types(table: Table) -> None: (spans, cell_margins, row_heights, rows[:]) +def exercise_rpptx_picture_crop_types(picture: Shape) -> None: + picture.crop_left = 0.25 + picture.crop_top = 0 + crop: tuple[float, float, float, float] = ( + picture.crop_left, + picture.crop_top, + picture.crop_right, + picture.crop_bottom, + ) + picture.crop_right = crop[0] + picture.crop_bottom = -0.1 + + def exercise_rpptx_text_layout_types(presentation: Presentation) -> None: frames: tuple[TextFrameLayout, ...] = presentation.text_layout() narrower: tuple[TextFrameLayout, ...] = presentation.text_layout(width_factor=0.95) diff --git a/crates/rpptx/src/lib.rs b/crates/rpptx/src/lib.rs index 58836716..1756c04b 100644 --- a/crates/rpptx/src/lib.rs +++ b/crates/rpptx/src/lib.rs @@ -22,10 +22,11 @@ use oxml_chart::CT_ChartSpace; pub use oxml_chart::{ChartData, ChartKind, RgbColor}; use oxml_core::OxmlError; pub use oxml_core::core_properties::CoreProperties; -pub use oxml_core::units::{Angle, Emu}; +pub use oxml_core::units::{Angle, Emu, Percent1000}; pub use oxml_drawing::color::ColorChoice; #[cfg(feature = "render")] use oxml_drawing::color::ColorMap; +use oxml_drawing::fill::RelativeRect; pub use oxml_drawing::fill::{Fill, NoFill, PatternFill, SolidFill}; use oxml_drawing::geometry::{Guide, GuideOp, GuideOperand}; pub use oxml_drawing::line::CT_LineProperties; @@ -6061,6 +6062,51 @@ impl<'a> ShapeMut<'a> { Ok(()) } + /// Crops a picture by left, top, right, and bottom insets of its image. + /// + /// Each inset is a share of the image, where `Percent1000(25_000)` is a + /// quarter, and a negative inset extends the image. An edge keeps its + /// stored attribute when its value is unchanged, a changed edge of zero + /// drops its attribute, and four zero insets add no `a:srcRect` to a + /// picture without one. Other shape kinds are rejected. + pub fn set_crop( + &mut self, + left: Percent1000, + top: Percent1000, + right: Percent1000, + bottom: Percent1000, + ) -> Result<()> { + const OPERATION: &str = "set crop"; + let shape_kind = shape_kind(self.child); + let ShapeTreeChild::Picture(picture) = self.child else { + return Err(Error::UnsupportedShapeMutation { + operation: OPERATION, + shape_kind, + }); + }; + let fill = picture + .blip_fill + .as_mut() + .ok_or_else(|| invalid_shape_mutation(OPERATION, "picture has no p:blipFill"))?; + let zero = Percent1000::default(); + if fill.source_rect.is_none() && [left, top, right, bottom] == [zero; 4] { + return Ok(()); + } + let edge = |stored: Option, value: Percent1000| { + if stored.unwrap_or_default() == value { + stored + } else { + (value != zero).then_some(value) + } + }; + let rect = fill.source_rect.get_or_insert_with(RelativeRect::default); + rect.left = edge(rect.left, left); + rect.top = edge(rect.top, top); + rect.right = edge(rect.right, right); + rect.bottom = edge(rect.bottom, bottom); + Ok(()) + } + /// Inserts or replaces one finite preset-geometry adjustment. pub fn set_adjust_value(&mut self, name: &str, value: f64) -> Result<()> { if !value.is_finite() { @@ -7438,6 +7484,28 @@ impl<'a> ShapeRef<'a> { shape_properties(self.child)?.line.as_ref() } + /// Returns a picture's `a:srcRect` crop as left, top, right, and bottom + /// insets of its image, reading an absent edge as zero. + /// + /// Other shape kinds return `None`. + pub fn crop(&self) -> Option<(Percent1000, Percent1000, Percent1000, Percent1000)> { + let ShapeTreeChild::Picture(picture) = self.child else { + return None; + }; + let rect = picture + .blip_fill + .as_ref() + .and_then(|fill| fill.source_rect.as_ref()); + Some(rect.map_or_else(Default::default, |rect| { + ( + rect.left.unwrap_or_default(), + rect.top.unwrap_or_default(), + rect.right.unwrap_or_default(), + rect.bottom.unwrap_or_default(), + ) + })) + } + /// Serialises the child as a self-contained element. /// /// Typed children declare the `p`, `a`, `r` and `mc` prefixes they use. diff --git a/crates/rpptx/tests/integration.rs b/crates/rpptx/tests/integration.rs index aaebc345..f6b644b5 100644 --- a/crates/rpptx/tests/integration.rs +++ b/crates/rpptx/tests/integration.rs @@ -7773,8 +7773,8 @@ use rpptx::{ ChartData, ChartKind, ConnectorType, EmbeddedContentKind, EmbeddedMediaInput, EmbeddedMutationPolicy, EmbeddedSignatureState, Emu, Error, Fill, HandoutLayout, MediaDiagnostic, MediaFallbackPolicy, MediaKind, MediaPlaybackPhase, MediaPlaybackSettings, - MediaPoster, MediaSourceInput, Presentation, PresentationPackageClass, ShapeKind, ShapeRef, - TextBullet, TextBulletCharacter, TextBulletChoice, TextFont, TimelinePosition, + MediaPoster, MediaSourceInput, Percent1000, Presentation, PresentationPackageClass, ShapeKind, + ShapeRef, TextBullet, TextBulletCharacter, TextBulletChoice, TextFont, TimelinePosition, }; use rpptx_layout::{ FlattenedItem, ResolveCtx, ResolvedContent, ResolvedSlide, ResolvedTextBody, ResolvedTextRun, @@ -22727,6 +22727,166 @@ fn replacing_a_picture_with_an_svg_alternate_is_rejected_without_change() { assert_eq!(presentation.to_bytes().unwrap(), before); } +#[test] +fn picture_crop_round_trips_and_rewrites_only_the_changed_edges() { + let zero = Percent1000(0); + let mut presentation = Presentation::new().unwrap(); + presentation.add_slide(6).unwrap(); + presentation + .add_picture( + 0, + &f226_blue_pixel_png(), + "crop.png", + Emu(0), + Emu(0), + Some(Emu(100)), + Some(Emu(100)), + ) + .unwrap(); + presentation + .slide_mut(0) + .unwrap() + .add_textbox(Emu(0), Emu(0), Emu(10), Emu(10)) + .unwrap(); + let slide = presentation.slide(0).unwrap(); + assert_eq!( + slide.shape(0).unwrap().crop(), + Some((zero, zero, zero, zero)) + ); + assert_eq!(slide.shape(1).unwrap().crop(), None); + let before = presentation.to_bytes().unwrap(); + { + let mut slide = presentation.slide_mut(0).unwrap(); + slide + .shape_mut(0) + .unwrap() + .set_crop(zero, zero, zero, zero) + .unwrap(); + assert!(matches!( + slide + .shape_mut(1) + .unwrap() + .set_crop(Percent1000(1), zero, zero, zero), + Err(Error::UnsupportedShapeMutation { .. }) + )); + } + assert_eq!(presentation.to_bytes().unwrap(), before); + + presentation + .slide_mut(0) + .unwrap() + .shape_mut(0) + .unwrap() + .set_crop( + Percent1000(25_000), + zero, + Percent1000(-10_000), + Percent1000(12_500), + ) + .unwrap(); + let saved = presentation.to_bytes().unwrap(); + let reopened = Presentation::from_bytes(&saved).unwrap(); + assert!(reopened.validate().is_empty(), "{:?}", reopened.validate()); + assert_eq!( + reopened.slide(0).unwrap().shape(0).unwrap().crop(), + Some(( + Percent1000(25_000), + zero, + Percent1000(-10_000), + Percent1000(12_500) + )) + ); + let picture = + String::from_utf8(reopened.slide(0).unwrap().shape(0).unwrap().xml().unwrap()).unwrap(); + let blip = picture.find(""#) + .unwrap(); + let stretch = picture.find("").unwrap(); + assert!(blip < crop && crop < stretch, "{picture}"); + + let explicit = r#""#; + let mut presentation = with_first_slide_children(&presentation, explicit, &[]); + let crop_xml = |presentation: &Presentation| { + let xml = String::from_utf8( + presentation + .slide(0) + .unwrap() + .shape(2) + .unwrap() + .xml() + .unwrap(), + ) + .unwrap(); + let start = xml.find("").unwrap() + 2].to_owned() + }; + let crop = |presentation: &mut Presentation, left: i32, top: i32| { + presentation + .slide_mut(0) + .unwrap() + .shape_mut(2) + .unwrap() + .set_crop(Percent1000(left), Percent1000(top), zero, zero) + .unwrap(); + }; + crop(&mut presentation, 10_000, 5_000); + assert_eq!( + crop_xml(&presentation), + r#""# + ); + crop(&mut presentation, 0, 0); + assert_eq!(crop_xml(&presentation), r#""#); + assert_eq!( + presentation.slide(0).unwrap().shape(2).unwrap().crop(), + Some((zero, zero, zero, zero)) + ); +} + +#[test] +#[ignore = "requires uv and pinned python-pptx 1.0.2"] +fn picture_crop_reads_back_in_pinned_python_pptx() { + let mut presentation = Presentation::new().unwrap(); + presentation.add_slide(6).unwrap(); + presentation + .add_picture( + 0, + &f226_blue_pixel_png(), + "crop.png", + Emu(0), + Emu(0), + None, + None, + ) + .unwrap(); + presentation + .slide_mut(0) + .unwrap() + .shape_mut(0) + .unwrap() + .set_crop( + Percent1000(25_000), + Percent1000(0), + Percent1000(-10_000), + Percent1000(12_500), + ) + .unwrap(); + let records = python_pptx_1_0_2_reads( + &presentation.to_bytes().unwrap(), + "picture-crop", + r#" +import sys +import pptx +from pptx import Presentation + +assert pptx.__version__ == "1.0.2", pptx.__version__ +picture = Presentation(sys.argv[1]).slides[0].shapes[0] +print(picture.crop_left, picture.crop_top, picture.crop_right, picture.crop_bottom) +"#, + ); + assert_eq!(records, "0.25 0.0 -0.1 0.125\n"); +} + #[test] fn removing_a_shape_deletes_only_relationships_and_parts_nothing_else_uses() { let png = f226_blue_pixel_png(); diff --git a/docs/hld/06-presentationml-model.md b/docs/hld/06-presentationml-model.md index 946c42fd..a5b5a75f 100644 --- a/docs/hld/06-presentationml-model.md +++ b/docs/hld/06-presentationml-model.md @@ -284,6 +284,7 @@ ShapeRef::shape_type(&self) -> Option; ShapeRef::rotation(&self) -> Option; ShapeRef::fill(&self) -> Option<&Fill>; ShapeRef::line(&self) -> Option<&CT_LineProperties>; +ShapeRef::crop(&self) -> Option<(Percent1000, Percent1000, Percent1000, Percent1000)>; ShapeRef::adjustments(&self) -> Result>; ShapeRef::xml(&self) -> Result>; ``` @@ -306,7 +307,8 @@ a literal `val` guide of the same name in the shape's own `a:avLst`. Only ordinary shapes with preset geometry have adjustments. `xml` serializes a typed child on its own with the prefixes it uses declared. Alternate content returns its preserved bytes, which may rely on prefixes only the slide root -declares. +declares. `crop` reads a picture's `a:srcRect` insets in left, top, right, +bottom order, with zero for an absent edge, and is `None` for other kinds. `slide_mut(index)` exposes a borrowed `SlideMut` handle. Its `shape(index)` method retains read access, while `shape_mut(index)` returns a `ShapeMut` for an @@ -316,7 +318,11 @@ children only. The selected `mc:Fallback` view remains read-only. Position, size, rotation, and name setters support ordinary shapes, pictures, graphic frames, groups, and connectors. Fill and line setters support ordinary shapes, pictures, and connectors because those kinds own typed shape -properties. Adjustment mutation supports finite values on preset geometry. +properties. `ShapeMut::set_crop(left, top, right, bottom)` writes the +`a:srcRect` of a picture between its blip and fill mode. It keeps the stored +attribute of an unchanged edge, drops a changed edge of zero, and adds no +element when a picture without one gets four zero insets. Adjustment mutation +supports finite values on preset geometry. Unsupported shape kinds and unsupported geometry return concrete facade errors. Indexed access remains total and returns `Option`. diff --git a/docs/hld/10-bindings-spec.md b/docs/hld/10-bindings-spec.md index 61094168..8b4beb85 100644 --- a/docs/hld/10-bindings-spec.md +++ b/docs/hld/10-bindings-spec.md @@ -318,7 +318,13 @@ the effective preset adjustments, normalized so that 1.0 is 100000, and assignment truncates as python-pptx does. `xml` returns the element serialized on its own as bytes. A picture's `image` is a frozen `Image` snapshot with `blob`, `content_type`, and the python-pptx `ext`, and `replace_image` changes -only that picture through the native staged replacement. +only that picture through the native staged replacement. `crop_left`, +`crop_top`, `crop_right`, and `crop_bottom` read and write the picture's +`a:srcRect` insets as python-pptx floats, where 0.25 is a quarter of the image. +A missing edge reads 0.0, a write rounds half to even as python-pptx does and +changes nothing when the value is unchanged, and a value that is not finite or +outside the `ST_Percentage` range raises `ValueError`. Other shape kinds raise +`ValueError`, and crop writes do not advance the revision. `ShapeCollection.add_shape` accepts a DrawingML preset name or an `MSO_SHAPE` member. `add_connector` follows the python-pptx signature, `add_group_shape` From 7b99c7f8c329b01f75ee635fc4dfb232cbc5aaf6 Mon Sep 17 00:00:00 2001 From: Hadrien Mary Date: Sun, 27 Sep 2026 20:21:03 +0200 Subject: [PATCH 4/6] Move a slide shape in the z-order from rpptx and its binding New shapes land on top of a slide's shape tree, and nothing in rpptx-oxml, the facade or the binding could bring one forward or send it back. CT_ShapeTree only appended and removed children. CT_ShapeTree::move_child(from, to) serializes the tree, moves the child's bytes past exactly the children between the two indices, and reparses, as remove_child_by_id already does, so unmodelled members such as p:contentPart and a schema-final p:extLst keep their bytes and their place. Both now share one finder that returns the byte range of every typed child in order. Presentation::move_shape moves an immediate slide child with final-index semantics like move_slide and rejects an index past the last child without change. Animations, connector glue and relationships name shape ids, so they need no rewrite. Python gains ShapeCollection.move(from_, to), mirroring SlideCollection.move: negative indices, one revision bump, and a ValueError on the read-only nested collections. GitHub issue #169. --- crates/rpptx-oxml/src/shape_tree.rs | 117 ++++++++++++++++-- crates/rpptx-py/README.md | 1 + crates/rpptx-py/python/rpptx/_rpptx.pyi | 1 + crates/rpptx-py/src/shape.rs | 18 +++ .../tests/test_documented_examples.py | 33 +++++ crates/rpptx-py/tests/typing_smoke.py | 4 + crates/rpptx/src/lib.rs | 29 +++++ crates/rpptx/tests/integration.rs | 95 ++++++++++++++ docs/hld/06-presentationml-model.md | 11 ++ docs/hld/10-bindings-spec.md | 5 +- 10 files changed, 300 insertions(+), 14 deletions(-) diff --git a/crates/rpptx-oxml/src/shape_tree.rs b/crates/rpptx-oxml/src/shape_tree.rs index 8b0b16df..10bac806 100644 --- a/crates/rpptx-oxml/src/shape_tree.rs +++ b/crates/rpptx-oxml/src/shape_tree.rs @@ -1470,9 +1470,12 @@ impl CT_ShapeTree { }; let removed = self.children[index].clone(); let xml = self.to_xml()?; - let range = direct_shape_child_range(&xml, id)?.ok_or_else(|| { - OxmlError::InvalidValue(format!("shape id {id} disappeared during removal")) - })?; + let range = direct_shape_child_ranges(&xml)? + .get(index) + .cloned() + .ok_or_else(|| { + OxmlError::InvalidValue(format!("shape id {id} disappeared during removal")) + })?; let mut rewritten = Vec::with_capacity(xml.len() - range.len()); rewritten.extend_from_slice(&xml[..range.start]); rewritten.extend_from_slice(&xml[range.end..]); @@ -1480,6 +1483,48 @@ impl CT_ShapeTree { Ok(Some(removed)) } + /// Moves one immediate child so that it ends up at index `to`, where + /// later children draw on top. + /// + /// The moved child passes only the children between its old and new + /// index. Unmodelled members, such as `p:contentPart`, and schema-final + /// content keep their bytes and their place among the other children. + pub fn move_child(&mut self, from: usize, to: usize) -> Result<()> { + let count = self.children.len(); + if let Some(index) = [from, to].into_iter().find(|index| *index >= count) { + return Err(OxmlError::InvalidValue(format!( + "shape-tree child index {index} is out of range for {count} children" + ))); + } + if from == to { + return Ok(()); + } + let xml = self.to_xml()?; + let ranges = direct_shape_child_ranges(&xml)?; + if ranges.len() != count { + return Err(OxmlError::InvalidValue( + "shape-tree children changed during a move".to_owned(), + )); + } + let moved = ranges[from].clone(); + let mut rewritten = Vec::with_capacity(xml.len()); + if from < to { + let after = ranges[to].end; + rewritten.extend_from_slice(&xml[..moved.start]); + rewritten.extend_from_slice(&xml[moved.end..after]); + rewritten.extend_from_slice(&xml[moved]); + rewritten.extend_from_slice(&xml[after..]); + } else { + let before = ranges[to].start; + rewritten.extend_from_slice(&xml[..before]); + rewritten.extend_from_slice(&xml[moved.clone()]); + rewritten.extend_from_slice(&xml[before..moved.start]); + rewritten.extend_from_slice(&xml[moved.end..]); + } + *self = Self::from_xml(&rewritten)?; + Ok(()) + } + /// Parses a complete `p:spTree` with any prefix bound to PresentationML. pub fn from_xml(xml: &[u8]) -> Result { Self::from_fragment(xml, &[]) @@ -1522,10 +1567,12 @@ impl CT_ShapeTree { } } -fn direct_shape_child_range(xml: &[u8], id: u32) -> Result>> { +/// Returns the byte range of every typed shape-tree child, in child order. +fn direct_shape_child_ranges(xml: &[u8]) -> Result>> { let mut reader = Reader::from_reader(xml); let mut buffer = Vec::new(); let mut root = None; + let mut ranges = Vec::new(); loop { match reader.read_event_into(&mut buffer)? { Event::Start(start) if root.is_none() => { @@ -1538,10 +1585,8 @@ fn direct_shape_child_range(xml: &[u8], id: u32) -> Result>> let start = shape_start_tag_range(xml, reader.buffer_position() as usize)?.start; let raw = capture_element(&mut reader, &child)?; let range = start..reader.buffer_position() as usize; - if parse_shape_tree_child(&name, uri, &raw, &namespaces)? - .is_some_and(|child| child.non_visual_id() == Some(id)) - { - return Ok(Some(range)); + if parse_shape_tree_child(&name, uri, &raw, &namespaces)?.is_some() { + ranges.push(range); } } Event::Empty(child) => { @@ -1550,13 +1595,11 @@ fn direct_shape_child_range(xml: &[u8], id: u32) -> Result>> let uri = namespaces.element_uri(child.name().as_ref()); let range = shape_start_tag_range(xml, reader.buffer_position() as usize)?; let raw = capture_empty_element(&child)?; - if parse_shape_tree_child(&name, uri, &raw, &namespaces)? - .is_some_and(|child| child.non_visual_id() == Some(id)) - { - return Ok(Some(range)); + if parse_shape_tree_child(&name, uri, &raw, &namespaces)?.is_some() { + ranges.push(range); } } - Event::End(_) | Event::Eof => return Ok(None), + Event::End(_) | Event::Eof => return Ok(ranges), _ => {} } buffer.clear(); @@ -2427,4 +2470,52 @@ mod style_tests { assert!(text.find(""# + ) + }; + let xml = format!( + r#"{}{}{}"#, + shape(2), + shape(3), + shape(4) + ); + let mut tree = CT_ShapeTree::from_xml(xml.as_bytes()).unwrap(); + let ids = |tree: &CT_ShapeTree| { + tree.children + .iter() + .filter_map(ShapeTreeChild::non_visual_id) + .collect::>() + }; + let order = |tree: &CT_ShapeTree| { + let text = String::from_utf8(tree.to_xml().unwrap()).unwrap(); + let mut marks = [r#"id="2""#, r#"id="3""#, r#"id="4""#, " Shape: ... def remove(self, shape: Shape) -> None: ... + def move(self, from_: int, to: int) -> None: ... @_final diff --git a/crates/rpptx-py/src/shape.rs b/crates/rpptx-py/src/shape.rs index 2a6b34e2..809a3827 100644 --- a/crates/rpptx-py/src/shape.rs +++ b/crates/rpptx-py/src/shape.rs @@ -850,6 +850,24 @@ impl PyShapeCollection { Ok(()) } + /// Moves the shape at `from_` so that it ends up at z-order index `to`, + /// where later shapes draw on top. + #[pyo3(name = "move")] + fn move_shape(&mut self, py: Python<'_>, from_: isize, to: isize) -> PyResult<()> { + self.require_slide_root()?; + let slide_index = self.validate(py)?; + let len = self.len(py)?; + let from_ = normalize_index(from_, len, "shape")?; + let to = normalize_index(to, len, "shape")?; + let mut presentation = self.presentation.borrow_mut(py); + presentation + .inner + .move_shape(slide_index, from_, to) + .map_err(|error| rpptx_to_pyerr(py, error))?; + presentation.revisions.bump(); + Ok(()) + } + #[allow(clippy::too_many_arguments)] fn add_table( &mut self, diff --git a/crates/rpptx-py/tests/test_documented_examples.py b/crates/rpptx-py/tests/test_documented_examples.py index f7e975c2..869306fe 100644 --- a/crates/rpptx-py/tests/test_documented_examples.py +++ b/crates/rpptx-py/tests/test_documented_examples.py @@ -2291,6 +2291,39 @@ def build(deck): assert (read.crop_left, read.crop_top, read.crop_right) == (0.0, 0.2, 0.33333) +def test_shapes_move_changes_the_z_order_and_stales_handles_once(tmp_path): + import rpptx + + prs = rpptx.Presentation() + prs.slides.add_slide(prs.slide_layouts[6]) + for name in ("back", "middle", "front"): + prs.slides[0].shapes.add_shape("rect", 0, 0, 100, 100).name = name + shapes = prs.slides[0].shapes + held = shapes[0] + shapes.move(0, -1) + assert [shape.name for shape in prs.slides[0].shapes] == ["middle", "front", "back"] + for operation in (lambda: held.name, lambda: len(shapes)): + _assert_stale_after_exactly_one_bump(rpptx, operation) + with pytest.raises(rpptx.StaleElementError, match=r"prs\.slides\[0\]\.shapes\[0\]\."): + _ = held.name + prs.slides[0].shapes.move(-1, 1) + assert [shape.name for shape in prs.slides[0].shapes] == ["middle", "back", "front"] + before = prs.to_bytes() + with pytest.raises(IndexError, match="shape index out of range"): + prs.slides[0].shapes.move(0, 3) + group = prs.slides[0].shapes.add_group_shape() + with pytest.raises(ValueError, match="nested shape collections are read-only"): + group.shapes.move(0, 0) + prs.slides[0].shapes.remove(prs.slides[0].shapes[3]) + assert prs.to_bytes() == before + output = tmp_path / "z-order.pptx" + prs.save(output) + + pptx = pytest.importorskip("pptx", reason="python-pptx is the differential oracle") + oracle = pptx.Presentation(output).slides[0].shapes + assert [shape.name for shape in oracle] == ["middle", "back", "front"] + + def test_shapes_and_slides_are_removed_and_reordered_with_stale_handles(tmp_path): import rpptx diff --git a/crates/rpptx-py/tests/typing_smoke.py b/crates/rpptx-py/tests/typing_smoke.py index 2796f637..aa5879cf 100644 --- a/crates/rpptx-py/tests/typing_smoke.py +++ b/crates/rpptx-py/tests/typing_smoke.py @@ -338,6 +338,10 @@ def exercise_rpptx_picture_crop_types(picture: Shape) -> None: picture.crop_bottom = -0.1 +def exercise_rpptx_z_order_types(slide: Slide) -> None: + slide.shapes.move(0, -1) + + def exercise_rpptx_text_layout_types(presentation: Presentation) -> None: frames: tuple[TextFrameLayout, ...] = presentation.text_layout() narrower: tuple[TextFrameLayout, ...] = presentation.text_layout(width_factor=0.95) diff --git a/crates/rpptx/src/lib.rs b/crates/rpptx/src/lib.rs index 1756c04b..5a878159 100644 --- a/crates/rpptx/src/lib.rs +++ b/crates/rpptx/src/lib.rs @@ -3193,6 +3193,35 @@ impl Presentation { self.commit_candidate(staged) } + /// Moves one immediate slide child so that it ends up at z-order index + /// `to_index`, where later children draw on top. + /// + /// Animations, connector glue, and relationships refer to shape ids, so + /// they follow the moved shape. Unmodelled shape-tree members keep their + /// place among the other children. + pub fn move_shape( + &mut self, + slide_index: usize, + from_index: usize, + to_index: usize, + ) -> Result<()> { + const OPERATION: &str = "move shape"; + self.require_slide_index(slide_index)?; + let tree = &mut self.slides[slide_index].slide.common_slide_data.shape_tree; + let count = tree.children.len(); + if let Some(index) = [from_index, to_index] + .into_iter() + .find(|index| *index >= count) + { + return Err(invalid_slide_mutation( + OPERATION, + format!("shape index {index} is out of range for {count} shapes"), + )); + } + tree.move_child(from_index, to_index) + .map_err(|error| invalid_shape_mutation(OPERATION, error.to_string())) + } + /// Inspects slide, layout, and master SmartArt in producing-scope order. pub fn smart_art(&self, slide_index: usize) -> Result> { let record = self diff --git a/crates/rpptx/tests/integration.rs b/crates/rpptx/tests/integration.rs index f6b644b5..5648bb4d 100644 --- a/crates/rpptx/tests/integration.rs +++ b/crates/rpptx/tests/integration.rs @@ -23060,6 +23060,101 @@ fn removing_a_shape_detaches_connectors_and_rejects_animated_targets_without_cha assert_eq!(first_slide_shape_id(&reopened, 1), group); } +fn z_order_fixture() -> Presentation { + let mut presentation = Presentation::new().unwrap(); + presentation.add_slide(6).unwrap(); + let mut slide = presentation.slide_mut(0).unwrap(); + for (name, color) in [("Red", "FF0000"), ("Blue", "0000FF")] { + let mut shape = slide + .add_shape("rect", Emu(0), Emu(0), Emu(914_400), Emu(914_400)) + .unwrap(); + shape.set_name(name).unwrap(); + shape + .set_fill( + Fill::from_xml( + format!(r#""#).as_bytes(), + ) + .unwrap(), + ) + .unwrap(); + } + presentation +} + +#[test] +fn moving_a_shape_changes_the_draw_order_and_keeps_every_shape_id() { + let mut presentation = z_order_fixture(); + let ids = |presentation: &Presentation| { + presentation + .slide(0) + .unwrap() + .shapes() + .map(|shape| shape.non_visual_id().unwrap()) + .collect::>() + }; + let top_color = |presentation: &Presentation| { + let png = presentation + .slide_png_deterministic(0, 72.0) + .unwrap() + .unwrap(); + let pixel = tiny_skia::Pixmap::decode_png(&png) + .unwrap() + .pixel(36, 36) + .unwrap(); + (pixel.red(), pixel.green(), pixel.blue()) + }; + let original = ids(&presentation); + assert_eq!(top_color(&presentation), (0, 0, 255)); + + presentation.move_shape(0, 1, 0).unwrap(); + assert_eq!(ids(&presentation), [original[1], original[0]]); + assert_eq!(top_color(&presentation), (255, 0, 0)); + let moved = presentation.to_bytes().unwrap(); + assert!(matches!( + presentation.move_shape(0, 0, 2), + Err(Error::InvalidSlideMutation { .. }) + )); + assert!(matches!( + presentation.move_shape(1, 0, 0), + Err(Error::UnknownSlideIndex { .. }) + )); + presentation.move_shape(0, 1, 1).unwrap(); + assert_eq!(presentation.to_bytes().unwrap(), moved); + + let reopened = Presentation::from_bytes(&moved).unwrap(); + assert!(reopened.validate().is_empty(), "{:?}", reopened.validate()); + assert_eq!(ids(&reopened), [original[1], original[0]]); + assert_eq!(top_color(&reopened), (255, 0, 0)); +} + +#[test] +#[ignore = "requires uv and pinned python-pptx 1.0.2"] +fn moved_shapes_read_back_in_pinned_python_pptx_order() { + let mut presentation = z_order_fixture(); + presentation + .slide_mut(0) + .unwrap() + .add_textbox(Emu(0), Emu(0), Emu(10), Emu(10)) + .unwrap() + .set_name("Text") + .unwrap(); + presentation.move_shape(0, 2, 0).unwrap(); + presentation.move_shape(0, 1, 2).unwrap(); + let records = python_pptx_1_0_2_reads( + &presentation.to_bytes().unwrap(), + "z-order", + r#" +import sys +import pptx +from pptx import Presentation + +assert pptx.__version__ == "1.0.2", pptx.__version__ +print([shape.name for shape in Presentation(sys.argv[1]).slides[0].shapes]) +"#, + ); + assert_eq!(records, "['Text', 'Blue', 'Red']\n"); +} + #[test] fn setting_notes_text_creates_the_notes_slide_and_a_missing_notes_master() { let mut presentation = Presentation::new().unwrap(); diff --git a/docs/hld/06-presentationml-model.md b/docs/hld/06-presentationml-model.md index a5b5a75f..e464bdad 100644 --- a/docs/hld/06-presentationml-model.md +++ b/docs/hld/06-presentationml-model.md @@ -478,6 +478,7 @@ pub struct PictureImage<'a> { pub fn picture_image(&self, slide_index: usize, shape_id: u32) -> Result>; pub fn replace_picture_image(&mut self, slide_index: usize, shape_id: u32, image_data: &[u8]) -> Result<()>; pub fn remove_shape(&mut self, slide_index: usize, shape_index: usize) -> Result<()>; +pub fn move_shape(&mut self, slide_index: usize, from_index: usize, to_index: usize) -> Result<()>; ``` `picture_image` finds the picture by `p:cNvPr/@id`, including inside groups @@ -502,6 +503,16 @@ delete. Slide relationships that only the removed subtree referenced are deleted, and their internal targets are pruned recursively once unreachable, so a removed chart also drops its embedded workbook. +`move_shape` changes the z-order of one immediate slide child so that it ends +up at `to_index`, where later children draw on top. An index past the last +child is rejected without change. Animations, connector glue, and +relationships name shape ids, so they need no rewrite. +`CT_ShapeTree::move_child` serializes the tree, moves the child's bytes past +exactly the children between its two indices, and reparses, the technique +`remove_child_by_id` uses. Unmodelled members such as `p:contentPart` and +schema-final `p:extLst` content therefore keep their bytes and their place +among the other children. + An ordinary shape has canonical non-visual properties, a typed transform, preset geometry, and a minimal text body. `add_shape` keeps the string API but accepts only names in the generated table of all 187 ECMA preset shapes. An diff --git a/docs/hld/10-bindings-spec.md b/docs/hld/10-bindings-spec.md index 8b4beb85..7deaf510 100644 --- a/docs/hld/10-bindings-spec.md +++ b/docs/hld/10-bindings-spec.md @@ -331,7 +331,10 @@ member. `add_connector` follows the python-pptx signature, `add_group_shape` appends an empty group, and `add_picture` accepts a path, bytes, or a binary file-like object, which is rewound first when it can seek. `remove` deletes one shape of a slide with the relationships and parts only it used and advances the -revision once. Nested collections stay read-only. +revision once. `move(from_, to)` changes the z-order like +`SlideCollection.move`, so the shape ends up at index `to` and draws above the +shapes before it, and advances the revision once. python-pptx has no z-order +API. Nested collections stay read-only. A table `Cell` follows python-pptx for `merge(other_cell)`, `split()`, `is_merge_origin`, `is_spanned`, `span_height`, and `span_width`. Merge and From 6fb8771a3e52e14f71f5875fb169eb1a0a9122d1 Mon Sep 17 00:00:00 2001 From: Hadrien Mary Date: Sun, 27 Sep 2026 20:29:35 +0200 Subject: [PATCH 5/6] Read and write run hyperlinks through the rpptx facade and binding A run's a:hlinkClick is modelled with only its relationship id, and nothing public resolved, added or retargeted the slide's hyperlink relationship, so neither Rust nor Python callers could read a link's address or point a run at a URL. Validation also reports a hyperlink relationship that nothing references, so a relationship-level helper would have left a window where the deck does not validate. Presentation::hyperlink_address resolves the r:id an a:hlinkClick stores, like python-pptx's address. Presentation::set_run_hyperlink does the write in one staged step: it finds the ordinary shape by its unique id, inside groups too, gives the regular run a fresh a:hlinkClick naming the slide's external hyperlink relationship to the address, reuses that relationship when it exists, and removes the old one once nothing on the slide names it, so repeated retargeting does not grow the part. None removes the link, the current address is a no-op, and a rejected address or run leaves the slide and its relationships as they were. Python gains Run.hyperlink, a Hyperlink view whose address getter and setter follow python-pptx, with None or an empty string removing the link. The write stays in place and does not advance the revision. GitHub issue #169. --- crates/rpptx-py/README.md | 1 + crates/rpptx-py/python/rpptx/_rpptx.pyi | 15 +- crates/rpptx-py/src/text.rs | 92 +++++++- .../tests/test_documented_examples.py | 77 +++++++ crates/rpptx-py/tests/typing_smoke.py | 10 + crates/rpptx/src/lib.rs | 171 ++++++++++++++- crates/rpptx/tests/integration.rs | 198 ++++++++++++++++++ docs/hld/06-presentationml-model.md | 29 +++ docs/hld/10-bindings-spec.md | 9 +- 9 files changed, 597 insertions(+), 5 deletions(-) diff --git a/crates/rpptx-py/README.md b/crates/rpptx-py/README.md index 50836fc6..62e3e46e 100644 --- a/crates/rpptx-py/README.md +++ b/crates/rpptx-py/README.md @@ -62,6 +62,7 @@ with open("review.pdf", "wb") as output: heights. - Picture crop, read and written as in python-pptx. - Shape z-order through `slide.shapes.move(from_, to)`. +- Run hyperlinks, read, added, retargeted, and removed as in python-pptx. ## Use it when diff --git a/crates/rpptx-py/python/rpptx/_rpptx.pyi b/crates/rpptx-py/python/rpptx/_rpptx.pyi index fe794728..5568f6dc 100644 --- a/crates/rpptx-py/python/rpptx/_rpptx.pyi +++ b/crates/rpptx-py/python/rpptx/_rpptx.pyi @@ -26,8 +26,8 @@ __all__ = [ "SlideCollection", "Shape", "ShapeCollection", "PlaceholderCollection", "Image", "AdjustmentCollection", "FillFormat", "LineFormat", "ColorFormat", "TextFrame", "Paragraph", "ParagraphCollection", "Run", "RunCollection", - "Font", "Table", "Column", "ColumnCollection", "Row", "RowCollection", - "Cell", + "Hyperlink", "Font", "Table", "Column", "ColumnCollection", "Row", + "RowCollection", "Cell", ] @@ -535,6 +535,17 @@ class Run: def text(self, value: str) -> None: ... @property def font(self) -> Font: ... + @property + def hyperlink(self) -> Hyperlink: ... + + +@_final +class Hyperlink: + def __new__(cls, *, _private: _Never) -> Hyperlink: ... + @property + def address(self) -> str | None: ... + @address.setter + def address(self, value: str | None) -> None: ... @_final diff --git a/crates/rpptx-py/src/text.rs b/crates/rpptx-py/src/text.rs index 8f6e8673..b1045790 100644 --- a/crates/rpptx-py/src/text.rs +++ b/crates/rpptx-py/src/text.rs @@ -12,7 +12,7 @@ use rpptx::{ use crate::normalize_index; use crate::presentation::PyPresentation; use crate::rpptx_to_pyerr; -use crate::shape::{shape_mut_at, shape_ref_at}; +use crate::shape::{shape_mut_at, shape_ref_at, slide_index}; use crate::validate_path; const EMU_PER_CENTIPOINT: i64 = 127; @@ -95,6 +95,7 @@ pub(crate) fn register(module: &Bound<'_, PyModule>) -> PyResult<()> { module.add_class::()?; module.add_class::()?; module.add_class::()?; + module.add_class::()?; module.add_class::()?; Ok(()) } @@ -898,6 +899,95 @@ impl PyRun { }, ) } + + #[getter] + fn hyperlink(&self, py: Python<'_>) -> PyResult> { + validate_path(py, &self.presentation.borrow(py), &self.path, "run", "")?; + Py::new( + py, + PyHyperlink { + presentation: self.presentation.clone_ref(py), + path: self.path.clone(), + }, + ) + } +} + +/// The click hyperlink of one run, like python-pptx `_Hyperlink`. +#[pyclass(name = "Hyperlink")] +pub struct PyHyperlink { + presentation: Py, + path: ContentPath, +} + +impl PyHyperlink { + /// Returns the run's direct character properties, raising when the run + /// no longer exists. + fn run_properties( + &self, + py: Python<'_>, + presentation: &PyPresentation, + ) -> PyResult> { + validate_path(py, presentation, &self.path, "hyperlink", ".hyperlink")?; + let paragraph = paragraph_index(&self.path) + .ok_or_else(|| PyIndexError::new_err("paragraph index is missing"))?; + let run = + run_index(&self.path).ok_or_else(|| PyIndexError::new_err("run index is missing"))?; + shape_ref_at(&presentation.inner, &self.path) + .and_then(|shape| shape.text_frame()) + .and_then(|frame| frame.paragraph(paragraph)) + .and_then(|paragraph| paragraph.run(run)) + .map(|run| run.properties().cloned()) + .ok_or_else(|| PyIndexError::new_err("run index out of range")) + } +} + +#[pymethods] +impl PyHyperlink { + /// The address the run's click hyperlink opens, or `None` without one. + #[getter] + fn address(&self, py: Python<'_>) -> PyResult> { + let presentation = self.presentation.borrow(py); + let properties = self.run_properties(py, &presentation)?; + let Some(relationship_id) = properties + .as_ref() + .and_then(|properties| properties.hyperlink_click.as_ref()) + .and_then(|hyperlink| hyperlink.relationship_id.as_deref()) + .filter(|id| !id.is_empty()) + else { + return Ok(None); + }; + Ok(presentation + .inner + .hyperlink_address(slide_index(&self.path)?, relationship_id) + .map(str::to_owned)) + } + + /// Points the run's click hyperlink at `value`, or removes it for `None` + /// or an empty string, as python-pptx does. The relationship the old + /// hyperlink used goes when nothing else on the slide uses it. + #[setter] + fn set_address(&self, py: Python<'_>, value: Option<&str>) -> PyResult<()> { + let mut presentation = self.presentation.borrow_mut(py); + self.run_properties(py, &presentation)?; + let paragraph = paragraph_index(&self.path) + .ok_or_else(|| PyIndexError::new_err("paragraph index is missing"))?; + let run = + run_index(&self.path).ok_or_else(|| PyIndexError::new_err("run index is missing"))?; + let shape_id = shape_ref_at(&presentation.inner, &self.path) + .and_then(|shape| shape.non_visual_id()) + .ok_or_else(|| PyValueError::new_err("shape has no id"))?; + presentation + .inner + .set_run_hyperlink( + slide_index(&self.path)?, + shape_id, + paragraph, + run, + value.filter(|value| !value.is_empty()), + ) + .map_err(|error| rpptx_to_pyerr(py, error)) + } } #[pyclass(name = "RunCollection")] diff --git a/crates/rpptx-py/tests/test_documented_examples.py b/crates/rpptx-py/tests/test_documented_examples.py index 869306fe..a3d4fbcd 100644 --- a/crates/rpptx-py/tests/test_documented_examples.py +++ b/crates/rpptx-py/tests/test_documented_examples.py @@ -1684,6 +1684,83 @@ def test_text_properties_agree_with_python_pptx_in_both_directions(tmp_path): assert font.color.rgb == OracleRGBColor(0xAA, 0xBB, 0xCC) +def _slide_hyperlink_targets(data): + import xml.etree.ElementTree as ElementTree + + relationships = _package_parts(data)["ppt/slides/_rels/slide1.xml.rels"] + return sorted( + relationship.get("Target") + for relationship in ElementTree.fromstring(relationships) + if relationship.get("Type").endswith("/hyperlink") + ) + + +def test_run_hyperlink_address_reads_writes_and_prunes_like_python_pptx(tmp_path): + import rpptx + + prs = _textbox_presentation(rpptx) + prs.slides[0].shapes[0].text_frame.paragraphs[0].add_run(" world") + first, second = prs.slides[0].shapes[0].text_frame.paragraphs[0].runs + link = first.hyperlink + assert link.address is None + before = prs.to_bytes() + link.address = None + link.address = "" + assert prs.to_bytes() == before + link.address = "https://example.com/a?x=1&y=2" + second.hyperlink.address = "https://example.com/a?x=1&y=2" + assert (first.hyperlink.address, second.text) == ("https://example.com/a?x=1&y=2", " world") + assert _slide_hyperlink_targets(prs.to_bytes()) == ["https://example.com/a?x=1&y=2"] + for _ in range(3): + link.address = "https://example.com/b" + link.address = "https://example.com/c" + assert _slide_hyperlink_targets(prs.to_bytes()) == [ + "https://example.com/a?x=1&y=2", + "https://example.com/c", + ] + second.hyperlink.address = None + assert (second.hyperlink.address, second.font.name) == (None, None) + assert _slide_hyperlink_targets(prs.to_bytes()) == ["https://example.com/c"] + before = prs.to_bytes() + with pytest.raises(rpptx.RpptxError, match="control characters"): + link.address = "https://example.com/\nnext" + assert prs.to_bytes() == before + assert b"https://example.com/c" in prs.to_pdf() + held = first.hyperlink + prs.slides.add_slide(prs.slide_layouts[6]) + with pytest.raises(rpptx.StaleElementError, match=r"\.runs\[0\]\.hyperlink\."): + _ = held.address + output = tmp_path / "hyperlinks.pptx" + prs.save(output) + + pptx = pytest.importorskip("pptx", reason="python-pptx is the differential oracle") + runs = pptx.Presentation(output).slides[0].shapes[0].text_frame.paragraphs[0].runs + assert [run.hyperlink.address for run in runs] == ["https://example.com/c", None] + + def build(deck): + frame = deck.slides.add_slide(deck.slide_layouts[6]).shapes.add_textbox(0, 0, 100, 100) + paragraph = frame.text_frame.paragraphs[0] + for text in ("one", "two"): + run = paragraph.add_run() + run.text = text + run.hyperlink.address = f"https://example.com/{text}" + + source = _python_pptx_deck(tmp_path / "python-pptx-links.pptx", build) + prs = rpptx.Presentation(source) + runs = prs.slides[0].shapes[0].text_frame.paragraphs[0].runs + assert [run.hyperlink.address for run in runs] == [ + "https://example.com/one", + "https://example.com/two", + ] + runs[0].hyperlink.address = "https://example.com/two" + runs[1].hyperlink.address = None + retargeted = tmp_path / "python-pptx-links-out.pptx" + prs.save(retargeted) + assert _slide_hyperlink_targets(retargeted.read_bytes()) == ["https://example.com/two"] + runs = pptx.Presentation(retargeted).slides[0].shapes[0].text_frame.paragraphs[0].runs + assert [run.hyperlink.address for run in runs] == ["https://example.com/two", None] + + def test_text_enums_match_python_pptx_member_values_and_xml_tokens(): if importlib.util.find_spec("pptx") is None: pytest.skip("python-pptx oracle is installed only for the differential gate") diff --git a/crates/rpptx-py/tests/typing_smoke.py b/crates/rpptx-py/tests/typing_smoke.py index aa5879cf..d193381b 100644 --- a/crates/rpptx-py/tests/typing_smoke.py +++ b/crates/rpptx-py/tests/typing_smoke.py @@ -29,6 +29,7 @@ ColumnCollection, FillFormat, Font, + Hyperlink, Image, LineFormat, Paragraph, @@ -342,6 +343,14 @@ def exercise_rpptx_z_order_types(slide: Slide) -> None: slide.shapes.move(0, -1) +def exercise_rpptx_hyperlink_types(run: Run) -> None: + hyperlink: Hyperlink = run.hyperlink + hyperlink.address = "https://example.com" + address: str | None = hyperlink.address + hyperlink.address = None + (address,) + + def exercise_rpptx_text_layout_types(presentation: Presentation) -> None: frames: tuple[TextFrameLayout, ...] = presentation.text_layout() narrower: tuple[TextFrameLayout, ...] = presentation.text_layout(width_factor=0.95) @@ -397,6 +406,7 @@ def exercise_rpptx_text_layout_types(presentation: Presentation) -> None: Column() # type: ignore[call-arg] ColumnCollection() # type: ignore[call-arg] Font() # type: ignore[call-arg] + Hyperlink() # type: ignore[call-arg] Paragraph() # type: ignore[call-arg] ParagraphCollection() # type: ignore[call-arg] PlaceholderCollection() # type: ignore[call-arg] diff --git a/crates/rpptx/src/lib.rs b/crates/rpptx/src/lib.rs index 5a878159..7e2aec2e 100644 --- a/crates/rpptx/src/lib.rs +++ b/crates/rpptx/src/lib.rs @@ -39,7 +39,7 @@ use oxml_drawing::table::{CT_Table, CT_TableCell, CT_TableCellProperties, CT_Tab use oxml_drawing::text::CT_TextListStyle; use oxml_drawing::text::{ CT_RegularTextRun, CT_TextBody, CT_TextParagraph, Coordinate32Value, NormalAutofit, - TextAutofit, TextRun, TextWrap, + TextAutofit, TextHyperlink, TextRun, TextWrap, }; pub use oxml_drawing::text::{ CT_TextCharacterProperties, CT_TextParagraphProperties, TextAlignment, TextAnchor, TextBullet, @@ -3193,6 +3193,144 @@ impl Presentation { self.commit_candidate(staged) } + /// Returns the target of one relationship of a slide, the address that a + /// hyperlink naming this relationship id opens. + /// + /// Pass the `r:id` of an `a:hlinkClick`. An external hyperlink returns + /// its URL, and an internal relationship, such as a jump to another + /// slide, returns its stored relative target, as python-pptx `address` + /// does. An unknown slide or id returns `None`. + pub fn hyperlink_address(&self, slide_index: usize, relationship_id: &str) -> Option<&str> { + let record = self.slides.get(slide_index)?; + self.package + .get_part_rels(&record.part_name)? + .get_by_id(relationship_id) + .map(|relationship| relationship.target.as_str()) + } + + /// Points one text run's click hyperlink at an external `address`, or + /// removes it with `None`, as python-pptx `hyperlink.address` does. + /// + /// The run is the zero-based regular run of a paragraph in the ordinary + /// shape whose `p:cNvPr/@id` is `shape_id`, inside groups too. It gets an + /// `a:hlinkClick` naming the slide's external hyperlink relationship to + /// `address`, which is reused when the slide has one. Assigning the + /// current address changes nothing. Every relationship that only the old + /// hyperlink named, its target or a click sound, is removed, so no + /// relationship is left unreferenced. An empty address, one with a + /// control character, or a shape id that is missing, shared, or not an + /// ordinary shape with that run is rejected without change. + pub fn set_run_hyperlink( + &mut self, + slide_index: usize, + shape_id: u32, + paragraph_index: usize, + run_index: usize, + address: Option<&str>, + ) -> Result<()> { + const OPERATION: &str = "set run hyperlink"; + self.require_slide_index(slide_index)?; + if address.is_some_and(|address| { + address.is_empty() + || address.chars().any(|character| { + character.is_control() || matches!(character, '\u{FFFE}' | '\u{FFFF}') + }) + }) { + return Err(invalid_shape_mutation( + OPERATION, + "a hyperlink address must be non-empty text without control characters", + )); + } + let record = &self.slides[slide_index]; + let part_name = record.part_name.clone(); + if shape_id_count( + &record.slide.common_slide_data.shape_tree.children, + shape_id, + ) > 1 + { + return Err(invalid_shape_mutation( + OPERATION, + format!("shape id {shape_id} is not unique on the slide"), + )); + } + let mut slide = record.slide.clone(); + let mut relationships = self + .package + .get_part_rels(&part_name) + .cloned() + .unwrap_or_default(); + let (old, new) = { + let missing = || { + invalid_shape_mutation( + OPERATION, + format!( + "shape id {shape_id} has no ordinary shape run {run_index} in paragraph {paragraph_index}" + ), + ) + }; + let body = find_shape_mut(&mut slide.common_slide_data.shape_tree.children, shape_id) + .and_then(|shape| shape.text_body.as_mut()) + .ok_or_else(missing)?; + let mut paragraph = TextFrame { body } + .into_paragraph_mut(paragraph_index) + .ok_or_else(missing)?; + let mut run = paragraph.run_mut(run_index).ok_or_else(missing)?; + let mut properties = run.properties().cloned().unwrap_or_default(); + let old = properties + .hyperlink_click + .as_ref() + .and_then(|hyperlink| hyperlink.relationship_id.clone()) + .filter(|id| !id.is_empty()); + let current = old + .as_deref() + .and_then(|id| relationships.get_by_id(id)) + .filter(|relationship| { + relationship.rel_type == rel_types::HYPERLINK + && relationship_is_external(relationship) + }) + .map(|relationship| relationship.target.as_str()); + let unchanged = match address { + None => properties.hyperlink_click.is_none(), + Some(address) => current == Some(address), + }; + if unchanged { + return Ok(()); + } + let new = address.map(|address| { + relationships + .items + .iter() + .find(|relationship| { + relationship.rel_type == rel_types::HYPERLINK + && relationship_is_external(relationship) + && relationship.target == address + }) + .map(|relationship| relationship.id.clone()) + .unwrap_or_else(|| relationships.add_external(rel_types::HYPERLINK, address)) + }); + properties.hyperlink_click = new.clone().map(|id| { + let mut hyperlink = TextHyperlink::default(); + hyperlink.relationship_id = Some(id); + hyperlink + }); + run.set_properties(properties); + (old, new) + }; + if old.is_some_and(|old| Some(&old) != new.as_ref()) { + // The old a:hlinkClick can carry more than its r:id, such as an + // a:snd click sound naming an audio relationship. Remove every + // relationship this edit left without a reference. + let before = slide_relationship_ids(&self.slides[slide_index].slide)?; + let after = slide_relationship_ids(&slide)?; + relationships.items.retain(|relationship| { + !before.contains(&relationship.id) || after.contains(&relationship.id) + }); + } + self.slides[slide_index].slide = slide; + self.package.set_part_rels(&part_name, relationships); + Ok(()) + } + /// Moves one immediate slide child so that it ends up at z-order index /// `to_index`, where later children draw on top. /// @@ -4043,6 +4181,37 @@ fn find_picture_mut(children: &mut [ShapeTreeChild], shape_id: u32) -> Option<&m None } +/// Counts the slide children and group members whose `p:cNvPr/@id` is +/// `shape_id`. +fn shape_id_count(children: &[ShapeTreeChild], shape_id: u32) -> usize { + children + .iter() + .map(|child| { + let nested = match child { + ShapeTreeChild::GroupShape(group) => shape_id_count(&group.children, shape_id), + _ => 0, + }; + usize::from(child.non_visual_id() == Some(shape_id)) + nested + }) + .sum() +} + +fn find_shape_mut(children: &mut [ShapeTreeChild], shape_id: u32) -> Option<&mut CT_Shape> { + for child in children { + let id = child.non_visual_id(); + match child { + ShapeTreeChild::Shape(shape) if id == Some(shape_id) => return Some(shape), + ShapeTreeChild::GroupShape(group) => { + if let Some(shape) = find_shape_mut(&mut group.children, shape_id) { + return Some(shape); + } + } + _ => {} + } + } + None +} + fn picture_image_relationship<'a>( picture: &'a CT_Picture, operation: &'static str, diff --git a/crates/rpptx/tests/integration.rs b/crates/rpptx/tests/integration.rs index 5648bb4d..901002bc 100644 --- a/crates/rpptx/tests/integration.rs +++ b/crates/rpptx/tests/integration.rs @@ -14458,6 +14458,204 @@ fn paragraph_run_font_and_bullet_properties_round_trip() { assert_eq!(properties.latin.as_ref().unwrap().typeface, "Carlito"); } +fn two_run_text_box() -> Presentation { + let mut presentation = Presentation::new().unwrap(); + presentation.add_slide(6).unwrap(); + let mut slide = presentation.slide_mut(0).unwrap(); + let mut shape = slide + .add_textbox(Emu(0), Emu(0), Emu(914_400), Emu(914_400)) + .unwrap(); + shape.set_text("first").unwrap(); + shape + .text_frame() + .unwrap() + .paragraph_mut(0) + .unwrap() + .add_run(" second"); + presentation +} + +fn run_hyperlink_id(presentation: &Presentation, run: usize) -> Option { + presentation + .slide(0) + .unwrap() + .shape(0) + .unwrap() + .text_frame() + .unwrap() + .paragraph(0) + .unwrap() + .run(run) + .unwrap() + .properties() + .and_then(|properties| properties.hyperlink_click.as_ref()) + .and_then(|hyperlink| hyperlink.relationship_id.clone()) +} + +fn slide_hyperlink_targets(presentation: &Presentation) -> Vec<(String, String)> { + open_opc(&presentation.to_bytes().unwrap(), "slide hyperlinks") + .get_part_rels("/ppt/slides/slide1.xml") + .unwrap() + .get_all_by_type(rel_types::HYPERLINK) + .into_iter() + .map(|relationship| { + assert_eq!(relationship.target_mode.as_deref(), Some("External")); + (relationship.id.clone(), relationship.target.clone()) + }) + .collect() +} + +#[test] +fn run_hyperlinks_reuse_resolve_and_prune_their_relationships() { + let mut presentation = two_run_text_box(); + let id = first_slide_shape_id(&presentation, 0); + let before = presentation.to_bytes().unwrap(); + for address in ["", "https://example.com/\nnext", "\u{FFFE}"] { + assert!(matches!( + presentation.set_run_hyperlink(0, id, 0, 0, Some(address)), + Err(Error::InvalidShapeMutation { .. }) + )); + } + for (slide, shape, paragraph, run) in [ + (1, id, 0, 0), + (0, id + 1, 0, 0), + (0, id, 1, 0), + (0, id, 0, 2), + ] { + assert!( + presentation + .set_run_hyperlink(slide, shape, paragraph, run, Some("https://example.com/a")) + .is_err() + ); + } + presentation.set_run_hyperlink(0, id, 0, 0, None).unwrap(); + assert_eq!(presentation.to_bytes().unwrap(), before); + + let a = "https://example.com/a?x=1&y=2"; + presentation + .set_run_hyperlink(0, id, 0, 0, Some(a)) + .unwrap(); + presentation + .set_run_hyperlink(0, id, 0, 1, Some(a)) + .unwrap(); + let first = run_hyperlink_id(&presentation, 0).unwrap(); + assert_eq!(run_hyperlink_id(&presentation, 1), Some(first.clone())); + assert_eq!(presentation.hyperlink_address(0, &first), Some(a)); + assert_eq!( + slide_hyperlink_targets(&presentation), + [(first.clone(), a.to_owned())] + ); + let linked = presentation.to_bytes().unwrap(); + presentation + .set_run_hyperlink(0, id, 0, 0, Some(a)) + .unwrap(); + assert_eq!(presentation.to_bytes().unwrap(), linked); + + presentation + .set_run_hyperlink(0, id, 0, 0, Some("https://example.com/b")) + .unwrap(); + let second = run_hyperlink_id(&presentation, 0).unwrap(); + assert_ne!(first, second); + assert_eq!(slide_hyperlink_targets(&presentation).len(), 2); + presentation.set_run_hyperlink(0, id, 0, 1, None).unwrap(); + assert_eq!(run_hyperlink_id(&presentation, 1), None); + assert_eq!(presentation.hyperlink_address(0, &first), None); + presentation + .set_run_hyperlink(0, id, 0, 0, Some("https://example.com/c")) + .unwrap(); + let third = run_hyperlink_id(&presentation, 0).unwrap(); + assert_eq!( + slide_hyperlink_targets(&presentation), + [(third.clone(), "https://example.com/c".to_owned())] + ); + let layout = open_opc(&presentation.to_bytes().unwrap(), "hyperlink layout") + .get_part_rels("/ppt/slides/slide1.xml") + .unwrap() + .get_by_type(rel_types::SLIDE_LAYOUT) + .unwrap() + .clone(); + assert_eq!( + presentation.hyperlink_address(0, &layout.id), + Some(layout.target.as_str()) + ); + assert_eq!(presentation.hyperlink_address(1, &third), None); + + let saved = presentation.to_bytes().unwrap(); + let reopened = Presentation::from_bytes(&saved).unwrap(); + assert!(reopened.validate().is_empty(), "{:?}", reopened.validate()); + assert_eq!( + ( + run_hyperlink_id(&reopened, 0), + run_hyperlink_id(&reopened, 1) + ), + (Some(third.clone()), None) + ); + assert_eq!( + reopened.hyperlink_address(0, &third), + Some("https://example.com/c") + ); + + let grouped = r#"inner"#; + let mut grouped = with_first_slide_children(&reopened, grouped, &[]); + grouped + .set_run_hyperlink(0, 51, 0, 0, Some("https://example.com/c")) + .unwrap(); + assert_eq!(slide_hyperlink_targets(&grouped).len(), 1); + let duplicate = format!( + r#"x"# + ); + let mut duplicated = with_first_slide_children(&reopened, &duplicate, &[]); + assert!( + duplicated + .set_run_hyperlink(0, id, 0, 0, None) + .unwrap_err() + .to_string() + .contains("not unique") + ); + + let sounding = r#"click"#; + let mut sounding = with_first_slide_children( + &reopened, + sounding, + &[ + ("rId80", rel_types::HYPERLINK, "https://example.com/sound"), + ("rId90", rel_types::AUDIO, "../media/click.wav"), + ], + ); + sounding.set_run_hyperlink(0, 60, 0, 0, None).unwrap(); + let relationships = open_opc(&sounding.to_bytes().unwrap(), "click sound") + .get_part_rels("/ppt/slides/slide1.xml") + .unwrap() + .clone(); + assert!(relationships.get_by_id("rId80").is_none()); + assert!(relationships.get_by_id("rId90").is_none()); + assert!(relationships.get_by_id(&third).is_some()); +} + +#[test] +#[ignore = "requires uv and pinned python-pptx 1.0.2"] +fn run_hyperlinks_read_back_in_pinned_python_pptx() { + let mut presentation = two_run_text_box(); + let id = first_slide_shape_id(&presentation, 0); + presentation + .set_run_hyperlink(0, id, 0, 0, Some("https://example.com/a?x=1&y=2")) + .unwrap(); + let records = python_pptx_1_0_2_reads( + &presentation.to_bytes().unwrap(), + "run-hyperlinks", + r#" +import sys +import pptx +from pptx import Presentation + +assert pptx.__version__ == "1.0.2", pptx.__version__ +runs = Presentation(sys.argv[1]).slides[0].shapes[0].text_frame.paragraphs[0].runs +print([run.hyperlink.address for run in runs]) +"#, + ); + assert_eq!(records, "['https://example.com/a?x=1&y=2', None]\n"); +} + fn plain_shape_fixture_bytes(from: &str, to: &str) -> Vec { let mut package = fixture_package(); let original = String::from_utf8(package.get_part(SLIDE_TWO_PART).unwrap().to_vec()).unwrap(); diff --git a/docs/hld/06-presentationml-model.md b/docs/hld/06-presentationml-model.md index e464bdad..f5ab327f 100644 --- a/docs/hld/06-presentationml-model.md +++ b/docs/hld/06-presentationml-model.md @@ -513,6 +513,35 @@ exactly the children between its two indices, and reparses, the technique schema-final `p:extLst` content therefore keep their bytes and their place among the other children. +The owning facade also resolves and writes run hyperlinks, because their +relationships belong to the slide part rather than to a borrowed run handle: + +```rust +pub fn hyperlink_address(&self, slide_index: usize, relationship_id: &str) -> Option<&str>; +pub fn set_run_hyperlink( + &mut self, + slide_index: usize, + shape_id: u32, + paragraph_index: usize, + run_index: usize, + address: Option<&str>, +) -> Result<()>; +``` + +`hyperlink_address` returns the target of the relationship an +`a:hlinkClick/@r:id` names, the URL of an external hyperlink or the stored +relative target of an internal one, as python-pptx `address` does. +`set_run_hyperlink` finds the ordinary shape by its `p:cNvPr/@id`, inside +groups too, and rejects an id that another slide child shares. The regular run +gets a fresh `a:hlinkClick` that names the slide's external hyperlink +relationship to the address, reused when the slide has one, and `None` removes +it. Assigning the current address changes nothing. Validation reports an +unreferenced hyperlink relationship, so the relationship the old hyperlink +named is removed once no other element of the slide names it, and one call +changes the run and the relationships together. An empty address, a control +character, or a missing shape, paragraph, or run leaves the slide and its +relationships unchanged. + An ordinary shape has canonical non-visual properties, a typed transform, preset geometry, and a minimal text body. `add_shape` keeps the string API but accepts only names in the generated table of all 187 ECMA preset shapes. An diff --git a/docs/hld/10-bindings-spec.md b/docs/hld/10-bindings-spec.md index 7deaf510..23d4adfe 100644 --- a/docs/hld/10-bindings-spec.md +++ b/docs/hld/10-bindings-spec.md @@ -359,7 +359,14 @@ as a column width keeps the frame width. A height that is not positive raises reports `none`, `normal`, or `shape` when the body carries an explicit choice. `Run.font` reads the run's direct Latin name, size, and sRGB colour, while the `Run.text` setter replaces only that run's text and preserves its typed and -unmodelled properties. +unmodelled properties. `Run.hyperlink` returns a live `Hyperlink` whose +`address` reads the target of the run's `a:hlinkClick`, or `None`. Assigning +an address goes through the native `set_run_hyperlink`, which reuses the +slide's relationship to the same address and removes the old relationship +once nothing on the slide names it, so retargeting does not grow the part. +`None` or an empty string removes the hyperlink, as in python-pptx, an address +with a control character raises `RpptxError`, and the write does not advance +the revision. Text formatting follows python-pptx names and value types. Every property reads the direct value only, `None` when the element or attribute is absent, From 6509eaf8da8867cc08b8d82ebb5daf602dd7a2f4 Mon Sep 17 00:00:00 2001 From: Hadrien Mary Date: Sun, 27 Sep 2026 20:37:46 +0200 Subject: [PATCH 6/6] Re-record the archive measurements of rpptx and rpptx-oxml The table, crop, z-order and hyperlink changes add public methods and their documentation to rpptx and a shape tree move to rpptx-oxml, so both packaged archives grew. The measurements in scripts/readme_doctests.py and the archive rows of both READMEs now match cargo package, which the Docs job checks. GitHub issue #169. --- crates/rpptx-oxml/README.md | 2 +- crates/rpptx/README.md | 2 +- scripts/readme_doctests.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/rpptx-oxml/README.md b/crates/rpptx-oxml/README.md index c84b6a68..3f113ebe 100644 --- a/crates/rpptx-oxml/README.md +++ b/crates/rpptx-oxml/README.md @@ -17,7 +17,7 @@ notes, comments, diagrams, and timing data. | Measurement | Value | Version | Platform | Build mode | Input | Command | Statistic | Measured on | |---|---|---|---|---|---|---|---|---| -| Crates.io archive: rpptx-oxml | 153,216 compressed bytes, 1,042,644 member bytes, 20 members | 0.12.1 | macOS 26.6.2, Apple M5 Max, arm64 | `cargo package --locked --no-verify` | Tracked `rpptx-oxml` package inventory | `python3 scripts/readme_doctests.py --record-measurements` | gzip archive bytes, tar member bytes, tar member count | 2026-09-19 | +| Crates.io archive: rpptx-oxml | 154,086 compressed bytes, 1,046,474 member bytes, 20 members | 0.12.1 | macOS 26.6.2, Apple M5 Max, arm64 | `cargo package --locked --no-verify` | Tracked `rpptx-oxml` package inventory | `python3 scripts/readme_doctests.py --record-measurements` | gzip archive bytes, tar member bytes, tar member count | 2026-09-19 | ## Use it when diff --git a/crates/rpptx/README.md b/crates/rpptx/README.md index 34eb7caa..91605a62 100644 --- a/crates/rpptx/README.md +++ b/crates/rpptx/README.md @@ -22,7 +22,7 @@ presentation, notes, handout, PDF, and animation outputs. | Measurement | Value | Version | Platform | Build mode | Input | Command | Statistic | Measured on | |---|---|---|---|---|---|---|---|---| -| Crates.io archive: rpptx | 407,658 compressed bytes, 2,122,094 member bytes, 16 members | 0.12.1 | macOS 26.6.2, Apple M5 Max, arm64 | `cargo package --locked --no-verify` | Tracked `rpptx` package inventory | `python3 scripts/readme_doctests.py --record-measurements` | gzip archive bytes, tar member bytes, tar member count | 2026-09-26 | +| Crates.io archive: rpptx | 414,268 compressed bytes, 2,157,689 member bytes, 16 members | 0.12.1 | macOS 26.6.2, Apple M5 Max, arm64 | `cargo package --locked --no-verify` | Tracked `rpptx` package inventory | `python3 scripts/readme_doctests.py --record-measurements` | gzip archive bytes, tar member bytes, tar member count | 2026-09-26 | ## Use it when diff --git a/scripts/readme_doctests.py b/scripts/readme_doctests.py index e24c643b..36a0602b 100644 --- a/scripts/readme_doctests.py +++ b/scripts/readme_doctests.py @@ -390,11 +390,11 @@ class ReadmeCase: "rdocx-opc": (3_655, 9_668, 6), "rdocx-oxml": (367_500, 2_380_047, 32), "rdocx-pdf": (8_111, 26_758, 6), - "rpptx": (407_658, 2_122_094, 16), + "rpptx": (414_268, 2_157_689, 16), "rpptx-chart": (6_648, 21_136, 6), "rpptx-cli": (36_709, 159_585, 8), "rpptx-layout": (79_109, 458_112, 11), - "rpptx-oxml": (153_216, 1_042_644, 20), + "rpptx-oxml": (154_086, 1_046_474, 20), "rpptx-render": (59_928, 329_994, 8), } PACKAGE_VERSIONS = {