Compare commits

..
Author SHA1 Message Date
Dylan M. Taylor 9c516fdf5a Merge branch 'main' into fix/sec-model-table-cells 2026-11-11 20:01:29 +00:00
Dylan M. Taylorandautocraft-bot[bot] <354215646+autocraft-bot[bot]@users.noreply.github.com> c6846f6a75 Photo album: apply the album limits when reopening an album (#1755)
* engine: apply the photo album limits when reopening an album

* address review: refuse an album Update when the editor seed omitted items, instead of deleting them

* Apply review fixes

- crates/engine/src/cmd/insert/album.rs

* address review: refuse an album replace when a slide past the item limit holds other content

* address review: notes and comments on a shapeless slide past the item limit block an album replace

* address review: slide background, transition, animations, colour map and tags count as slide content

---------

Co-authored-by: autocraft-bot[bot] <354215646+autocraft-bot[bot]@users.noreply.github.com>
2026-11-11 14:59:47 -05:00
Dylan M. Taylorandautocraft-bot[bot] <354215646+autocraft-bot[bot]@users.noreply.github.com> a3f2f215b0 Metafile: stop shading on a full canvas, charge nested pictures and clip regions to the budgets (#1777)
* Metafile: stop shading on a full canvas, charge nested pictures and clip regions to the budgets

* address review: count shared tiles once, include text advances, check clip region before keeping an item

* address review: count EMF+ object bytes against the canvas byte budget

* Apply review fixes

- crates/metafile/src/canvas.rs
- crates/metafile/src/tests.rs

* Apply review fixes

- crates/metafile/src/canvas.rs

* address review: count pieces of a continued EMF+ object against the shared byte budget

* Apply review fixes

- crates/metafile/src/canvas.rs

* address review: count the current and saved clips against the shared byte budget

---------

Co-authored-by: autocraft-bot[bot] <354215646+autocraft-bot[bot]@users.noreply.github.com>
2026-11-11 14:59:32 -05:00
Dylan M. Taylor 4106e85ad7 address review: clip merge spans to the rows and columns left after the cell cap 2026-11-11 14:48:18 +00:00
Dylan M. Taylor 761c1428bb Merge branch 'main' into fix/sec-model-table-cells 2026-11-11 10:37:29 +00:00
Dylan M. Taylor 9b77b29dcb address review: refuse table edits past the cell cap, warn on oversized PPTX tables, fail export instead of trimming 2026-11-11 04:29:28 +00:00
Dylan M. Taylor f2a12bfde7 address review: build the table measurement copy from only the cells that survive normalization 2026-11-11 02:17:34 +00:00
Dylan M. Taylor 33108d5fb9 address review: make merge work-bound test use in-range coordinates 2026-11-10 22:45:36 +00:00
Dylan M. Taylor b396f94f0d address review: bound merge expansion work; measure a normalized copy of an unnormalized table 2026-11-03 15:19:12 +00:00
Dylan M. Taylor 469eede715 model: cap a table's grid and merge area before edits allocate or walk it 2026-11-03 11:20:29 +00:00
19 changed files with 692 additions and 77 deletions
Generated
+24 -24
View File
@@ -1038,7 +1038,7 @@ checksum = "0c87e182de0887fd5361989c677c4e8f5000cd9491d6d563161a8f3a5519fc7f"
[[package]]
name = "deckcraft"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"arboard",
"chrono",
@@ -1060,7 +1060,7 @@ dependencies = [
[[package]]
name = "deckcraft-anim"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"deckcraft-geom",
"deckcraft-model",
@@ -1069,7 +1069,7 @@ dependencies = [
[[package]]
name = "deckcraft-av1"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"deckcraft-bitstream",
"rayon",
@@ -1078,14 +1078,14 @@ dependencies = [
[[package]]
name = "deckcraft-bitstream"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"craft-media",
]
[[package]]
name = "deckcraft-cli"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"deckcraft-engine",
"deckcraft-mcp",
@@ -1095,14 +1095,14 @@ dependencies = [
[[package]]
name = "deckcraft-color"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"serde",
]
[[package]]
name = "deckcraft-engine"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"chrono",
"deckcraft-anim",
@@ -1128,7 +1128,7 @@ dependencies = [
[[package]]
name = "deckcraft-fonts"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"epaint_default_fonts",
"harfrust 0.13.3",
@@ -1140,7 +1140,7 @@ dependencies = [
[[package]]
name = "deckcraft-format"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"deckcraft-model",
"serde",
@@ -1151,7 +1151,7 @@ dependencies = [
[[package]]
name = "deckcraft-geom"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"kurbo",
"serde",
@@ -1159,7 +1159,7 @@ dependencies = [
[[package]]
name = "deckcraft-h264"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"deckcraft-bitstream",
"rayon",
@@ -1168,7 +1168,7 @@ dependencies = [
[[package]]
name = "deckcraft-hevc"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"deckcraft-bitstream",
"rayon",
@@ -1177,7 +1177,7 @@ dependencies = [
[[package]]
name = "deckcraft-mcp"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"deckcraft-engine",
"deckcraft-render",
@@ -1187,7 +1187,7 @@ dependencies = [
[[package]]
name = "deckcraft-media"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"craft-audio",
"craft-media",
@@ -1203,7 +1203,7 @@ dependencies = [
[[package]]
name = "deckcraft-metafile"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"image",
"kurbo",
@@ -1212,7 +1212,7 @@ dependencies = [
[[package]]
name = "deckcraft-model"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"deckcraft-color",
"deckcraft-geom",
@@ -1224,7 +1224,7 @@ dependencies = [
[[package]]
name = "deckcraft-pdf"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"craft-pdf",
"deckcraft-fonts",
@@ -1242,7 +1242,7 @@ dependencies = [
[[package]]
name = "deckcraft-pptx"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"deckcraft-color",
"deckcraft-engine",
@@ -1258,7 +1258,7 @@ dependencies = [
[[package]]
name = "deckcraft-render"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"deckcraft-anim",
"deckcraft-color",
@@ -1277,7 +1277,7 @@ dependencies = [
[[package]]
name = "deckcraft-text"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"chrono",
"deckcraft-color",
@@ -1291,7 +1291,7 @@ dependencies = [
[[package]]
name = "deckcraft-ui-egui"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"craft-ui",
"craft-update",
@@ -1317,7 +1317,7 @@ dependencies = [
[[package]]
name = "deckcraft-vp9"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"deckcraft-bitstream",
"rayon",
@@ -1326,7 +1326,7 @@ dependencies = [
[[package]]
name = "deckcraft-web"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"craft-web-api",
"craft-web-host",
@@ -6311,7 +6311,7 @@ checksum = "9440ea3e5aeabb0ac63af70daf835274065238cdd0cec83418f417eae38bacee"
[[package]]
name = "xtask"
version = "0.18.1"
version = "0.18.0"
dependencies = [
"serde_json",
]
+1 -1
View File
@@ -4,7 +4,7 @@ members = ["crates/*", "apps/*", "xtask"]
default-members = ["crates/*", "apps/deckcraft", "apps/deckcraft-cli"]
[workspace.package]
version = "0.18.1"
version = "0.18.0"
edition = "2024"
license = "MIT OR Apache-2.0"
rust-version = "1.95"
+3
View File
@@ -148,6 +148,9 @@ fn warn_if_damaged(path: &str, opened: &serde_json::Value) {
if slides > 0 {
eprintln!("deckcraft-cli: warning: {path}: {slides} slide{} couldn't be read and left out", if slides == 1 { "" } else { "s" });
}
for w in opened.get("importWarnings").and_then(|v| v.as_array()).into_iter().flatten().filter_map(|w| w.as_str()) {
eprintln!("deckcraft-cli: warning: {path}: {w}");
}
if media > 0 {
eprintln!("deckcraft-cli: warning: {path}: {media} picture or media file{} couldn't be read", if media == 1 { "" } else { "s" });
}
+9
View File
@@ -433,6 +433,15 @@ pub(crate) fn shape_of(s: &Session, id: ShapeId) -> Result<deckcraft_model::Shap
pub fn xfrm_of(doc: &deckcraft_model::Presentation, sel: &crate::Selection, shape: &deckcraft_model::Shape) -> Xfrm {
let mut x = shape.xfrm.unwrap_or_else(|| ctx_xfrm(doc, sel, shape));
if let deckcraft_model::ShapeKind::Table(t) = &shape.kind {
// A loaded table may not be normalized yet: measure the bounded grid an edit will
// leave, not every cell the file stored.
let normalized;
let t = if t.is_normalized() {
t
} else {
normalized = t.normalized();
&normalized
};
let drawn: f64 = table_drawn_heights(doc, sel, t).iter().sum();
if drawn.is_finite() && drawn > x.h {
x.h = drawn;
+17 -9
View File
@@ -139,11 +139,13 @@ pub struct Damage {
pub media: Vec<String>,
/// Slides a `.pptx` lists but that couldn't be read, so they are not in the presentation.
pub slides: Vec<String>,
/// Content a `.pptx` had that was cut to fit a limit; the file still opened.
pub warnings: Vec<String>,
}
impl Damage {
pub fn is_empty(&self) -> bool {
self.media.is_empty() && self.slides.is_empty()
self.media.is_empty() && self.slides.is_empty() && self.warnings.is_empty()
}
}
@@ -151,17 +153,17 @@ impl Damage {
pub fn open_checked(name: &str, bytes: &[u8]) -> Result<(Presentation, Damage)> {
if deckcraft_format::sniff(bytes) {
let (p, media) = deckcraft_format::load_checked(bytes).map_err(|e| EngineError::Other(e.to_string()))?;
return Ok((p, Damage { media, slides: vec![] }));
return Ok((p, Damage { media, ..Default::default() }));
}
open_other(name, bytes).map(|(p, slides)| (p, Damage { media: vec![], slides }))
open_other(name, bytes).map(|(p, slides, warnings)| (p, Damage { slides, warnings, ..Default::default() }))
}
fn open_other(name: &str, bytes: &[u8]) -> Result<(Presentation, Vec<String>)> {
fn open_other(name: &str, bytes: &[u8]) -> Result<(Presentation, Vec<String>, Vec<String>)> {
if deckcraft_pptx::sniff(bytes) {
let (mut p, lost_slides) = deckcraft_pptx::import_checked(bytes).map_err(|e| EngineError::Other(e.to_string()))?;
let (mut p, lost_slides, warnings) = deckcraft_pptx::import_report(bytes).map_err(|e| EngineError::Other(e.to_string()))?;
deckcraft_format::repair(&mut p);
probe_clips(&mut p);
return Ok((p, lost_slides));
return Ok((p, lost_slides, warnings));
}
if deckcraft_pptx::is_encrypted(bytes) {
return Err(EngineError::Other(format!("{name} is password-protected. DeckCraft can't open password-protected presentations yet.")));
@@ -180,7 +182,7 @@ fn open_other(name: &str, bytes: &[u8]) -> Result<(Presentation, Vec<String>)> {
if p.slides.is_empty() {
p = defaults::new_presentation(None);
}
return Ok((p, vec![]));
return Ok((p, vec![], vec![]));
}
Err(EngineError::Other(format!("{name}: not a presentation DeckCraft can read")))
}
@@ -276,7 +278,7 @@ fn adopt_loaded(s: &mut Session, f: LoadedFile) -> Result<Value> {
let st = s.doc()?;
Ok(json!({
"document": i, "slides": st.doc.slides.len(), "missingFonts": st.missing_fonts,
"damagedMedia": damaged.media, "lostSlides": damaged.slides,
"damagedMedia": damaged.media, "lostSlides": damaged.slides, "importWarnings": damaged.warnings,
}))
}
@@ -311,7 +313,9 @@ fn open_bytes(s: &mut Session, p: &Value) -> Result<Value> {
let (doc, damaged) = open_checked(name, &data)?;
let i = add_opened(s, name, None, doc);
report_damage(s, &damaged)?;
Ok(json!({"document": i, "missingFonts": s.doc()?.missing_fonts, "damagedMedia": damaged.media, "lostSlides": damaged.slides}))
Ok(
json!({"document": i, "missingFonts": s.doc()?.missing_fonts, "damagedMedia": damaged.media, "lostSlides": damaged.slides, "importWarnings": damaged.warnings}),
)
}
/// Tell the user which pictures, media and slides a damaged file lost, and mark the copy repaired.
@@ -339,6 +343,10 @@ fn report_damage(s: &mut Session, damaged: &Damage) -> Result<()> {
if n == 1 { "was" } else { "were" },
));
}
for w in &damaged.warnings {
text.push_str(w);
text.push_str(". ");
}
text.push_str("It was opened as a repaired copy; save it under a new name to keep the original file.");
s.ui_requests.push(crate::UiRequest::Message { text });
Ok(())
+144 -6
View File
@@ -80,6 +80,12 @@ pub(super) fn photo_album_edit(s: &mut Session, p: &Value) -> Result<Value> {
let opts = album_opts(p)?;
let items = album_items(p)?;
let (id, ids) = album_target(s, p)?;
// The editor is seeded within the album limits; replacing the album from a trimmed seed would
// delete what it left out, so refuse rather than lose it.
let st = s.doc()?;
if items_from_slides(&st.doc, &album_slides(st, &ids)).1 {
return Err(bad("insert.photoAlbumEdit", "this album holds more than the editor can carry; editing it would delete the rest"));
}
commit_album(s, items, opts, Some((id, ids)))
}
@@ -545,6 +551,19 @@ fn album_target(s: &Session, p: &Value) -> Result<(u32, Vec<u64>)> {
Err(bad("insert.photoAlbumEdit", "this slide is not part of a photo album"))
}
/// Whether a slide holds anything besides album items: shapes, notes, comments, a background, a
/// transition, animations, a colour map or tags.
fn has_slide_content(s: &deckcraft_model::Slide) -> bool {
!s.shapes.is_empty()
|| !s.notes.text().trim().is_empty()
|| !s.comments.is_empty()
|| s.background.is_some()
|| s.transition.is_some()
|| !s.animations.is_empty()
|| s.color_map.is_some()
|| !s.tags.is_empty()
}
fn pic_count(slide: &deckcraft_model::Slide) -> usize {
slide.shapes.iter().filter(|sh| matches!(sh.kind, ShapeKind::Picture { .. })).count()
}
@@ -592,9 +611,16 @@ fn infer_album(slides: &[&deckcraft_model::Slide], theme_now: &str) -> (String,
(layout, frame.into(), captions, any && gray, theme_now.to_string())
}
fn items_from_slides(doc: &deckcraft_model::Presentation, slides: &[&deckcraft_model::Slide]) -> Vec<Value> {
/// The album's items for the dialog, within the limits `insert.photoAlbum` itself takes (items,
/// bytes per picture and in all), checked before a picture is encoded: an album slide set can
/// name one large picture many times.
///
/// The flag is true when something was left out for a limit.
fn items_from_slides(doc: &deckcraft_model::Presentation, slides: &[&deckcraft_model::Slide]) -> (Vec<Value>, bool) {
let mut items = Vec::new();
for slide in slides {
let mut total = 0usize;
let mut omitted = false;
'slides: for slide in slides {
let pics: Vec<&deckcraft_model::Shape> = slide.shapes.iter().filter(|sh| matches!(sh.kind, ShapeKind::Picture { .. })).collect();
if pics.is_empty() {
let text = slide
@@ -605,13 +631,27 @@ fn items_from_slides(doc: &deckcraft_model::Presentation, slides: &[&deckcraft_m
.find(|t| !t.trim().is_empty())
.unwrap_or_default();
if !text.trim().is_empty() {
if items.len() >= ALBUM_MAX_ITEMS {
omitted = true;
break 'slides;
}
items.push(json!({"text": text.trim()}));
} else if items.len() >= ALBUM_MAX_ITEMS && has_slide_content(slide) {
// Past the item limit, a slide with other content would be deleted by a replace.
omitted = true;
break 'slides;
}
continue;
}
for sh in pics {
let ShapeKind::Picture { fill } = &sh.kind else { continue };
let media = doc.media(fill.media);
let size = media.map_or(0, |m| m.data.len());
if items.len() >= ALBUM_MAX_ITEMS || size > ALBUM_MAX_BYTES || total.saturating_add(size) > ALBUM_MAX_TOTAL {
omitted = true;
break 'slides;
}
total += size;
let name = if !sh.descr.is_empty() {
sh.descr.clone()
} else {
@@ -626,14 +666,19 @@ fn items_from_slides(doc: &deckcraft_model::Presentation, slides: &[&deckcraft_m
}));
}
}
items
(items, omitted)
}
/// The slides of album `ids` as the editor would seed them.
fn album_slides<'a>(st: &'a crate::DocState, ids: &[u64]) -> Vec<&'a deckcraft_model::Slide> {
ids.iter().filter_map(|sid| u32::try_from(*sid).ok()).filter_map(|n| st.doc.slide(deckcraft_model::SlideId(n))).collect()
}
fn album_seed(s: &Session) -> Result<Value> {
let (id, ids) = album_target(s, &Value::Null)?;
let st = s.doc()?;
let slides: Vec<&deckcraft_model::Slide> =
ids.iter().filter_map(|sid| u32::try_from(*sid).ok()).filter_map(|n| st.doc.slide(deckcraft_model::SlideId(n))).collect();
let slides = album_slides(st, &ids);
let (items, _) = items_from_slides(&st.doc, &slides);
let theme_now = st.doc.masters.first().map(|m| m.theme.name.clone()).filter(|n| !n.is_empty()).unwrap_or_else(|| "DeckCraft".into());
let inferred = infer_album(&slides, &theme_now);
let mark = slides.first().and_then(|sl| sl.raw_ext.as_deref().and_then(parse_album_mark));
@@ -655,7 +700,7 @@ fn album_seed(s: &Session) -> Result<Value> {
"captions": captions,
"grayscale": grayscale,
"theme": theme,
"items": items_from_slides(&st.doc, &slides),
"items": items,
}))
}
@@ -722,3 +767,96 @@ fn insert_album(s: &mut Session, items: Vec<AlbumItem>, opts: &AlbumOpts, at: Op
s.execute("slide.go", &json!({"index": first}))?;
Ok(json!({"slides": slides, "first": first, "pictures": pictures, "album": album_id}))
}
#[cfg(test)]
mod seed_tests {
use std::sync::Arc;
use deckcraft_model::style::PictureFill;
use deckcraft_model::{Shape, ShapeId, defaults};
use super::*;
#[test]
fn reopening_an_album_seeds_at_most_the_items_an_album_takes() {
// One picture named by 500 shapes: the dialog gets 48 items, not 500 encoded copies.
let mut p = defaults::new_presentation(None);
let img = p.add_media("a.png", "image/png", vec![1; 64 * 1024]).unwrap();
let pic = |k: u32| Shape {
id: ShapeId(10_000 + k),
kind: ShapeKind::Picture { fill: PictureFill { media: img, ..Default::default() } },
..Default::default()
};
Arc::make_mut(&mut p.slides[0]).shapes = (0..500).map(pic).collect();
let slides: Vec<&deckcraft_model::Slide> = p.slides.iter().map(|s| s.as_ref()).collect();
let (items, omitted) = items_from_slides(&p, &slides);
assert_eq!(items.len(), ALBUM_MAX_ITEMS);
assert!(omitted);
let few: Vec<&deckcraft_model::Slide> = Vec::new();
assert!(!items_from_slides(&p, &few).1);
}
/// An album too big for the editor's seed cannot be replaced from that seed: the replace would
/// delete what the seed left out.
#[test]
fn editing_an_album_the_editor_cannot_hold_is_refused() {
let mut s = crate::Session::new();
s.execute("file.new", &json!({})).unwrap();
let texts: Vec<Value> = (0..ALBUM_MAX_ITEMS).map(|i| json!({"text": format!("t{i}")})).collect();
s.execute("insert.photoAlbum", &json!({"pictures": texts, "layout": "1"})).unwrap();
let st = s.doc_mut().unwrap();
let doc = Arc::make_mut(&mut st.doc);
let mut extra = doc.slides.last().unwrap().as_ref().clone();
extra.id = deckcraft_model::SlideId(doc.alloc_id().unwrap());
doc.slides.push(Arc::new(extra));
let n = doc.slides.len();
let e = s.execute("insert.photoAlbumEdit", &json!({"pictures": [{"text": "x"}], "layout": "1"})).unwrap_err().to_string();
assert!(e.contains("delete the rest"), "{e}");
assert_eq!(s.doc().unwrap().doc.slides.len(), n);
}
/// Past the item limit, a slide holding only a drawing counts as content the replace would delete.
#[test]
fn editing_an_album_with_a_drawing_slide_past_the_limit_is_refused() {
let mut s = crate::Session::new();
s.execute("file.new", &json!({})).unwrap();
let texts: Vec<Value> = (0..ALBUM_MAX_ITEMS).map(|i| json!({"text": format!("t{i}")})).collect();
s.execute("insert.photoAlbum", &json!({"pictures": texts, "layout": "1"})).unwrap();
let st = s.doc_mut().unwrap();
let doc = Arc::make_mut(&mut st.doc);
let mut extra = doc.slides.last().unwrap().as_ref().clone();
extra.id = deckcraft_model::SlideId(doc.alloc_id().unwrap());
extra.shapes = vec![Shape { id: ShapeId(90_000), ..Default::default() }];
doc.slides.push(Arc::new(extra));
let e = s.execute("insert.photoAlbumEdit", &json!({"pictures": [{"text": "x"}], "layout": "1"})).unwrap_err().to_string();
assert!(e.contains("delete the rest"), "{e}");
}
/// Notes or comments on a shapeless slide past the limit are content a replace would delete.
#[test]
fn editing_an_album_with_a_notes_only_slide_past_the_limit_is_refused() {
let mut s = crate::Session::new();
s.execute("file.new", &json!({})).unwrap();
let texts: Vec<Value> = (0..ALBUM_MAX_ITEMS).map(|i| json!({"text": format!("t{i}")})).collect();
s.execute("insert.photoAlbum", &json!({"pictures": texts, "layout": "1"})).unwrap();
let st = s.doc_mut().unwrap();
let doc = Arc::make_mut(&mut st.doc);
let mut extra = doc.slides.last().unwrap().as_ref().clone();
extra.id = deckcraft_model::SlideId(doc.alloc_id().unwrap());
extra.shapes.clear();
extra.notes = deckcraft_model::text::TextBody::from_text("speaker notes");
doc.slides.push(Arc::new(extra));
let e = s.execute("insert.photoAlbumEdit", &json!({"pictures": [{"text": "x"}], "layout": "1"})).unwrap_err().to_string();
assert!(e.contains("delete the rest"), "{e}");
}
/// A shapeless slide with a background, transition, colour map or tags counts as content; a plain empty one does not.
#[test]
fn slide_level_settings_count_as_slide_content() {
use deckcraft_model::Slide;
assert!(!has_slide_content(&Slide::default()));
assert!(has_slide_content(&Slide { tags: vec![("k".into(), "v".into())], ..Default::default() }));
assert!(has_slide_content(&Slide { color_map: Some(vec![("bg1".into(), "lt1".into())]), ..Default::default() }));
assert!(has_slide_content(&Slide { transition: Some(Default::default()), ..Default::default() }));
}
}
+17 -4
View File
@@ -286,6 +286,12 @@ fn with_table(s: &mut Session, p: &Value, cmd: &str, f: impl Fn(&mut Table) -> R
};
let r = edit_shapes(s, &json!({"id": id}), cmd, |sh| match &mut sh.kind {
ShapeKind::Table(t) => {
// Edits walk the grid: bound it first (a loaded table need not be normalized yet).
// An over-cap table is refused, not trimmed: a trim would delete rows while reporting success.
if t.over_cap() {
return Err(bad(cmd, format!("the table is larger than {} cells; edit refused", deckcraft_model::table::MAX_CELLS)));
}
t.normalize();
f(t)?;
t.normalize();
let (w, h) = (t.width(), t.height());
@@ -319,10 +325,17 @@ fn rows_cols(s: &mut Session, p: &Value, op: &str, name: &'static str) -> Result
let op = op.to_string();
with_table(s, p, name, move |t| {
match op.as_str() {
"rowAbove" => t.insert_row(r),
"rowBelow" => t.insert_row(r.saturating_add(1)),
"colLeft" => t.insert_col(c),
"colRight" => t.insert_col(c.saturating_add(1)),
"rowAbove" | "rowBelow" | "colLeft" | "colRight" => {
let ok = match op.as_str() {
"rowAbove" => t.insert_row(r),
"rowBelow" => t.insert_row(r.saturating_add(1)),
"colLeft" => t.insert_col(c),
_ => t.insert_col(c.saturating_add(1)),
};
if !ok {
return Err(bad(name, format!("the table is at its {}-cell limit", deckcraft_model::table::MAX_CELLS)));
}
}
"delRow" => {
if !t.delete_row(r) {
return Err(bad(name, "can't delete the last row"));
+39
View File
@@ -6921,6 +6921,45 @@ fn master_body_placeholder_format_keeps_per_level_formatting() {
assert_eq!(shown(&s), vec![Some(28.0), Some(24.0)]);
}
#[test]
fn table_edits_past_the_cell_cap_are_refused_not_trimmed() {
use deckcraft_model::table::{MAX_CELLS, Row};
let mut s = session();
let id = table_at(&mut s, &[100.0, 100.0], json!([["a", "b"], ["c", "d"]]));
let cols = 200;
let rows = MAX_CELLS / cols;
let big = deckcraft_model::Table {
cols: vec![1.0; cols],
rows: vec![Row { height: 1.0, cells: vec![Default::default(); cols] }; rows],
..Default::default()
};
for sl in std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc).slides.iter_mut() {
for sh in std::sync::Arc::make_mut(sl).shapes.iter_mut() {
if sh.id == id {
sh.kind = deckcraft_model::ShapeKind::Table(big.clone());
}
}
}
for cmd in ["table.insertRowAbove", "table.insertRowBelow"] {
let e = s.execute(cmd, &json!({"id": id.0, "row": 0})).unwrap_err().to_string();
assert!(e.contains("cell limit"), "{cmd}: {e}");
}
let e = s.execute("table.insertColumnLeft", &json!({"id": id.0, "col": 0})).unwrap_err().to_string();
assert!(e.contains("cell limit"), "{e}");
assert_eq!(table_of(&s, id), big, "a refused insert leaves every row in place");
// A table already over the cap is refused too, not trimmed behind a success.
let huge = deckcraft_model::Table { rows: vec![Row { height: 1.0, cells: vec![Default::default(); cols] }; rows + 1], ..big.clone() };
for sl in std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc).slides.iter_mut() {
for sh in std::sync::Arc::make_mut(sl).shapes.iter_mut() {
if sh.id == id {
sh.kind = deckcraft_model::ShapeKind::Table(huge.clone());
}
}
}
assert!(s.execute("table.deleteRow", &json!({"id": id.0, "row": 0})).is_err());
assert_eq!(table_of(&s, id).n_rows(), rows + 1);
}
/// A shape named many times in one edit is edited once and not copied per mention.
#[test]
fn repeated_target_ids_edit_the_shape_once() {
+26 -2
View File
@@ -313,6 +313,8 @@ pub(crate) struct Canvas {
saved: Vec<Saved>,
/// Bytes held by the items so far (path ops and pixels), capped at [`MAX_BYTES`].
used: usize,
/// Bytes retained outside the canvas (EMF+ objects) that count against the same cap.
held: usize,
objects: Vec<Option<Obj>>,
pen: Pen,
brush: Brush,
@@ -345,6 +347,7 @@ impl Canvas {
viewport_set: false,
saved: Vec::new(),
used: 0,
held: 0,
objects: Vec::new(),
pen: Pen { style: 0, width: 0.0, color: [0, 0, 0, 255] },
brush: Brush::new(BS_SOLID, [255, 255, 255, 255]),
@@ -368,6 +371,17 @@ impl Canvas {
self.surface = clip::normalize(rect);
}
/// Bytes retained outside the canvas that share its budget (see [`Canvas::hold`]).
#[cfg(test)]
pub(crate) fn held(&self) -> usize {
self.held
}
/// Bytes the canvas has retained so far.
pub fn used(&self) -> usize {
self.used
}
/// True once the item cap is hit; the readers stop there and keep what they have.
pub fn full(&self) -> bool {
self.full
@@ -385,6 +399,12 @@ impl Canvas {
}
}
/// Records the bytes another part of the player retains, so the items and they stay within
/// [`MAX_BYTES`] together.
pub fn hold(&mut self, bytes: usize) {
self.held = bytes;
}
fn push_raw(&mut self, mut raw: Raw) {
if self.clip.as_ref().is_some_and(Vec::is_empty) {
return;
@@ -402,7 +422,7 @@ impl Canvas {
Raw::Bitmap { img, .. } => img.px.len(),
Raw::Text { text: t, .. } => t.text.len() + t.dx.len() * 4 + t.font.family.len() + 128,
} + region_cost;
if self.items.len() >= MAX_ITEMS || self.used.saturating_add(cost) > MAX_BYTES {
if self.items.len() >= MAX_ITEMS || self.used.saturating_add(self.held).saturating_add(cost) > MAX_BYTES {
self.full = true;
} else {
self.used += cost;
@@ -551,6 +571,10 @@ impl Canvas {
/// of at most [`SHADE_RES`] cells on the longer side, laid out so that its one repeat covers
/// the shape.
pub fn shade_shape(&mut self, ops: &[Op], shade: &dyn Fn(P) -> [u8; 4]) {
// A full canvas or an empty clip keeps nothing more, so do not sample a tile for it.
if self.full || self.clip.as_ref().is_some_and(Vec::is_empty) {
return;
}
let dev: Vec<Op> = ops.iter().map(|o| self.map_op(*o)).collect();
let Some([l, t, r, b]) = crate::cliprect::ops_bbox(&dev) else { return };
let side = (r - l).max(b - t);
@@ -666,7 +690,7 @@ impl Canvas {
// A pattern brush's pixels count against the byte budget like drawn items.
if let Obj::Brush(Brush { pattern: Some(p), .. }) = &obj {
let cost = p.px.len();
if self.used.saturating_add(cost) > MAX_BYTES {
if self.used.saturating_add(self.held).saturating_add(cost) > MAX_BYTES {
self.full = true;
return;
}
+90 -10
View File
@@ -288,13 +288,23 @@ impl Plus {
if !self.active {
return None;
}
match typ {
0x4008 => self.object(flags, r),
let done = match typ {
0x4008 => self.object(flags, r, cv.used().saturating_add(self.clip_bytes())),
0x4009 | 0x4013 | 0x401A..=0x401C | 0x4036 => self.record_image_text_region(cv, typ, flags, r),
0x4025..=0x4035 => self.record_state(cv, typ, flags, r),
0x4010..=0x4012 | 0x4016..=0x4019 => self.record_curve(cv, typ, flags, r),
_ => self.record_shape(cv, typ, flags, r),
}
};
// What this player retains (objects, a continued object's pieces, the current and saved
// clips) shares the canvas's byte budget; clip changes are charged as they happen.
cv.hold(self.object_bytes.saturating_add(self.partial.as_ref().map_or(0, |p| p.data.len())).saturating_add(self.clip_bytes()));
done
}
/// Bytes of the current clip and of every saved one.
fn clip_bytes(&self) -> usize {
let area = |a: &Area| a.as_ref().map_or(0, |v| v.len().saturating_mul(std::mem::size_of::<P>()));
self.stack.iter().map(|s| area(&s.clip)).fold(area(&self.clip), usize::saturating_add)
}
/// Rectangle, polygon, ellipse and path records.
@@ -559,7 +569,7 @@ impl Plus {
Some(())
}
fn object(&mut self, flags: u16, r: Bytes) -> Option<()> {
fn object(&mut self, flags: u16, r: Bytes, canvas_used: usize) -> Option<()> {
let id = usize::from(flags & 0xFF);
let kind = (flags >> 8) & 0x7F;
if flags & 0x8000 != 0 {
@@ -575,7 +585,7 @@ impl Plus {
p.data.extend_from_slice(&chunk[..chunk.len().min(room)]);
if p.data.len() >= p.total {
let obj = parse_object(p.kind, Bytes(&p.data));
self.store(p.id, obj);
self.store(p.id, obj, canvas_used);
} else {
self.partial = Some(p);
}
@@ -589,22 +599,23 @@ impl Plus {
let room = p.total.saturating_sub(p.data.len());
p.data.extend_from_slice(&r.0[..r.0.len().min(room)]);
let obj = parse_object(kind, Bytes(&p.data));
self.store(id, obj);
self.store(id, obj, canvas_used);
return Some(());
}
let obj = parse_object(kind, r);
self.store(id, obj);
self.store(id, obj, canvas_used);
Some(())
}
fn store(&mut self, id: usize, obj: Option<Obj>) {
/// `canvas_used` is what the canvas already retains: objects and items share one budget.
fn store(&mut self, id: usize, obj: Option<Obj>, canvas_used: usize) {
let Some(slot) = self.objects.get_mut(id) else { return };
if let Some(old) = slot.take() {
self.object_bytes = self.object_bytes.saturating_sub(Self::obj_cost(&old));
}
if let Some(new_obj) = obj {
let cost = Self::obj_cost(&new_obj);
if self.object_bytes.saturating_add(cost) <= crate::MAX_BYTES {
if self.object_bytes.saturating_add(cost).saturating_add(canvas_used) <= crate::MAX_BYTES {
self.object_bytes += cost;
*slot = Some(new_obj);
}
@@ -615,7 +626,7 @@ impl Plus {
match obj {
Obj::Image(img) => match img.as_ref() {
ImageData::Bitmap(rgba) => rgba.px.len(),
ImageData::Meta(pic) => pic.items.len().saturating_mul(128),
ImageData::Meta(pic) => picture_cost(&pic.items),
},
Obj::Brush(Fill::Texture { img, .. }) => img.px.len(),
Obj::Path(ops) => ops.len().saturating_mul(std::mem::size_of::<Op>()),
@@ -1282,6 +1293,33 @@ fn metafile(r: Bytes) -> Option<Picture> {
pic.filter(|p| p.width > 0.0 && p.height > 0.0)
}
/// Bytes the items of a nested picture own: path segments, clip rectangles, pixels and text, with
/// each shared pattern tile counted once.
fn picture_cost(items: &[crate::Item]) -> usize {
use crate::Item;
let region = |r: &Option<Vec<[f32; 4]>>| r.as_ref().map_or(0, |v| v.len().saturating_mul(16));
let mut tiles = std::collections::HashSet::new();
let mut total = 0usize;
for item in items {
let own = match item {
Item::Path { segs, region: r, .. } => segs.len().saturating_mul(std::mem::size_of::<crate::Seg>()).saturating_add(region(r)),
Item::Bitmap { rgba, region: r, .. } => rgba.len().saturating_add(region(r)),
Item::Pattern { segs, tile, region: r, .. } => {
let tile_bytes = if tiles.insert(Arc::as_ptr(tile)) { tile.rgba.len() } else { 0 };
segs.len().saturating_mul(std::mem::size_of::<crate::Seg>()).saturating_add(tile_bytes).saturating_add(region(r))
}
Item::Text(t) => t
.text
.len()
.saturating_add(t.dx.len().saturating_mul(std::mem::size_of::<f32>()))
.saturating_add(t.font.family.len())
.saturating_add(region(&t.region)),
};
total = total.saturating_add(128).saturating_add(own);
}
total
}
fn bitmap(r: Bytes) -> Option<Rgba> {
let (w, h, stride) = (r.u32(8)? as i32, r.u32(12)? as i32, r.u32(16)? as i32);
let format = r.u32(20)?;
@@ -1559,3 +1597,45 @@ fn wrap_line(line: &[u16], width: f64, em: f64) -> Vec<Vec<u16>> {
out.push(cur);
out
}
#[cfg(test)]
mod cost_tests {
use super::*;
use crate::{Item, Seg, Tile};
#[test]
fn a_shared_pattern_tile_is_charged_once() {
let tile = Arc::new(Tile { width: 512, height: 512, rgba: vec![0; 1 << 20], cell: [1.0, 1.0], origin: [0.0, 0.0] });
let item = || Item::Pattern { segs: vec![Seg::Move(0.0, 0.0), Seg::Line(1.0, 1.0)], even_odd: false, tile: tile.clone(), region: None };
let items: Vec<Item> = (0..257).map(|_| item()).collect();
assert!(picture_cost(&items) < 2 << 20, "{}", picture_cost(&items));
}
#[test]
fn pieces_of_a_continued_object_count_against_the_shared_budget() {
let (mut plus, mut cv) = (Plus::new(), Canvas::new());
plus.active = true;
// Image object (kind 5), id 0, continued: 1000 bytes announced, 100 delivered.
let mut rec = 1000u32.to_le_bytes().to_vec();
rec.extend([0u8; 100]);
plus.record(&mut cv, 0x4008, 0x8000 | (5 << 8), Bytes(&rec));
assert_eq!(cv.held(), 100);
}
#[test]
fn current_and_saved_clips_count_against_the_shared_budget() {
let (mut plus, mut cv) = (Plus::new(), Canvas::new());
plus.active = true;
let pts = vec![(0.0, 0.0); 1000];
plus.clip = Some(pts.clone());
plus.stack.push(Saved { id: 1, world: IDENTITY, page: 1.0, clip: Some(pts) });
// Any record refreshes the held count (an unknown record draws nothing).
plus.record(&mut cv, 0x40FF, 0, Bytes(&[]));
assert_eq!(cv.held(), 2 * 1000 * std::mem::size_of::<P>());
// Restoring the saved state releases the charge of what it dropped.
plus.stack.clear();
plus.clip = None;
plus.record(&mut cv, 0x40FF, 0, Bytes(&[]));
assert_eq!(cv.held(), 0);
}
}
+12 -1
View File
@@ -58,6 +58,7 @@ impl Picture {
let k = (c[0].abs() + c[3].abs()) / 2.0;
let mut out = Vec::with_capacity(self.items.len());
let mut total_elements = 0usize;
let region_elements = std::cell::Cell::new(0usize);
let mut cached_region: Option<(*const [f32; 4], usize, Option<Option<Arc<BezPath>>>)> = None;
let mut get_region = |reg: &Option<Vec<[f32; 4]>>| -> Option<Option<Arc<BezPath>>> {
let (ptr, len) = match reg {
@@ -71,12 +72,15 @@ impl Picture {
return res.clone();
}
let res = placed_region(&tf, reg);
// Each distinct clip region is expanded once; its elements count against the placement budget.
let n = res.as_ref().and_then(Option::as_ref).map_or(0, |p| p.elements().len());
region_elements.set(region_elements.get().saturating_add(n));
cached_region = Some((ptr, len, res.clone()));
res
};
for (index, item) in self.items.iter().enumerate() {
if total_elements >= MAX_PLACED_ELEMENTS {
if total_elements.saturating_add(region_elements.get()) >= MAX_PLACED_ELEMENTS {
break;
}
match item {
@@ -84,6 +88,10 @@ impl Picture {
let path = tf * bez(segs);
let Some(region) = get_region(region) else { continue };
if in_range(path.bounding_box()) {
// This item's own region is expanded by now: it counts before it is kept.
if total_elements.saturating_add(path.elements().len()).saturating_add(region_elements.get()) > MAX_PLACED_ELEMENTS {
break;
}
total_elements = total_elements.saturating_add(path.elements().len());
let stroke = stroke.map(|(c, w)| (c, f64::from(w) * k));
out.push(PlacedItem::Path { path, fill: *fill, stroke, even_odd: *even_odd, region });
@@ -97,6 +105,9 @@ impl Picture {
* Affine::scale_non_uniform(f64::from(t.cell[0]), f64::from(t.cell[1]));
let det = tile.determinant();
if in_range(path.bounding_box()) && det.is_finite() && det.abs() > 1e-12 {
if total_elements.saturating_add(path.elements().len()).saturating_add(region_elements.get()) > MAX_PLACED_ELEMENTS {
break;
}
total_elements = total_elements.saturating_add(path.elements().len());
out.push(PlacedItem::Pattern { index, path, even_odd: *even_odd, tile, region });
}
+39
View File
@@ -897,6 +897,24 @@ fn place_fits_the_frame_scales_strokes_and_drops_far_paths() {
assert_eq!(stroke.map(|s| s.1), Some(8.0));
}
#[test]
fn place_counts_the_items_own_clip_region_before_keeping_it() {
// A path of 499,998 elements nearly fills the 500,000 budget; a second one whose new clip
// region adds elements goes past it and is left out.
let mut segs = vec![Seg::Move(0.0, 0.0)];
segs.extend((0..499_997).map(|i| Seg::Line(1.0 + (i % 50) as f32, 1.0)));
let big = Item::Path { segs, fill: None, stroke: None, even_odd: false, region: None };
let small = Item::Path {
segs: vec![Seg::Move(0.0, 0.0), Seg::Line(1.0, 1.0)],
fill: None,
stroke: None,
even_odd: false,
region: Some(vec![[0.0, 0.0, 50.0, 25.0], [10.0, 10.0, 20.0, 20.0]]),
};
let pic = crate::Picture { width: 100.0, height: 50.0, items: vec![big, small] };
assert_eq!(pic.place(kurbo::Rect::new(0.0, 0.0, 100.0, 50.0), [0.0; 4]).len(), 1);
}
#[test]
fn place_keeps_only_finite_bounded_bitmaps_with_area() {
let bitmap = |rect: [f32; 4]| Item::Bitmap { rect, width: 1, height: 1, rgba: vec![0; 4], region: None };
@@ -2155,3 +2173,24 @@ fn region_intersection_over_budget_keeps_the_previous_area() {
assert!(crate::cliprect::clip_polygon_within(&gon(0.0), &gon(30.0), &mut 10).is_none());
assert!(crate::cliprect::clip_polygon_within(&gon(0.0), &gon(30.0), &mut crate::cliprect::MAX_AREA_CLIP_WORK.clone()).is_some());
}
#[test]
fn full_canvas_does_not_sample_gradient_tiles() {
let mut cv = Canvas::new();
let big = || Rgba { w: 1, h: 1, bottom_up: false, px: vec![0u8; MAX_BYTES / 2] };
for _ in 0..3 {
cv.bitmap([0.0, 0.0, 1.0, 1.0], big());
}
assert!(cv.full());
cv.shade_shape(&crate::geom::rect(0.0, 0.0, 10.0, 10.0), &|_| panic!("sampled a full canvas"));
assert_eq!(cv.items.len(), 2);
}
#[test]
fn canvas_budget_counts_bytes_held_elsewhere() {
let mut cv = Canvas::new();
cv.hold(MAX_BYTES - 100);
cv.bitmap([0.0, 0.0, 1.0, 1.0], Rgba { w: 1, h: 1, bottom_up: false, px: vec![0u8; 200] });
assert!(cv.full());
assert!(cv.items.is_empty());
}
+186 -16
View File
@@ -83,6 +83,12 @@ impl Default for Table {
}
}
/// Cells a table may hold after [`Table::normalize`] (PowerPoint's own limit is 75 x 75).
pub const MAX_CELLS: usize = 100_000;
/// Cell visits [`Table::merge`] may spend growing its rectangle over existing merges: a few
/// passes over the largest grid, where a real table needs one or two.
const MAX_MERGE_WORK: usize = 16 * MAX_CELLS;
impl Table {
pub fn new(rows: usize, cols: usize, width: f64, row_height: f64) -> Self {
let rows = rows.clamp(1, 1000);
@@ -113,15 +119,24 @@ impl Table {
self.rows.iter().map(|r| r.height).sum()
}
/// Insert an empty row before `at`. A merge the new row falls inside grows to cover it.
pub fn insert_row(&mut self, at: usize) {
/// Returns false, leaving the table as it was, when the row would take it past [`MAX_CELLS`].
pub fn insert_row(&mut self, at: usize) -> bool {
if self.cols.len().saturating_mul(self.rows.len().saturating_add(1)) > MAX_CELLS {
return false;
}
let at = at.min(self.rows.len());
let merges = self.merges();
let h = self.rows.get(at.saturating_sub(1)).map(|r| r.height).unwrap_or(30.0);
self.rows.insert(at, Row { height: h, cells: (0..self.cols.len()).map(|_| Cell::new("")).collect() });
self.set_merges(merges.into_iter().map(|m| m.insert_row(at)));
true
}
/// Insert an empty column before `at`. A merge the new column falls inside grows to cover it.
pub fn insert_col(&mut self, at: usize) {
/// Returns false, leaving the table as it was, when the column would take it past [`MAX_CELLS`].
pub fn insert_col(&mut self, at: usize) -> bool {
if self.cols.len().saturating_add(1).saturating_mul(self.rows.len()) > MAX_CELLS {
return false;
}
let at = at.min(self.cols.len());
let merges = self.merges();
let w = self.cols.get(at.saturating_sub(1)).copied().unwrap_or(80.0);
@@ -131,6 +146,7 @@ impl Table {
r.cells.insert(i, Cell::new(""));
}
self.set_merges(merges.into_iter().map(|m| m.insert_col(at)));
true
}
/// Delete row `at`. Merges through it shrink; a merge anchored on it moves its anchor (and
/// text) to the next row it covers.
@@ -170,8 +186,12 @@ impl Table {
true
}
/// The merged areas: every uncovered cell spanning more than one cell, clipped to the grid.
/// Merges of a well-formed table do not overlap, so together they cover at most the grid; a
/// file's overlapping spans past that are dropped, so rebuilding them cannot revisit cells
/// without bound.
fn merges(&self) -> Vec<Merge> {
let (nr, nc) = (self.rows.len(), self.cols.len());
let mut area = nr.saturating_mul(nc);
let mut out = vec![];
for (r, row) in self.rows.iter().enumerate() {
for (c, cell) in row.cells.iter().enumerate().take(nc) {
@@ -180,7 +200,10 @@ impl Table {
}
let rs = (cell.row_span.max(1) as usize).min(nr - r);
let gs = (cell.grid_span.max(1) as usize).min(nc - c);
if rs > 1 || gs > 1 {
if (rs > 1 || gs > 1)
&& let Some(left) = area.checked_sub(rs.saturating_mul(gs))
{
area = left;
out.push(Merge { r, c, rs, gs });
}
}
@@ -233,9 +256,17 @@ impl Table {
if r1 >= self.rows.len() || c1 >= self.cols.len() || (r0 == r1 && c0 == c1) {
return false;
}
// Grow the rectangle until no existing merge sticks out of it.
// Grow the rectangle until no existing merge sticks out of it. Each pass reads the whole
// grid and may grow it by one row, so the passes are charged against
// [`MAX_MERGE_WORK`]: past it the merge is refused, before anything changed.
let (mut r0, mut c0, mut r1, mut c1) = (r0, c0, r1, c1);
let pass = self.rows.len().saturating_mul(self.cols.len()).max(1);
let mut work = 0usize;
loop {
work = work.saturating_add(pass);
if work > MAX_MERGE_WORK {
return false;
}
let before = (r0, c0, r1, c1);
for (r, row) in self.rows.iter().enumerate() {
for (c, cell) in row.cells.iter().enumerate().take(self.cols.len()) {
@@ -289,21 +320,50 @@ impl Table {
return false;
}
// Spans come from the file and may reach past the table; only walk the cells that exist.
for rr in r..r.saturating_add(rs).min(self.rows.len()) {
for cc in c..c.saturating_add(gs).min(self.cols.len()) {
if let Some(x) = self.cell_mut(rr, cc) {
x.h_merge = false;
x.v_merge = false;
x.grid_span = 1;
x.row_span = 1;
}
let end = r.saturating_add(rs).min(self.rows.len());
let gs = gs.min(self.cols.len().saturating_sub(c));
for row in self.rows.get_mut(r..end).unwrap_or_default() {
for x in row.cells.iter_mut().skip(c).take(gs) {
x.h_merge = false;
x.v_merge = false;
x.grid_span = 1;
x.row_span = 1;
}
}
true
}
/// Make every row have exactly `cols.len()` cells (repair after loading).
pub fn normalize(&mut self) {
/// Whether the grid is as [`Table::normalize`] leaves it: within [`MAX_CELLS`], every row
/// exactly `cols.len()` cells wide.
pub fn is_normalized(&self) -> bool {
let n = self.cols.len();
n <= MAX_CELLS && self.rows.len() <= (MAX_CELLS / n.max(1)).max(1) && self.rows.iter().all(|r| r.cells.len() == n)
}
/// Whether [`Table::normalize`] would drop rows or columns to fit [`MAX_CELLS`].
pub fn over_cap(&self) -> bool {
let n = self.cols.len();
n > MAX_CELLS || self.rows.len() > (MAX_CELLS / n.max(1)).max(1)
}
/// What [`Table::normalize`] would leave of this table, cloning only the rows and cells that
/// survive (an oversized table is not copied whole first).
pub fn normalized(&self) -> Table {
let n = self.cols.len().min(MAX_CELLS);
let keep = (MAX_CELLS / n.max(1)).max(1);
let rows = self.rows.iter().take(keep).map(|r| Row { height: r.height, cells: r.cells.iter().take(n).cloned().collect() }).collect();
let mut t = Table { cols: self.cols[..n].to_vec(), rows, background: self.background.clone(), style: self.style.clone(), ..*self };
t.normalize();
if self.over_cap() {
t.clip_spans();
}
t
}
/// Make every row have exactly `cols.len()` cells (repair after loading). A grid over
/// [`MAX_CELLS`] loses its last rows (and columns) first, so a file naming many rows and many
/// columns but few cells cannot make this allocate their product.
pub fn normalize(&mut self) {
let truncated = self.over_cap();
self.cols.truncate(MAX_CELLS);
let n = self.cols.len();
self.rows.truncate((MAX_CELLS / n.max(1)).max(1));
for r in &mut self.rows {
r.cells.resize_with(n, || Cell::new(""));
for c in &mut r.cells {
@@ -311,6 +371,20 @@ impl Table {
c.row_span = c.row_span.max(1);
}
}
if truncated {
self.clip_spans();
}
}
/// Cut every merge span to the rows and columns that remain, so no merge reaches past a
/// table that lost rows or columns to the cell cap.
fn clip_spans(&mut self) {
let (nr, nc) = (self.rows.len(), self.cols.len());
for (ri, r) in self.rows.iter_mut().enumerate() {
for (ci, c) in r.cells.iter_mut().enumerate() {
c.row_span = c.row_span.min((nr - ri) as u32).max(1);
c.grid_span = c.grid_span.min((nc - ci) as u32).max(1);
}
}
}
}
@@ -519,6 +593,102 @@ mod tests {
}
}
/// Each cell spans two rows, uncovered: merging the last two rows would grow the rectangle a
/// row per pass over the whole grid, 10,000 passes. The merge is refused at its work bound
/// instead, leaving the table as it was; a small merge still works.
#[test]
fn merge_expansion_stops_at_its_work_bound() {
let mut t = Table::new(1_000, 10, 400.0, 30.0);
for row in &mut t.rows {
for c in &mut row.cells {
c.row_span = 2;
}
}
let before = t.clone();
assert!(!t.merge(998, 0, 999, 9));
assert_eq!(t, before);
let mut small = Table::new(4, 4, 400.0, 30.0);
assert!(small.merge(0, 0, 1, 1));
assert!(small.is_normalized() && !Table { cols: vec![1.0; 3], rows: vec![Row::default(); 2], ..Default::default() }.is_normalized());
}
#[test]
fn normalize_caps_a_sparse_grid_at_max_cells() {
// 1,000 columns and 1,000 empty rows would be a million cells.
let mut t = Table { cols: vec![10.0; 1000], rows: vec![Row::default(); 1000], ..Default::default() };
t.normalize();
assert_eq!(t.cols.len(), 1000);
assert_eq!(t.rows.len(), MAX_CELLS / 1000);
assert!(t.rows.iter().all(|r| r.cells.len() == 1000));
// Columns alone past the cap keep one row of MAX_CELLS cells.
let mut t = Table { cols: vec![1.0; MAX_CELLS + 5], rows: vec![Row::default(); 3], ..Default::default() };
t.normalize();
assert_eq!((t.cols.len(), t.rows.len()), (MAX_CELLS, 1));
// `normalized` leaves what `normalize` does.
let wide = Table { cols: vec![10.0; 1000], rows: vec![Row { height: 1.0, cells: vec![Cell::new("x"); 1500] }; 150], ..Default::default() };
let mut n = wide.clone();
n.normalize();
assert_eq!(wide.normalized(), n);
// A normal table is untouched.
let mut t = Table::new(4, 4, 400.0, 30.0);
t.normalize();
assert_eq!((t.n_rows(), t.n_cols()), (4, 4));
}
#[test]
fn overlapping_file_spans_rebuild_in_bounded_work() {
// Every cell an uncovered anchor spanning to the far corner: their areas add up to about
// n^4 / 4, but only what fits the grid is kept, so an edit's rebuild stays near n^2.
let n = 200;
let mut t = Table::new(n, n, 400.0, 30.0);
for row in &mut t.rows {
for c in &mut row.cells {
c.grid_span = u32::MAX;
c.row_span = u32::MAX;
}
}
assert_eq!(t.merges().len(), 1, "the first anchor already covers the whole grid");
assert!(t.insert_row(n));
assert_eq!(t.n_rows(), n + 1);
assert!(t.split(0, 0));
}
/// A table at the cell cap refuses another row or column instead of losing rows to
/// normalization, and the cap sits above PowerPoint's own 75 x 75.
#[test]
fn insert_past_the_cap_is_refused() {
const { assert!(MAX_CELLS >= 75 * 75) };
let mut t =
Table { cols: vec![1.0; 200], rows: vec![Row { height: 1.0, cells: vec![Cell::new(""); 200] }; MAX_CELLS / 200], ..Default::default() };
assert!(!t.over_cap());
let before = t.clone();
assert!(!t.insert_row(0));
assert!(!t.insert_col(0));
assert_eq!(t, before);
assert!(Table::new(75, 75, 100.0, 10.0).insert_row(0));
}
/// A merge that crosses the truncation boundary is clipped to the rows and columns that remain.
#[test]
fn merges_are_clipped_when_the_cap_drops_rows() {
let cols = 1000;
let rows = MAX_CELLS / cols + 1;
let mut t = Table { cols: vec![1.0; cols], rows: vec![Row { height: 1.0, cells: vec![Cell::new(""); cols] }; rows], ..Default::default() };
t.rows[0].cells[0].row_span = rows as u32;
t.rows[0].cells[cols - 1].grid_span = 5;
let n = t.normalized();
assert_eq!(n.rows.len(), rows - 1);
assert_eq!(n.rows[0].cells[0].row_span as usize, rows - 1);
assert_eq!(n.rows[0].cells[cols - 1].grid_span, 1);
t.normalize();
assert_eq!(t.rows[0].cells[0].row_span as usize, rows - 1);
// Within the cap, spans are left as the file wrote them.
let mut small = Table::new(3, 3, 30.0, 10.0);
small.rows[2].cells[2].grid_span = 4;
small.normalize();
assert_eq!(small.rows[2].cells[2].grid_span, 4);
}
fn merged() -> Table {
// 4x4 with a 2x2 merge at (1,1) holding "M".
let mut t = Table::new(4, 4, 400.0, 30.0);
@@ -552,10 +722,10 @@ mod tests {
fn row_and_column_edits_keep_merges_consistent() {
for at in 0..=4 {
let mut t = merged();
t.insert_row(at);
assert!(t.insert_row(at));
assert_topology(&t);
let mut t = merged();
t.insert_col(at);
assert!(t.insert_col(at));
assert_topology(&t);
}
for at in 0..4 {
+6
View File
@@ -70,6 +70,12 @@ pub fn import_checked(bytes: &[u8]) -> Result<(Presentation, Vec<String>), PptxE
read::import_checked(bytes)
}
/// [`import_checked`], plus warnings for content cut to fit a limit (a table past the cell cap
/// keeps its leading rows); the file still opens.
pub fn import_report(bytes: &[u8]) -> Result<(Presentation, Vec<String>, Vec<String>), PptxError> {
read::import_report(bytes)
}
/// [`import`] with a lower presentation-wide chart point budget, so tests reach the over-budget
/// path without a multi-million-point chart.
#[doc(hidden)]
+27 -2
View File
@@ -55,12 +55,24 @@ pub struct Imp<'a> {
pub tag_bytes: usize,
/// The tag budget ran out and the warning was given.
pub tags_capped: bool,
/// What the file lost that the reader kept going past (see [`MAX_IMPORT_WARNINGS`]).
pub warnings: Vec<String>,
/// Whether a part has relationships of its own, by normalized part name: looked up once
/// however many embedded objects point at the part.
pub has_rels: HashMap<String, bool>,
}
/// Import warnings kept; a hostile file with many oversized tables adds no more than this.
const MAX_IMPORT_WARNINGS: usize = 32;
impl Imp<'_> {
/// Record something the file lost, and log it.
pub fn warn(&mut self, msg: String) {
log::warn!("pptx: {msg}");
if self.warnings.len() < MAX_IMPORT_WARNINGS {
self.warnings.push(msg);
}
}
pub fn alloc(&mut self) -> u32 {
self.next = self.next.saturating_add(1);
self.next
@@ -462,6 +474,7 @@ pub fn import_theme(bytes: &[u8]) -> Result<(deckcraft_model::theme::Theme, Vec<
tags_by_part: HashMap::new(),
tag_bytes: 0,
tags_capped: false,
warnings: vec![],
has_rels: HashMap::new(),
};
let part = Part::new(&pkg, &theme_path, &doc);
@@ -476,11 +489,22 @@ pub fn import(bytes: &[u8]) -> Result<Presentation, PptxError> {
/// [`import`], plus the parts of the slide list the file names but that couldn't be read (a
/// missing or truncated slide): those slides are left out, so a caller can say so (#203).
pub fn import_checked(bytes: &[u8]) -> Result<(Presentation, Vec<String>), PptxError> {
import_with_chart_points(bytes, chart::MAX_PRESENTATION_CHART_POINTS)
import_report(bytes).map(|(p, lost, _)| (p, lost))
}
/// [`import_checked`], plus warnings for content the file had that was cut to fit a limit (a
/// table past the cell cap keeps its leading rows).
pub fn import_report(bytes: &[u8]) -> Result<(Presentation, Vec<String>, Vec<String>), PptxError> {
import_report_with_chart_points(bytes, chart::MAX_PRESENTATION_CHART_POINTS)
}
/// [`import_checked`] with the presentation's chart point budget set (tests lower it).
pub(crate) fn import_with_chart_points(bytes: &[u8], chart_points_cap: usize) -> Result<(Presentation, Vec<String>), PptxError> {
import_report_with_chart_points(bytes, chart_points_cap).map(|(p, lost, _)| (p, lost))
}
/// [`import_report`] with the presentation's chart point budget set.
fn import_report_with_chart_points(bytes: &[u8], chart_points_cap: usize) -> Result<(Presentation, Vec<String>, Vec<String>), PptxError> {
let mut lost_slides: Vec<String> = vec![];
let pkg = Package::open(bytes)?;
let root_rels = pkg.rels("");
@@ -513,6 +537,7 @@ pub(crate) fn import_with_chart_points(bytes: &[u8], chart_points_cap: usize) ->
tags_by_part: HashMap::new(),
tag_bytes: 0,
tags_capped: false,
warnings: vec![],
has_rels: HashMap::new(),
};
// Slide ids first so hyperlinks anywhere can point at slides.
@@ -544,7 +569,7 @@ pub(crate) fn import_with_chart_points(bytes: &[u8], chart_points_cap: usize) ->
p.media = std::mem::take(&mut imp.media);
p.next_id = imp.next;
p.fix_next_id();
Ok((p, lost_slides))
Ok((p, lost_slides, std::mem::take(&mut imp.warnings)))
}
/// Masters, layouts and slides to read, in file order and without duplicates: (master paths,
+3
View File
@@ -785,6 +785,9 @@ fn table(imp: &mut Imp, part: &Part, tbl: &El) -> Table {
let n = t.rows.iter().map(|r| r.cells.len()).max().unwrap_or(0);
t.cols = vec![72.0; n];
}
if t.over_cap() {
imp.warn(format!("a table in {} is over {} cells; its last rows were left out", part.name, deckcraft_model::table::MAX_CELLS));
}
t.normalize();
t
}
+9
View File
@@ -65,6 +65,8 @@ pub struct Exp<'a> {
n_kept: u32,
pub slide_parts: HashMap<SlideId, String>,
pub table_styles: Vec<String>,
/// A table past the cell cap was met; export fails rather than write a trimmed one.
pub table_over_cap: bool,
/// The ids in `table_styles`, for checking membership without a scan.
pub table_style_set: std::collections::HashSet<String>,
placeholder: Option<String>,
@@ -558,6 +560,7 @@ pub fn export(p: &Presentation, main_ct: &str) -> Result<Vec<u8>, PptxError> {
n_kept: 0,
slide_parts: HashMap::new(),
table_styles: vec![],
table_over_cap: false,
table_style_set: Default::default(),
placeholder: None,
};
@@ -766,6 +769,12 @@ pub fn export(p: &Presentation, main_ct: &str) -> Result<Vec<u8>, PptxError> {
root.add_new(&k.rel_type, &k.name, false);
}
x.pkg.rels("", &root);
if x.table_over_cap {
return Err(PptxError::Unsupported(format!(
"a table has more than {} cells; export refused rather than drop rows",
deckcraft_model::table::MAX_CELLS
)));
}
x.pkg.finish()
}
+2 -2
View File
@@ -721,8 +721,8 @@ fn frame_close(w: &mut W) {
}
fn table(w: &mut W, x: &mut Exp, o: &mut Out, t: &Table) {
let mut t = t.clone();
t.normalize();
x.table_over_cap |= t.over_cap();
let t = t.normalized();
w.open0("a:tbl");
let (guid, _) = crate::tables::guid_for_style(&t.style);
if x.table_style_set.insert(t.style.clone()) {
+38
View File
@@ -288,6 +288,44 @@ fn unreadable_slide_is_reported() {
assert_eq!(deckcraft_pptx::import(&broken).expect("import").slides.len(), 1);
}
fn table_deck(rows: usize, cols: usize) -> deckcraft_model::Presentation {
use deckcraft_model::table::Row;
use deckcraft_model::{Shape, ShapeId, ShapeKind, Table, Xfrm};
let mut p = defaults::new_presentation(None);
let id = ShapeId(p.alloc_id().expect("ids"));
let t = Table { cols: vec![1.0; cols], rows: vec![Row { height: 1.0, cells: vec![Default::default(); cols] }; rows], ..Default::default() };
let slide = std::sync::Arc::make_mut(&mut p.slides[0]);
slide.shapes.push(Shape { id, name: "T".into(), xfrm: Some(Xfrm::new(0.0, 0.0, 10.0, 10.0)), kind: ShapeKind::Table(t), ..Default::default() });
p
}
/// A table past the cell cap opens with its leading rows and an import warning; saving one that
/// is over the cap is an error, never a silent trim.
#[test]
fn oversized_table_warns_on_import_and_refuses_export() {
let max = deckcraft_model::table::MAX_CELLS;
let cols = 1000;
let rows = max / cols;
// A table exactly at the cap round-trips with no warning.
let bytes = deckcraft_pptx::export(&table_deck(rows, cols)).expect("export at the cap");
let (_, _, warnings) = deckcraft_pptx::import_report(&bytes).expect("import");
assert!(warnings.is_empty(), "{warnings:?}");
// One more row in the file: kept truncated, and reported.
let mut es = entries(&bytes);
let slide = es.iter_mut().find(|(n, _)| n == "ppt/slides/slide1.xml").expect("slide1");
let xml = String::from_utf8(std::mem::take(&mut slide.1)).expect("utf8");
let start = xml.rfind("<a:tr ").expect("row");
let end = xml.rfind("</a:tr>").expect("row end") + "</a:tr>".len();
slide.1 = format!("{}{}{}", &xml[..end], &xml[start..end], &xml[end..]).into_bytes();
let (q, _, warnings) = deckcraft_pptx::import_report(&rezip(&es)).expect("import");
assert_eq!(warnings.len(), 1, "{warnings:?}");
let t = q.slides[0].shapes.iter().find_map(|s| if let deckcraft_model::ShapeKind::Table(t) = &s.kind { Some(t) } else { None }).expect("table");
assert_eq!((t.rows.len(), t.cols.len()), (rows, cols));
// A model table past the cap is refused on export.
let err = deckcraft_pptx::export(&table_deck(rows + 1, cols)).expect_err("over the cap");
assert!(err.to_string().contains("table"), "{err}");
}
/// Many slides naming one notes part: it is read for the first slide only, not copied onto every
/// slide that names it.
#[test]