Merge branch 'main' into fix/sec-numbervalue-percent-linear

Each trailing % copied the rest of the text again, so a long run of
them made one call quadratic. The signs are now counted and cut off in
place.

crates/chart/src/render/kinds.rs

@@ -203,11 +203,12 @@ pub(super) fn pie(ctx: &mut Ctx, chart: &Chart, data: &ChartData, area: Bx, inne
 
 // ---------------------------------------------------------------- treemap
 
-/// Worst aspect ratio of a row with the given sum, smallest and largest area.
-fn worst(s: f64, mn: f64, mx: f64, side: f64) -> f64 {
+fn worst(row: &[f64], side: f64) -> f64 {
+    let s: f64 = row.iter().sum();
     if s <= 0.0 || side <= 0.0 {
         return f64::INFINITY;
     }
+    let (mn, mx) = row.iter().fold((f64::INFINITY, 0.0f64), |(a, b), v| (a.min(*v), b.max(*v)));
     let s2 = s * s;
     let w2 = side * side;
     (w2 * mx / s2).max(s2 / (w2 * mn))
@@ -224,8 +225,6 @@ fn squarify(items: &[(usize, f64)], r: Bx) -> Vec<(usize, Bx)> {
     let mut out = Vec::with_capacity(items.len());
     let mut rem = r;
     let mut row: Vec<(usize, f64)> = Vec::new();
-    // The row's sum, smallest and largest area, kept as items join so each step is O(1).
-    let (mut sum, mut mn, mut mx) = (0.0f64, f64::INFINITY, 0.0f64);
     let mut i = 0;
     let layout = |row: &[(usize, f64)], rem: &mut Bx, out: &mut Vec<(usize, Bx)>| {
         let s: f64 = row.iter().map(|x| x.1).sum();
@@ -257,14 +256,15 @@ fn squarify(items: &[(usize, f64)], r: Bx) -> Vec<(usize, Bx)> {
     while let Some(&(j, v)) = items.get(i) {
         let a = v * scale;
         let side = rem.w.min(rem.h) as f64;
-        if row.is_empty() || worst(sum + a, mn.min(a), mx.max(a), side) <= worst(sum, mn, mx, side) {
+        let cur: Vec<f64> = row.iter().map(|x| x.1).collect();
+        let mut with = cur.clone();
+        with.push(a);
+        if row.is_empty() || worst(&with, side) <= worst(&cur, side) {
             row.push((j, a));
-            (sum, mn, mx) = (sum + a, mn.min(a), mx.max(a));
             i += 1;
         } else {
             layout(&row, &mut rem, &mut out);
             row.clear();
-            (sum, mn, mx) = (0.0, f64::INFINITY, 0.0);
         }
     }
     if !row.is_empty() {
@@ -279,7 +279,6 @@ pub(super) fn treemap(ctx: &mut Ctx, chart: &Chart, data: &ChartData, area: Bx)
         s.values.iter().take(MAX_MARKERS).enumerate().filter_map(|(j, v)| v.filter(|v| *v > 0.0).map(|v| (j, v))).collect();
     items.sort_by(|a, b| b.1.total_cmp(&a.1));
     let f = Fmt::new(&s.number_format);
-    let value_of: std::collections::HashMap<usize, f64> = items.iter().copied().collect();
     for (j, b) in squarify(&items, area) {
         let fill = point_color(data, j);
         ctx.rect(b.x, b.y, b.w, b.h, fill, Some((WHITE, 1.5)));
@@ -287,7 +286,7 @@ pub(super) fn treemap(ctx: &mut Ctx, chart: &Chart, data: &ChartData, area: Bx)
             let name = ctx.fit(&cat_label(data, j), LABEL_SIZE, b.w - 8.0);
             ctx.text(b.x + 4.0, b.y + 4.0, &name, LABEL_SIZE, on(fill), (HAlign::Left, VAlign::Top));
             if chart.data_labels && b.h > LABEL_SIZE * 3.2 {
-                let v = value_of.get(&j).copied().unwrap_or(0.0);
+                let v = items.iter().find(|x| x.0 == j).map(|x| x.1).unwrap_or(0.0);
                 let t = ctx.fit(&f.format(v, None), LABEL_SIZE, b.w - 8.0);
                 ctx.text(b.x + 4.0, b.y + 4.0 + LABEL_SIZE * 1.4, &t, LABEL_SIZE, on(fill), (HAlign::Left, VAlign::Top));
             }

crates/functions/src/text.rs

@@ -339,11 +339,11 @@ fn numbervalue(a: &[Arg], c: &mut dyn Ctx) -> R<Value> {
     if body.is_empty() {
         return num_val(0.0);
     }
-    let mut pct = 0;
-    while let Some(b) = body.strip_suffix('%') {
-        body = b.to_string();
-        pct += 1;
-    }
+    // Count the trailing percent signs and cut them off in place: copying the rest once per sign
+    // made a long run of them quadratic.
+    let kept = body.trim_end_matches('%').len();
+    let pct = body.len() - kept;
+    body.truncate(kept);
     let mut seen_dec = false;
     let mut out = String::new();
     for c in body.chars() {
@@ -1084,6 +1084,15 @@ mod tests {
 
     use crate::util::testutil::*;
 
+    /// Trailing percent signs each divide by 100, and a long run of them is cut off in one pass.
+    #[test]
+    fn numbervalue_percent_signs() {
+        close(ev("NUMBERVALUE", vec![t("5%%")]), 0.0005);
+        close(ev("NUMBERVALUE", vec![t("1,5%"), t(","), t(".")]), 0.015);
+        let many = format!("1{}", "%".repeat(32_000));
+        close(ev("NUMBERVALUE", vec![t(&many)]), 0.0);
+    }
+
     #[test]
     fn bahttext() {
         let bt = |x: f64| ev("BAHTTEXT", vec![n(x)]);

crates/numfmt/src/decimal.rs

@@ -136,9 +136,6 @@ pub(crate) fn fixed(a: f64, decimals: usize) -> (String, String) {
 /// General format squeezed into `max_chars` characters. Excel shows fewer decimals, then
 /// scientific notation, then "####" when a column is narrow; `None` means the caller paints "####".
 pub fn format_general_fit(n: f64, max_chars: usize) -> Option<String> {
-    // The widest fixed rendering of a double is about 330 characters; a larger request only
-    // pads zero decimals the trim below throws away.
-    let max_chars = max_chars.min(1024);
     if !n.is_finite() {
         return None;
     }
@@ -217,8 +214,6 @@ mod tests {
     fn general_fit() {
         assert_eq!(format_general_fit(1e11, 11).as_deref(), Some("1E+11"));
         assert_eq!(format_general_fit(12345678901.0, 11).as_deref(), Some("12345678901"));
-        // A huge width neither allocates width-sized padding nor changes the text.
-        assert_eq!(format_general_fit(1.5, usize::MAX).as_deref(), Some("1.5"));
         assert_eq!(format_general_fit(123456789012.0, 11).as_deref(), Some("1.23457E+11"));
         assert_eq!(format_general_fit(0.1 + 0.2, 11).as_deref(), Some("0.3"));
         assert_eq!(format_general_fit(1.0 / 3.0, 11).as_deref(), Some("0.333333333"));