aleleba/deckcraft
1
0
Fork 0
mirror of https://github.com/storytold/deckcraft.git synced 2026-10-31 05:56:55 -06:00
Code Issues Packages Projects Releases Wiki Activity

[NN6] review.compare treats an absent box like an unrotated, unflipped one

Browse Source 
slide.reset drops a placeholder's stored box, and compare called that
"rotated" because None differed from a stored rotation of 0. Absent now
equals 0 for rotation and no flips; a real flip difference reads "flipped".
PowerPoint's Compare reports a rotation only when the angle differs.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
Zach Berry
2026-10-27 21:43:36 -05:00
co-authored by Claude Sonnet 5.5
parent 48535be970
commit cd5ff01810
1 changed files with 178 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/engine/src/compare.rs
+178 -15
View File
@@ -7,6 +7,7 @@
use std::collections::{HashMap, HashSet};
use std::sync::Arc;
use deckcraft_model::resolve::Ctx;
use deckcraft_model::{MediaId, Presentation, Shape, ShapeId, ShapeKind, Slide, SlideId};
use serde::Serialize;
@@ -192,7 +193,18 @@ fn plain_shape(s: &Shape) -> Shape {
c
}
fn shape_details(cp: &Presentation, a: &Shape, rp: &Presentation, b: &Shape, out: &mut Vec<String>) {
/// The box a shape is drawn with: its own, else the one it inherits from its layout or master
/// placeholder (`ctx`).
fn drawn_box(ctx: Option<&Ctx>, s: &Shape) -> Option<deckcraft_model::Xfrm> {
s.xfrm.or_else(|| ctx.map(|c| deckcraft_model::resolve::xfrm(c, s)))
}
/// The rotation and flips a shape is drawn with (none without a box).
fn turn(ctx: Option<&Ctx>, s: &Shape) -> (f64, bool, bool) {
drawn_box(ctx, s).map_or((0.0, false, false), |x| (x.rot, x.flip_h, x.flip_v))
}
fn shape_details(cp: &Presentation, ca: Option<&Ctx>, a: &Shape, rp: &Presentation, cb: Option<&Ctx>, b: &Shape, out: &mut Vec<String>) {
let label = shape_label(b);
let text = |s: &Shape| s.text.as_ref().map(|t| t.text()).unwrap_or_default();
let mut what = vec![];
@@ -201,12 +213,24 @@ fn shape_details(cp: &Presentation, a: &Shape, rp: &Presentation, b: &Shape, out
} else if a.text != b.text {
what.push("text formatting");
}
match (&a.xfrm, &b.xfrm) {
// Boxes as drawn: a stored box equal to the inherited one is no change.
match (&drawn_box(ca, a), &drawn_box(cb, b)) {
(Some(x), Some(y)) if (x.x, x.y) != (y.x, y.y) && (x.w, x.h) == (y.w, y.h) => what.push("moved"),
(Some(x), Some(y)) if (x.w, x.h) != (y.w, y.h) => what.push("resized"),
(x, y) if x.as_ref().map(|x| x.rot) != y.as_ref().map(|y| y.rot) => what.push("rotated"),
_ => {}
}
// Rotation and flips are reported alongside a move or resize, not hidden behind it. No stored
// box means the inherited one (the layout's or master's), as the shape is drawn.
let (ta, tb) = (turn(ca, a), turn(cb, b));
if ta.0 != tb.0 {
what.push("rotated");
}
if (ta.1, ta.2) != (tb.1, tb.2) {
what.push("flipped");
}
if a.geom != b.geom {
what.push("shape");
}
if a.fill != b.fill || a.line != b.line || a.style != b.style || a.effects != b.effects {
what.push("formatting");
}
@@ -217,17 +241,24 @@ fn shape_details(cp: &Presentation, a: &Shape, rp: &Presentation, b: &Shape, out
if pic(cp, a) != pic(rp, b) {
what.push("picture");
}
if what.is_empty() {
let (mut x, mut y) = (plain_shape(a), plain_shape(b));
x.text = None;
y.text = None;
if let (ShapeKind::Picture { .. }, ShapeKind::Picture { .. }) = (&x.kind, &y.kind) {
x.kind = ShapeKind::Shape;
y.kind = ShapeKind::Shape;
}
if x != y {
what.push("properties");
}
// Anything the checks above do not cover (alt text, hidden, a link…) is reported too, also
// beside them: the fields they compared are left out of this comparison.
let (mut x, mut y) = (plain_shape(a), plain_shape(b));
for z in [&mut x, &mut y] {
z.text = None;
z.xfrm = None;
z.geom = Default::default();
z.fill = None;
z.line = None;
z.style = None;
z.effects = Default::default();
}
if let (ShapeKind::Picture { .. }, ShapeKind::Picture { .. }) = (&x.kind, &y.kind) {
x.kind = ShapeKind::Shape;
y.kind = ShapeKind::Shape;
}
if x != y {
what.push("properties");
}
if !what.is_empty() {
out.push(format!("{label}: {}", what.join(", ")));
@@ -237,6 +268,7 @@ fn shape_details(cp: &Presentation, a: &Shape, rp: &Presentation, b: &Shape, out
fn slide_details(cp: &Presentation, a: &Slide, rp: &Presentation, b: &Slide) -> Vec<String> {
let mut out = vec![];
let mut used: HashSet<usize> = HashSet::new();
let (ca, cb) = (Ctx::for_slide(cp, a), Ctx::for_slide(rp, b));
for sa in &a.shapes {
let k = b
.shapes
@@ -247,7 +279,7 @@ fn slide_details(cp: &Presentation, a: &Slide, rp: &Presentation, b: &Slide) ->
Some(k) if !used.contains(&k) => {
used.insert(k);
if let Some(sb) = b.shapes.get(k) {
shape_details(cp, sa, rp, sb, &mut out);
shape_details(cp, ca.as_ref(), sa, rp, cb.as_ref(), sb, &mut out);
}
}
_ => out.push(format!("{} deleted (only in the deck)", shape_label(sa))),
@@ -374,6 +406,25 @@ pub fn adopt(doc: &mut Presentation, rev: &Presentation, slide: &Slide, replacin
let by_name = name.and_then(|n| doc.masters.iter().flat_map(|m| &m.layouts).find(|l| l.name == n).map(|l| l.id));
sl.layout = by_name.or_else(|| doc.masters.first().and_then(|m| m.layouts.first()).map(|l| l.id)).unwrap_or_default();
}
// A placeholder without a box of its own is drawn with its layout's; where this deck's layout
// draws it otherwise (placed or turned differently), the file's box is stored on it, so the
// accepted slide looks as the file does and compares equal to it.
if let (Some(file), Some(here)) = (Ctx::for_slide(rev, slide), Ctx::for_slide(doc, &sl)) {
let boxes: Vec<Option<deckcraft_model::Xfrm>> = slide
.shapes
.iter()
.zip(&sl.shapes)
.map(|(f, h)| {
let want = deckcraft_model::resolve::xfrm(&file, f);
(h.xfrm.is_none() && deckcraft_model::resolve::xfrm(&here, h) != want).then_some(want)
})
.collect();
for (h, b) in sl.shapes.iter_mut().zip(boxes) {
if b.is_some() {
h.xfrm = b;
}
}
}
match replacing {
Some(id) => sl.id = id,
None => {
@@ -514,6 +565,118 @@ mod tests {
assert_eq!(bytes(back.media), b"open background");
}
/// NN6: a slide reset drops the placeholders' stored boxes; comparing it with the copy from
/// before the reset must not call that a rotation (no stored box is rotation 0).
#[test]
fn a_reset_slide_is_not_rotated_against_its_earlier_copy() {
let mut s = Session::with_new();
s.execute("slide.new", &json!({"layout": "titleAndContent", "title": "T", "body": "b"})).unwrap();
let ids: Vec<u32> = s.doc().unwrap().doc.slides[1].shapes.iter().map(|x| x.id.0).collect();
for id in &ids {
s.execute("shape.move", &json!({"ids": [id], "dx": 10, "dy": 0})).unwrap();
s.execute("shape.move", &json!({"ids": [id], "dx": -10, "dy": 0})).unwrap();
}
let before = s.execute("file.saveBytes", &json!({})).unwrap()["data"].as_str().unwrap().to_string();
s.execute("slide.reset", &json!({"index": 1})).unwrap();
let v = s.execute("review.compare", &json!({"name": "before.deckcraft", "data": before})).unwrap();
assert!(!v.to_string().contains("rotated"), "{v}");
}
/// A shape both moved and rotated reports both, not only the move.
#[test]
fn a_moved_and_rotated_shape_reports_both() {
let mut s = Session::with_new();
let id = s.execute("shape.insert", &json!({"preset": "rect", "rect": [100, 100, 100, 50]})).unwrap()["id"].as_u64().unwrap();
let before = s.execute("file.saveBytes", &json!({})).unwrap()["data"].as_str().unwrap().to_string();
s.execute("shape.move", &json!({"ids": [id], "dx": 40, "dy": 0})).unwrap();
s.execute("shape.rotate", &json!({"ids": [id], "deg": 30})).unwrap();
let v = s.execute("review.compare", &json!({"name": "before.deckcraft", "data": before})).unwrap().to_string();
assert!(v.contains("moved") && v.contains("rotated"), "{v}");
}
/// A property no other check covers (here the alt text) is reported beside a flip.
#[test]
fn a_flipped_shape_with_new_alt_text_reports_both() {
let mut s = Session::with_new();
let id = s.execute("shape.insert", &json!({"preset": "rect", "rect": [100, 100, 100, 50]})).unwrap()["id"].as_u64().unwrap();
let before = s.execute("file.saveBytes", &json!({})).unwrap()["data"].as_str().unwrap().to_string();
s.execute("shape.flip", &json!({"ids": [id]})).unwrap();
let st = s.doc_mut().unwrap();
let doc = std::sync::Arc::make_mut(&mut st.doc);
let sl = std::sync::Arc::make_mut(&mut doc.slides[0]);
sl.shapes.iter_mut().find(|x| x.id.0 as u64 == id).unwrap().descr = "A red box".into();
let v = s.execute("review.compare", &json!({"name": "before.deckcraft", "data": before})).unwrap().to_string();
assert!(v.contains("flipped") && v.contains("properties"), "{v}");
}
/// A flip and a change of shape (rectangle to ellipse) are both reported.
#[test]
fn a_flipped_shape_that_changed_shape_reports_both() {
let mut s = Session::with_new();
let id = s.execute("shape.insert", &json!({"preset": "rect", "rect": [100, 100, 100, 50]})).unwrap()["id"].as_u64().unwrap();
let before = s.execute("file.saveBytes", &json!({})).unwrap()["data"].as_str().unwrap().to_string();
s.execute("shape.flip", &json!({"ids": [id]})).unwrap();
s.execute("shape.change", &json!({"ids": [id], "preset": "ellipse"})).unwrap();
let v = s.execute("review.compare", &json!({"name": "before.deckcraft", "data": before})).unwrap().to_string();
assert!(v.contains("flipped") && v.contains("shape\""), "{v}");
}
/// A placeholder with no box of its own is drawn with its layout's rotation; setting it to 0
/// on the slide is a rotation change, and compare says so.
#[test]
fn an_inherited_rotation_counts_when_compared() {
let mut s = Session::with_new();
s.execute("slide.new", &json!({"layout": "titleOnly", "title": "T"})).unwrap();
s.execute("slide.reset", &json!({"index": 1})).unwrap();
let st = s.doc_mut().unwrap();
let doc = std::sync::Arc::make_mut(&mut st.doc);
let layout = doc.slides[1].layout;
let title = |sh: &deckcraft_model::Shape| sh.ph.as_ref().is_some_and(|p| matches!(p.kind, deckcraft_model::PhType::Title));
let (mi, li) = doc.masters.iter().enumerate().find_map(|(m, x)| x.layouts.iter().position(|l| l.id == layout).map(|l| (m, l))).unwrap();
let ph = std::sync::Arc::make_mut(&mut doc.masters[mi]).layouts[li].shapes.iter_mut().find(|x| title(x)).unwrap();
let mut x = ph.xfrm.unwrap();
x.rot = 30.0;
ph.xfrm = Some(x);
assert!(doc.slides[1].shapes.iter().find(|x| title(x)).unwrap().xfrm.is_none(), "the title inherits its box");
let before = s.execute("file.saveBytes", &json!({})).unwrap()["data"].as_str().unwrap().to_string();
let st = s.doc_mut().unwrap();
let doc = std::sync::Arc::make_mut(&mut st.doc);
let sl = std::sync::Arc::make_mut(&mut doc.slides[1]);
let t = sl.shapes.iter_mut().find(|x| title(x)).unwrap();
x.rot = 0.0;
t.xfrm = Some(x);
let v = s.execute("review.compare", &json!({"name": "before.deckcraft", "data": before})).unwrap();
assert!(v.to_string().contains("rotated"), "{v}");
}
/// The file's placeholder inherits an unturned box from its layout while this deck's layout
/// turns it: accepting stores the file's box on the placeholder, so the slide is drawn as in the
/// file and nothing is left to accept.
#[test]
fn accepting_an_inherited_box_takes_the_files_box() {
let mut s = Session::with_new();
s.execute("slide.new", &json!({"layout": "titleOnly", "title": "T"})).unwrap();
s.execute("slide.reset", &json!({"index": 1})).unwrap();
let file = s.execute("file.saveBytes", &json!({})).unwrap()["data"].as_str().unwrap().to_string();
let title = |sh: &deckcraft_model::Shape| sh.ph.as_ref().is_some_and(|p| matches!(p.kind, deckcraft_model::PhType::Title));
let st = s.doc_mut().unwrap();
let doc = std::sync::Arc::make_mut(&mut st.doc);
let layout = doc.slides[1].layout;
let (mi, li) = doc.masters.iter().enumerate().find_map(|(m, x)| x.layouts.iter().position(|l| l.id == layout).map(|l| (m, l))).unwrap();
let ph = std::sync::Arc::make_mut(&mut doc.masters[mi]).layouts[li].shapes.iter_mut().find(|x| title(x)).unwrap();
let mut x = ph.xfrm.unwrap();
x.rot = 30.0;
ph.xfrm = Some(x);
let v = s.execute("review.compare", &json!({"name": "file.deckcraft", "data": file})).unwrap();
assert!(v.to_string().contains("rotated"), "{v}");
s.execute("review.accept", &json!({"all": true})).unwrap();
let st = s.doc().unwrap();
let t = st.doc.slides[1].shapes.iter().find(|x| title(x)).unwrap();
assert_eq!(t.xfrm.map(|x| x.rot), Some(0.0), "the file's box is stored: {:?}", t.xfrm);
let v = s.execute("review.compare", &json!({"name": "file.deckcraft", "data": file})).unwrap();
assert!(v["changes"].as_array().is_some_and(|c| c.is_empty()), "nothing left to accept: {v}");
}
#[test]
fn each_change_says_which_side_it_is_on() {
let (mut s, rev, _) = decks();