From 4bb73d5d3477ef4dc5ec119995e56a73fdc3b74a Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Mon, 26 Oct 2026 16:01:53 -0400 Subject: [PATCH 1/2] refactor: reduce cognitive complexity in five functions (S3776) --- crates/engine/src/cmd/points.rs | 106 ++++++++++++---------- crates/h264/src/dpb.rs | 63 ++++++------- crates/hevc/src/mvpred.rs | 81 ++++++++--------- crates/model/src/chart.rs | 99 ++++++++++++--------- crates/render/src/table.rs | 151 ++++++++++++++++++-------------- crates/render/src/tests.rs | 37 ++++++++ 6 files changed, 301 insertions(+), 236 deletions(-) diff --git a/crates/engine/src/cmd/points.rs b/crates/engine/src/cmd/points.rs index baa94232..28eb0dd3 100644 --- a/crates/engine/src/cmd/points.rs +++ b/crates/engine/src/cmd/points.rs @@ -658,6 +658,62 @@ pub fn vertex_at(doc: &deckcraft_model::Presentation, sel: &crate::Selection, sh None } +/// The point at parameter `t` of segment `si`, scaled by `scale` into the shape's box. +fn segment_point(segs: &[Seg], si: usize, p0: Point, t: f64, scale: (f64, f64)) -> Option { + let q = match segs.get(si)? { + Seg::L(p1) => p0.lerp(*p1, t), + Seg::Q(c, p1) => p0.lerp(*c, t).lerp(c.lerp(*p1, t), t), + Seg::C(c1, c2, p1) => { + let (a, b, c) = (p0.lerp(*c1, t), c1.lerp(*c2, t), c2.lerp(*p1, t)); + a.lerp(b, t).lerp(b.lerp(c, t), t) + } + _ => return None, + }; + Some(Point::new(q.x * scale.0, q.y * scale.1)) +} + +/// The vertex that ends segment `si` (a closing edge ends on the start). +fn segment_end_vertex(segs: &[Seg], vs: &[usize], si: usize) -> Option { + if closes_onto_start(segs, si) { + let start = segs.get(..si).and_then(|s| s.iter().rposition(|x| matches!(x, Seg::M(_)))); + start.and_then(|m| vs.iter().position(|v| *v == m)) + } else { + vs.iter().position(|v| *v == si) + } +} + +/// The distance and `t` of the point on segment `si` nearest `local`, if it lies within `tol`. +fn nearest_on_segment(segs: &[Seg], si: usize, p0: Point, scale: (f64, f64), local: Point, tol: f64) -> Option<(f64, f64)> { + let at = |t: f64| segment_point(segs, si, p0, t, scale); + let mut near: Option<(f64, f64)> = None; + for k in 1..32 { + let t = k as f64 / 32.0; + if let Some(q) = at(t) { + let d = (q - local).hypot(); + if d <= tol && near.is_none_or(|n| d < n.0) { + near = Some((d, t)); + } + } + } + let (mut d, mut t) = near?; + // Narrow `t` down to the nearest place on the segment, where Add Point puts the point. + let mut step = 1.0 / 32.0; + for _ in 0..4 { + step /= 8.0; + let c = t; + for k in -8..=8 { + let u = (c + f64::from(k) * step).clamp(0.0, 1.0); + if let Some(q) = at(u) { + let e = (q - local).hypot(); + if e < d { + (d, t) = (e, u); + } + } + } + } + Some((d, t)) +} + /// The segment of `sh`'s outline near slide point `p`: (path, index of the vertex ending it, t). pub fn segment_at(doc: &deckcraft_model::Presentation, sel: &crate::Selection, sh: &Shape, p: Point, tol: f64) -> Option<(usize, usize, f64)> { let Geom::Custom { paths } = &sh.geom else { return None }; @@ -665,57 +721,13 @@ pub fn segment_at(doc: &deckcraft_model::Presentation, sel: &crate::Selection, s let local = x.affine().inverse() * p; let mut best: Option<(f64, usize, usize, f64)> = None; for (pi, cp) in paths.iter().enumerate() { - let (sx, sy) = (if cp.w > 0.0 { x.w / cp.w } else { 1.0 }, if cp.h > 0.0 { x.h / cp.h } else { 1.0 }); + let scale = (if cp.w > 0.0 { x.w / cp.w } else { 1.0 }, if cp.h > 0.0 { x.h / cp.h } else { 1.0 }); let segs = parse(&cp.d); let vs = vertices(&segs); for si in 0..segs.len() { let Some(p0) = seg_start(&segs, si) else { continue }; - let at = |t: f64| -> Option { - let q = match segs.get(si)? { - Seg::L(p1) => p0.lerp(*p1, t), - Seg::Q(c, p1) => p0.lerp(*c, t).lerp(c.lerp(*p1, t), t), - Seg::C(c1, c2, p1) => { - let (a, b, c) = (p0.lerp(*c1, t), c1.lerp(*c2, t), c2.lerp(*p1, t)); - a.lerp(b, t).lerp(b.lerp(c, t), t) - } - _ => return None, - }; - Some(Point::new(q.x * sx, q.y * sy)) - }; - // The vertex that ends this segment (a closing edge ends on the start). - let end_vertex = if closes_onto_start(&segs, si) { - let start = segs.get(..si).and_then(|s| s.iter().rposition(|x| matches!(x, Seg::M(_)))); - start.and_then(|m| vs.iter().position(|v| *v == m)) - } else { - vs.iter().position(|v| *v == si) - }; - let Some(ev) = end_vertex else { continue }; - let mut near: Option<(f64, f64)> = None; - for k in 1..32 { - let t = k as f64 / 32.0; - if let Some(q) = at(t) { - let d = (q - local).hypot(); - if d <= tol && near.is_none_or(|n| d < n.0) { - near = Some((d, t)); - } - } - } - let Some((mut d, mut t)) = near else { continue }; - // Narrow `t` down to the nearest place on the segment, where Add Point puts the point. - let mut step = 1.0 / 32.0; - for _ in 0..4 { - step /= 8.0; - let c = t; - for k in -8..=8 { - let u = (c + f64::from(k) * step).clamp(0.0, 1.0); - if let Some(q) = at(u) { - let e = (q - local).hypot(); - if e < d { - (d, t) = (e, u); - } - } - } - } + let Some(ev) = segment_end_vertex(&segs, &vs, si) else { continue }; + let Some((d, t)) = nearest_on_segment(&segs, si, p0, scale, local, tol) else { continue }; if best.is_none_or(|b| d < b.0) { best = Some((d, pi, ev, t)); } diff --git a/crates/h264/src/dpb.rs b/crates/h264/src/dpb.rs index a74799d9..708ea7a1 100644 --- a/crates/h264/src/dpb.rs +++ b/crates/h264/src/dpb.rs @@ -301,6 +301,25 @@ impl Dpb { Ok(lists) } + /// Marks as unused every entry in state `mark` that `hit` selects. + fn unmark_where(&mut self, mark: RefMark, hit: impl Fn(&DpbEntry) -> bool) { + for e in &mut self.entries { + if e.mark == mark && hit(e) { + e.mark = RefMark::Unused; + } + } + } + + /// Makes every short-term entry that `hit` selects a long-term one with index `idx`. + fn mark_long_where(&mut self, hit: impl Fn(&DpbEntry) -> bool, idx: u32) { + for e in &mut self.entries { + if e.mark == RefMark::Short && hit(e) { + e.mark = RefMark::Long; + e.long_term_frame_idx = idx; + } + } + } + /// One memory management control operation; returns true for MMCO 5. fn apply_mmco(&mut self, m: &Mmco, cur_frame_num: u32, max_frame_num: u32, cur_long: &mut Option) -> bool { let mut had5 = false; @@ -308,40 +327,17 @@ impl Dpb { match m.op { 1 => { let pn = curr_pic_num - (m.difference_of_pic_nums_minus1 as i32 + 1); - for e in &mut self.entries { - if e.mark == RefMark::Short && Self::frame_num_wrap(e, cur_frame_num, max_frame_num) == pn { - e.mark = RefMark::Unused; - } - } - } - 2 => { - for e in &mut self.entries { - if e.mark == RefMark::Long && e.long_term_frame_idx == m.long_term_pic_num { - e.mark = RefMark::Unused; - } - } + self.unmark_where(RefMark::Short, |e| Self::frame_num_wrap(e, cur_frame_num, max_frame_num) == pn); } + 2 => self.unmark_where(RefMark::Long, |e| e.long_term_frame_idx == m.long_term_pic_num), 3 => { let pn = curr_pic_num - (m.difference_of_pic_nums_minus1 as i32 + 1); - for e in &mut self.entries { - if e.mark == RefMark::Long && e.long_term_frame_idx == m.long_term_frame_idx { - e.mark = RefMark::Unused; - } - } - for e in &mut self.entries { - if e.mark == RefMark::Short && Self::frame_num_wrap(e, cur_frame_num, max_frame_num) == pn { - e.mark = RefMark::Long; - e.long_term_frame_idx = m.long_term_frame_idx; - } - } + self.unmark_where(RefMark::Long, |e| e.long_term_frame_idx == m.long_term_frame_idx); + self.mark_long_where(|e| Self::frame_num_wrap(e, cur_frame_num, max_frame_num) == pn, m.long_term_frame_idx); } 4 => { let max = m.max_long_term_frame_idx_plus1; - for e in &mut self.entries { - if e.mark == RefMark::Long && (max == 0 || e.long_term_frame_idx > max - 1) { - e.mark = RefMark::Unused; - } - } + self.unmark_where(RefMark::Long, |e| max == 0 || e.long_term_frame_idx > max - 1); self.max_long_term_frame_idx = if max == 0 { None } else { Some(max - 1) }; } 5 => { @@ -353,16 +349,9 @@ impl Dpb { } 6 => { // Only an index within the allowed range may be assigned (8.2.5.4.6). - let allowed = match self.max_long_term_frame_idx { - Some(max) => m.long_term_frame_idx <= max, - None => false, - }; + let allowed = self.max_long_term_frame_idx.is_some_and(|max| m.long_term_frame_idx <= max); if allowed { - for e in &mut self.entries { - if e.mark == RefMark::Long && e.long_term_frame_idx == m.long_term_frame_idx { - e.mark = RefMark::Unused; - } - } + self.unmark_where(RefMark::Long, |e| e.long_term_frame_idx == m.long_term_frame_idx); *cur_long = Some(m.long_term_frame_idx); } } diff --git a/crates/hevc/src/mvpred.rs b/crates/hevc/src/mvpred.rs index 236c14de..29b77e3b 100644 --- a/crates/hevc/src/mvpred.rs +++ b/crates/hevc/src/mvpred.rs @@ -261,68 +261,59 @@ impl<'a> SliceDecoder<'a> { if cur.long_term || col_diff == cur_diff { Some(mv) } else { Some(scale_mv(mv, col_diff, cur_diff)) } } + /// AMVP neighbour motion with the target's reference picture (no scaling), 8.5.3.2.7. + fn amvp_exact(&self, f: &MvField, l: usize, tpoc: i32) -> Option<[i16; 2]> { + let y = 1 - l; + if f.pred(l) && self.ref_pic(l, f.ref_idx[l]).poc == tpoc { + Some(f.mv[l]) + } else if f.pred(y) && self.ref_pic(y, f.ref_idx[y]).poc == tpoc { + Some(f.mv[y]) + } else { + None + } + } + + /// AMVP neighbour motion with a long-term-compatible reference, scaled when both are short-term. + fn amvp_scaled(&self, f: &MvField, l: usize, tpoc: i32, tlt: bool, cur_poc: i32) -> Option<[i16; 2]> { + let y = 1 - l; + let pick = if f.pred(l) && self.ref_pic(l, f.ref_idx[l]).long_term == tlt { + Some((f.mv[l], self.ref_pic(l, f.ref_idx[l]))) + } else if f.pred(y) && self.ref_pic(y, f.ref_idx[y]).long_term == tlt { + Some((f.mv[y], self.ref_pic(y, f.ref_idx[y]))) + } else { + None + }; + pick.map(|(mv, r)| if r.poc != tpoc && !r.long_term && !tlt { scale_mv(mv, cur_poc - r.poc, cur_poc - tpoc) } else { mv }) + } + + /// The first neighbour whose position is available and whose motion `pick` accepts. + fn first_amvp_neighbour(&self, pos: &[(i32, i32)], avail: &[bool], pick: impl Fn(&MvField) -> Option<[i16; 2]>) -> Option<[i16; 2]> { + pos.iter().zip(avail).find_map(|(&(x, y), &ok)| if ok { pick(&self.mv_at(x, y)) } else { None }) + } + /// AMVP predictor (8.5.3.2.6, 8.5.3.2.7). pub fn derive_amvp(&self, g: PbGeom, l: usize, ref_idx: usize, mvp_flag: usize) -> [i16; 2] { let PbGeom { xp, yp, w, h, .. } = g; let target = &self.refs[l][ref_idx]; let (tpoc, tlt) = (target.poc, target.long_term); let cur_poc = self.pic.poc; - let y = 1 - l; - // exact reference match (no scaling) - let unscaled = |f: &MvField| -> Option<[i16; 2]> { - if f.pred(l) && self.ref_pic(l, f.ref_idx[l]).poc == tpoc { - Some(f.mv[l]) - } else if f.pred(y) && self.ref_pic(y, f.ref_idx[y]).poc == tpoc { - Some(f.mv[y]) - } else { - None - } - }; - // long-term-compatible match, scaled when both references are short-term - let scaled = |f: &MvField| -> Option<[i16; 2]> { - let pick = if f.pred(l) && self.ref_pic(l, f.ref_idx[l]).long_term == tlt { - Some((f.mv[l], self.ref_pic(l, f.ref_idx[l]))) - } else if f.pred(y) && self.ref_pic(y, f.ref_idx[y]).long_term == tlt { - Some((f.mv[y], self.ref_pic(y, f.ref_idx[y]))) - } else { - None - }; - pick.map(|(mv, r)| if r.poc != tpoc && !r.long_term && !tlt { scale_mv(mv, cur_poc - r.poc, cur_poc - tpoc) } else { mv }) - }; + let exact = |f: &MvField| self.amvp_exact(f, l, tpoc); + let scaled = |f: &MvField| self.amvp_scaled(f, l, tpoc, tlt, cur_poc); let pa = [(xp - 1, yp + h), (xp - 1, yp + h - 1)]; let av_a = [self.pb_avail(&g, pa[0].0, pa[0].1), self.pb_avail(&g, pa[1].0, pa[1].1)]; let is_scaled = av_a[0] || av_a[1]; - let mut a = None; - for k in 0..2 { - if av_a[k] && a.is_none() { - a = unscaled(&self.mv_at(pa[k].0, pa[k].1)); - } - } + let mut a = self.first_amvp_neighbour(&pa, &av_a, exact); if a.is_none() { - for k in 0..2 { - if av_a[k] && a.is_none() { - a = scaled(&self.mv_at(pa[k].0, pa[k].1)); - } - } + a = self.first_amvp_neighbour(&pa, &av_a, scaled); } let pb = [(xp + w, yp - 1), (xp + w - 1, yp - 1), (xp - 1, yp - 1)]; let av_b = [0, 1, 2].map(|k| self.pb_avail(&g, pb[k].0, pb[k].1)); - let mut b = None; - for k in 0..3 { - if av_b[k] && b.is_none() { - b = unscaled(&self.mv_at(pb[k].0, pb[k].1)); - } - } + let mut b = self.first_amvp_neighbour(&pb, &av_b, exact); if !is_scaled && b.is_some() { a = b; } if !is_scaled { - b = None; - for k in 0..3 { - if av_b[k] && b.is_none() { - b = scaled(&self.mv_at(pb[k].0, pb[k].1)); - } - } + b = self.first_amvp_neighbour(&pb, &av_b, scaled); } let mut list: [[i16; 2]; 3] = [[0; 2]; 3]; let mut n = 0; diff --git a/crates/model/src/chart.rs b/crates/model/src/chart.rs index 9d3dc738..7e8311c4 100644 --- a/crates/model/src/chart.rs +++ b/crates/model/src/chart.rs @@ -608,64 +608,73 @@ impl Chart { if ser.is_empty() { return (!secondary && self.kind != ChartType::Combo).then_some((0.0, 0.0)); } - let (mut lo, mut hi) = (f64::INFINITY, f64::NEG_INFINITY); + let mut ext = (f64::INFINITY, f64::NEG_INFINITY); let has_stacked_or_percent = ser.iter().any(|s| { let k = self.series_kind(s); k.stacked() || k.percent() }); - - let mut percent = false; - if has_stacked_or_percent { - let n = ser.iter().map(|s| s.values.len()).max().unwrap_or(0); - let budget = (ser.len() * n).min(10_000_000); - let n_clamped = if ser.is_empty() { 0 } else { n.min(budget / ser.len()) }; - - for i in 0..n_clamped { - let (mut p, mut m, mut stacked) = (0.0, 0.0, false); - for s in &ser { - let k = self.series_kind(s); - let Some(v) = s.values.get(i).copied().flatten().filter(|v| v.is_finite()) else { continue }; - if k.percent() { - percent = true; - } else if k.stacked() { - stacked = true; - if v >= 0.0 { p += v } else { m += v } - } else { - hi = hi.max(v); - lo = lo.min(v); - } - } - if stacked { - hi = hi.max(p); - lo = lo.min(m); - } - } + let percent = if has_stacked_or_percent { + self.fold_stacked(&ser, &mut ext) } else { for s in &ser { for v in s.values.iter().flatten().copied().filter(|v| v.is_finite()) { - hi = hi.max(v); - lo = lo.min(v); + widen(&mut ext, v); } } - } + false + }; if !percent { - for s in &ser { - if self.series_kind(s).stacked() { - continue; - } - for y in s.trendline_ys(crate::trendline::series_is_scatter(self.series_kind(s))) { - hi = hi.max(y); - lo = lo.min(y); - } - } + self.fold_trendlines(&ser, &mut ext); } if percent { - lo = lo.min(0.0); - hi = hi.max(1.0); + ext.0 = ext.0.min(0.0); + ext.1 = ext.1.max(1.0); } + let (lo, hi) = ext; Some(if lo <= hi { (lo, hi) } else { (0.0, 0.0) }) } + /// Widens `ext` by the points of the stacked and 100 % series in `ser`; returns whether a + /// 100 % series is among them. + fn fold_stacked(&self, ser: &[&Series], ext: &mut (f64, f64)) -> bool { + let mut percent = false; + let n = ser.iter().map(|s| s.values.len()).max().unwrap_or(0); + let budget = (ser.len() * n).min(10_000_000); + let n_clamped = if ser.is_empty() { 0 } else { n.min(budget / ser.len()) }; + for i in 0..n_clamped { + let (mut p, mut m, mut stacked) = (0.0, 0.0, false); + for s in ser { + let k = self.series_kind(s); + let Some(v) = s.values.get(i).copied().flatten().filter(|v| v.is_finite()) else { continue }; + if k.percent() { + percent = true; + } else if k.stacked() { + stacked = true; + if v >= 0.0 { p += v } else { m += v } + } else { + widen(ext, v); + } + } + if stacked { + ext.1 = ext.1.max(p); + ext.0 = ext.0.min(m); + } + } + percent + } + + /// Widens `ext` by the trendlines of the series in `ser` that are not stacked. + fn fold_trendlines(&self, ser: &[&Series], ext: &mut (f64, f64)) { + for s in ser { + if self.series_kind(s).stacked() { + continue; + } + for y in s.trendline_ys(crate::trendline::series_is_scatter(self.series_kind(s))) { + widen(ext, y); + } + } + } + /// A waterfall's bars as (from, to) on the value axis: each point floats from the running /// total, a total point stands on zero and sets it. pub fn waterfall_bars(&self) -> Vec<(f64, f64)> { @@ -1346,6 +1355,12 @@ impl Chart { } } +/// Widens the (min, max) extent `ext` to include `v`. +fn widen(ext: &mut (f64, f64), v: f64) { + ext.0 = ext.0.min(v); + ext.1 = ext.1.max(v); +} + /// Excel's QUARTILE.EXC position `p·(n+1)` (1-based) on sorted data, clamped to the ends. fn quantile_exc(v: &[f64], p: f64) -> f64 { let n = v.len(); diff --git a/crates/render/src/table.rs b/crates/render/src/table.rs index 9c720628..5be9900d 100644 --- a/crates/render/src/table.rs +++ b/crates/render/src/table.rs @@ -147,6 +147,86 @@ pub fn look(ctx: &Ctx, t: &Table, r: usize, c: usize) -> CellLook { look_with_def(ctx, resolve_style(ctx, t), t, r, c) } +/// Which parts of the table a cell sits in, for the preset styles. +#[derive(Clone, Copy)] +struct PresetPart { + header: bool, + total: bool, + first_col: bool, + last_col: bool, + banded: bool, +} + +impl PresetPart { + /// In a header, total, first or last column: the parts a preset style makes strong. + fn strong(self) -> bool { + self.header || self.total || self.first_col || self.last_col + } +} + +/// Tint `k` of accent `a`. +fn tint(a: Rgba, k: i32) -> Rgba { + deckcraft_color::apply(a, &[ColorTransform::Tint(k)]) +} + +fn light1_look(a: Rgba, p: PresetPart, on: impl Fn(Rgba) -> Rgba) -> CellLook { + let fill = p.banded.then(|| tint(a, 20000)); + CellLook { fill, text: fill.map(on), bold: p.strong(), border: if p.header { Some((a, 1.0)) } else { None }, sides: [None; 4], paint: None } +} + +fn light2_look(a: Rgba, p: PresetPart) -> CellLook { + CellLook { + fill: if p.header { Some(a) } else { None }, + text: if p.header { Some(Rgba::WHITE) } else { None }, + bold: p.header || p.total || p.first_col, + border: Some((a, 1.0)), + sides: [None; 4], + paint: None, + } +} + +fn dark1_look(a: Rgba, p: PresetPart) -> CellLook { + CellLook { + fill: Some(if p.header { + Rgba::BLACK + } else if p.banded { + deckcraft_color::apply(a, &[ColorTransform::Shade(60000)]) + } else { + deckcraft_color::apply(a, &[ColorTransform::Shade(75000)]) + }), + text: Some(Rgba::WHITE), + bold: p.header || p.total || p.first_col, + border: Some((Rgba::WHITE, 1.0)), + sides: [None; 4], + paint: None, + } +} + +fn medium4_look(a: Rgba, p: PresetPart, on: impl Fn(Rgba) -> Rgba) -> CellLook { + let fill = if p.header || p.banded { tint(a, 40000) } else { tint(a, 20000) }; + CellLook { fill: Some(fill), text: Some(on(fill)), bold: p.strong(), border: Some((a, 0.75)), sides: [None; 4], paint: None } +} + +/// medium2: strong header, banded light tints, white grid. +fn medium2_look(a: Rgba, p: PresetPart, on: impl Fn(Rgba) -> Rgba) -> CellLook { + let strong = p.strong(); + let fill = if strong { + a + } else if p.banded { + tint(a, 40000) + } else { + tint(a, 20000) + }; + CellLook { + fill: Some(fill), + text: Some(if strong { Rgba::WHITE } else { on(fill) }), + bold: strong, + border: Some((Rgba::WHITE, 1.0)), + sides: [None; 4], + paint: None, + } +} + pub fn look_with_def(ctx: &Ctx, def: Option<&TableStyleDef>, t: &Table, r: usize, c: usize) -> CellLook { if let Some(def) = def { return look_def(ctx, def, t, r, c); @@ -160,8 +240,7 @@ pub fn look_with_def(ctx: &Ctx, def: Option<&TableStyleDef>, t: &Table, r: usize let body_r = if t.first_row { r.wrapping_sub(1) } else { r }; let band = t.band_row && !header && !total && body_r % 2 == 0; let band_c = t.band_col && c.is_multiple_of(2); - let tint = |k: i32| deckcraft_color::apply(a, &[ColorTransform::Tint(k)]); - let white = Rgba::WHITE; + let p = PresetPart { header, total, first_col, last_col, banded: band || band_c }; let text_color = ctx.color(&ColorRef::scheme(SchemeSlot::Tx1), None); // Text on a tinted cell never inherits the text colour, which a dark theme makes light and // unreadable on the light tint: PowerPoint's medium styles set it to the theme's dark colour @@ -173,69 +252,11 @@ pub fn look_with_def(ctx: &Ctx, def: Option<&TableStyleDef>, t: &Table, r: usize match kind { "none" => CellLook { fill: None, text: None, bold: false, border: None, sides: [None; 4], paint: None }, "grid" => CellLook { fill: None, text: None, bold: false, border: Some((text_color, 0.75)), sides: [None; 4], paint: None }, - "light1" => { - let fill = (band || band_c).then(|| tint(20000)); - CellLook { - fill, - text: fill.map(on), - bold: header || total || first_col || last_col, - border: if header { Some((a, 1.0)) } else { None }, - sides: [None; 4], - paint: None, - } - } - "light2" => CellLook { - fill: if header { Some(a) } else { None }, - text: if header { Some(white) } else { None }, - bold: header || total || first_col, - border: Some((a, 1.0)), - sides: [None; 4], - paint: None, - }, - "dark1" => CellLook { - fill: Some(if header { - Rgba::BLACK - } else if band || band_c { - deckcraft_color::apply(a, &[ColorTransform::Shade(60000)]) - } else { - deckcraft_color::apply(a, &[ColorTransform::Shade(75000)]) - }), - text: Some(white), - bold: header || total || first_col, - border: Some((white, 1.0)), - sides: [None; 4], - paint: None, - }, - "medium4" => { - let fill = if header || band || band_c { tint(40000) } else { tint(20000) }; - CellLook { - fill: Some(fill), - text: Some(on(fill)), - bold: header || total || first_col || last_col, - border: Some((a, 0.75)), - sides: [None; 4], - paint: None, - } - } - _ => { - // medium2: strong header, banded light tints, white grid. - let strong = header || total || first_col || last_col; - let fill = if strong { - a - } else if band || band_c { - tint(40000) - } else { - tint(20000) - }; - CellLook { - fill: Some(fill), - text: Some(if strong { white } else { on(fill) }), - bold: strong, - border: Some((white, 1.0)), - sides: [None; 4], - paint: None, - } - } + "light1" => light1_look(a, p, on), + "light2" => light2_look(a, p), + "dark1" => dark1_look(a, p), + "medium4" => medium4_look(a, p, on), + _ => medium2_look(a, p, on), } } diff --git a/crates/render/src/tests.rs b/crates/render/src/tests.rs index 29e1fe42..bcddf4c9 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -1439,6 +1439,43 @@ fn table_body_text_is_dark_on_the_tinted_cells_of_a_dark_theme() { } } +#[test] +fn table_light_and_dark_presets_give_their_own_header_and_body_looks() { + // light1, light2 and dark1 each style the header and banded body cells their own way. + use deckcraft_color::{ColorTransform, Rgba, SchemeSlot}; + use deckcraft_model::style::ColorRef; + let p = Presentation::default(); + let ctx = Ctx::for_slide(&p, &p.slides[0]).unwrap(); + let a = ctx.color(&ColorRef::scheme(SchemeSlot::Accent1), None); + let look = |style: &str, r: usize| { + let mut t = Table::new(3, 1, 100.0, 90.0); + t.style = style.to_string(); + t.first_row = true; + t.band_row = true; + crate::table::look_with_def(&ctx, None, &t, r, 0) + }; + + // light1: header has no fill, bold text and an accent border; banded body rows get a light tint. + let h = look("light1", 0); + assert_eq!((h.fill, h.bold, h.border), (None, true, Some((a, 1.0)))); + let b = look("light1", 1); + assert_eq!(b.fill, Some(deckcraft_color::apply(a, &[ColorTransform::Tint(20000)]))); + assert!(b.text.is_some() && !b.bold); + + // light2: accent-filled header with white bold text; body has no fill. + let h = look("light2", 0); + assert_eq!((h.fill, h.text, h.bold, h.border), (Some(a), Some(Rgba::WHITE), true, Some((a, 1.0)))); + let b = look("light2", 1); + assert_eq!((b.fill, b.text, b.bold), (None, None, false)); + + // dark1: black header, shaded accent body (darker when banded), white text and white borders. + let h = look("dark1", 0); + assert_eq!((h.fill, h.text, h.bold, h.border), (Some(Rgba::BLACK), Some(Rgba::WHITE), true, Some((Rgba::WHITE, 1.0)))); + let b = look("dark1", 1); + assert_eq!(b.fill, Some(deckcraft_color::apply(a, &[ColorTransform::Shade(60000)]))); + assert_eq!(b.text, Some(Rgba::WHITE)); +} + /// A white 280x200 pt rectangle at (20, 20) with `effects`, rendered at 1 px/pt. fn effect_box(effects: deckcraft_model::Effects) -> Image { use deckcraft_model::style::{Fill, Line}; From 27009cd941b22cb0788cd5040f9fad91472230c7 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Mon, 26 Oct 2026 16:01:57 -0400 Subject: [PATCH 2/2] refactor: reduce cognitive complexity in five more functions (S3776) --- crates/engine/src/cmd/shape.rs | 149 +++++++++++++++++++-------------- crates/h264/src/dpb.rs | 75 ++++++++++------- crates/hevc/src/slicedec.rs | 41 +++++---- crates/model/src/chart.rs | 82 +++++++++++------- crates/pptx/src/read/dml.rs | 74 +++++++++------- 5 files changed, 249 insertions(+), 172 deletions(-) diff --git a/crates/engine/src/cmd/shape.rs b/crates/engine/src/cmd/shape.rs index 9ae6ae90..f2a5da07 100644 --- a/crates/engine/src/cmd/shape.rs +++ b/crates/engine/src/cmd/shape.rs @@ -5,7 +5,7 @@ use std::collections::HashMap; use deckcraft_color::{ColorTransform, Rgba, SchemeSlot}; use deckcraft_model::style::{Dash, Glow, Gradient, GradientShape, GradientStop, LineEnd, PatternFill, PictureFill, PictureMode, Reflection, Shadow}; -use deckcraft_model::{ColorRef, Effects, Fill, Geom, Line, ShapeStyle}; +use deckcraft_model::{ColorRef, Effects, Fill, Geom, Line, ShapeId, ShapeStyle}; use serde_json::{Value, json}; use super::{ @@ -556,81 +556,102 @@ fn edit_group_appearance(s: &mut Session, p: &Value, cmd: &str, f: impl Fn(&mut edit_shapes(s, p, cmd, |sh| apply(sh, 0, &f)) } -fn fill(s: &mut Session, p: &Value) -> Result { - let new_fill: Option> = if bool_or(p, "reset", false) { - Some(None) - } else if bool_or(p, "none", false) { - Some(Some(Fill::None)) - } else if let Some(c) = color_param(p, "color") { - Some(Some(Fill::solid(c))) - } else if let Some(g) = p.get("gradient") { - Some(Some(Fill::Gradient( - gradient_param(g).ok_or_else(|| bad("shape.fill", "gradient needs at least two `stops`: [[0, \"#fff\"], [1, \"#000\"]]"))?, - ))) - } else if let Some(pt) = p.get("pattern") { - Some(Some(Fill::Pattern(PatternFill { +/// The fill a `shape.fill` request sets: `Some(None)` clears it, `Some(Some(_))` sets it, and `None` +/// leaves it as it is. +fn requested_fill(s: &mut Session, p: &Value) -> Result>> { + if bool_or(p, "reset", false) { + return Ok(Some(None)); + } + if bool_or(p, "none", false) { + return Ok(Some(Some(Fill::None))); + } + if let Some(c) = color_param(p, "color") { + return Ok(Some(Some(Fill::solid(c)))); + } + if let Some(g) = p.get("gradient") { + let gradient = gradient_param(g).ok_or_else(|| bad("shape.fill", "gradient needs at least two `stops`: [[0, \"#fff\"], [1, \"#000\"]]"))?; + return Ok(Some(Some(Fill::Gradient(gradient)))); + } + if let Some(pt) = p.get("pattern") { + return Ok(Some(Some(Fill::Pattern(PatternFill { preset: pt.get("preset").and_then(Value::as_str).unwrap_or("pct50").to_string(), fg: pt.get("fg").and_then(color_value).unwrap_or(ColorRef::scheme(SchemeSlot::Accent1)), bg: pt.get("bg").and_then(color_value).unwrap_or(ColorRef::scheme(SchemeSlot::Bg1)), - }))) - } else if p.get("picture").is_some() { + })))); + } + if p.get("picture").is_some() { let (name, bytes) = super::insert::media_bytes(&with_param(p, "data", p.get("picture").cloned().unwrap_or_default()), "shape.fill")?; let ct = super::insert::content_type(&name, &bytes); let media = s.edit(|doc, _| doc.add_media(&name, ct, bytes).ok_or_else(|| bad("shape.fill", NO_ID_SPACE)))?; - Some(Some(Fill::Picture(PictureFill { media, ..Default::default() }))) - } else { - None - }; + return Ok(Some(Some(Fill::Picture(PictureFill { media, ..Default::default() })))); + } + Ok(None) +} + +/// The effective fill of each targeted shape, for a transparency change on an inherited (style) fill. +fn inherited_fills(s: &Session, p: &Value) -> Result>> { + let st = s.doc()?; + let ctx = super::text::ctx_for(&st.doc, &st.selection); + let mut fills = HashMap::new(); + for id in targets(s, p)? { + if let Some(sh) = st.shape(id) { + deckcraft_model::walk(std::slice::from_ref(sh), &mut |sh, _| { + let fill = ctx.as_ref().and_then(|c| { + let (f, ph) = deckcraft_model::resolve::fill(c, sh); + f.map(|f| match f { + Fill::Solid { color } => Fill::solid(ColorRef::rgb(c.color(&color, ph))), + other => other, + }) + }); + fills.insert(sh.id, fill); + }); + } + } + Ok(fills) +} + +/// Applies a `shape.fill` request to one shape: the fill, then its picture options, then transparency. +fn apply_fill( + sh: &mut deckcraft_model::Shape, + new_fill: Option<&Option>, + pic_props: Option<&Value>, + transparency: Option, + inherited: &HashMap>, +) -> Result<()> { + if let Some(f) = new_fill { + sh.fill.clone_from(f); + } + if let (Some(pp), Some(Fill::Picture(pf))) = (pic_props, sh.fill.as_mut()) { + picture_props(pf, pp); + } + if let (Some(t), Some(Fill::Picture(pf))) = (transparency, sh.fill.as_mut()) { + let a = 1.0 - t.clamp(0.0, 1.0); + pf.alpha = (a < 1.0).then_some(a); + return Ok(()); + } + if let Some(t) = transparency { + let base = sh.fill.clone().or_else(|| inherited.get(&sh.id).cloned().flatten()); + if let Some(Fill::Solid { mut color }) = base { + color.mods.retain(|m| !matches!(m, ColorTransform::Alpha(_))); + if t > 0.0 { + color.mods.push(ColorTransform::Alpha(((1.0 - t.clamp(0.0, 1.0)) * 100_000.0).round() as i32)); + } + sh.fill = Some(Fill::Solid { color }); + } + } + Ok(()) +} + +fn fill(s: &mut Session, p: &Value) -> Result { + let new_fill = requested_fill(s, p)?; let transparency = f64_param(p, "transparency"); let pic_props = p.get("pictureProps").filter(|v| v.is_object()); if new_fill.is_none() && transparency.is_none() && pic_props.is_none() { return Err(bad("shape.fill", "give `color`, `none`, `gradient`, `pattern`, `picture`, `pictureProps`, `transparency` or `reset`")); } // Transparency on an inherited (style) fill needs the effective colour. - let st = s.doc()?; - let ctx_fill = { - let ctx = super::text::ctx_for(&st.doc, &st.selection); - let mut fills = HashMap::new(); - for id in targets(s, p)? { - if let Some(sh) = st.shape(id) { - deckcraft_model::walk(std::slice::from_ref(sh), &mut |sh, _| { - let fill = ctx.as_ref().and_then(|c| { - let (f, ph) = deckcraft_model::resolve::fill(c, sh); - f.map(|f| match f { - Fill::Solid { color } => Fill::solid(ColorRef::rgb(c.color(&color, ph))), - other => other, - }) - }); - fills.insert(sh.id, fill); - }); - } - } - fills - }; - edit_group_appearance(s, p, "shape.fill", |sh| { - if let Some(f) = &new_fill { - sh.fill.clone_from(f); - } - if let (Some(pp), Some(Fill::Picture(pf))) = (pic_props, sh.fill.as_mut()) { - picture_props(pf, pp); - } - if let (Some(t), Some(Fill::Picture(pf))) = (transparency, sh.fill.as_mut()) { - let a = 1.0 - t.clamp(0.0, 1.0); - pf.alpha = (a < 1.0).then_some(a); - return Ok(()); - } - if let Some(t) = transparency { - let base = sh.fill.clone().or_else(|| ctx_fill.get(&sh.id).cloned().flatten()); - if let Some(Fill::Solid { mut color }) = base { - color.mods.retain(|m| !matches!(m, ColorTransform::Alpha(_))); - if t > 0.0 { - color.mods.push(ColorTransform::Alpha(((1.0 - t.clamp(0.0, 1.0)) * 100_000.0).round() as i32)); - } - sh.fill = Some(Fill::Solid { color }); - } - } - Ok(()) - }) + let inherited = inherited_fills(s, p)?; + edit_group_appearance(s, p, "shape.fill", |sh| apply_fill(sh, new_fill.as_ref(), pic_props, transparency, &inherited)) } /// Applies Format Shape ▸ Picture or texture fill options to a picture fill. diff --git a/crates/h264/src/dpb.rs b/crates/h264/src/dpb.rs index 708ea7a1..46e8f060 100644 --- a/crates/h264/src/dpb.rs +++ b/crates/h264/src/dpb.rs @@ -204,6 +204,48 @@ impl Dpb { entry_lists } + /// picNumLXNoWrap (8-34, 8-35) of a short-term modification from the predictor `pred`, and the + /// picNum it selects. + fn short_term_pic_nums(pred: i32, idc: u32, value: u32, curr_pic_num: i32, max_pic_num: i32) -> (i32, i32) { + let d = value as i32 + 1; + let no_wrap = if idc == 0 { + if pred - d < 0 { pred - d + max_pic_num } else { pred - d } + } else if pred + d >= max_pic_num { + pred + d - max_pic_num + } else { + pred + d + }; + let pn = if no_wrap > curr_pic_num { no_wrap - max_pic_num } else { no_wrap }; + (no_wrap, pn) + } + + /// Puts `target` at `ref_idx` and shifts the rest of the list up, then drops the later entries + /// that repeat it (8.2.4.3.1, 8.2.4.3.2) and keeps the list `n` long. + fn insert_ref<'a>(list: &mut Vec>, ref_idx: usize, n: usize, target: &'a DpbEntry, is_long: bool) { + for c in (ref_idx + 1..=n).rev() { + list[c] = list[c - 1]; + } + list[ref_idx] = Some(target); + let mut nidx = ref_idx + 1; + for c in ref_idx + 1..=n { + let keep = match list[c] { + None => true, + Some(e) => { + if is_long { + !(e.mark == RefMark::Long && e.long_term_frame_idx == target.long_term_frame_idx) + } else { + !(e.mark == RefMark::Short && std::ptr::eq(e, target)) + } + } + }; + if keep { + list[nidx] = list[c]; + nidx += 1; + } + } + list.truncate(n); + } + /// Reference picture list modification (8.2.4.3) of one list; `n` is num_ref_idx_active. fn modify_ref_list<'a>( list: &mut Vec>, @@ -223,16 +265,8 @@ impl Dpb { list.push(None); // temporarily one longer let (target, is_long): (Option<&DpbEntry>, bool) = match m.idc { 0 | 1 => { - let d = m.value as i32 + 1; - let no_wrap = if m.idc == 0 { - if pred - d < 0 { pred - d + max_pic_num } else { pred - d } - } else if pred + d >= max_pic_num { - pred + d - max_pic_num - } else { - pred + d - }; + let (no_wrap, pn) = Self::short_term_pic_nums(pred, m.idc, m.value, curr_pic_num, max_pic_num); pred = no_wrap; - let pn = if no_wrap > curr_pic_num { no_wrap - max_pic_num } else { no_wrap }; (short.iter().copied().find(|e| Self::frame_num_wrap(e, cur_fn, max_frame_num) == pn), false) } 2 => (long.iter().copied().find(|e| e.long_term_frame_idx == m.value), true), @@ -244,29 +278,8 @@ impl Dpb { ref_idx += 1; continue; }; - for c in (ref_idx + 1..=n).rev() { - list[c] = list[c - 1]; - } - list[ref_idx] = Some(target); + Self::insert_ref(list, ref_idx, n, target, is_long); ref_idx += 1; - let mut nidx = ref_idx; - for c in ref_idx..=n { - let keep = match list[c] { - None => true, - Some(e) => { - if is_long { - !(e.mark == RefMark::Long && e.long_term_frame_idx == target.long_term_frame_idx) - } else { - !(e.mark == RefMark::Short && std::ptr::eq(e, target)) - } - } - }; - if keep { - list[nidx] = list[c]; - nidx += 1; - } - } - list.truncate(n); } Ok(()) } diff --git a/crates/hevc/src/slicedec.rs b/crates/hevc/src/slicedec.rs index 5bf9f07b..6cc24cdc 100644 --- a/crates/hevc/src/slicedec.rs +++ b/crates/hevc/src/slicedec.rs @@ -277,6 +277,30 @@ pub struct SliceDecoder<'a> { pub ctb_y: i32, } +/// Scaling process (8.6.3): dequantises the top-left `max_x` × `max_y` coefficients of the `n` × `n` +/// block `coeffs` with scaling matrix `m` (flat when `None`). +fn dequantize(coeffs: &mut [i32], n: usize, (max_x, max_y): (usize, usize), m: Option<&[u8]>, scale: i64, round: i64, bd_shift: u32) { + for y in 0..=max_y { + for x in 0..=max_x { + let v = coeffs[y * n + x]; + if v != 0 { + let f = m.map_or(16, |m| m[y * n + x] as i64); + coeffs[y * n + x] = ((v as i64 * f * scale + round) >> bd_shift).clamp(-32768, 32767) as i32; + } + } + } +} + +/// Adds the `n` × `n` residual `coeffs` to the samples of `plane` at (x0, y0), clipped to 0..=`max`. +fn add_residual(plane: &mut [u16], stride: usize, (x0, y0): (usize, usize), n: usize, coeffs: &[i32], max: i32) { + for y in 0..n { + let row = &mut plane[(y0 + y) * stride + x0..][..n]; + for (x, s) in row.iter_mut().enumerate() { + *s = (*s as i32 + coeffs[y * n + x]).clamp(0, max) as u16; + } + } +} + impl<'a> SliceDecoder<'a> { #[inline(always)] fn dd(&mut self, ctx: usize) -> u32 { @@ -1331,15 +1355,7 @@ impl<'a> SliceDecoder<'a> { } _ => None, }; - for y in 0..=max_y { - for x in 0..=max_x { - let v = coeffs[y * n + x]; - if v != 0 { - let f = m.map_or(16, |m| m[y * n + x] as i64); - coeffs[y * n + x] = ((v as i64 * f * scale + round) >> bd_shift).clamp(-32768, 32767) as i32; - } - } - } + dequantize(coeffs, n, (max_x, max_y), m, scale, round, bd_shift); let bd_shift = 20 - bd; if ts { transform::transform_skip(coeffs, n, bd_shift); @@ -1355,12 +1371,7 @@ impl<'a> SliceDecoder<'a> { let _ = dc_only; let (stride, max) = (if c == 0 { self.pic.width } else { self.pic.cwidth }, (1i32 << bd) - 1); let plane = &mut self.pic.planes[c]; - for y in 0..n { - let row = &mut plane[(y0 as usize + y) * stride + x0 as usize..][..n]; - for (x, s) in row.iter_mut().enumerate() { - *s = (*s as i32 + coeffs[y * n + x]).clamp(0, max) as u16; - } - } + add_residual(plane, stride, (x0 as usize, y0 as usize), n, coeffs, max); Ok(()) } diff --git a/crates/model/src/chart.rs b/crates/model/src/chart.rs index 7e8311c4..7ad61a50 100644 --- a/crates/model/src/chart.rs +++ b/crates/model/src/chart.rs @@ -811,6 +811,7 @@ impl Chart { /// The (min, max, major unit) of an axis over the data range [lo, hi]: `fmin`/`fmax`/`major` /// where the chart fixes them; otherwise Office's automatic limits, rounded out to the smallest /// 1-2-5 unit giving at most `max_intervals` intervals: +/// The data range [lo, hi] an axis covers, clamped to ±1e100; (0, 1) without finite values. /// /// - **Maximum:** `hi + (hi − lo)/20` (with `headroom`), rounded up to the unit. /// - **Minimum:** zero when every value is ≥ 0 and `lo` is below 5/6 of `hi`; otherwise @@ -818,22 +819,14 @@ impl Chart { /// near them. /// - All-negative values mirror this: the axis ends at zero when `hi` is above 5/6 of `lo`. /// Mixed signs get headroom at both ends. -pub fn axis_scale( - lo: f64, - hi: f64, - fmin: Option, - fmax: Option, - major: Option, - max_intervals: usize, - headroom: bool, -) -> (f64, f64, f64) { - // Values past ±1e100 aren't axis values; clamping keeps the arithmetic finite. - let fin = |v: Option| v.filter(|v| v.is_finite()).map(|v| v.clamp(-1e100, 1e100)); - let (fmin, fmax) = (fin(fmin), fin(fmax)); - let (lo, hi) = if lo.is_finite() && hi.is_finite() { (lo.min(hi).clamp(-1e100, 1e100), lo.max(hi).clamp(-1e100, 1e100)) } else { (0.0, 1.0) }; - // Office's automatic limits ("How chart axis limits are determined", as Jon Peltier - // summarises it): 5 % of the data range past the data, and zero when the values don't - // sit within the top sixth of their largest magnitude. A single value is measured from zero. +fn axis_data_range(lo: f64, hi: f64) -> (f64, f64) { + if lo.is_finite() && hi.is_finite() { (lo.min(hi).clamp(-1e100, 1e100), lo.max(hi).clamp(-1e100, 1e100)) } else { (0.0, 1.0) } +} + +/// Office's automatic (min, max) of the data range [lo, hi]. "How chart axis limits are determined", +/// as Jon Peltier summarises it: 5 % of the data range past the data, and zero when the values don't +/// sit within the top sixth of their largest magnitude. A single value is measured from zero. +fn auto_limits(lo: f64, hi: f64, headroom: bool) -> (f64, f64) { let flat = hi - lo <= 1e-12 * lo.abs().max(hi.abs()); let pad = if !headroom { 0.0 @@ -844,21 +837,23 @@ pub fn axis_scale( }; let auto_lo = if lo >= 0.0 && (flat || lo < hi * 5.0 / 6.0) { 0.0 } else { lo - pad }; let auto_hi = if hi <= 0.0 && (flat || hi > lo * 5.0 / 6.0) { 0.0 } else { hi + pad }; - let mut lo = fmin.unwrap_or(auto_lo); - let mut hi = fmax.unwrap_or(auto_hi); - let mag = lo.abs().max(hi.abs()); - // Relative to the data's magnitude, so tiny values don't count as flat; all-zero falls back to 1. - let unit = if mag == 0.0 { 1.0 } else { mag }; - if hi - lo <= 1e-12 * mag { - // A fixed limit on the far side of the data: the automatic end gives way. - match (fmin, fmax) { - (None, Some(_)) => lo = if hi > 0.0 { 0.0 } else { hi - hi.abs().max(unit) }, - (Some(_), None) if lo < 0.0 => hi = 0.0, - _ => hi = lo + unit, - } + (auto_lo, auto_hi) +} + +/// The ends of a span too small to show: with one fixed limit on the far side of the data, the +/// automatic end gives way; otherwise the span is one `unit` wide. +fn flat_span_ends(fmin: Option, fmax: Option, lo: f64, hi: f64, unit: f64) -> (f64, f64) { + match (fmin, fmax) { + (None, Some(_)) => (if hi > 0.0 { 0.0 } else { hi - hi.abs().max(unit) }, hi), + (Some(_), None) if lo < 0.0 => (lo, 0.0), + _ => (lo, lo + unit), } - let n = max_intervals.clamp(1, 50) as f64; - let step = match fin(major).filter(|m| *m > 0.0 && (hi - lo) / m <= 1000.0) { +} + +/// The major step: `major` when it gives at most 1000 intervals, else the smallest 1-2-5 step +/// that fits `n` intervals over [lo, hi]. +fn axis_step(major: Option, lo: f64, hi: f64, n: f64, unit: f64) -> f64 { + match major.filter(|m| *m > 0.0 && (hi - lo) / m <= 1000.0) { Some(m) => m, None => { let mut step = 10f64.powf(((hi - lo) / n).log10().floor()); @@ -874,7 +869,32 @@ pub fn axis_scale( } step } - }; + } +} + +pub fn axis_scale( + lo: f64, + hi: f64, + fmin: Option, + fmax: Option, + major: Option, + max_intervals: usize, + headroom: bool, +) -> (f64, f64, f64) { + // Values past ±1e100 aren't axis values; clamping keeps the arithmetic finite. + let fin = |v: Option| v.filter(|v| v.is_finite()).map(|v| v.clamp(-1e100, 1e100)); + let (fmin, fmax) = (fin(fmin), fin(fmax)); + let (lo, hi) = axis_data_range(lo, hi); + let (auto_lo, auto_hi) = auto_limits(lo, hi, headroom); + let (mut lo, mut hi) = (fmin.unwrap_or(auto_lo), fmax.unwrap_or(auto_hi)); + let mag = lo.abs().max(hi.abs()); + // Relative to the data's magnitude, so tiny values don't count as flat; all-zero falls back to 1. + let unit = if mag == 0.0 { 1.0 } else { mag }; + if hi - lo <= 1e-12 * mag { + (lo, hi) = flat_span_ends(fmin, fmax, lo, hi, unit); + } + let n = max_intervals.clamp(1, 50) as f64; + let step = axis_step(fin(major), lo, hi, n, unit); let min = fmin.unwrap_or((lo / step + 1e-9).floor() * step); let max = fmax.unwrap_or((hi / step - 1e-9).ceil() * step).max(min + step); (min, max, step) diff --git a/crates/pptx/src/read/dml.rs b/crates/pptx/src/read/dml.rs index f664a79c..e96fcd22 100644 --- a/crates/pptx/src/read/dml.rs +++ b/crates/pptx/src/read/dml.rs @@ -576,6 +576,44 @@ pub fn text_body(imp: &mut Imp, part: &Part, tx: &El) -> TextBody { const MAX_PARAGRAPH_RUNS: usize = 10_000; +/// The run properties of `e`'s `rPr` child, or the defaults. +fn run_props(imp: &mut Imp, part: &Part, e: &El) -> RunProps { + e.child("rPr").map(|r| rpr(imp, part, r)).unwrap_or_default() +} + +/// Appends the runs of an `a:r` text, where a vertical tab or line feed is a line break, as it +/// lays out and as export writes it. +fn push_text_runs(para: &mut Paragraph, props: RunProps, text: &str) { + for (i, piece) in text.split(['\u{b}', '\n']).enumerate() { + if para.runs.len() >= MAX_PARAGRAPH_RUNS { + break; + } + if i > 0 { + para.runs.push(Run { text: String::new(), props: props.clone(), kind: RunKind::Break }); + } + if !piece.is_empty() { + if para.runs.len() >= MAX_PARAGRAPH_RUNS { + break; + } + para.runs.push(Run { text: piece.to_string(), props: props.clone(), kind: RunKind::Text }); + } + } +} + +/// Appends an Office Math (a14:m) run: its plain text and its OMML. +fn push_math_run(para: &mut Paragraph, part: &Part, e: &El) { + if para.runs.len() < MAX_PARAGRAPH_RUNS { + let mut text = String::new(); + let mut ts = vec![]; + e.find_all("t", &mut ts); + for t in ts { + text.push_str(&t.text()); + } + let omml = e.elements().map(|c| part.keep_inner(c)).collect::(); + para.runs.push(Run { text, props: RunProps::default(), kind: RunKind::Math { omml } }); + } +} + pub fn paragraph(imp: &mut Imp, part: &Part, p: &El) -> Paragraph { let mut para = Paragraph::default(); for e in p.elements() { @@ -589,50 +627,24 @@ pub fn paragraph(imp: &mut Imp, part: &Part, p: &El) -> Paragraph { } } "r" => { - let props = e.child("rPr").map(|r| rpr(imp, part, r)).unwrap_or_default(); + let props = run_props(imp, part, e); let text: String = e.child("t").map(|t| t.text()).unwrap_or_default(); - // A vertical tab or line feed inside text is a line break, as it lays out and as export writes it. - for (i, piece) in text.split(['\u{b}', '\n']).enumerate() { - if para.runs.len() >= MAX_PARAGRAPH_RUNS { - break; - } - if i > 0 { - para.runs.push(Run { text: String::new(), props: props.clone(), kind: RunKind::Break }); - } - if !piece.is_empty() { - if para.runs.len() >= MAX_PARAGRAPH_RUNS { - break; - } - para.runs.push(Run { text: piece.to_string(), props: props.clone(), kind: RunKind::Text }); - } - } + push_text_runs(&mut para, props, &text); } "br" => { if para.runs.len() < MAX_PARAGRAPH_RUNS { - let props = e.child("rPr").map(|r| rpr(imp, part, r)).unwrap_or_default(); + let props = run_props(imp, part, e); para.runs.push(Run { text: String::new(), props, kind: RunKind::Break }); } } "fld" => { if para.runs.len() < MAX_PARAGRAPH_RUNS { - let props = e.child("rPr").map(|r| rpr(imp, part, r)).unwrap_or_default(); + let props = run_props(imp, part, e); let text = e.child("t").map(|t| t.text()).unwrap_or_default(); para.runs.push(Run { text, props, kind: RunKind::Field { field: e.attr("type").unwrap_or("").to_string() } }); } } - "m" => { - if para.runs.len() < MAX_PARAGRAPH_RUNS { - // Office Math (a14:m) — keep the OMML and its plain text. - let mut text = String::new(); - let mut ts = vec![]; - e.find_all("t", &mut ts); - for t in ts { - text.push_str(&t.text()); - } - let omml = e.elements().map(|c| part.keep_inner(c)).collect::(); - para.runs.push(Run { text, props: RunProps::default(), kind: RunKind::Math { omml } }); - } - } + "m" => push_math_run(&mut para, part, e), "endParaRPr" => para.end_props = rpr(imp, part, e), _ => {} }