From aef364867d5a316780e1c5b5370e81f6294266ce Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Mon, 26 Oct 2026 01:56:23 -0700 Subject: [PATCH] Format: Slide background fill applies to shapes and text boxes only shape.fill {background: true} leaves the fill of pictures, connectors, tables and charts unchanged. Only a shape or text box (p:sp) saves useBgFill in a .pptx. With a group selected, its shapes and text boxes take the fill and the group's own fill is cleared. When no selected object can take the fill, the command returns an error. The Format pane leaves Slide background fill out of a picture's Fill options, and shows it chosen for a group whose shapes and text boxes all have it. --- crates/engine/src/cmd/shape.rs | 55 +++++++++++++++++++++++++- crates/ui-egui/src/panes.rs | 72 +++++++++++++++++++++++++++++++++- 2 files changed, 125 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/shape.rs b/crates/engine/src/cmd/shape.rs index dfffc4ecf..a6cc60368 100644 --- a/crates/engine/src/cmd/shape.rs +++ b/crates/engine/src/cmd/shape.rs @@ -90,7 +90,7 @@ pub fn specs() -> Vec { "Shape Fill", ["Shape Format", "Shape Styles"], None, - "{color? | none?: true | background?: true (the slide background shows through) | gradient?: {stops: [[pos, color]], angle?, kind?: linear|radial} | picture?: base64 | pattern?: {preset, fg, bg} | transparency?: 0..1 | reset?: true, ids?}; pictureProps?: {tile?: bool, offsetX?, offsetY?: pt, scaleX?, scaleY?: 0..10, align?: tl|t|tr|l|ctr|r|bl|b|br, mirror?: none|x|y|xy, stretch?: [l, t, r, b] fractions, rotateWithShape?: bool} adjusts a picture fill; a group also formats its members", + "{color? | none?: true | background?: true (the slide background shows through; shapes and text boxes only) | gradient?: {stops: [[pos, color]], angle?, kind?: linear|radial} | picture?: base64 | pattern?: {preset, fg, bg} | transparency?: 0..1 | reset?: true, ids?}; pictureProps?: {tile?: bool, offsetX?, offsetY?: pt, scaleX?, scaleY?: 0..10, align?: tl|t|tr|l|ctr|r|bl|b|br, mirror?: none|x|y|xy, stretch?: [l, t, r, b] fractions, rotateWithShape?: bool} adjusts a picture fill; a group also formats its members", has_selection, fill ), @@ -612,7 +612,27 @@ fn fill(s: &mut Session, p: &Value) -> Result { } fills }; + // Only a shape or text box (`p:sp`) saves a slide background fill (`useBgFill`). Pictures, + // connectors, tables and charts keep their fill, and a group's own fill is cleared. + let background = matches!(new_fill, Some(Some(Fill::Background))); + if background { + let mut any = false; + for id in targets(s, p)? { + if let Some(sh) = st.shape(id) { + deckcraft_model::walk(std::slice::from_ref(sh), &mut |sh, _| any |= matches!(sh.kind, deckcraft_model::ShapeKind::Shape)); + } + } + if !any { + return Err(bad("shape.fill", "slide background fill applies to shapes and text boxes")); + } + } edit_group_appearance(s, p, "shape.fill", |sh| { + if background && !matches!(sh.kind, deckcraft_model::ShapeKind::Shape) { + if matches!(sh.kind, deckcraft_model::ShapeKind::Group { .. }) { + sh.fill = None; + } + return Ok(()); + } if let Some(f) = &new_fill { sh.fill.clone_from(f); } @@ -1645,6 +1665,39 @@ mod tests { assert_eq!(filled, 1, "the shape keeps the background fill after a save and reopen"); } + /// A picture cannot save a slide background fill, so `background` leaves its fill alone. + #[test] + fn background_fill_skips_pictures() { + let mut s = Session::with_new(); + s.execute("slide.new", &json!({"layout": "blank"})).unwrap(); + let shape = s.execute("shape.insert", &json!({"preset": "rect", "rect": [100, 100, 200, 100]})).unwrap()["id"].as_u64().unwrap(); + let png = deckcraft_render::Image { width: 2, height: 2, pixels: [255, 0, 0, 255].repeat(4) }.to_png(); + let pic = s + .execute("insert.picture", &json!({"data": crate::cmd::base64_encode(&png), "name": "red.png", "rect": [400, 100, 100, 100]})) + .unwrap()["id"] + .as_u64() + .unwrap(); + let fill = |s: &Session, id: u64| s.doc().unwrap().shape(ShapeId(id as u32)).unwrap().fill.clone(); + let err = s.execute("shape.fill", &json!({"background": true, "ids": [pic]})).unwrap_err(); + assert!(err.to_string().contains("shapes and text boxes"), "{err}"); + assert_eq!(fill(&s, pic), None); + + s.execute("edit.select", &json!({"ids": [shape, pic]})).unwrap(); + let group = s.execute("arrange.group", &json!({})).unwrap()["id"].as_u64().unwrap(); + s.execute("shape.fill", &json!({"color": "#ff0000"})).unwrap(); + s.execute("shape.fill", &json!({"background": true})).unwrap(); + assert_eq!(fill(&s, shape), Some(Fill::Background)); + assert!(matches!(fill(&s, pic), Some(Fill::Solid { .. })), "the picture keeps its fill"); + assert_eq!(fill(&s, group), None, "the group's own fill is cleared"); + let bytes = deckcraft_pptx::export(&s.doc().unwrap().doc).unwrap(); + let back = deckcraft_pptx::import(&bytes).unwrap(); + let mut filled = 0; + for slide in &back.slides { + deckcraft_model::walk(&slide.shapes, &mut |sh, _| filled += usize::from(sh.fill == Some(Fill::Background))); + } + assert_eq!(filled, 1, "the shape keeps the background fill after a save and reopen"); + } + fn rotation(s: &Session) -> f64 { let st = s.active().unwrap(); let id = st.selection.shapes[0]; diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index 9f7dbfd7b..42cad56ac 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -449,7 +449,20 @@ fn format_shape(app: &mut SlideApp, ui: &mut Ui) { _ => { // Fill & Line. ui.label(egui::RichText::new("Fill").font(theme::bold(13.0))); + // A group keeps no slide background fill of its own. It shows Slide background fill + // when every shape or text box in it has one. + let members_background = sh.fill.is_none() && matches!(sh.kind, ShapeKind::Group { .. }) && { + let (mut any, mut all) = (false, true); + deckcraft_model::walk(sh.children(), &mut |m, _| { + if matches!(m.kind, ShapeKind::Shape) { + any = true; + all &= m.fill == Some(Fill::Background); + } + }); + any && all + }; let kind = match &sh.fill { + None if members_background => "background", None => "auto", Some(Fill::None) => "none", Some(Fill::Solid { .. }) => "solid", @@ -459,6 +472,14 @@ fn format_shape(app: &mut SlideApp, ui: &mut Ui) { Some(Fill::Background) => "background", _ => "auto", }; + // Only a shape or text box saves a slide background fill. A group offers it when it + // holds one. + let mut background_ok = false; + if let Some(st) = app.session.active() { + for s in st.selected_shapes() { + deckcraft_model::walk(std::slice::from_ref(s), &mut |m, _| background_ok |= matches!(m.kind, ShapeKind::Shape)); + } + } for (l, k) in [ ("No fill", "none"), ("Solid fill", "solid"), @@ -468,6 +489,9 @@ fn format_shape(app: &mut SlideApp, ui: &mut Ui) { ("Slide background fill", "background"), ("Automatic", "auto"), ] { + if k == "background" && !background_ok { + continue; + } if ui.radio(kind == k, l).clicked() { match k { "none" => run(app, "shape.fill", json!({"none": true})), @@ -495,7 +519,7 @@ fn format_shape(app: &mut SlideApp, ui: &mut Ui) { } // `shape.fill {transparency}` changes a solid fill (an automatic one through its // effective color) and a picture fill; a gradient has a transparency per stop. - let transparency = matches!(sh.fill, None | Some(Fill::Solid { .. } | Fill::Picture(_))); + let transparency = !members_background && matches!(sh.fill, None | Some(Fill::Solid { .. } | Fill::Picture(_))); ui.horizontal(|ui| { if !transparency { return; @@ -1703,6 +1727,52 @@ mod tests { let automatic = h.query_all_by_label("Automatic").filter(|n| n.accesskit_node().toggled() == Some(egui::accesskit::Toggled::True)).count(); assert_eq!(automatic, 1, "only the line's Automatic is chosen"); } + + /// A picture cannot save a slide background fill, so its Fill options leave it out. A group + /// that holds a shape keeps the option. + #[test] + fn a_picture_has_no_slide_background_fill_option() { + let pane = |group: bool| { + let mut s = Session::with_new(); + s.execute("slide.new", &json!({"layout": "blank"})).unwrap(); + let png = deckcraft_render::Image { width: 2, height: 2, pixels: [255, 0, 0, 255].repeat(4) }.to_png(); + let pic = + s.execute("insert.picture", &json!({"data": deckcraft_engine::cmd::base64_encode(&png), "name": "red.png"})).unwrap()["id"].clone(); + if group { + let shape = s.execute("shape.insert", &json!({"preset": "rect", "rect": [500.0, 40.0, 120.0, 80.0]})).unwrap()["id"].clone(); + s.execute("edit.select", &json!({"ids": [pic, shape]})).unwrap(); + s.execute("arrange.group", &json!({})).unwrap(); + s.execute("shape.fill", &json!({"color": "#ff0000"})).unwrap(); + } + let mut app = SlideApp::new(s, Services::default()); + app.ui.pane = Some("format".into()); + app.ui.format_tab = "fill".into(); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_ui_state( + |ui, app: &mut SlideApp| { + app.logic(ui.ctx()); + app.ui(ui); + }, + app, + ); + h.run_steps(3); + h + }; + let h = pane(false); + assert!(h.query_by_label("Pattern fill").is_some(), "the Fill options are shown"); + assert!(h.query_by_label("Slide background fill").is_none()); + let mut h = pane(true); + h.get_by_label("Slide background fill").click(); + h.run_steps(3); + let st = h.state().session.active().unwrap(); + let group = st.selected_shapes()[0]; + let fills: Vec<_> = group.children().iter().map(|m| m.fill.clone()).collect(); + assert_eq!(fills[1], Some(Fill::Background), "the group's shape takes the fill"); + assert!(matches!(fills[0], Some(Fill::Solid { .. })), "the picture keeps its fill: {:?}", fills[0]); + assert_eq!(group.fill, None, "the group's own fill is cleared"); + assert_eq!(h.get_by_label("Slide background fill").accesskit_node().toggled(), Some(egui::accesskit::Toggled::True)); + assert_eq!(h.query_all_by_role_and_label(egui::accesskit::Role::Slider, "Transparency").count(), 0); + assert_eq!(color_buttons(&h), 1, "only the line's Color button"); + } } /// Review › Thesaurus: look a word up; clicking a synonym puts it in place of the selected word