From 8d46e1df7f57541b45612b998a54e46e7da19ba3 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 10 Nov 2026 17:59:59 -0500 Subject: [PATCH] Gradients: bound the stops sampled per pixel (#2950) * Gradients: bound the stops sampled per pixel Gradient sampling scans the stops linearly and shape painting samples once per pixel, so a gradient with a huge stop list multiplied every frame's work. Sampling now looks at the first 256 colour and opacity stops, and the plain JSON form (coerce_json) refuses gradients with more than 256. * address review: bound tagged gradients to the stop limit --- crates/keyframe/src/tests.rs | 21 +++++++++++++++++++++ crates/keyframe/src/value.rs | 16 ++++++++++++---- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/crates/keyframe/src/tests.rs b/crates/keyframe/src/tests.rs index 88f9f3249..68823b76b 100644 --- a/crates/keyframe/src/tests.rs +++ b/crates/keyframe/src/tests.rs @@ -165,6 +165,27 @@ fn gradient_tracks_sample_separately() { assert_eq!((empty.color_at(0.3), empty.opacity_at(0.3)), ([1.0; 4], 1.0)); } +/// A gradient's stop count multiplies every painted pixel's sampling: plain JSON with more than +/// `MAX_GRADIENT_STOPS` is refused, and a longer stored list is sampled through its first ones. +#[test] +fn gradient_stop_count_is_bounded() { + use crate::value::MAX_GRADIENT_STOPS; + let g = Value::Gradient(Gradient::default()); + let many: Vec = (0..=MAX_GRADIENT_STOPS).map(|_| serde_json::json!("#ff0000")).collect(); + assert_eq!(g.coerce_json(&serde_json::Value::Array(many.clone())), None); + assert!(g.coerce_json(&serde_json::Value::Array(many[..MAX_GRADIENT_STOPS].to_vec())).is_some()); + let ops: Vec = (0..=MAX_GRADIENT_STOPS).map(|i| serde_json::json!([i as f64 / MAX_GRADIENT_STOPS as f64, 1.0])).collect(); + assert_eq!(g.coerce_json(&serde_json::json!({"colors": ["#000000", "#ffffff"], "opacities": ops})), None); + // The tagged (typed) form is bounded the same way. + let tagged = Value::Gradient(Gradient { colors: (0..=MAX_GRADIENT_STOPS).map(|i| (i as f64, [0.0; 4])).collect(), opacities: vec![(0.0, 1.0)] }); + assert_eq!(g.coerce_json(&tagged.to_json()), None); + // A stored gradient with a huge tail: stops past the limit don't take part in sampling. + let mut colors: Vec<(f64, [f32; 4])> = (0..MAX_GRADIENT_STOPS).map(|i| (i as f64 / MAX_GRADIENT_STOPS as f64 * 0.5, [0.0, 0.0, 0.0, 1.0])).collect(); + colors.extend((0..100_000).map(|_| (1.0, [1.0, 1.0, 1.0, 1.0]))); + let big = Gradient { colors, opacities: vec![(0.0, 1.0)] }; + assert_eq!(big.sample(1.0), [0.0, 0.0, 0.0, 1.0]); +} + /// #612: the editor saves the typed form, so one colour stop (which the plain JSON parser /// rejects) and each stop's RGBA come back exactly. #[test] diff --git a/crates/keyframe/src/value.rs b/crates/keyframe/src/value.rs index 6609aed68..6e87f8933 100644 --- a/crates/keyframe/src/value.rs +++ b/crates/keyframe/src/value.rs @@ -210,7 +210,13 @@ impl Gradient { } } +/// Most stops (colour or opacity) a gradient is sampled through. Gradients are sampled per +/// pixel, so the stop count multiplies every painted pixel's work: stops past this are ignored +/// when sampling, and a gradient written as JSON with more is refused. +pub const MAX_GRADIENT_STOPS: usize = 256; + fn sample_stops(stops: &[(f64, T)], t: f64, lerp: impl Fn(T, T, f64) -> T) -> Option { + let stops = &stops[..stops.len().min(MAX_GRADIENT_STOPS)]; let first = stops.first()?; if t <= first.0 { return Some(first.1); @@ -243,7 +249,7 @@ fn gradient_json(j: &serde_json::Value) -> Option { } fn stops(a: &[J]) -> Option> { let n = a.len(); - if n < 2 { + if !(2..=MAX_GRADIENT_STOPS).contains(&n) { return None; } a.iter() @@ -259,6 +265,7 @@ fn gradient_json(j: &serde_json::Value) -> Option { J::Object(o) => { let colors = stops(o.get("colors")?.as_array()?)?; let op = match o.get("opacities") { + Some(J::Array(a)) if a.len() > MAX_GRADIENT_STOPS => return None, Some(J::Array(a)) => Some( a.iter() .map(|s| { @@ -469,9 +476,10 @@ impl Value { } Value::Layer(_) => Some(Value::Layer(j.as_u64())), Value::Path(_) => serde_json::from_value::(j.clone()).ok().map(Value::Path).or_else(|| serde_json::from_value::(j.clone()).ok()), - Value::Gradient(_) => { - serde_json::from_value::(j.clone()).ok().filter(|v| matches!(v, Value::Gradient(_))).or_else(|| gradient_json(j).map(Value::Gradient)) - } + Value::Gradient(_) => serde_json::from_value::(j.clone()) + .ok() + .filter(|v| matches!(v, Value::Gradient(g) if g.colors.len() <= MAX_GRADIENT_STOPS && g.opacities.len() <= MAX_GRADIENT_STOPS)) + .or_else(|| gradient_json(j).map(Value::Gradient)), } } /// JSON for display/automation (numbers/arrays for numeric kinds).