From fe35df948cdc354cfe8defeb5cf9ae79c60acbfd Mon Sep 17 00:00:00 2001 From: Zach Berry <1112351+zberry@users.noreply.github.com> Date: Wed, 4 Nov 2026 20:09:40 -0600 Subject: [PATCH 1/5] Flip is refused on a SmartArt graphic instead of turning it into a plain group MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PowerPoint greys Flip out for a SmartArt graphic (SmartArt Design ▸ Right to Left is the way to mirror it), also inside a group, as ungrouping would hand the flip to the graphic. Before, a flip turned it into a plain group that the smartArt.* commands then refused. arrange.flipHorizontal and flipVertical reach the check through shape.flip. Split out of the SmartArt PR at review. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/shape/size.rs | 10 ++++++++++ crates/engine/src/tests.rs | 24 ++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/crates/engine/src/cmd/shape/size.rs b/crates/engine/src/cmd/shape/size.rs index af235e403..5a573c625 100644 --- a/crates/engine/src/cmd/shape/size.rs +++ b/crates/engine/src/cmd/shape/size.rs @@ -191,6 +191,16 @@ pub(super) fn rotate(s: &mut Session, p: &Value) -> Result { pub(super) fn flip(s: &mut Session, p: &Value) -> Result { let v = str_param(p, "axis").unwrap_or("horizontal").starts_with('v'); edit_geometry(s, p, "shape.flip", |sh| { + // PowerPoint greys Flip out for a SmartArt graphic (SmartArt Design > Right to Left mirrors it), + // also inside a group: ungrouping would hand the flip to the graphic. + let mut graphic = false; + deckcraft_model::walk(std::slice::from_ref(&*sh), &mut |x, _| graphic |= x.diagram.is_some()); + if graphic { + return Err(bad( + "shape.flip", + "Flip is not available for a SmartArt graphic (it would turn it into a plain group); use `smartArt.layout` to change how it is laid out", + )); + } if let Some(x) = sh.xfrm.as_mut() { if v { x.flip_v = !x.flip_v; diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 12fac5efc..2410a38bc 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -6509,3 +6509,27 @@ fn save_to_image_path_is_an_error_not_zip_bytes() { } let _ = std::fs::remove_dir_all(&dir); } + +#[test] +fn flip_is_refused_on_a_smartart_graphic_and_leaves_it_a_graphic() { + let mut s = session(); + s.execute("slide.new", &json!({"layout": "blank"})).unwrap(); + s.execute("insert.textBox", &json!({"text": "Goals\nRevenue"})).unwrap(); + let gid = s.execute("smartArt.fromText", &json!({"kind": "blockList"})).unwrap()["id"].as_u64().unwrap(); + for cmd in ["arrange.flipHorizontal", "arrange.flipVertical", "shape.flip"] { + let e = s.execute(cmd, &json!({"ids": [gid]})).unwrap_err().to_string(); + assert!(e.contains("SmartArt") && e.contains("smartArt.layout"), "{cmd}: {e}"); + } + // Grouped with another shape it cannot be flipped either: ungrouping would hand it the flip. + let other = s.execute("insert.textBox", &json!({"text": "x"})).unwrap()["id"].as_u64().unwrap(); + let group = s.execute("arrange.group", &json!({"ids": [gid, other]})).unwrap()["id"].as_u64().unwrap(); + let e = s.execute("shape.flip", &json!({"ids": [group]})).unwrap_err().to_string(); + assert!(e.contains("SmartArt"), "{e}"); + s.execute("arrange.ungroup", &json!({"ids": [group]})).unwrap(); + // Still a SmartArt graphic: smartArt commands keep working. + s.execute("smartArt.layout", &json!({"id": gid, "kind": "cycle"})).unwrap(); + // Ordinary shapes still flip. + let b = s.execute("insert.textBox", &json!({"text": "x"})).unwrap(); + let id = b["id"].as_u64().unwrap(); + assert_eq!(s.execute("arrange.flipHorizontal", &json!({"ids": [id]})).unwrap(), json!(1)); +} From b06d60b8ed9985610e033df98610561f9b41d170 Mon Sep 17 00:00:00 2001 From: sbjwin Date: Thu, 5 Nov 2026 12:35:59 +0900 Subject: [PATCH 2/5] UI: follow Grayscale view mode in slide thumbnails In PowerPoint, turning on Grayscale view mode turns both the active slide canvas and the slide thumbnail pane to grayscale, while Print retains the original full color for output. Previously, DeckCraft only passed `app.ui.grayscale` to the main canvas texture renderer (`Textures::slide`), while thumbnail rendering (`Textures::thumb`) hardcoded `false` and lacked `grayscale` in its cache key. As a result, when Grayscale mode was enabled, the canvas rendered in grayscale but the thumbnail pane remained in full color. - Update `Textures::thumb` to accept `grayscale: bool` and include it in the cache key (`extra: grayscale as u64`). - Pass `app.ui.grayscale` to `Textures::thumb` in the thumbnail pane (`panels/thumbs.rs`), Slide Sorter & Notes Page (`sorter.rs`), and Notes Master view (`canvas.rs`). - Keep Print preview (`backstage/print.rs`) in full color (`false`) to match the actual exported PDF print output. - Add regression test `to_color_image_turns_colors_to_grayscale_when_flag_is_set`. --- crates/ui-egui/src/backstage/print.rs | 3 ++- crates/ui-egui/src/canvas.rs | 2 +- crates/ui-egui/src/panels/thumbs.rs | 2 +- crates/ui-egui/src/sorter.rs | 5 +++-- crates/ui-egui/src/textures.rs | 29 ++++++++++++++++++++++++--- 5 files changed, 33 insertions(+), 8 deletions(-) diff --git a/crates/ui-egui/src/backstage/print.rs b/crates/ui-egui/src/backstage/print.rs index 612ecaf75..db5d6b972 100644 --- a/crates/ui-egui/src/backstage/print.rs +++ b/crates/ui-egui/src/backstage/print.rs @@ -207,7 +207,8 @@ fn preview(app: &mut SlideApp, ui: &mut Ui, bs: &mut Backstage, doc: &deckcraft_ let mut budget = 4u32; let mut draw_slide = |ui: &mut Ui, i: usize, r: Rect| { let px = (r.width() * ui.ctx().pixels_per_point()).round().clamp(16.0, 1600.0) as u32; - match app.textures.thumb(ui.ctx(), doc, i, px, &mut budget) { + // Print preview matches the actual exported PDF output (always full color). + match app.textures.thumb(ui.ctx(), doc, i, px, false, &mut budget) { Some(tex) => { ui.painter().image(tex.id(), r, Rect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 1.0)), Color32::WHITE); } diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 922435ca5..ca1029cbc 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -299,7 +299,7 @@ pub fn show(app: &mut SlideApp, ui: &mut Ui) { vec2(w as f32 * scale, h as f32 * scale), ); let mut budget = 2; - if let Some(tex) = app.textures.thumb(ui.ctx(), &doc, idx, (r.width() * ppp).round().max(1.0) as u32, &mut budget) { + if let Some(tex) = app.textures.thumb(ui.ctx(), &doc, idx, (r.width() * ppp).round().max(1.0) as u32, app.ui.grayscale, &mut budget) { painter.image(tex.id(), r, Rect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 1.0)), Color32::WHITE); } } diff --git a/crates/ui-egui/src/panels/thumbs.rs b/crates/ui-egui/src/panels/thumbs.rs index 946391c35..4956f2b0b 100644 --- a/crates/ui-egui/src/panels/thumbs.rs +++ b/crates/ui-egui/src/panels/thumbs.rs @@ -219,7 +219,7 @@ fn list(app: &mut SlideApp, ui: &mut Ui) { // Only rows in view (plus a little margin) ask for a thumbnail: with every row asking, a // deck bigger than the thumbnail cache evicts its own thumbnails and never settles (#969). let near = ui.clip_rect().expand2(vec2(0.0, 2.0 * row.height())).intersects(row); - let tex = if near { app.textures.thumb(ui.ctx(), &doc, i, wpx, &mut budget) } else { None }; + let tex = if near { app.textures.thumb(ui.ctx(), &doc, i, wpx, app.ui.grayscale, &mut budget) } else { None }; match tex { Some(tex) => { p.image( diff --git a/crates/ui-egui/src/sorter.rs b/crates/ui-egui/src/sorter.rs index 6c9626380..755f1f4a1 100644 --- a/crates/ui-egui/src/sorter.rs +++ b/crates/ui-egui/src/sorter.rs @@ -209,7 +209,8 @@ fn place_slides(app: &mut SlideApp, ui: &mut Ui, g: &Grid<'_>, budget: &mut u32, // Only cells in view (plus a little margin) ask for a thumbnail, so a deck bigger // than the thumbnail cache does not evict its own thumbnails forever (#969). let near = ui.clip_rect().expand2(vec2(0.0, 2.0 * cell.height())).intersects(cell); - let tex = if near { app.textures.thumb(ui.ctx(), g.doc, i, (img.width() * ppp).round() as u32, budget) } else { None }; + let tex = + if near { app.textures.thumb(ui.ctx(), g.doc, i, (img.width() * ppp).round() as u32, app.ui.grayscale, budget) } else { None }; match tex { Some(tex) => { p.image(tex.id(), img, Rect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 1.0)), Color32::WHITE); @@ -391,7 +392,7 @@ pub fn notes_page(app: &mut SlideApp, ui: &mut Ui) { } let ppp = ui.ctx().pixels_per_point(); let mut budget = 2; - if let Some(tex) = app.textures.thumb(ui.ctx(), &doc, idx, (srect.width() * ppp).round() as u32, &mut budget) { + if let Some(tex) = app.textures.thumb(ui.ctx(), &doc, idx, (srect.width() * ppp).round() as u32, app.ui.grayscale, &mut budget) { ui.painter().image(tex.id(), srect, Rect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 1.0)), Color32::WHITE); } ui.painter().rect_stroke(srect, CornerRadius::ZERO, Stroke::new(1.0, Color32::GRAY), egui::StrokeKind::Outside); diff --git a/crates/ui-egui/src/textures.rs b/crates/ui-egui/src/textures.rs index 19a4b4f68..f7accb8b3 100644 --- a/crates/ui-egui/src/textures.rs +++ b/crates/ui-egui/src/textures.rs @@ -363,7 +363,16 @@ impl Textures { /// A thumbnail texture; renders at most `budget` new thumbnails per frame (returns None when /// it is over budget, so the caller draws a placeholder and asks for another frame). - pub fn thumb(&mut self, ctx: &egui::Context, p: &Presentation, index: usize, width_px: u32, budget: &mut u32) -> Option { + /// When `grayscale` is true, the thumbnail renders in black-and-white (matching Grayscale view). + pub fn thumb( + &mut self, + ctx: &egui::Context, + p: &Presentation, + index: usize, + width_px: u32, + grayscale: bool, + budget: &mut u32, + ) -> Option { let slide = p.slides.get(index)?.clone(); let master = master_of(p, &slide); let h = (width_px as f64 * p.slide_size.height / p.slide_size.width.max(1.0)).round().max(1.0) as u32; @@ -372,7 +381,7 @@ impl Textures { master: master.as_ref().map(|m| Arc::as_ptr(m) as usize).unwrap_or(0), size: (width_px, h), edit: false, - extra: 0, + extra: grayscale as u64, want: Region::slide(width_px, h), editing_text: None, }; @@ -393,7 +402,7 @@ impl Textures { let scale = tw as f64 / p.slide_size.width.max(1.0); let img = deckcraft_render::render_slide(p, index, &RenderOpts { scale, size: Some((tw, th)), threads: 0, ..Default::default() }); self.thumb_renders += 1; - let tex = upload_editor(ctx, format!("thumb-{index}"), to_color_image(&img, false)); + let tex = upload_editor(ctx, format!("thumb-{index}"), to_color_image(&img, grayscale)); // Drop the old entry for this slide address; entries for slides that no longer exist age out. let region = key.want; self.thumbs.insert(key.slide, Entry { key, tex: tex.clone(), region, _slide: slide, _master: master, stamp: clock }); @@ -433,6 +442,20 @@ mod tests { assert_eq!(device_pixel_rect(aligned, ppp), aligned); } + #[test] + fn to_color_image_turns_colors_to_grayscale_when_flag_is_set() { + let img = deckcraft_render::Image { width: 2, height: 1, pixels: vec![0, 0, 255, 255, 255, 0, 0, 255] }; + let color = super::to_color_image(&img, false); + assert_eq!(color.pixels[0], egui::Color32::from_rgba_premultiplied(0, 0, 255, 255)); + assert_eq!(color.pixels[1], egui::Color32::from_rgba_premultiplied(255, 0, 0, 255)); + + let gray = super::to_color_image(&img, true); + assert_eq!(gray.pixels[0].r(), gray.pixels[0].g()); + assert_eq!(gray.pixels[0].g(), gray.pixels[0].b()); + assert_eq!(gray.pixels[1].r(), gray.pixels[1].g()); + assert_eq!(gray.pixels[1].g(), gray.pixels[1].b()); + } + #[test] fn a_bad_scale_does_not_move_the_rect() { let r = Rect::from_min_size(pos2(3.0, 4.0), vec2(10.0, 12.0)); From ebd306acc12e75a5666d3b7a8010daabdeefb26b Mon Sep 17 00:00:00 2001 From: sbjwin Date: Thu, 5 Nov 2026 12:51:29 +0900 Subject: [PATCH 3/5] UI: request repaint on stale thumbnail return to finish grayscale refresh --- crates/ui-egui/src/textures.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/textures.rs b/crates/ui-egui/src/textures.rs index f7accb8b3..d0f764066 100644 --- a/crates/ui-egui/src/textures.rs +++ b/crates/ui-egui/src/textures.rs @@ -394,7 +394,8 @@ impl Textures { return Some(e.tex.clone()); } if *budget == 0 { - // Keep showing the stale thumbnail while the new one waits. + // Keep showing the stale thumbnail while the new one waits; request a repaint to finish rendering. + ctx.request_repaint(); return self.thumbs.get(&key.slide).map(|e| e.tex.clone()); } *budget -= 1; From d9186c7ce125924db4c7487842a9559d916e1ef2 Mon Sep 17 00:00:00 2001 From: aiongg <67543387+aiongg@users.noreply.github.com> Date: Thu, 5 Nov 2026 04:04:02 +0000 Subject: [PATCH 4/5] Apply review fixes - crates/engine/src/cmd/shape/size.rs --- crates/engine/src/cmd/shape/size.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/shape/size.rs b/crates/engine/src/cmd/shape/size.rs index 5a573c625..7568a0632 100644 --- a/crates/engine/src/cmd/shape/size.rs +++ b/crates/engine/src/cmd/shape/size.rs @@ -196,10 +196,7 @@ pub(super) fn flip(s: &mut Session, p: &Value) -> Result { let mut graphic = false; deckcraft_model::walk(std::slice::from_ref(&*sh), &mut |x, _| graphic |= x.diagram.is_some()); if graphic { - return Err(bad( - "shape.flip", - "Flip is not available for a SmartArt graphic (it would turn it into a plain group); use `smartArt.layout` to change how it is laid out", - )); + return Err(bad("shape.flip", "Flip is not available for a SmartArt graphic; use `smartArt.layout` to change how it is laid out")); } if let Some(x) = sh.xfrm.as_mut() { if v { From 480cbfd4effc3a2e45c20cd74dffa814ec9805c5 Mon Sep 17 00:00:00 2001 From: FredApps <153173854+FredApps@users.noreply.github.com> Date: Thu, 5 Nov 2026 05:18:38 +0100 Subject: [PATCH 5/5] Textures::thumb: document the stale thumbnail returned over budget --- crates/ui-egui/src/textures.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/textures.rs b/crates/ui-egui/src/textures.rs index d0f764066..e6e051457 100644 --- a/crates/ui-egui/src/textures.rs +++ b/crates/ui-egui/src/textures.rs @@ -361,8 +361,9 @@ impl Textures { self.thumbs.clear(); } - /// A thumbnail texture; renders at most `budget` new thumbnails per frame (returns None when - /// it is over budget, so the caller draws a placeholder and asks for another frame). + /// A thumbnail texture; renders at most `budget` new thumbnails per frame. Over budget it + /// returns the slide's previous thumbnail if there is one (and asks for another frame), else + /// None, so the caller draws a placeholder. /// When `grayscale` is true, the thumbnail renders in black-and-white (matching Grayscale view). pub fn thumb( &mut self,