Reduce cognitive complexity in variables, PDF graphics, compose frames and menu shortcuts (#1847)

* refactor(engine): split convert_all's per-story plan into convert_plan

Resolves SonarCloud rust:S3776 on convert_all (cmd/variables.rs).

* refactor(pdf): split graphic_unclipped into per-kind helpers

Resolves SonarCloud rust:S3776 on graphic_unclipped (pdf/src/export.rs).

* refactor(compose): extract wrap exclusions and frame text area from frame_specs_placed

Resolves SonarCloud rust:S3776 on frame_specs_placed (compose/src/lib.rs).

* refactor(ui): extract command shortcut lookup from shortcut_key

Resolves SonarCloud rust:S3776 on shortcut_key (ui-egui/src/menus.rs).
This commit is contained in:
Dylan M. Taylor
2026-10-25 21:25:52 -04:00
committed by GitHub
parent e413cdf59b
commit 42463ce99d
4 changed files with 243 additions and 213 deletions
+47 -36
View File
@@ -662,6 +662,51 @@ fn wrap_obstacles(it: &Item, parent: Affine, inv: Affine, skip: ItemId, left_pag
}
}
/// Adds to `exclusions` the objects on `sp` that wrap the text of frame `item`. Only objects that
/// reach the frame's text area wrap its text: one elsewhere on the spread would otherwise cost a
/// narrow column its room (the 1.5 em minimum segment of `free_slots` applies once an object cuts
/// the column).
fn frame_wrap_exclusions(doc: &Document, sp: &designcraft_doc::Spread, item: &Item, inv: Affine, exclusions: &mut Vec<Exclusion>) {
let left = |x: f64| sp.page_at_x(x).and_then(|pi| sp.pages.get(pi)).is_some_and(|p| p.side == designcraft_doc::PageSide::Left);
for other in &sp.items {
if doc.layer(other.layer).is_some_and(|l| !l.visible) {
continue;
}
wrap_obstacles(other, Affine::IDENTITY, inv, item.id, &left, 0, exclusions);
}
let a = item.text_area();
exclusions.retain(|e| {
let r = e.rect;
r.is_finite() && r.x0 < a.x1 && r.x1 > a.x0 && r.y0 < a.y1 && r.y1 > a.y0
});
}
/// The text area and options a frame composes into. On a path, one line as long as the path (from
/// the start offset).
fn frame_text_area(doc: &Document, st: &Story, item: &Item, tf: &designcraft_doc::TextFrame) -> (Rect, TextFrameOptions) {
match &tf.options.path {
Some(pt) => {
let len = designcraft_geom::warp::PathWarp::new(&item.path.to_bezpath(), false).length();
let size = st
.paras
.first()
.map(|p| doc.styles.resolve_para(p).1)
.map(|base| st.runs().map(|(_, f)| doc.styles.resolve_char(&base, f).size).fold(base.size, f64::max))
.unwrap_or(12.0);
let opts = TextFrameOptions {
columns: 1,
inset: [0.0; 4],
first_baseline: designcraft_doc::FirstBaseline::Ascent,
// Keep the path identity: its synthetic area uses full line extents.
..tf.options.clone()
};
let end = pt.end.map_or(len, |e| e.max(0.0).min(len));
(Rect::new(0.0, 0.0, (end - pt.start).max(1.0), size * 1.6), opts)
}
None => (item.text_area(), tf.options.clone()),
}
}
/// Build the frame specs of a story from the document (geometry, wrap, page names, grid).
pub fn frame_specs(doc: &Document, sid: StoryId) -> Vec<FrameSpec> {
frame_specs_placed(doc, sid, &[])
@@ -694,21 +739,7 @@ pub(crate) fn frame_specs_placed(doc: &Document, sid: StoryId, placed: &[(ItemId
&& loc.is_some()
&& let Some(sp) = spread
{
let left = |x: f64| sp.page_at_x(x).and_then(|pi| sp.pages.get(pi)).is_some_and(|p| p.side == designcraft_doc::PageSide::Left);
for other in &sp.items {
if doc.layer(other.layer).is_some_and(|l| !l.visible) {
continue;
}
wrap_obstacles(other, Affine::IDENTITY, inv, item.id, &left, 0, &mut exclusions);
}
// Only objects that reach the frame's text area wrap its text: one elsewhere on the
// spread would otherwise cost a narrow column its room (the 1.5 em minimum segment of
// `free_slots` applies once an object cuts the column).
let a = item.text_area();
exclusions.retain(|e| {
let r = e.rect;
r.is_finite() && r.x0 < a.x1 && r.x1 > a.x0 && r.y0 < a.y1 && r.y1 > a.y0
});
frame_wrap_exclusions(doc, sp, item, inv, &mut exclusions);
}
let (page_name, page, side) = match (spread_ref, spread) {
(Some(designcraft_doc::SpreadRef::Doc(si)), Some(sp)) => {
@@ -750,27 +781,7 @@ pub(crate) fn frame_specs_placed(doc: &Document, sid: StoryId, placed: &[(ItemId
};
let grid = (inc > 0.0 && inc.is_finite() && start.is_finite()).then_some((start - ty, inc));
// Type on a path: one line as long as the path (from the start offset).
let (area, opts) = match &tf.options.path {
Some(pt) => {
let len = designcraft_geom::warp::PathWarp::new(&item.path.to_bezpath(), false).length();
let size = st
.paras
.first()
.map(|p| doc.styles.resolve_para(p).1)
.map(|base| st.runs().map(|(_, f)| doc.styles.resolve_char(&base, f).size).fold(base.size, f64::max))
.unwrap_or(12.0);
let opts = TextFrameOptions {
columns: 1,
inset: [0.0; 4],
first_baseline: designcraft_doc::FirstBaseline::Ascent,
// Keep the path identity: its synthetic area uses full line extents.
..tf.options.clone()
};
let end = pt.end.map_or(len, |e| e.max(0.0).min(len));
(Rect::new(0.0, 0.0, (end - pt.start).max(1.0), size * 1.6), opts)
}
None => (item.text_area(), tf.options.clone()),
};
let (area, opts) = frame_text_area(doc, st, item, tf);
// A non-rectangular frame keeps the top inset inward from its outline; its rectangular text
// area (all four insets) still bounds the lines.
let outline = match &tf.options.path {
+56 -45
View File
@@ -273,56 +273,67 @@ fn convert_all(s: &mut Session, p: &Value) -> Result<Value> {
};
s.edit(|d, sel| {
for plan in &plans {
// A selection in a table cell or a footnote keeps its place in that text: it is
// converted first, so the loop below finds nothing left there.
if let Some(t) = sel.text.as_mut().filter(|t| t.story == plan.sid)
&& let Some(c) = t.cell
{
let value = match c.footnote_id() {
Some(id) => d.story(plan.sid).and_then(|st| st.notes.iter().position(|n| n.id == id)).and_then(|k| plan.notes.get(k)),
None => plan.tables.iter().find(|(id, _)| *id == c.table).map(|(_, v)| v),
};
if let Some(v) = value
&& let Some(text) = d.text_story_mut(plan.sid, Some(c))
{
let grown = replace_nested(text, i, v, 1);
t.anchor = shifted(t.anchor, &grown);
t.focus = shifted(t.focus, &grown);
}
}
let st = d.story_mut(plan.sid).ok_or(designcraft_doc::DocError::NoStory(plan.sid))?;
let at: Vec<(usize, usize)> = plan.own.iter().map(|(pos, _)| (*pos, i)).collect();
let grown = replace_instances(st, &at, &|pos| plan.own.iter().find(|(k, _)| *k == pos).map(|(_, v)| v.clone()).unwrap_or_default());
let mut nested = false;
for (id, v) in &plan.tables {
if let Some(t) = st.tables.get_mut(id)
&& t.cells.iter().any(|c| holds(&c.text, i, 1))
{
for c in &mut std::sync::Arc::make_mut(t).cells {
replace_nested(&mut c.text, i, v, 1);
}
nested = true;
}
}
for (note, v) in st.notes.iter_mut().zip(&plan.notes).chain(st.endnotes.iter_mut().zip(&plan.endnotes)) {
if holds(&note.text, i, 1) {
replace_nested(&mut std::sync::Arc::make_mut(note).text, i, v, 1);
nested = true;
}
}
if nested {
st.rev = st.rev.wrapping_add(1);
}
// A selection in the story keeps its place in the text.
if let Some(t) = sel.text.as_mut().filter(|t| t.story == plan.sid && t.cell.is_none()) {
t.anchor = shifted(t.anchor, &grown);
t.focus = shifted(t.focus, &grown);
}
convert_plan(d, sel, i, plan)?;
}
Ok(Value::Null)
})
}
/// Converts one story's instances of variable `i` as planned, keeping the selection in place.
fn convert_plan(d: &mut designcraft_doc::Document, sel: &mut designcraft_doc::Selection, i: usize, plan: &StoryPlan) -> Result<()> {
// A selection in a table cell or a footnote keeps its place in that text: it is
// converted first, so the loop below finds nothing left there.
if let Some(t) = sel.text.as_mut().filter(|t| t.story == plan.sid)
&& let Some(c) = t.cell
{
let value = match c.footnote_id() {
Some(id) => d.story(plan.sid).and_then(|st| st.notes.iter().position(|n| n.id == id)).and_then(|k| plan.notes.get(k)),
None => plan.tables.iter().find(|(id, _)| *id == c.table).map(|(_, v)| v),
};
if let Some(v) = value
&& let Some(text) = d.text_story_mut(plan.sid, Some(c))
{
let grown = replace_nested(text, i, v, 1);
t.anchor = shifted(t.anchor, &grown);
t.focus = shifted(t.focus, &grown);
}
}
let st = d.story_mut(plan.sid).ok_or(designcraft_doc::DocError::NoStory(plan.sid))?;
let at: Vec<(usize, usize)> = plan.own.iter().map(|(pos, _)| (*pos, i)).collect();
let grown = replace_instances(st, &at, &|pos| plan.own.iter().find(|(k, _)| *k == pos).map(|(_, v)| v.clone()).unwrap_or_default());
if replace_in_tables_and_notes(st, i, plan) {
st.rev = st.rev.wrapping_add(1);
}
// A selection in the story keeps its place in the text.
if let Some(t) = sel.text.as_mut().filter(|t| t.story == plan.sid && t.cell.is_none()) {
t.anchor = shifted(t.anchor, &grown);
t.focus = shifted(t.focus, &grown);
}
Ok(())
}
/// Replaces variable `i` in the tables and foot/endnotes of `st` that hold it. Returns whether any did.
fn replace_in_tables_and_notes(st: &mut designcraft_doc::Story, i: usize, plan: &StoryPlan) -> bool {
let mut nested = false;
for (id, v) in &plan.tables {
if let Some(t) = st.tables.get_mut(id)
&& t.cells.iter().any(|c| holds(&c.text, i, 1))
{
for c in &mut std::sync::Arc::make_mut(t).cells {
replace_nested(&mut c.text, i, v, 1);
}
nested = true;
}
}
for (note, v) in st.notes.iter_mut().zip(&plan.notes).chain(st.endnotes.iter_mut().zip(&plan.endnotes)) {
if holds(&note.text, i, 1) {
replace_nested(&mut std::sync::Arc::make_mut(note).text, i, v, 1);
nested = true;
}
}
nested
}
/// Nesting limit for tables in cells and notes (hostile documents).
const MAX_DEPTH: usize = 32;
+116 -110
View File
@@ -1451,116 +1451,7 @@ impl Exporter<'_> {
}
fn graphic_unclipped(&mut self, s: &mut Surface, g: &designcraft_doc::Graphic, hidden: Option<&[String]>) {
// Video / sound without a poster: a dark frame with a play mark.
if let Some(asset) = self.doc.assets.get(&g.asset)
&& designcraft_doc::media_kind(&asset.mime).is_some()
{
let (w, h) = g.size;
let (cx, cy, r) = (w / 2.0, h / 2.0, w.min(h) * 0.18);
let mut tri = BezPath::new();
tri.move_to((cx - r * 0.6, cy - r));
tri.line_to((cx + r, cy));
tri.line_to((cx - r * 0.6, cy + r));
tri.close_path();
s.push_transform(&tf(g.xf));
s.set_stroke(None);
for (bp, c) in [(Rect::new(0.0, 0.0, w, h).to_path(0.1), rgb::Color::new(48, 48, 52)), (tri, rgb::Color::new(235, 235, 235))] {
if let Some(p) = to_path(&bp) {
s.set_fill(Some(Fill { paint: c.into(), ..Default::default() }));
s.draw_path(&p);
}
}
s.set_fill(None);
s.pop();
return;
}
// Placed PDFs go in as vectors (the page as a form XObject).
let doc = self.doc;
if let Some(asset) = doc.assets.get(&g.asset)
&& asset.data.starts_with(b"%PDF")
{
// Cropped to a box: the whole page, placed so that the box spans graphic space.
let shown = asset.shown_box();
let [l, t, r, b] = shown.unwrap_or([0.0, 0.0, 1.0, 1.0]);
let (page_w, page_h) = (g.size.0 / (r - l), g.size.1 / (b - t));
let Some(size) = Size::from_wh(page_w.max(1e-3) as f32, page_h.max(1e-3) as f32) else { return };
let request = (g.asset, hidden.map(<[String]>::to_vec));
let variant = match self.variants.get(&request) {
Some(variant) => variant.clone(),
None => {
#[cfg(test)]
{
self.pdf_preparations += 1;
}
match crate::optional_content::prepare(&asset.data, hidden) {
Ok(variant) => {
self.variants.insert(request, variant.clone());
variant
}
Err(error) => {
let message = match error {
PdfError::Write(message) => message,
other => other.to_string(),
};
self.layer_error.get_or_insert(PdfError::Write(format!("{}: {message}", asset.name)));
return;
}
}
}
};
if let Some(warning) = variant.warning {
self.warn(format!("{}: {warning}", asset.name));
}
let key = (g.asset, variant.hidden);
let placed = match self.pdfs.get(&key) {
Some(p) => p.clone(),
None => {
let p = self.placed_pdf(asset, variant.data);
self.pdfs.insert(key, p.clone());
p
}
};
match placed {
// krilla would fail the whole export at the end on a page the PDF doesn't have.
Some((_, pages)) if asset.page as usize >= pages => {
self.warn(format!("{}: the placed PDF has no page {}", asset.name, u64::from(asset.page) + 1));
}
Some((placed, _)) => {
self.has_optional_content |= variant.has_optional_content;
s.push_transform(&tf(g.xf));
let clip = shown.and_then(|_| to_path(&Rect::new(0.0, 0.0, g.size.0, g.size.1).to_path(0.1)));
if let Some(clip) = &clip {
s.push_clip_path(clip, &FillRule::NonZero);
}
s.push_transform(&tf(Affine::translate((-l * page_w, -t * page_h))));
s.draw_pdf_page(&placed, size, asset.page as usize);
s.pop();
if clip.is_some() {
s.pop();
}
s.pop();
}
None => {}
}
return;
}
// Placed SVGs go in as vectors too.
if let Some(asset) = self.doc.assets.get(&g.asset)
&& designcraft_images::is_svg(&asset.data)
{
let Some(size) = Size::from_wh(g.size.0.max(1e-3) as f32, g.size.1.max(1e-3) as f32) else { return };
let tree = self.svgs.entry(g.asset).or_insert_with(|| designcraft_images::svg_tree(&asset.data).map(Arc::new)).clone();
match tree {
Some(tree) => {
use krilla_svg::SurfaceExt;
s.push_transform(&tf(g.xf));
if s.draw_svg(&tree, size, krilla_svg::SvgSettings::default()).is_none() {
self.warn(format!("{}: parts of the placed SVG could not be drawn", asset.name));
}
s.pop();
}
None => self.warn(format!("{}: can't read the placed SVG", asset.name)),
}
if self.graphic_media_placeholder(s, g) || self.graphic_placed_pdf(s, g, hidden) || self.graphic_placed_svg(s, g) {
return;
}
if self.colorized(s, g) {
@@ -1573,6 +1464,121 @@ impl Exporter<'_> {
s.pop();
}
/// Video or sound without a poster: a dark frame with a play mark. `false` when `g` is not one.
fn graphic_media_placeholder(&self, s: &mut Surface, g: &designcraft_doc::Graphic) -> bool {
let Some(asset) = self.doc.assets.get(&g.asset) else { return false };
if designcraft_doc::media_kind(&asset.mime).is_none() {
return false;
}
let (w, h) = g.size;
let (cx, cy, r) = (w / 2.0, h / 2.0, w.min(h) * 0.18);
let mut tri = BezPath::new();
tri.move_to((cx - r * 0.6, cy - r));
tri.line_to((cx + r, cy));
tri.line_to((cx - r * 0.6, cy + r));
tri.close_path();
s.push_transform(&tf(g.xf));
s.set_stroke(None);
for (bp, c) in [(Rect::new(0.0, 0.0, w, h).to_path(0.1), rgb::Color::new(48, 48, 52)), (tri, rgb::Color::new(235, 235, 235))] {
if let Some(p) = to_path(&bp) {
s.set_fill(Some(Fill { paint: c.into(), ..Default::default() }));
s.draw_path(&p);
}
}
s.set_fill(None);
s.pop();
true
}
/// A placed PDF goes in as vectors (the page as a form XObject). `false` when `g` is not one.
fn graphic_placed_pdf(&mut self, s: &mut Surface, g: &designcraft_doc::Graphic, hidden: Option<&[String]>) -> bool {
let doc = self.doc;
let Some(asset) = doc.assets.get(&g.asset).filter(|asset| asset.data.starts_with(b"%PDF")) else { return false };
// Cropped to a box: the whole page, placed so that the box spans graphic space.
let shown = asset.shown_box();
let [l, t, r, b] = shown.unwrap_or([0.0, 0.0, 1.0, 1.0]);
let (page_w, page_h) = (g.size.0 / (r - l), g.size.1 / (b - t));
let Some(size) = Size::from_wh(page_w.max(1e-3) as f32, page_h.max(1e-3) as f32) else { return true };
let request = (g.asset, hidden.map(<[String]>::to_vec));
let variant = match self.variants.get(&request) {
Some(variant) => variant.clone(),
None => {
#[cfg(test)]
{
self.pdf_preparations += 1;
}
match crate::optional_content::prepare(&asset.data, hidden) {
Ok(variant) => {
self.variants.insert(request, variant.clone());
variant
}
Err(error) => {
let message = match error {
PdfError::Write(message) => message,
other => other.to_string(),
};
self.layer_error.get_or_insert(PdfError::Write(format!("{}: {message}", asset.name)));
return true;
}
}
}
};
if let Some(warning) = variant.warning {
self.warn(format!("{}: {warning}", asset.name));
}
let key = (g.asset, variant.hidden);
let placed = match self.pdfs.get(&key) {
Some(p) => p.clone(),
None => {
let p = self.placed_pdf(asset, variant.data);
self.pdfs.insert(key, p.clone());
p
}
};
match placed {
// krilla would fail the whole export at the end on a page the PDF doesn't have.
Some((_, pages)) if asset.page as usize >= pages => {
self.warn(format!("{}: the placed PDF has no page {}", asset.name, u64::from(asset.page) + 1));
}
Some((placed, _)) => {
self.has_optional_content |= variant.has_optional_content;
s.push_transform(&tf(g.xf));
let clip = shown.and_then(|_| to_path(&Rect::new(0.0, 0.0, g.size.0, g.size.1).to_path(0.1)));
if let Some(clip) = &clip {
s.push_clip_path(clip, &FillRule::NonZero);
}
s.push_transform(&tf(Affine::translate((-l * page_w, -t * page_h))));
s.draw_pdf_page(&placed, size, asset.page as usize);
s.pop();
if clip.is_some() {
s.pop();
}
s.pop();
}
None => {}
}
true
}
/// A placed SVG goes in as vectors too. `false` when `g` is not one.
fn graphic_placed_svg(&mut self, s: &mut Surface, g: &designcraft_doc::Graphic) -> bool {
let Some(asset) = self.doc.assets.get(&g.asset).filter(|asset| designcraft_images::is_svg(&asset.data)) else { return false };
let Some(size) = Size::from_wh(g.size.0.max(1e-3) as f32, g.size.1.max(1e-3) as f32) else { return true };
let tree = self.svgs.entry(g.asset).or_insert_with(|| designcraft_images::svg_tree(&asset.data).map(Arc::new)).clone();
match tree {
Some(tree) => {
use krilla_svg::SurfaceExt;
s.push_transform(&tf(g.xf));
if s.draw_svg(&tree, size, krilla_svg::SvgSettings::default()).is_none() {
self.warn(format!("{}: parts of the placed SVG could not be drawn", asset.name));
}
s.pop();
}
None => self.warn(format!("{}: can't read the placed SVG", asset.name)),
}
true
}
/// A greyscale or bitmap image coloured by the swatch applied to it (#1094): the swatch is
/// painted through the image's ink coverage (its darkness), over 0% ink for a greyscale
/// image, whose white is opaque. `false`, drawing nothing, when the image keeps its own
+24 -22
View File
@@ -3260,6 +3260,29 @@ fn keys_taken(app: &DesignApp, ctx: &egui::Context) -> bool {
|| crate::chrome::search_had_focus(ctx)
}
/// The command whose shortcut `key` with `modifiers` stands for, and whether the modifiers match
/// that shortcut exactly.
fn command_shortcut(app: &DesignApp, key: egui::Key, modifiers: egui::Modifiers, typing: bool) -> Option<(String, bool)> {
for (id, sc) in &effective_shortcuts(app) {
let id = id.as_str();
if let Some((m, k)) = parse_shortcut(sc)
&& let Some(exact) = key_matches(m, k, key, modifiers)
&& (m.command || m.alt || !typing)
&& !(id == "edit.clear" && typing)
{
// Single-key shortcuts (W) only when not typing.
if !m.command && !m.alt && !m.shift && typing {
continue;
}
if id == "edit.clear" || (typing && matches!(id, "edit.copy" | "edit.cut" | "edit.paste" | "edit.pasteInPlace")) {
continue; // Handled by the active tool / text clipboard events.
}
return Some((id.to_string(), exact));
}
}
None
}
/// Runs the presentation key, command shortcut or tool shortcut a key press stands for. `consume`
/// takes the press from the input where nothing else may see it. Returns whether the press was used.
fn shortcut_key(app: &mut DesignApp, key: egui::Key, modifiers: egui::Modifiers, typing: bool, consume: &mut dyn FnMut()) -> bool {
@@ -3285,28 +3308,7 @@ fn shortcut_key(app: &mut DesignApp, key: egui::Key, modifiers: egui::Modifiers,
return true;
}
}
// Command shortcuts.
let mut fired: Option<(String, bool)> = None;
let all = effective_shortcuts(app);
for (id, sc) in &all {
let id = id.as_str();
if let Some((m, k)) = parse_shortcut(sc)
&& let Some(exact) = key_matches(m, k, key, modifiers)
&& (m.command || m.alt || !typing)
&& !(id == "edit.clear" && typing)
{
// Single-key shortcuts (W) only when not typing.
if !m.command && !m.alt && !m.shift && typing {
continue;
}
if id == "edit.clear" || (typing && matches!(id, "edit.copy" | "edit.cut" | "edit.paste" | "edit.pasteInPlace")) {
continue; // Handled by the active tool / text clipboard events.
}
fired = Some((id.to_string(), exact));
break;
}
}
if let Some((id, exact)) = fired {
if let Some((id, exact)) = command_shortcut(app, key, modifiers, typing) {
// The native menu bar carries the shortcuts in effect, the user's included, and its
// accelerator fires only for the shortcut's own key and modifiers; the Plus key standing
// in for = never reaches it.