diff --git a/ROADMAP.md b/ROADMAP.md index 76e5c428..a192ae59 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ **Stage: alpha** · next: beta, ~20 points and ~120–210 h away -> **Last reviewed:** 2026-11-02 · **Last updated:** 2026-11-03 · **Change:** trivial (Reuse Slides brings along referenced custom table styles, remapping conflicting style ids; parity scores unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) +> **Last reviewed:** 2026-11-02 · **Last updated:** 2026-11-03 · **Change:** trivial (Reuse Slides brings along referenced custom table styles, remapping conflicting style ids; previous: tiled picture fills keep their size in scaled groups and upright tiles (#1693); parity scores unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) > **Last reviewed:** 2026-10-14 · **Last updated:** 2026-11-01 · **Change:** trivial (locked No Fill and no-outline click-through regression coverage added; No Fill hit-testing follows resolved fill instead of the presence of a style (#1621); scores unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) > **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-30 · **Change:** trivial (progress log: read-only `slide.actions` lists shape and table text actions; parity scores unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) > **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-27 · **Change:** trivial (progress log: Set Up Slide Show ▸ Keep mouse cursor visible, #1445; stage and scores unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) @@ -151,6 +151,7 @@ app's JSON control channel (`deckcraft --control PORT`). |---|---| | 2026-11-02 | File › About replaces the unused Account page, with version, project links, contributor/model credits and appearance preferences | | 2026-11-03 | Reuse Slides brings along referenced custom table styles, including grouped tables, and remaps conflicting style ids without changing destination tables | +| 2026-11-02 | Tiled picture fills in groups: a tile (`a:blipFill` with `a:tile`) keeps its size, offset and anchor in slide points whatever the group's child-coordinate scale, uniform or stretched unevenly, nested or flipped (also a group's own tile fill passed to its members, and a picture shape's own tile, with its shadow and reflection), and tiles with `rotWithShape="0"` keep their size in points at any zoom, as PowerPoint draws them (measured on its own PNG export of a synthetic probe deck: every grouped case within ~3/255 of the ungrouped control's own difference from PowerPoint, which is ~13/255 from resampling). Before, a group whose child space was ~1800 times smaller than its box showed one flat texel of the texture (a Venn diagram with paper-textured circles and a hatched intersection) | | 2026-10-31 | No Fill shape interiors allow selection and editing of underlying objects, even when a quick style remains; hit-testing uses the renderer’s fill resolution. Outlines, rotation, text targets and actual fills keep their hit behaviour (#1621) | | 2026-10-30 | Print-preview tests use the URL emitted by egui and its exact file path; previews are explicitly removed before the macOS process exits | | 2026-10-30 | `slide.actions` lists shape click and hover actions and text-run hyperlinks (including table cells and grouped shapes) for audit; listing never runs an action. The UI pane is a follow-up | @@ -324,6 +325,7 @@ app's JSON control channel (`deckcraft --control PORT`). |---|---|---| | 2026-11-03 | trivial | Reuse Slides brings referenced custom table styles along; scores unchanged | | 2026-11-02 | trivial | File › About replaces Account; product links, credits and appearance retained | +| 2026-11-02 | trivial | Progress log: tiled picture fills keep their size in scaled groups. Scores and milestone names unchanged | | 2026-10-31 | trivial | No Fill hit-testing follows resolved fill instead of the presence of a style (#1621); scores unchanged | | 2026-10-30 | trivial | Progress log: read-only slide action inspection; parity scores unchanged | | 2026-10-29 | trivial | Progress log: going back in the slide show reverses the left slide's transition (#1531). Scores and milestone names unchanged | diff --git a/crates/engine/src/cmd/design/master_view.rs b/crates/engine/src/cmd/design/master_view.rs index 5dfe0496..c03bfa57 100644 --- a/crates/engine/src/cmd/design/master_view.rs +++ b/crates/engine/src/cmd/design/master_view.rs @@ -1,17 +1,17 @@ use super::{ Arc, Fill, NO_ID_SPACE, PhType, Result, Run, RunKind, Session, Shape, ShapeId, Size, Target, TextBody, Value, bad, bool_param, json, - master_index, ok, opt_usize_param, rect_param, str_param, usize_param, + master_index, ok, opt_usize_param, rect_param, str_param, }; pub(super) fn master_view(s: &mut Session, p: &Value) -> Result { let st = s.doc()?; - let mi = usize_param(p, "master").unwrap_or_else(|| master_index(&st.doc, &st.selection)); + let mi = opt_usize_param(p, "master", "view.slideMaster")?.unwrap_or_else(|| master_index(&st.doc, &st.selection)); // A master thumbnail sends `{master}` and means the master itself. `{master, layout}` selects // that layout. With neither key, follow the current slide's layout. let layout = if p.get("master").is_some() && p.get("layout").is_none() { None } else { - usize_param(p, "layout") + opt_usize_param(p, "layout", "view.slideMaster")? .or_else(|| st.current_slide().and_then(|sl| st.doc.masters.get(mi).and_then(|m| m.layouts.iter().position(|l| l.id == sl.layout)))) }; let target = match layout { diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 0d52e635..8943eb9a 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -345,6 +345,7 @@ pub fn save_bytes(doc: &Presentation, format: &str) -> Result> { "rtf" => Ok(deckcraft_format::rtf::slides_to_rtf(doc).into_bytes()), "pdf" => deckcraft_pdf::export(doc, &Default::default()).map_err(|e| EngineError::Other(e.to_string())), "gif" => gif_bytes(doc, &Value::Null), + "png" | "jpeg" | "jpg" => Err(EngineError::Other("saving a presentation as a single image isn't supported; use file.export".into())), _ => deckcraft_format::save(doc).map_err(|e| EngineError::Other(e.to_string())), } } @@ -359,7 +360,13 @@ pub fn pdf_options(p: &Value, count: usize) -> Result let mut o = deckcraft_pdf::PdfOptions { layout: deckcraft_pdf::PageLayout::Slides, ..Default::default() }; o.layout = match str_param(p, "layout").unwrap_or("slides") { "notes" => deckcraft_pdf::PageLayout::Notes, - "handouts" => deckcraft_pdf::PageLayout::Handouts { per_page: usize_param(p, "perPage").unwrap_or(6).clamp(1, 9) as u8 }, + "handouts" => { + let per_page = opt_usize_param(p, "perPage", "file.export")?.unwrap_or(6); + if ![1, 2, 3, 4, 6, 9].contains(&per_page) { + return Err(bad("file.export", format!("perPage {per_page} isn't one of 1, 2, 3, 4, 6, 9"))); + } + deckcraft_pdf::PageLayout::Handouts { per_page: per_page as u8 } + } _ => deckcraft_pdf::PageLayout::Slides, }; o.dpi = f64_or(p, "dpi", o.dpi).clamp(36.0, 600.0); diff --git a/crates/engine/src/cmd/format/paragraph.rs b/crates/engine/src/cmd/format/paragraph.rs index f036068f..6d575ff1 100644 --- a/crates/engine/src/cmd/format/paragraph.rs +++ b/crates/engine/src/cmd/format/paragraph.rs @@ -55,6 +55,7 @@ pub(super) fn bullets(s: &mut Session, p: &Value) -> Result { } if size.is_some() { para.props.bullet_size = size; + para.props.bullet_size_points = None; para.props.bullet_size_follow_text = None; } if para.props.indent.is_none_or(|i| i >= 0.0) && para.props.margin_left.is_none_or(|m| m == 0.0) { @@ -85,6 +86,7 @@ pub(super) fn numbering(s: &mut Session, p: &Value) -> Result { } if size.is_some() { para.props.bullet_size = size; + para.props.bullet_size_points = None; para.props.bullet_size_follow_text = None; } para.props.margin_left = Some(27.0 + 36.0 * para.level as f64); diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 7d21bb4b..38f4de8d 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -41,12 +41,13 @@ pub fn write_file(path: &str, bytes: &[u8]) -> Result<(), String> { Ok(canonical) => canonical, Err(_) => std::path::PathBuf::from(path), }; - let target_str = target_path.to_str().unwrap_or(path); let existing_perms = std::fs::metadata(&target_path).ok().map(|m| m.permissions()); let mut attempts = 0; let (tmp, mut file) = loop { - let tmp = format!("{target_str}.{}.{}.saving", std::process::id(), COUNTER.fetch_add(1, Ordering::Relaxed)); + // A short fixed-size name: appending to the target's name could exceed the file name limit. + let tmp_name = format!(".deckcraft.{}.{}.saving", std::process::id(), COUNTER.fetch_add(1, Ordering::Relaxed)); + let tmp = target_path.with_file_name(tmp_name); match std::fs::OpenOptions::new().write(true).create_new(true).open(&tmp) { Ok(f) => break (tmp, f), Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists && attempts < 16 => attempts += 1, @@ -120,4 +121,18 @@ mod tests { let _ = std::fs::remove_file(&test_file); let _ = std::fs::remove_dir(&tmp_dir); } + + #[test] + fn write_file_overwrites_file_with_name_near_the_length_limit() { + let dir = std::env::temp_dir().join("deckcraft_io_long_name_test"); + let _ = std::fs::create_dir_all(&dir); + let file = dir.join(format!("{}.pptx", "a".repeat(250))); + std::fs::write(&file, b"old").unwrap(); + let path_str = file.to_str().unwrap(); + + write_file(path_str, b"new").unwrap(); + assert_eq!(read_file(path_str).unwrap(), b"new"); + + let _ = std::fs::remove_file(&file); + } } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 68de1341..4e75e8b4 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2410,6 +2410,23 @@ fn pdf_options_take_paper_and_orientation() { assert!(cmd::file::pdf_options(&json!({"orientation": "sideways"}), 1).is_err()); } +/// Handout `perPage` accepts only 1, 2, 3, 4, 6 and 9; anything else is an error, not a 9-up page. +#[test] +fn pdf_options_reject_unsupported_per_page() { + for n in [1, 2, 3, 4, 6, 9] { + let o = cmd::file::pdf_options(&json!({"layout": "handouts", "perPage": n}), 1).unwrap(); + assert!(matches!(o.layout, deckcraft_pdf::PageLayout::Handouts { per_page } if per_page as usize == n)); + } + for n in [0, 5, 7, 8, 10] { + let e = cmd::file::pdf_options(&json!({"layout": "handouts", "perPage": n}), 1).unwrap_err(); + assert!(e.to_string().contains("1, 2, 3, 4, 6, 9"), "perPage {n}: {e}"); + } + // Not a count at all: refused too, instead of falling back to 6. + for v in [json!(-1), json!(5.5), json!("6")] { + assert!(cmd::file::pdf_options(&json!({"layout": "handouts", "perPage": v}), 1).is_err(), "perPage {v}"); + } +} + #[test] fn shape_line_sets_compound_and_arrow_sizes() { // The Format Shape pane's Line page: compound type, arrowheads and their sizes. @@ -3206,6 +3223,7 @@ fn numbering_takes_color_and_size() { props.bullet_font_follow_text = Some(true); props.bullet_color_follow_text = Some(true); props.bullet_size_follow_text = Some(true); + props.bullet_size_points = Some(12.0); } s.execute("format.numbering", &json!({"on": true, "scheme": "romanUcPeriod", "start": 3, "color": "accent2", "size": 150})).unwrap(); let para = |s: &Session| { @@ -3219,9 +3237,11 @@ fn numbering_takes_color_and_size() { assert_eq!(pp.bullet_font_follow_text, Some(true)); assert_eq!(pp.bullet_color_follow_text, None); assert_eq!(pp.bullet_size_follow_text, None); + assert_eq!(pp.bullet_size_points, None); assert_eq!(pp.bullet_color, Some(deckcraft_model::ColorRef::scheme(deckcraft_color::SchemeSlot::Accent2))); s.execute("format.bullets", &json!({"on": true, "char": "★", "size": 9000})).unwrap(); assert_eq!(para(&s).bullet_size, Some(4.0)); + assert_eq!(para(&s).bullet_size_points, None); assert_eq!(para(&s).bullet_font_follow_text, None); } @@ -6458,3 +6478,30 @@ fn master_body_placeholder_format_keeps_per_level_formatting() { assert_eq!((level(&s, 0).run.size, level(&s, 1).run.size), (Some(28.0), Some(24.0))); assert_eq!(shown(&s), vec![Some(28.0), Some(24.0)]); } + +#[test] +fn slide_master_view_rejects_invalid_indices() { + let mut s = Session::with_new(); + for p in [json!({"master": -1}), json!({"master": 0.5}), json!({"master": 0, "layout": -1}), json!({"master": 0, "layout": 1.5})] { + let r = s.execute("view.slideMaster", &p); + assert!(r.is_err(), "invalid index accepted for {p}: {r:?}"); + } + assert!(s.execute("view.slideMaster", &json!({})).is_ok()); + assert!(s.execute("view.slideMaster", &json!({"master": 0})).is_ok()); +} + +#[test] +fn save_to_image_path_is_an_error_not_zip_bytes() { + let dir = std::env::temp_dir().join(format!("deckcraft-save-img-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + for (cmd, name) in [("file.save", "a.png"), ("file.saveAs", "a.jpg"), ("file.save", "a.jpeg")] { + let mut s = Session::new(); + sample::open_sample(&mut s).unwrap(); + let path = dir.join(name).to_string_lossy().to_string(); + let before = s.doc().unwrap().path.clone(); + assert!(s.execute(cmd, &json!({"path": path})).is_err(), "{cmd} {name} should fail"); + assert!(!std::path::Path::new(&path).exists(), "{name} must not be written"); + assert_eq!(s.doc().unwrap().path, before); + } + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/crates/model/src/text.rs b/crates/model/src/text.rs index 362f4393..ca992c38 100644 --- a/crates/model/src/text.rs +++ b/crates/model/src/text.rs @@ -242,6 +242,8 @@ pub struct ParaProps { pub bullet_color_follow_text: Option, /// Bullet size as a fraction of the text size. pub bullet_size: Option, + /// Absolute bullet font size in points, independent of the paragraph's run size. + pub bullet_size_points: Option, pub bullet_size_follow_text: Option, pub tabs: Option>, pub default_tab: Option, @@ -286,11 +288,13 @@ impl ParaProps { self.bullet_color.clone_from(&p.bullet_color); } } - if self.bullet_size_follow_text == Some(false) || self.bullet_size.is_some() { + if self.bullet_size_follow_text == Some(false) || self.bullet_size.is_some() || self.bullet_size_points.is_some() { self.bullet_size_follow_text = None; } - if self.bullet_size.is_none() && self.bullet_size_follow_text.is_none() { - if p.bullet_size_follow_text == Some(true) { + if self.bullet_size.is_none() && self.bullet_size_points.is_none() && self.bullet_size_follow_text.is_none() { + if p.bullet_size_points.is_some() { + self.bullet_size_points = p.bullet_size_points; + } else if p.bullet_size_follow_text == Some(true) { self.bullet_size_follow_text = Some(true); } else { self.bullet_size = p.bullet_size; @@ -310,6 +314,33 @@ impl ParaProps { } } +#[cfg(test)] +mod bullet_size_tests { + use super::ParaProps; + + #[test] + fn bullet_size_inheritance_keeps_percentage_and_points_as_one_mode() { + let parent = ParaProps { bullet_size_points: Some(12.0), ..Default::default() }; + let mut child = ParaProps { bullet_size: Some(2.0), ..Default::default() }; + child.inherit(&parent); + assert_eq!(child.bullet_size, Some(2.0)); + assert_eq!(child.bullet_size_points, None); + + let parent = ParaProps { bullet_size: Some(1.5), ..Default::default() }; + let mut child = ParaProps { bullet_size_points: Some(14.0), ..Default::default() }; + child.inherit(&parent); + assert_eq!(child.bullet_size, None); + assert_eq!(child.bullet_size_points, Some(14.0)); + + let parent = ParaProps { bullet_size_points: Some(12.0), ..Default::default() }; + let mut child = ParaProps { bullet_size_follow_text: Some(true), ..Default::default() }; + child.inherit(&parent); + assert_eq!(child.bullet_size, None); + assert_eq!(child.bullet_size_points, None); + assert_eq!(child.bullet_size_follow_text, Some(true)); + } +} + #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub enum Caps { diff --git a/crates/pptx/src/read/dml.rs b/crates/pptx/src/read/dml.rs index a2ed6abe..300773a5 100644 --- a/crates/pptx/src/read/dml.rs +++ b/crates/pptx/src/read/dml.rs @@ -23,6 +23,16 @@ pub fn pct(e: &El, name: &str) -> Option { e.f64(name).map(|v| v / 100_000.0) } +fn bullet_size_points(value: Option<&str>) -> Option { + value.and_then(|v| v.trim().parse::().ok()).filter(|v| (100..=400_000).contains(v)).map(|v| v as f64 / 100.0) +} + +fn bullet_size_percent(value: Option<&str>) -> Option { + let value = value?.trim(); + let (value, scale) = if let Some(value) = value.strip_suffix('%') { (value.trim(), 100.0) } else { (value, 100_000.0) }; + value.parse::().ok().filter(|v| v.is_finite()).map(|v| v / scale) +} + /// 1/60 000 degree → degrees. pub fn ang(e: &El, name: &str) -> Option { e.f64(name).map(|v| v / 60_000.0) @@ -454,9 +464,22 @@ pub fn ppr(imp: &mut Imp, part: &Part, p: &El) -> (Option, ParaProps, Option "buFont" => pp.bullet_font = e.attr("typeface").map(String::from), "buFontTx" => pp.bullet_font_follow_text = Some(true), "buClr" => pp.bullet_color = color(e), + "buSzPct" => { + pp.bullet_size_points = None; + pp.bullet_size_follow_text = None; + pp.bullet_size = bullet_size_percent(e.attr("val")).map(|v| v.clamp(0.25, 4.0)); + } + "buSzPts" => { + pp.bullet_size = None; + pp.bullet_size_follow_text = None; + pp.bullet_size_points = bullet_size_points(e.attr("val")); + } "buClrTx" => pp.bullet_color_follow_text = Some(true), - "buSzPct" => pp.bullet_size = pct(e, "val").map(|v| v.clamp(0.25, 4.0)), - "buSzTx" => pp.bullet_size_follow_text = Some(true), + "buSzTx" => { + pp.bullet_size = None; + pp.bullet_size_points = None; + pp.bullet_size_follow_text = Some(true); + } "tabLst" => { pp.tabs = Some( e.children_named("tab") @@ -692,6 +715,21 @@ mod tests { assert_eq!(color(&d.root).unwrap().base, ColorBase::Rgb { rgb: Rgba::rgb(255, 0, 0) }); } + #[test] + fn bullet_point_size_trims_schema_whitespace() { + let d = parse(br#""#).unwrap(); + assert_eq!(bullet_size_points(d.root.attr("val")), Some(14.0)); + assert_eq!(bullet_size_points(Some("1400")), Some(14.0)); + } + + #[test] + fn bullet_percentage_size_accepts_numeric_and_percent_forms() { + assert_eq!(bullet_size_percent(Some("100000")), Some(1.0)); + assert_eq!(bullet_size_percent(Some("100%")), Some(1.0)); + assert_eq!(bullet_size_percent(Some(" 125% ")), Some(1.25)); + assert_eq!(bullet_size_percent(Some("invalid")), None); + } + #[test] fn miter_limit_accepts_both_percentage_forms_with_schema_bounds() { assert_eq!(parse_miter_limit("200000"), Some(200_000)); diff --git a/crates/pptx/src/write/dml.rs b/crates/pptx/src/write/dml.rs index 32081095..84b30c83 100644 --- a/crates/pptx/src/write/dml.rs +++ b/crates/pptx/src/write/dml.rs @@ -543,6 +543,7 @@ pub fn ppr(w: &mut W, x: &mut Exp, o: &mut Out, tag: &str, level: Option, p: || p.bullet_color.is_some() || p.bullet_color_follow_text.is_some() || p.bullet_size.is_some() + || p.bullet_size_points.is_some() || p.bullet_size_follow_text.is_some() || p.tabs.is_some() || def.is_some(); @@ -567,8 +568,10 @@ pub fn ppr(w: &mut W, x: &mut Exp, o: &mut Out, tag: &str, level: Option, p: color(w, c); w.close("a:buClr"); } - if p.bullet_size.is_none() && p.bullet_size_follow_text == Some(true) { + if p.bullet_size.is_none() && p.bullet_size_points.is_none() && p.bullet_size_follow_text == Some(true) { w.empty0("a:buSzTx"); + } else if let Some(s) = p.bullet_size_points.filter(|s| s.is_finite() && (1.0..=4000.0).contains(s)) { + w.val("a:buSzPts", ((s * 100.0).round() as u32).to_string()); } else if let Some(s) = p.bullet_size { w.val("a:buSzPct", pct(s.clamp(0.25, 4.0))); } diff --git a/crates/pptx/tests/it/roundtrip.rs b/crates/pptx/tests/it/roundtrip.rs index f651fc5c..1ed97f54 100644 --- a/crates/pptx/tests/it/roundtrip.rs +++ b/crates/pptx/tests/it/roundtrip.rs @@ -490,6 +490,33 @@ fn shapes_fills_lines_effects_round_trip() { assert_eq!(by_name(&q.slides[1], "Group").id, by_name(&p.slides[1], "Group").id); } +#[test] +fn bullet_size_in_points_survives_two_pptx_round_trips() { + let mut p = Presentation::default(); + let mut textbox = shape(&mut p, "Point-sized bullet", 20.0, 20.0, 400.0, 100.0); + textbox.text = Some(TextBody { + paragraphs: [12.0, 500.0, 4000.0] + .into_iter() + .map(|size| Paragraph { + props: deckcraft_model::text::ParaProps { + bullet: Some(Bullet::Char { char: "•".into() }), + bullet_size_points: Some(size), + ..Default::default() + }, + runs: vec![Run::with("Bullet text", RunProps::default())], + ..Default::default() + }) + .collect(), + ..Default::default() + }); + Arc::make_mut(&mut p.slides[0]).shapes.push(textbox); + + let q = round(&round(&p)); + let props = &by_name(&q.slides[0], "Point-sized bullet").text.as_ref().expect("text").paragraphs; + assert_eq!(props.iter().map(|p| p.props.bullet_size_points).collect::>(), vec![Some(12.0), Some(500.0), Some(4000.0)]); + assert!(props.iter().all(|p| p.props.bullet_size.is_none())); +} + #[test] fn line_miter_limit_survives_two_pptx_round_trips() { let mut p = Presentation::default(); diff --git a/crates/render/src/paint.rs b/crates/render/src/paint.rs index ee572fd9..74d11123 100644 --- a/crates/render/src/paint.rs +++ b/crates/render/src/paint.rs @@ -91,7 +91,10 @@ pub fn fill_path( let on_path = m * path.clone(); ctx.set_transform(Affine::IDENTITY); ctx.push_clip_layer(&on_path); - picture(ctx, pres, &resolved(rctx, pf, ph), on_slide, Affine::IDENTITY, None); + // Drawn in device pixels, so a tile (given in shape units) is sized to pixels first. + let tiled = tiled_per_point(fill, (c[0].hypot(c[1]), c[2].hypot(c[3]))); + let upright = if let Fill::Picture(p) = tiled.as_ref() { p } else { pf }; + picture(ctx, pres, &resolved(rctx, upright, ph), on_slide, Affine::IDENTITY, None); ctx.pop_layer(); return; } @@ -226,9 +229,12 @@ pub fn picture(ctx: &mut RenderContext, pres: &Presentation, pf: &PictureFill, b } let (nw, nh) = images::natural_size(&item.data).unwrap_or((iw, ih)); // PowerPoint draws the tile at a whole number of output pixels. - let k = px_scale.max(1e-9); - let snap = |v: f64| (v * k).round().max(1.0) / k; - let (tile_w, tile_h) = (snap(nw * 0.75 * sx), snap(nh * 0.75 * sy)); + // Per axis: a group can stretch its child space unevenly, and a pixel is then a different + // number of shape units across and down. + let c = m.as_coeffs(); + let (kx, ky) = (c[0].hypot(c[1]).max(1e-9), c[2].hypot(c[3]).max(1e-9)); + let snap = |v: f64, k: f64| (v * k).round().max(1.0) / k; + let (tile_w, tile_h) = (snap(nw * 0.75 * sx, kx), snap(nh * 0.75 * sy, ky)); if !(tile_w > 1e-6 && tile_h > 1e-6 && tile_w.is_finite() && tile_h.is_finite()) { return; } @@ -260,6 +266,31 @@ pub fn picture(ctx: &mut RenderContext, pres: &Presentation, pf: &PictureFill, b draw_image(ctx, &image, iw, ih, dest, m, alpha); } +/// `fill`, with a tiled picture's tile size and offset moved from points to shape units, given +/// the shape units to the point along x and y. A group's child coordinates scale its shapes' +/// boxes (a group can stretch them unevenly), but a tile keeps its size on the slide as PowerPoint +/// draws it, so in a group with a legacy child space the tile would otherwise cover hundreds of +/// boxes and the fill show one flat texel. Anything but a tiled picture, or a unit scale, is +/// returned as it is. +pub fn tiled_per_point(fill: &Fill, per_point: (f64, f64)) -> std::borrow::Cow<'_, Fill> { + let Fill::Picture(pf) = fill else { return std::borrow::Cow::Borrowed(fill) }; + match tiled_picture_per_point(pf, per_point) { + std::borrow::Cow::Borrowed(_) => std::borrow::Cow::Borrowed(fill), + std::borrow::Cow::Owned(p) => std::borrow::Cow::Owned(Fill::Picture(p)), + } +} + +/// [`tiled_per_point`] for a picture fill itself (a picture shape's own fill is not a [`Fill`]). +pub fn tiled_picture_per_point(pf: &PictureFill, per_point: (f64, f64)) -> std::borrow::Cow<'_, PictureFill> { + let PictureMode::Tile { tx, ty, sx, sy, flip, align } = &pf.mode else { return std::borrow::Cow::Borrowed(pf) }; + let (kx, ky) = per_point; + if !(kx.is_finite() && ky.is_finite() && kx > 1e-12 && ky > 1e-12) || ((kx - 1.0).abs() < 1e-9 && (ky - 1.0).abs() < 1e-9) { + return std::borrow::Cow::Borrowed(pf); + } + let mode = PictureMode::Tile { tx: tx * kx, ty: ty * ky, sx: sx * kx, sy: sy * ky, flip: flip.clone(), align: align.clone() }; + std::borrow::Cow::Owned(PictureFill { mode, ..pf.clone() }) +} + /// Where the first tile of a tiled picture fill goes in `bounds`: its corner, edge middle or /// centre named by `align` (`tl`, `t`, `tr`, `l`, `ctr`, `r`, `bl`, `b`, `br`) on the same point /// of the box, then moved by the offset (points). diff --git a/crates/render/src/renderer.rs b/crates/render/src/renderer.rs index c25f6bab..e7cd5ff3 100644 --- a/crates/render/src/renderer.rs +++ b/crates/render/src/renderer.rs @@ -346,6 +346,14 @@ impl Renderer { if k.is_finite() && k > 1e-12 { 1.0 / k } else { 1.0 } } + /// [`Renderer::per_point`] along the shape's own x and y axes: a group can stretch its child + /// space unevenly, and a tiled fill keeps its size in points on the slide on both axes. + fn per_point_xy(&self, m: Affine, depth: usize) -> (f64, f64) { + let c = self.line_space(m, depth).1.as_coeffs(); + let inv = |s: f64| if s.is_finite() && s > 1e-12 { 1.0 / s } else { 1.0 }; + (inv(c[0].hypot(c[1])), inv(c[2].hypot(c[3]))) + } + /// The size and transform a shape's text is set in: inside a group whose child coordinates /// scale the shape, the shape's size in slide points and `m` with that scale taken out, so /// insets and font sizes stay in points; otherwise `w`, `h` and `m` themselves. @@ -428,7 +436,8 @@ impl Renderer { // A group's own fill (a group filled with its parent's passes that on). let own = match &s.fill { Some(Fill::Group) | None => self.group_fill.clone(), - Some(fl) => Some((fl.clone(), m, Rect::new(0.0, 0.0, x.w, x.h))), + // A tile is given in points; this group's own space may be scaled by its parents. + Some(fl) => Some((paint::tiled_per_point(fl, self.per_point_xy(m, depth)).into_owned(), m, Rect::new(0.0, 0.0, x.w, x.h))), }; let outer = std::mem::replace(&mut self.group_fill, own); // A group's outer shadow is cast by everything in it, as PowerPoint draws it: the @@ -541,6 +550,7 @@ impl Renderer { geo: &Geometry, (w, h): (f64, f64), m: Affine, + per_point: (f64, f64), fade: &dyn Fn(usize) -> deckcraft_geom::morph::Fade, ) { for (i, sp) in geo.paths.iter().enumerate() { @@ -566,7 +576,10 @@ impl Renderer { paint::fill_path(ctx, rctx, gf, None, &path, *gbox, *gm, sp.fill, fade(i).fill, f.pres); } } - _ => paint::fill_path(ctx, rctx, fl, fill_ph, &sp.path, Rect::new(0.0, 0.0, w, h), m, sp.fill, fade(i).fill, f.pres), + _ => { + let fl = paint::tiled_per_point(fl, per_point); + paint::fill_path(ctx, rctx, &fl, fill_ph, &sp.path, Rect::new(0.0, 0.0, w, h), m, sp.fill, fade(i).fill, f.pres) + } } ctx.set_fill_rule(peniko::Fill::NonZero); } @@ -655,9 +668,13 @@ impl Renderer { let line = &d.line; // Effects below the shape. if let Some(e) = &effects { - // A picture (or picture fill) casts its shadow from its pixels, not its frame. + // A picture (or picture fill) casts its shadow from its pixels, not its frame. A tile + // is sized in points on the slide, as the face's is, and so is its shadow and reflection. let content = match (&s.kind, &fill) { - (ShapeKind::Picture { fill: pf }, _) | (_, Some(Fill::Picture(pf))) => Some(paint::resolved(rctx, pf, None).into_owned()), + (ShapeKind::Picture { fill: pf }, _) | (_, Some(Fill::Picture(pf))) => { + let pf = paint::tiled_picture_per_point(pf, self.per_point_xy(m, depth)); + Some(paint::resolved(rctx, &pf, None).into_owned()) + } _ => None, }; // What casts the shadow: the fill as see-through as it is, and the outline. @@ -736,6 +753,7 @@ impl Renderer { let fill = &d.fill; let outline = &d.outline; let fade = |i: usize| d.fade(i); + let pt_xy = self.per_point_xy(m, d.depth); // Fill. match &s.kind { ShapeKind::Picture { fill: pf } => { @@ -744,13 +762,14 @@ impl Renderer { if s.fill.as_ref().is_some_and(|f| !f.is_none()) && let Some(fl) = &fill { - self.fill_geometry(ctx, f, rctx, fl, fill_ph, geo, (w, h), m, &fade); + self.fill_geometry(ctx, f, rctx, fl, fill_ph, geo, (w, h), m, pt_xy, &fade); } ctx.set_transform(m); let bounds = Rect::new(0.0, 0.0, w, h); ctx.push_clip_layer(outline); let t = f.opts.gif_times.iter().find(|g| g.0 == s.id).map(|g| g.1); - paint::picture(ctx, f.pres, &paint::resolved(rctx, pf, None), bounds, m, t); + let pf = paint::tiled_picture_per_point(pf, pt_xy); + paint::picture(ctx, f.pres, &paint::resolved(rctx, &pf, None), bounds, m, t); ctx.pop_layer(); } // An object DeckCraft doesn't model (an embedded OLE object, an unknown graphic): its @@ -761,7 +780,7 @@ impl Renderer { if s.fill.as_ref().is_some_and(|f| !f.is_none()) && let Some(fl) = &fill { - self.fill_geometry(ctx, f, rctx, fl, fill_ph, geo, (w, h), m, &fade); + self.fill_geometry(ctx, f, rctx, fl, fill_ph, geo, (w, h), m, pt_xy, &fade); } ctx.set_transform(m); let pf = deckcraft_model::style::PictureFill { media: *media, ..Default::default() }; @@ -787,7 +806,7 @@ impl Renderer { if let Some(fl) = &fill && !(empty_ph && f.opts.edit && s.fill.is_none() && s.line.is_none() && s.style.is_none()) { - self.fill_geometry(ctx, f, rctx, fl, fill_ph, geo, (w, h), m, &fade); + self.fill_geometry(ctx, f, rctx, fl, fill_ph, geo, (w, h), m, pt_xy, &fade); } } } diff --git a/crates/render/src/tests_wild.rs b/crates/render/src/tests_wild.rs index ae78f64b..987781f1 100644 --- a/crates/render/src/tests_wild.rs +++ b/crates/render/src/tests_wild.rs @@ -454,6 +454,198 @@ fn tiled_pictures_flip_align_and_skip_non_positive_scales() { assert!(close(img.pixel(115, 115), WHITE, 5) && close(img.pixel(300, 150), WHITE, 5)); } +#[allow(clippy::unwrap_used)] +fn red_over_blue() -> Vec { + let img = image::RgbImage::from_fn(4, 8, |_, y| if y < 4 { image::Rgb([255, 0, 0]) } else { image::Rgb([0, 0, 255]) }); + let mut out = std::io::Cursor::new(vec![]); + image::DynamicImage::ImageRgb8(img).write_to(&mut out, image::ImageFormat::Png).unwrap(); + out.into_inner() +} + +#[test] +fn a_tile_keeps_its_size_in_a_group_with_scaled_child_coordinates() { + use deckcraft_model::style::{PictureFill, PictureMode}; + // A group's child space scales its shapes' boxes, not the tile: the same tiles show whether + // the group's child space is 1600 times smaller (old decks write them so), 4 times larger, + // the slide's own or stretched unevenly (PowerPoint draws all of them alike, measured on its + // own export of a probe deck). Before, one tile covered hundreds of boxes and the fill was one + // flat texel, and an uneven group stretched the tile. + // Wide: 8x4 px = 6x3 pt, ten times = 60x30 pt tiles, red then blue across. + // Tall: 4x8 px = 3x6 pt, ten times = 30x60 pt tiles, red over blue down. + let blue = Rgba::rgb(0, 0, 255); + let scales = [(1.0, 1.0), (1600.0, 1600.0), (0.25, 0.25), (2.0, 0.5), (0.5, 2.0), (1800.0, 1.0), (1.0, 1800.0)]; + for (kx, ky) in scales { + for tall in [false, true] { + let mut p = Presentation::default(); + let png = if tall { red_over_blue() } else { half_red_half_blue() }; + let media = p.add_media("t.png", "image/png", png).expect("media"); + let mode = PictureMode::Tile { tx: 0.0, ty: 0.0, sx: 10.0, sy: 10.0, flip: "none".into(), align: "tl".into() }; + let member = Shape { + id: ShapeId(22), + xfrm: Some(Xfrm::new(0.0, 0.0, 240.0 / kx, 60.0 / ky)), + fill: Some(Fill::Picture(PictureFill { media, mode, ..Default::default() })), + line: no_line(), + ..Default::default() + }; + let group = Shape { + id: ShapeId(21), + xfrm: Some(Xfrm::new(100.0, 100.0, 240.0, 60.0)), + kind: ShapeKind::Group { children: vec![member], child: Xfrm::new(0.0, 0.0, 240.0 / kx, 60.0 / ky) }, + ..Default::default() + }; + std::sync::Arc::make_mut(&mut p.slides[0]).shapes = vec![group]; + let img = render_slide(&p, 0, &RenderOpts { scale: 1.0, ..Default::default() }); + // Along the tile's long side: first half red, second blue, then red again. + let (a, b, c) = if tall { ((115, 115), (115, 145), (115, 175)) } else { ((115, 115), (145, 115), (175, 115)) }; + let (pa, pb, pc) = (img.pixel(a.0, a.1), img.pixel(b.0, b.1), img.pixel(c.0, c.1)); + assert!( + close(pa, RED, 30) && close(pb, blue, 30) && (tall || close(pc, RED, 30)) && (!tall || close(pc, WHITE, 5) || close(pc, RED, 30)), + "child space {kx} x {ky}, tall={tall}: {pa:?} {pb:?} {pc:?}" + ); + } + } +} + +/// A slide with `shape` (built for a box of `w` x `h` child units) in a group at (100, 100) that is +/// 240 x 60 pt on the slide and scaled `kx` x `ky` against its child space, drawn at 1 px per point. +fn in_a_scaled_group(kx: f64, ky: f64, p: Presentation, shape: impl FnOnce(f64, f64) -> Shape) -> Image { + let (w, h) = (240.0 / kx, 60.0 / ky); + let group = Shape { + id: ShapeId(21), + xfrm: Some(Xfrm::new(100.0, 100.0, 240.0, 60.0)), + kind: ShapeKind::Group { children: vec![shape(w, h)], child: Xfrm::new(0.0, 0.0, w, h) }, + ..Default::default() + }; + let mut p = p; + std::sync::Arc::make_mut(&mut p.slides[0]).shapes = vec![group]; + render_slide(&p, 0, &RenderOpts { scale: 1.0, ..Default::default() }) +} + +#[test] +fn a_tile_a_group_passes_to_its_members_keeps_its_size_inside_a_scaled_group() { + use deckcraft_model::style::{PictureFill, PictureMode}; + // The group that owns the tiled fill sits inside another group whose child space is scaled: the + // members that take it (`grpFill`) get tiles of 60 x 30 pt, as with no outer group. + let blue = Rgba::rgb(0, 0, 255); + for (kx, ky) in [(1.0, 1.0), (0.25, 0.25), (1600.0, 1600.0), (2.0, 0.5)] { + let mut p = Presentation::default(); + let media = p.add_media("t.png", "image/png", half_red_half_blue()).expect("media"); + let mode = PictureMode::Tile { tx: 0.0, ty: 0.0, sx: 10.0, sy: 10.0, flip: "none".into(), align: "tl".into() }; + let img = in_a_scaled_group(kx, ky, p, |w, h| { + let member = + Shape { id: ShapeId(23), xfrm: Some(Xfrm::new(0.0, 0.0, w, h)), fill: Some(Fill::Group), line: no_line(), ..Default::default() }; + Shape { + id: ShapeId(22), + xfrm: Some(Xfrm::new(0.0, 0.0, w, h)), + fill: Some(Fill::Picture(PictureFill { media, mode, ..Default::default() })), + kind: ShapeKind::Group { children: vec![member], child: Xfrm::new(0.0, 0.0, w, h) }, + ..Default::default() + } + }); + let (a, b, c) = (img.pixel(115, 115), img.pixel(145, 115), img.pixel(175, 115)); + assert!(close(a, RED, 30) && close(b, blue, 30) && close(c, RED, 30), "outer group {kx} x {ky}: {a:?} {b:?} {c:?}"); + } +} + +#[test] +fn a_picture_shapes_own_tile_keeps_its_size_inside_a_scaled_group() { + use deckcraft_model::style::{PictureFill, PictureMode}; + // A picture (`p:pic`) whose blip fill is a tile rather than a stretch. + let blue = Rgba::rgb(0, 0, 255); + for (kx, ky) in [(1.0, 1.0), (0.25, 0.25), (1600.0, 1600.0), (2.0, 0.5)] { + let mut p = Presentation::default(); + let media = p.add_media("t.png", "image/png", half_red_half_blue()).expect("media"); + let mode = PictureMode::Tile { tx: 0.0, ty: 0.0, sx: 10.0, sy: 10.0, flip: "none".into(), align: "tl".into() }; + let img = in_a_scaled_group(kx, ky, p, |w, h| Shape { + id: ShapeId(22), + xfrm: Some(Xfrm::new(0.0, 0.0, w, h)), + kind: ShapeKind::Picture { fill: PictureFill { media, mode, ..Default::default() } }, + line: no_line(), + ..Default::default() + }); + let (a, b, c) = (img.pixel(115, 115), img.pixel(145, 115), img.pixel(175, 115)); + assert!(close(a, RED, 30) && close(b, blue, 30) && close(c, RED, 30), "group {kx} x {ky}: {a:?} {b:?} {c:?}"); + } +} + +#[test] +fn a_tiled_pictures_reflection_keeps_the_tile_size_inside_a_scaled_group() { + use deckcraft_model::style::{PictureFill, PictureMode, Reflection}; + // The reflection mirrors the picture's pixels, so its tiles must be the face's, 60 x 30 pt, + // whatever the group's child scale: just below the box it shows the same colours. + let reflected = |kx: f64, ky: f64| { + let mut p = Presentation::default(); + let media = p.add_media("t.png", "image/png", half_red_half_blue()).expect("media"); + let mode = PictureMode::Tile { tx: 0.0, ty: 0.0, sx: 10.0, sy: 10.0, flip: "none".into(), align: "tl".into() }; + let rf = Reflection { blur: 0.0, start_alpha: 1.0, end_alpha: 0.0, end_pos: 0.5, dist: 0.0 }; + let img = in_a_scaled_group(kx, ky, p, |w, h| Shape { + id: ShapeId(22), + xfrm: Some(Xfrm::new(0.0, 0.0, w, h)), + kind: ShapeKind::Picture { fill: PictureFill { media, mode, ..Default::default() } }, + line: no_line(), + effects: Some(deckcraft_model::Effects { reflection: Some(rf), ..Default::default() }), + ..Default::default() + }); + [img.pixel(115, 163), img.pixel(145, 163), img.pixel(175, 163)] + }; + let plain = reflected(1.0, 1.0); + assert!(plain[0][0] > plain[0][2] && plain[1][2] > plain[1][0] && plain[2][0] > plain[2][2], "red, blue, red in the reflection: {plain:?}"); + for (kx, ky) in [(0.25, 0.25), (1600.0, 1600.0), (2.0, 0.5)] { + let got = reflected(kx, ky); + for (g, e) in got.iter().zip(&plain) { + assert!(close(*g, Rgba::rgb(e[0], e[1], e[2]), 40), "child scale {kx} x {ky}: {got:?} against {plain:?}"); + } + } +} + +#[test] +fn an_upright_tile_in_a_turned_shape_keeps_its_size_in_points_at_any_zoom() { + use deckcraft_model::style::{PictureFill, PictureMode}; + // `rotWithShape="0"` draws the tiles on the slide's axes, in device pixels: they were sized in + // pixels, so zooming (or a group's child scale) changed their size on the slide. PowerPoint's + // export of the same shapes shows 60x30 pt tiles at every zoom. + let draw = |scale: f64, group: f64| { + let mut p = Presentation::default(); + let media = p.add_media("t.png", "image/png", half_red_half_blue()).expect("media"); + let mode = PictureMode::Tile { tx: 0.0, ty: 0.0, sx: 10.0, sy: 10.0, flip: "none".into(), align: "tl".into() }; + let fill = Fill::Picture(PictureFill { media, mode, upright: true, ..Default::default() }); + let shape = Shape { + id: ShapeId(22), + xfrm: Some(Xfrm { rot: 20.0, ..Xfrm::new(0.0, 0.0, 240.0 / group, 120.0 / group) }), + fill: Some(fill), + line: no_line(), + ..Default::default() + }; + let shape = if group == 1.0 { + Shape { xfrm: Some(Xfrm { rot: 20.0, ..Xfrm::new(100.0, 100.0, 240.0, 120.0) }), ..shape } + } else { + Shape { + id: ShapeId(21), + xfrm: Some(Xfrm::new(100.0, 100.0, 240.0, 120.0)), + kind: ShapeKind::Group { children: vec![shape], child: Xfrm::new(0.0, 0.0, 240.0 / group, 120.0 / group) }, + ..Default::default() + } + }; + std::sync::Arc::make_mut(&mut p.slides[0]).shapes = vec![shape]; + render_slide(&p, 0, &RenderOpts { scale, ..Default::default() }) + }; + for group in [1.0, 0.25] { + let (one, two) = (draw(1.0, group), draw(2.0, group)); + let (mut same, mut total) = (0, 0); + for y in (120..220).step_by(7) { + for x in (120..320).step_by(9) { + let (a, b) = (one.pixel(x, y), two.pixel(2 * x, 2 * y)); + if close(a, WHITE, 5) && close(b, WHITE, 5) { + continue; // outside the turned rectangle + } + total += 1; + same += usize::from(close(a, Rgba::rgb(b[0], b[1], b[2]), 40)); + } + } + assert!(total > 40 && same * 10 >= total * 9, "group scale {group}: {same} of {total} samples agree between zoom 1 and 2"); + } +} + #[test] fn a_tiles_first_copy_sits_on_the_point_its_alignment_names() { let b = Rect::new(0.0, 0.0, 100.0, 50.0); diff --git a/crates/text/src/shape.rs b/crates/text/src/shape.rs index 2be8424f..d5558001 100644 --- a/crates/text/src/shape.rs +++ b/crates/text/src/shape.rs @@ -76,7 +76,7 @@ pub(crate) fn shape_para( if let Some(c) = &props.bullet_color { rp.fill = Some(Fill::solid(c.clone())); } - rp.size = Some(lead.size.unwrap_or(18.0) * props.bullet_size.unwrap_or(1.0)); + rp.size = Some(props.bullet_size_points.unwrap_or(lead.size.unwrap_or(18.0) * props.bullet_size.unwrap_or(1.0))); rp.underline = None; rp.strike = None; rp.highlight = None; @@ -105,7 +105,7 @@ pub(crate) fn shape_para( if let Some(c) = &props.bullet_color { rp.fill = Some(Fill::solid(c.clone())); } - rp.size = Some(lead.size.unwrap_or(18.0) * props.bullet_size.unwrap_or(1.0)); + rp.size = Some(props.bullet_size_points.unwrap_or(lead.size.unwrap_or(18.0) * props.bullet_size.unwrap_or(1.0))); rp.underline = None; let mut st = make_style(ctx, &rp, scale, opts.prompt_color); let label = autonum(scheme, start_at.saturating_sub(1).saturating_add(number.unwrap_or(1))); diff --git a/crates/ui-egui/src/panels/comments.rs b/crates/ui-egui/src/panels/comments.rs index edf0e8b8..5acb7140 100644 --- a/crates/ui-egui/src/panels/comments.rs +++ b/crates/ui-egui/src/panels/comments.rs @@ -130,29 +130,34 @@ fn comment_card(app: &mut SlideApp, ui: &mut Ui, slide: usize, index: usize, c: for r in &c.replies { ui.label(egui::RichText::new(format!("↳ {}: {}", r.author, r.text)).color(t.text_dim)); } + // The buttons are laid out first, from the right, so the reply field takes exactly the + // width they leave. A fixed allowance overflowed the card and widened the pane every + // frame until it stuck at its widest (#1431). ui.horizontal(|ui| { - let id = ui.scope_id().with(("reply", slide, index)); - let mut txt: String = ui.data_mut(|d| d.get_temp(id).unwrap_or_default()); - let r = ui.add(egui::TextEdit::singleline(&mut txt).hint_text("Reply…").desired_width(ui.available_width() - 70.0)); - if r.lost_focus() && ui.input(|i| i.key_pressed(egui::Key::Enter)) && !txt.trim().is_empty() { - focus_comment(app, slide, index); - run(app, "comment.reply", json!({"index": index, "text": txt})); - txt.clear(); - } - ui.data_mut(|d| d.insert_temp(id, txt)); - if ui.small_button(if c.resolved { "Reopen" } else { "Resolve" }).clicked() { - focus_comment(app, slide, index); - run(app, "comment.resolve", json!({"index": index, "resolved": !c.resolved})); - } - if ui.small_button("🗑").clicked() { - let _ = app.run("slide.go", json!({"index": slide})); - let _ = app.run("comment.delete", json!({"index": index})); - match app.ui.comment_focus { - Some((fs, fi)) if fs == slide && fi == index => app.ui.comment_focus = None, - Some((fs, fi)) if fs == slide && fi > index => app.ui.comment_focus = Some((fs, fi - 1)), - _ => {} + ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { + if ui.small_button("🗑").clicked() { + let _ = app.run("slide.go", json!({"index": slide})); + let _ = app.run("comment.delete", json!({"index": index})); + match app.ui.comment_focus { + Some((fs, fi)) if fs == slide && fi == index => app.ui.comment_focus = None, + Some((fs, fi)) if fs == slide && fi > index => app.ui.comment_focus = Some((fs, fi - 1)), + _ => {} + } } - } + if ui.small_button(if c.resolved { "Reopen" } else { "Resolve" }).clicked() { + focus_comment(app, slide, index); + run(app, "comment.resolve", json!({"index": index, "resolved": !c.resolved})); + } + let id = ui.scope_id().with(("reply", slide, index)); + let mut txt: String = ui.data_mut(|d| d.get_temp(id).unwrap_or_default()); + let r = ui.add(egui::TextEdit::singleline(&mut txt).hint_text("Reply…").desired_width(ui.available_width())); + if r.lost_focus() && ui.input(|i| i.key_pressed(egui::Key::Enter)) && !txt.trim().is_empty() { + focus_comment(app, slide, index); + run(app, "comment.reply", json!({"index": index, "text": txt})); + txt.clear(); + } + ui.data_mut(|d| d.insert_temp(id, txt)); + }); }); }); if focused { diff --git a/crates/ui-egui/src/ribbon/animations.rs b/crates/ui-egui/src/ribbon/animations.rs index 67f0bbd0..d4380160 100644 --- a/crates/ui-egui/src/ribbon/animations.rs +++ b/crates/ui-egui/src/ribbon/animations.rs @@ -1,4 +1,3 @@ -use super::transitions::option_label; use super::{ Align2, AnimClass, CornerRadius, Icon, Rect, Sense, SlideApp, Tokens, Ui, big, big_button, big_button_ex, dropdown, enabled, group, icons, json, pos2, rows, run, theme, vec2, widgets, @@ -81,12 +80,13 @@ pub(super) fn animations(app: &mut SlideApp, ui: &mut Ui) { } }); let effect_rows = cur.as_ref().map(|c| animation_effect_rows(&c.effect, c.class, Some(c.option.as_str()))).unwrap_or_default(); + let effect = cur.as_ref().map(|c| c.effect.as_str()).unwrap_or_default(); let build = cur.as_ref().map(|c| text_build_id(c.text_build)); let build_rows = marked_options(&["asOne", "byParagraph"], build); let r = big_button(ui, Icon::EffectOptions, "Effect\nOptions", !effect_rows.is_empty()); egui::Popup::menu(&r).show(|ui| { for &(o, on) in &effect_rows { - if ui.selectable_label(on, option_label(o)).clicked() { + if ui.selectable_label(on, animation_option_label(effect, o)).clicked() { run(app, "animation.options", json!({"option": o})); ui.close(); } @@ -216,6 +216,70 @@ pub(super) fn animation_effect_rows(effect: &str, class: AnimClass, current: Opt marked_options(opts, current) } +fn animation_option_label(effect: &str, option: &str) -> String { + let direction = match option { + "b" => Some("From Bottom"), + "lb" => Some("From Bottom-Left"), + "l" => Some("From Left"), + "lt" => Some("From Top-Left"), + "t" => Some("From Top"), + "rt" => Some("From Top-Right"), + "r" => Some("From Right"), + "rb" => Some("From Bottom-Right"), + _ => None, + }; + if matches!(effect, "fly" | "flyOut" | "wipe" | "wipeOut" | "peek" | "peekOut" | "crawl" | "crawlOut") + && let Some(label) = direction + { + if matches!(effect, "flyOut" | "peekOut" | "crawlOut") { + return label.replacen("From", "To", 1); + } + return label.into(); + } + + match (effect, option) { + ("float" | "floatOut", "u") => "Up".into(), + ("float" | "floatOut", "d") => "Down".into(), + ("split" | "splitOut", "horzIn") => "Horizontal In".into(), + ("split" | "splitOut", "horzOut") => "Horizontal Out".into(), + ("split" | "splitOut", "vertIn") => "Vertical In".into(), + ("split" | "splitOut", "vertOut") => "Vertical Out".into(), + ("shape" | "shapeOut", o) if o.ends_with("In") => format!("{} In", title_first(shape_option_name(o))), + ("shape" | "shapeOut", o) if o.ends_with("Out") => format!("{} Out", title_first(shape_option_name(o))), + ("wheel" | "wheelOut", "1") => "1 Spoke".into(), + ("wheel" | "wheelOut", n) => format!("{n} Spokes"), + ("box" | "boxOut" | "circle" | "diamond" | "diamondOut", "in") => format!("{} In", title_first(effect.trim_end_matches("Out"))), + ("box" | "boxOut" | "circle" | "diamond" | "diamondOut", "out") => format!("{} Out", title_first(effect.trim_end_matches("Out"))), + ("zoom" | "zoomOut", "in") => "Zoom In".into(), + ("zoom" | "zoomOut", "out") => "Zoom Out".into(), + ("strips" | "stripsOut", "lu") => "Left Up".into(), + ("strips" | "stripsOut", "ru") => "Right Up".into(), + ("strips" | "stripsOut", "ld") => "Left Down".into(), + ("strips" | "stripsOut", "rd") => "Right Down".into(), + (_, "horz") => "Horizontal".into(), + (_, "vert") => "Vertical".into(), + (_, "across") => "Across".into(), + (_, "down") => "Down".into(), + (_, "up") => "Up".into(), + (_, "right") => "Right".into(), + (_, "left") => "Left".into(), + (_, "clockwise") => "Clockwise".into(), + (_, "counterClockwise") => "Counterclockwise".into(), + (_, "in") => "In".into(), + (_, "out") => "Out".into(), + (_, other) => super::transitions::option_label(other), + } +} + +fn shape_option_name(option: &str) -> &str { + option.strip_suffix("In").or_else(|| option.strip_suffix("Out")).unwrap_or(option) +} + +fn title_first(value: &str) -> String { + let mut chars = value.chars(); + chars.next().map(|c| c.to_uppercase().collect::() + chars.as_str()).unwrap_or_default() +} + pub(super) fn text_build_id(build: deckcraft_model::anim::TextBuild) -> &'static str { match build { deckcraft_model::anim::TextBuild::ByParagraph => "byParagraph", @@ -224,3 +288,45 @@ pub(super) fn text_build_id(build: deckcraft_model::anim::TextBuild) -> &'static deckcraft_model::anim::TextBuild::AsOne => "asOne", } } + +#[cfg(test)] +mod animation_option_label_tests { + use super::animation_option_label; + use deckcraft_model::anim::ANIMATIONS; + + #[test] + fn labels_every_animation_gallery_option_without_changing_its_code() { + for &(effect, _, _, _, _, options) in ANIMATIONS { + for &option in options { + let label = animation_option_label(effect, option); + assert!(!label.is_empty(), "{effect}/{option}"); + assert_ne!(label, option, "raw option shown for {effect}/{option}"); + } + } + assert_eq!(animation_option_label("fly", "lb"), "From Bottom-Left"); + assert_eq!(animation_option_label("flyOut", "rt"), "To Top-Right"); + assert_eq!(animation_option_label("flyOut", "b"), "To Bottom"); + assert_eq!(animation_option_label("peekOut", "l"), "To Left"); + assert_eq!(animation_option_label("wipe", "b"), "From Bottom"); + assert_eq!(animation_option_label("wheel", "1"), "1 Spoke"); + assert_eq!(animation_option_label("split", "vertOut"), "Vertical Out"); + assert_eq!(animation_option_label("shape", "circleIn"), "Circle In"); + assert_eq!(animation_option_label("diamond", "out"), "Diamond Out"); + assert_eq!(animation_option_label("diamondOut", "in"), "Diamond In"); + assert_eq!(animation_option_label("diamondOut", "out"), "Diamond Out"); + assert_eq!(animation_option_label("strips", "lu"), "Left Up"); + assert_eq!(animation_option_label("strips", "ru"), "Right Up"); + assert_eq!(animation_option_label("strips", "ld"), "Left Down"); + assert_eq!(animation_option_label("strips", "rd"), "Right Down"); + assert_eq!(animation_option_label("crawl", "b"), "From Bottom"); + assert_eq!(animation_option_label("crawl", "l"), "From Left"); + assert_eq!(animation_option_label("crawl", "t"), "From Top"); + assert_eq!(animation_option_label("crawl", "r"), "From Right"); + assert_eq!(animation_option_label("crawlOut", "l"), "To Left"); + assert_eq!(animation_option_label("crawlOut", "r"), "To Right"); + assert_eq!(animation_option_label("plus", "in"), "In"); + assert_eq!(animation_option_label("plus", "out"), "Out"); + assert_eq!(animation_option_label("changeFontSize", "increase"), "Increase"); + assert_eq!(animation_option_label("changeFontSize", "decrease"), "Decrease"); + } +} diff --git a/crates/ui-egui/src/ribbon/transitions.rs b/crates/ui-egui/src/ribbon/transitions.rs index 3f43ebd9..6986d828 100644 --- a/crates/ui-egui/src/ribbon/transitions.rs +++ b/crates/ui-egui/src/ribbon/transitions.rs @@ -12,11 +12,14 @@ pub(super) fn transitions(app: &mut SlideApp, ui: &mut Ui) { let cur = app.session.active().and_then(|d| d.current_slide().and_then(|s| s.transition.clone())); let kind = cur.as_ref().map(|t| t.kind.clone()).unwrap_or_else(|| "none".into()); group(ui, |ui| { - if big_button(ui, Icon::Preview, "Preview", true).clicked() { - let from = app.session.active().map(|d| d.selection.slide).unwrap_or(0); - app.start_show(from, true); - if let Some(s) = app.show.as_mut() { - s.preview_only = true; + let playing = app.show.as_ref().is_some_and(|s| s.preview_only); + let (resp, _) = widgets::big_button_ex(ui, Icon::Preview, "Preview", true, false, playing); + if resp.on_hover_text("Play this slide's transition").clicked() { + let to = app.session.active().map(|d| d.selection.slide).unwrap_or(0); + app.start_show(to, true); + let now = ui.input(|i| i.time); + if let (Some(s), Some(d)) = (app.show.as_mut(), app.session.active()) { + s.start_transition_preview(&d.doc, now); } } }); diff --git a/crates/ui-egui/src/show.rs b/crates/ui-egui/src/show.rs index a8521d3d..0cb2b458 100644 --- a/crates/ui-egui/src/show.rs +++ b/crates/ui-egui/src/show.rs @@ -269,6 +269,27 @@ impl Show { } } + /// Play this slide's transition from the previous slide (or black for slide 0) as a preview (#1654). + pub fn start_transition_preview(&mut self, doc: &Presentation, now: f64) { + let i = self.state.slide; + let from = i.checked_sub(1); + let (kind, option, dur) = doc + .slides + .get(i) + .and_then(|s| s.transition.as_ref()) + .map(|t| (t.kind.clone(), t.option.clone(), t.duration_ms as f64 / 1000.0)) + .unwrap_or(("none".into(), String::new(), 0.0)); + self.trans = if kind != "none" && dur > 0.0 && !doc.show.without_animation { + Some(Transition { old: None, from, morph: None, parts: None, start: now, kind, option, dur, reverse: false }) + } else { + None + }; + self.preview_only = true; + self.slide_start = now; + self.step_start = now; + self.step_end = now; + } + /// Identity of the show state a queued window-point event belongs to. pub(crate) fn input_state(&self) -> ShowInputState { let bits = |v: f32| if v.is_finite() { v.to_bits() } else { 0 }; diff --git a/crates/ui-egui/src/show/tests.rs b/crates/ui-egui/src/show/tests.rs index 23909fe4..1dd6b09e 100644 --- a/crates/ui-egui/src/show/tests.rs +++ b/crates/ui-egui/src/show/tests.rs @@ -1298,3 +1298,37 @@ fn a_preview_of_an_edited_presentation_is_rendered_again() { let tex = harness.state().1.next_tex.as_ref().map(|(d, k, _)| (std::sync::Arc::ptr_eq(d, &edited), *k)); assert_eq!(tex, Some((true, 1)), "the preview comes from the edited presentation"); } + +/// #1654: Preview on the Transitions ribbon plays the current slide's transition from the +/// previous slide into the current slide, and exits when finished. +#[test] +fn transition_preview_plays_from_the_previous_slide() { + let mut s = deckcraft_engine::Session::with_new(); + s.execute("slide.new", &json!({})).unwrap(); + s.execute("transition.set", &json!({"kind": "push", "index": 1, "duration": 1000})).unwrap(); + let doc = s.active().unwrap().doc.clone(); + let mut show = Show::new(&doc, 1, true, false); + show.start_transition_preview(&doc, 10.0); + assert!(show.preview_only); + let tr = show.trans.as_ref().expect("transition preview is playing"); + assert_eq!(tr.kind, "push"); + assert_eq!(tr.from, Some(0), "enters from slide 0 into slide 1"); + assert_eq!(tr.dur, 1.0); + assert!(!tr.reverse); + assert_eq!(tr.start, 10.0); +} + +/// #1654: Transition preview for slide 0 plays with from = None (transitioning from black). +#[test] +fn transition_preview_for_first_slide_transitions_from_black() { + let mut s = deckcraft_engine::Session::with_new(); + s.execute("transition.set", &json!({"kind": "fade", "index": 0, "duration": 500})).unwrap(); + let doc = s.active().unwrap().doc.clone(); + let mut show = Show::new(&doc, 0, true, false); + show.start_transition_preview(&doc, 5.0); + assert!(show.preview_only); + let tr = show.trans.as_ref().expect("fade transition preview is playing"); + assert_eq!(tr.kind, "fade"); + assert_eq!(tr.from, None, "slide 0 transitions from black (None)"); + assert_eq!(tr.dur, 0.5); +} diff --git a/crates/ui-egui/src/show/ui.rs b/crates/ui-egui/src/show/ui.rs index a0a1fe29..072714a3 100644 --- a/crates/ui-egui/src/show/ui.rs +++ b/crates/ui-egui/src/show/ui.rs @@ -10,7 +10,7 @@ use super::zoom::{ }; use super::{ Align2, BackKey, Click, Color32, CornerRadius, Key, Layer, LinkJump, Pos2, Rect, RenderOpts, Sense, ShapeId, ShapeState, Show, ShowAction, - SlideApp, Source, Stroke, TextureHandle, Ui, json, pos2, show_fills_screen, state_for, theme, vec2, viewport_visible, + ShowState, SlideApp, Source, Stroke, TextureHandle, Ui, json, pos2, show_fills_screen, state_for, theme, vec2, viewport_visible, }; use deckcraft_anim::{StepStamp, TriggerPlay}; use deckcraft_model::Presentation; @@ -458,6 +458,17 @@ fn draw_transition(show: &mut Show, f: &Fr, sl: &Sl, painter: &egui::Painter, te if tr.parts.is_none() && deckcraft_anim::uses_slide_parts(&tr.kind, &tr.option) { tr.parts = Some(part_textures(ctx, doc, tr.from, idx, size)); } + if tr.old.is_none() + && let Some(prev) = tr.from + && !deckcraft_anim::uses_slide_parts(&tr.kind, &tr.option) + { + let threads = + if cfg!(target_arch = "wasm32") { 0 } else { std::thread::available_parallelism().map(|n| n.get().min(8) as u16).unwrap_or(0) }; + let timeline = ShowState::timeline(doc, prev); + let step = timeline.steps(); + let ci = render_frame(doc, prev, timeline, step, 0.0, size, vec![], &[], threads, &[], &[], None); + tr.old = Some(crate::textures::upload(ctx, "trans-old", ci)); + } let aspect = doc.slide_size.width / doc.slide_size.height.max(1.0); let part = |k: usize| tr.parts.as_ref().and_then(|p| p.get(k).cloned().flatten()); let parts = [part(0), part(1), part(2), part(3)]; diff --git a/crates/ui-egui/src/tests.rs b/crates/ui-egui/src/tests.rs index 29fa755f..5221544a 100644 --- a/crates/ui-egui/src/tests.rs +++ b/crates/ui-egui/src/tests.rs @@ -2361,3 +2361,47 @@ fn overdue_visible_logic_does_not_reanchor_a_later_hidden_show() { let _ = ctx.run_logic(&raw_at(12.0, true, false), |ctx| app.tick_hidden_show(ctx, visible_anchor + std::time::Duration::from_secs(3))); assert_eq!(app.show.as_ref().unwrap().state.slide, 1, "time from the full visible frame is preserved through the logic-only pass"); } + +/// #1431: a comment card's reply row was wider than the pane, so the Comments pane grew every +/// frame until it stuck at its widest and dragging its edge did nothing. +#[test] +fn comments_pane_keeps_its_width_and_resizes() { + let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_ui_state( + |ui, app: &mut SlideApp| { + app.logic(ui.ctx()); + app.ui(ui); + }, + SlideApp::new(Session::with_new(), Services::default()), + ); + h.run_steps(3); + h.state_mut().run("comment.add", json!({"text": "Check the figures on this slide"})).unwrap(); + h.state_mut().run("comment.add", json!({"text": "Resolved one"})).unwrap(); + h.state_mut().run("comment.resolve", json!({"index": 1, "resolved": true})).unwrap(); + h.state_mut().run("view.pane", json!({"pane": "comments"})).unwrap(); + h.run_steps(30); + let width = h.state().ui.pane_width; + assert!(width < 320.0, "the pane keeps its default width, not {width}"); + // The cards stay compact: the second one starts near the top, not below a stretched first. + let second = h.get_by_label("Resolved one").rect(); + assert!(second.top() < 450.0, "the second comment sits near the top, at {second:?}"); + + // Drag the pane's left edge: right narrows it, left widens it. + let drag = |h: &mut egui_kittest::Harness<'_, SlideApp>, dx: f32| { + let start = egui::pos2(1200.0 - h.state().ui.pane_width - 1.0, 500.0); + let end = start + egui::vec2(dx, 0.0); + h.event(egui::Event::PointerMoved(start)); + h.run_steps(1); + h.event(egui::Event::PointerButton { pos: start, button: egui::PointerButton::Primary, pressed: true, modifiers: egui::Modifiers::NONE }); + for i in 1..=6 { + h.event(egui::Event::PointerMoved(start + (end - start) * (i as f32 / 6.0))); + h.run_steps(1); + } + h.event(egui::Event::PointerButton { pos: end, button: egui::PointerButton::Primary, pressed: false, modifiers: egui::Modifiers::NONE }); + h.run_steps(10); + h.state().ui.pane_width + }; + let narrow = drag(&mut h, 50.0); + assert!(narrow < width - 40.0, "dragging right narrows the pane: {width} -> {narrow}"); + let wide = drag(&mut h, -150.0); + assert!(wide > narrow + 140.0, "dragging left widens the pane: {narrow} -> {wide}"); +} diff --git a/docs/animation-parity.md b/docs/animation-parity.md index b7f840a9..ffca88c9 100644 --- a/docs/animation-parity.md +++ b/docs/animation-parity.md @@ -1,6 +1,6 @@ # Transitions and animations: parity with PowerPoint -> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-29 · **Change:** minor (going back in the show reverses the left slide's transition, #1531; previous: minor, Animation Pane ▸ Play From plays from the selected effect; previous: community PR wave 2026-10-14: interactive-deck triggers, canvas preview, the Animation Pane timeline, repeat and bounce end round-trip; duplicated rows and headers folded; previous: major, transitions measured frame by frame against PowerPoint for Windows' video export) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) +> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-30 · **Change:** trivial (animation Effect Options translate stored codes into readable labels; previous: minor, going back in the show reverses the left slide's transition, #1531; previous: minor, Animation Pane ▸ Play From plays from the selected effect; previous: community PR wave 2026-10-14: interactive-deck triggers, canvas preview, the Animation Pane timeline, repeat and bounce end round-trip; duplicated rows and headers folded; previous: major, transitions measured frame by frame against PowerPoint for Windows' video export) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) Transitions and animations are what make a deck a presentation, and the part of PowerPoint people notice first when it's missing. Scores roll up into [target-app-parity.md](target-app-parity.md) @@ -65,7 +65,7 @@ In, Wipe, Zoom, Float In, Grow/Shrink, Pulse, Spin, motion lines): weighted, ~85 | Animation feature | PowerPoint | DeckCraft | |---|---|---| | Start On Click / With Previous / After Previous, duration, delay | yes | yes | -| Effect Options (direction, sequence by paragraph) | yes | yes | +| Effect Options (direction, sequence by paragraph) | yes | yes; every gallery option has a readable label while retaining its stored code | | Effect dialog: sound, after-animation dim/hide, animate text by word/letter, smooth start/end, bounce end, repeat, rewind | yes | partly; repeat, bounce end and rewind round-trip | | By-paragraph text builds, sub-bullets follow parents | yes | yes; builds edited in PowerPoint (own start per paragraph, other effects in between, by 2nd-level paragraphs) import as one effect per paragraph and keep their clicks (#844) | | Triggers (on click of a shape) | yes | yes; played in the show: a step per click, the sequence starts over after its last step, a click completes a playing step. Separate click steps, mixed starts and paragraph builds survive PPTX save (`trigger_steps.rs`) | @@ -94,6 +94,7 @@ In, Wipe, Zoom, Float In, Grow/Shrink, Pulse, Spin, motion lines): weighted, ~85 | Date | Change | Summary | |---|---|---| +| 2026-10-30 | trivial | Animation Effect Options show readable, effect-aware labels for all gallery option codes; stored values and command parameters stay unchanged | | 2026-10-29 | minor | Going back in the show plays the transition of the slide being left, reversed, instead of the previous slide's entrance transition (#1531); new row in the transition features. Scores unchanged | | 2026-10-22 | minor | Animation Pane ▸ Play From: with an effect selected, the preview starts at that effect on the play-all clock; the row says yes and the estimate drops to ~14–26 h | | 2026-10-15 | trivial | PPTX interactive sequences preserve later On Click effects and one click per paragraph; no change to playback or feature scores | diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index b29dbd58..e08249ec 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -2,7 +2,11 @@ > **Generated:** 2026-10-14 (bullets/numbering remain D after empty-line marker review; summary by `cargo xtask parity`; rows re-scored by the 2026-10-14 community PR wave, duplicate Thesaurus, AutoCorrect, Artistic effects and Corrections rows folded; Preview in editor re-scored 2026-10-13; earlier rows re-scored 2026-10-10 against PowerPoint for Mac 16.113.4; video/animated GIF row re-scored 2026-10-11, #171) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) -> **Last updated:** 2026-11-03 · Reuse Slides now transfers referenced custom table styles and safely remaps conflicts; feature status and summary scores unchanged. +> **Last updated:** 2026-11-03 · Reuse Slides transfers referenced custom table styles; feature status and summary scores unchanged. +> +> **Previously (2026-11-02):** Tiled picture fills keep their size in scaled groups and upright tiles (#1693); Picture and texture fill row remains P. Scores unchanged. +> +> **Previously (2026-11-01):** Locked-shape and no-outline regressions added for #1621; fill row re-assessed: remains D; No Fill hit-testing now follows resolved appearance. Scores unchanged. The summary between ` **Weighted breadth parity: 84%** over 191 features. @@ -241,4 +245,5 @@ Open P0 items: Contextual tabs (Shape Format, Picture Format, Table Design/Layou | Date | Change | Summary | |---|---|---| | 2026-11-03 | trivial | Reuse Slides transfers referenced custom table styles; row remains P because other reuse behavior is still partial, summary scores unchanged | +| 2026-11-02 | trivial | Tiled picture fills keep their size in scaled groups and upright tiles (#1693); Picture and texture fill row remains P and summary scores are unchanged | | 2026-10-31 | trivial | No Fill interior hit-testing follows resolved fill (#1621); fill row remains D and summary scores are unchanged |