From adcfa70b4a1ae68c364b42ef1015f0e14c71a2a5 Mon Sep 17 00:00:00 2001 From: xc4pr <88853037+xc4pr@users.noreply.github.com> Date: Fri, 23 Oct 2026 00:45:54 +0200 Subject: [PATCH 1/3] Design Ideas: no ideas for slides with diagrams, connectors or other graphics (#1200) --- crates/engine/src/cmd/ideas.rs | 45 +++++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/ideas.rs b/crates/engine/src/cmd/ideas.rs index 45676754..3931cb58 100644 --- a/crates/engine/src/cmd/ideas.rs +++ b/crates/engine/src/cmd/ideas.rs @@ -57,9 +57,23 @@ fn roles(shapes: &[Shape]) -> Roles { r } -/// The ideas for slide `index` (none when it has nothing to arrange). +/// Whether an idea may move this shape: a picture, a placeholder or a plain text box. Diagram +/// boxes, connectors, groups, tables, charts and media are laid out by their own geometry, which +/// the ideas do not know, so a slide holding one gets no ideas. +fn arrangeable(s: &Shape) -> bool { + match s.kind { + ShapeKind::Picture { .. } => true, + ShapeKind::Shape => s.ph.is_some() || s.text_box, + _ => false, + } +} + +/// The ideas for slide `index` (none when it has nothing to arrange, or other content in the way). pub fn ideas_for(doc: &Presentation, index: usize) -> Vec { let Some(slide) = doc.slides.get(index) else { return vec![] }; + if slide.shapes.iter().any(|s| !s.hidden && !arrangeable(s)) { + return vec![]; + } let r = roles(&slide.shapes); if r.picture.is_some() { PICTURE_IDEAS.to_vec() @@ -228,6 +242,7 @@ mod tests { }; let body = Shape { id: ShapeId(501), + text_box: true, xfrm: Some(Xfrm::new(50.0, 140.0, 860.0, 350.0)), text: Some(deckcraft_model::text::TextBody::from_text("one\ntwo")), ..Default::default() @@ -314,6 +329,34 @@ mod tests { assert!(s.execute("design.applyIdea", &json!({"id": "nope"})).is_err()); } + #[test] + fn slides_with_diagrams_or_other_graphics_get_no_ideas() { + // A filled autoshape with text (a diagram box) is not text to restack. + let mut s = deck(false); + let slide = Arc::make_mut(&mut Arc::make_mut(&mut s.doc_mut().unwrap().doc).slides[0]); + slide.shapes.push(Shape { + id: ShapeId(510), + xfrm: Some(Xfrm::new(300.0, 200.0, 200.0, 80.0)), + fill: Some(Fill::solid(deckcraft_model::ColorRef::rgb(Rgba::rgba(200, 80, 20, 255)))), + text: Some(deckcraft_model::text::TextBody::from_text("Solar wind")), + ..Default::default() + }); + assert!(ideas_for(&s.doc().unwrap().doc, 0).is_empty()); + assert!(s.execute("design.applyIdea", &json!({"id": "titleColumn"})).is_err()); + // A connector too, and a hidden one does not count. + let mut s = deck(false); + let slide = Arc::make_mut(&mut Arc::make_mut(&mut s.doc_mut().unwrap().doc).slides[0]); + slide.shapes.push(Shape { + id: ShapeId(511), + kind: ShapeKind::Connector { start: None, end: None }, + xfrm: Some(Xfrm::new(300.0, 200.0, 100.0, 0.0)), + ..Default::default() + }); + assert!(ideas_for(&s.doc().unwrap().doc, 0).is_empty()); + Arc::make_mut(&mut Arc::make_mut(&mut s.doc_mut().unwrap().doc).slides[0]).shapes[2].hidden = true; + assert_eq!(ideas_for(&s.doc().unwrap().doc, 0).len(), 3); + } + #[test] fn cover_crop_matches_the_box_aspect() { let c = cover_crop(2.0, 1.0); From 6d312e0722b687c8d70210615c870f2237dae135 Mon Sep 17 00:00:00 2001 From: xc4pr <88853037+xc4pr@users.noreply.github.com> Date: Fri, 23 Oct 2026 02:27:59 +0200 Subject: [PATCH 2/3] Design Ideas: an empty text box that shows also blocks the ideas (#1200) --- crates/engine/src/cmd/ideas.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/ideas.rs b/crates/engine/src/cmd/ideas.rs index 3931cb58..a5482336 100644 --- a/crates/engine/src/cmd/ideas.rs +++ b/crates/engine/src/cmd/ideas.rs @@ -63,7 +63,13 @@ fn roles(shapes: &[Shape]) -> Roles { fn arrangeable(s: &Shape) -> bool { match s.kind { ShapeKind::Picture { .. } => true, - ShapeKind::Shape => s.ph.is_some() || s.text_box, + ShapeKind::Shape => { + let is_title = s.ph.as_ref().is_some_and(|p| p.kind.is_title()); + let has_text = s.text.as_ref().is_some_and(|t| !t.text().trim().is_empty()); + // Only titles and shapes with text are moved (see `roles`): an empty one that shows + // (filled or outlined) would stay where it is, over the rearranged slide. + (s.ph.is_some() || s.text_box) && (is_title || has_text || (s.fill.is_none() && s.line.is_none())) + } _ => false, } } @@ -357,6 +363,17 @@ mod tests { assert_eq!(ideas_for(&s.doc().unwrap().doc, 0).len(), 3); } + #[test] + fn an_empty_text_box_blocks_ideas_only_when_it_shows() { + let mut s = deck(false); + let slide = Arc::make_mut(&mut Arc::make_mut(&mut s.doc_mut().unwrap().doc).slides[0]); + slide.shapes.push(Shape { id: ShapeId(512), text_box: true, xfrm: Some(Xfrm::new(10.0, 10.0, 100.0, 40.0)), ..Default::default() }); + assert_eq!(ideas_for(&s.doc().unwrap().doc, 0).len(), 3, "invisible and empty: nothing in the way"); + let slide = Arc::make_mut(&mut Arc::make_mut(&mut s.doc_mut().unwrap().doc).slides[0]); + slide.shapes[2].fill = Some(Fill::solid(deckcraft_model::ColorRef::rgb(Rgba::rgba(200, 80, 20, 255)))); + assert!(ideas_for(&s.doc().unwrap().doc, 0).is_empty(), "a filled empty box would stay on top"); + } + #[test] fn cover_crop_matches_the_box_aspect() { let c = cover_crop(2.0, 1.0); From e126fad9a7e4c5bf184e881d3eb2f3a7aa30590d Mon Sep 17 00:00:00 2001 From: xc4pr <88853037+xc4pr@users.noreply.github.com> Date: Fri, 23 Oct 2026 18:17:39 +0200 Subject: [PATCH 3/3] Design Ideas: decide whether an empty box shows from its resolved fill and outline (#1200) --- crates/engine/src/cmd/ideas.rs | 48 +++++++++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/ideas.rs b/crates/engine/src/cmd/ideas.rs index a5482336..2cbd380e 100644 --- a/crates/engine/src/cmd/ideas.rs +++ b/crates/engine/src/cmd/ideas.rs @@ -4,6 +4,7 @@ //! slide. Everything is computed here, offline. use deckcraft_geom::Xfrm; +use deckcraft_model::resolve::{self, Ctx}; use deckcraft_model::{Fill, Presentation, Rgba, Shape, ShapeKind}; use serde_json::{Value, json}; @@ -57,10 +58,20 @@ fn roles(shapes: &[Shape]) -> Roles { r } +/// Whether `s` paints a fill or an outline once inherited and style-referenced values are +/// resolved, as the renderer does (`fill: None` alone means inherited, not invisible). +fn shows(ctx: Option<&Ctx>, s: &Shape) -> bool { + let visible = |f: &Option| f.as_ref().is_some_and(|f| !f.is_none()); + match ctx { + Some(ctx) => visible(&resolve::fill(ctx, s).0) || visible(&resolve::line(ctx, s).0.fill), + None => visible(&s.fill) || visible(&s.line.as_ref().and_then(|l| l.fill.clone())), + } +} + /// Whether an idea may move this shape: a picture, a placeholder or a plain text box. Diagram /// boxes, connectors, groups, tables, charts and media are laid out by their own geometry, which /// the ideas do not know, so a slide holding one gets no ideas. -fn arrangeable(s: &Shape) -> bool { +fn arrangeable(ctx: Option<&Ctx>, s: &Shape) -> bool { match s.kind { ShapeKind::Picture { .. } => true, ShapeKind::Shape => { @@ -68,7 +79,7 @@ fn arrangeable(s: &Shape) -> bool { let has_text = s.text.as_ref().is_some_and(|t| !t.text().trim().is_empty()); // Only titles and shapes with text are moved (see `roles`): an empty one that shows // (filled or outlined) would stay where it is, over the rearranged slide. - (s.ph.is_some() || s.text_box) && (is_title || has_text || (s.fill.is_none() && s.line.is_none())) + (s.ph.is_some() || s.text_box) && (is_title || has_text || !shows(ctx, s)) } _ => false, } @@ -77,7 +88,8 @@ fn arrangeable(s: &Shape) -> bool { /// The ideas for slide `index` (none when it has nothing to arrange, or other content in the way). pub fn ideas_for(doc: &Presentation, index: usize) -> Vec { let Some(slide) = doc.slides.get(index) else { return vec![] }; - if slide.shapes.iter().any(|s| !s.hidden && !arrangeable(s)) { + let ctx = Ctx::for_slide(doc, slide); + if slide.shapes.iter().any(|s| !s.hidden && !arrangeable(ctx.as_ref(), s)) { return vec![]; } let r = roles(&slide.shapes); @@ -374,6 +386,36 @@ mod tests { assert!(ideas_for(&s.doc().unwrap().doc, 0).is_empty(), "a filled empty box would stay on top"); } + #[test] + fn an_empty_box_with_an_explicit_no_fill_does_not_block_ideas() { + let mut s = deck(false); + let slide = Arc::make_mut(&mut Arc::make_mut(&mut s.doc_mut().unwrap().doc).slides[0]); + slide.shapes.push(Shape { + id: ShapeId(512), + text_box: true, + xfrm: Some(Xfrm::new(10.0, 10.0, 100.0, 40.0)), + fill: Some(Fill::None), + ..Default::default() + }); + assert_eq!(ideas_for(&s.doc().unwrap().doc, 0).len(), 3, "an explicit no fill shows nothing"); + } + + #[test] + fn an_empty_box_filled_through_its_style_blocks_ideas() { + let mut s = deck(false); + let slide = Arc::make_mut(&mut Arc::make_mut(&mut s.doc_mut().unwrap().doc).slides[0]); + let mut style = deckcraft_model::style::ShapeStyle::accent(deckcraft_model::SchemeSlot::Accent1); + style.line_ref.0 = 0; + slide.shapes.push(Shape { + id: ShapeId(512), + text_box: true, + xfrm: Some(Xfrm::new(10.0, 10.0, 100.0, 40.0)), + style: Some(style), + ..Default::default() + }); + assert!(ideas_for(&s.doc().unwrap().doc, 0).is_empty(), "the theme fill shows, so the box would stay on top"); + } + #[test] fn cover_crop_matches_the_box_aspect() { let c = cover_crop(2.0, 1.0);