Wrap serialized gradient stops in a new GradientRamp struct and unify FillChoice (#4400)

* Introduce the GradientRamp exchange struct as the serialized TaggedValue::Gradient payload

* Unify FillChoice and FillChoiceUI into one enum generic over color format, carrying GradientRamp stops

* Rename the TaggedValue::Gradient variant to GradientRamp to match its payload

* Move the Color variant into the tagged_value macro list since its stored and wire forms match
This commit is contained in:
Keavon Chambers
2026-08-03 23:23:41 -07:00
committed by GitHub
parent 808662e3d2
commit 79136c95be
32 changed files with 414 additions and 299 deletions

View File

@@ -15,7 +15,7 @@ use graphene_application_io::resource::ResourceId;
use graphic_types::raster_types::{CPU, Image, Raster};
use graphic_types::vector_types::vector::misc::BoxCorners;
use graphic_types::vector_types::vector::style::DashPattern;
use graphic_types::vector_types::vector::style::Gradient;
use graphic_types::vector_types::vector::style::{Gradient, GradientRamp};
use graphic_types::vector_types::vector::{self, ReferencePoint};
use graphic_types::{Artboard, Graphic, Vector};
use rendering::RenderMetadata;
@@ -89,15 +89,10 @@ macro_rules! tagged_value {
DashPattern(Vec<f64>),
/// Stored compactly as a `Vec<f64>` of corner values, materializes as an `Item<BoxCorners>` at runtime via `to_dynany`/`to_any`.
BoxCorners(Vec<f64>),
/// A plain, always-present color. Aliases recover legacy on-disk shapes; a legacy `null` payload (the old "no color")
/// is routed to [`TaggedValue::no_paint`] by `deserialize_tagged_value_with_legacy_migration`.
#[serde(deserialize_with = "core_types::misc::migrate_to_color")] // TODO: Eventually remove this document upgrade code
#[serde(alias = "ColorTable", alias = "OptionalColor", alias = "ColorNotInTable")]
Color(Color),
/// Stored as the `{ color, position?, midpoint? }` stops struct, materializing as an `Item<Gradient>` at runtime. Aliases recover legacy on-disk shapes.
/// (Old documents that stored a full `Gradient` struct under this same `"Gradient"` tag are routed to `LegacyGradient` by `deserialize_tagged_value_with_legacy_migration`.)
#[serde(alias = "GradientTable", alias = "GradientPositions", alias = "GradientStops")]
Gradient(Gradient),
/// Stored as the `GradientRamp` exchange struct (nested `{ stops: { color, position?, midpoint? } }`), materializing as an `Item<Gradient>` at runtime. Aliases recover legacy on-disk shapes.
/// (Old documents stored flat stops, a tuple list, or the ancient full `Gradient` struct under the legacy `"Gradient"` tag, all routed by `deserialize_tagged_value_with_legacy_migration`.)
#[serde(alias = "Gradient", alias = "GradientTable", alias = "GradientPositions", alias = "GradientStops")]
GradientRamp(GradientRamp),
/// Stored compactly as a `Vec<BrushStroke>`, materializes as the single-value `Item<BrushTrace>` at runtime via `to_dynany`/`to_any`. Aliases recover legacy on-disk shapes.
#[serde(deserialize_with = "brush_nodes::migrations::migrate_to_brush_strokes")] // TODO: Eventually remove this document upgrade code
#[serde(alias = "BrushStrokeTable")]
@@ -143,8 +138,7 @@ macro_rules! tagged_value {
Self::F64Array(values) => values.cache_hash(state),
Self::DashPattern(lengths) => lengths.cache_hash(state),
Self::BoxCorners(values) => values.cache_hash(state),
Self::Color(color) => color.cache_hash(state),
Self::Gradient(stops) => stops.cache_hash(state),
Self::GradientRamp(ramp) => ramp.cache_hash(state),
Self::BrushStrokes(strokes) => strokes.cache_hash(state),
// =======================
// NON-SERIALIZED VARIANTS
@@ -207,8 +201,7 @@ macro_rules! tagged_value {
}
Self::DashPattern(lengths) => Box::new(Item::new_from_element(DashPattern::from(lengths))),
Self::BoxCorners(values) => Box::new(Item::new_from_element(BoxCorners::from(values))),
Self::Color(color) => Box::new(Item::new_from_element(color)),
Self::Gradient(stops) => Box::new(Item::new_from_element(stops)),
Self::GradientRamp(ramp) => Box::new(Item::new_from_element(Gradient::from(ramp))),
Self::BrushStrokes(strokes) => Box::new(core_types::list::Item::new_from_element(BrushTrace::from(strokes))),
// =======================
// AUTO-GENERATED VARIANTS
@@ -271,8 +264,7 @@ macro_rules! tagged_value {
}
Self::DashPattern(lengths) => Arc::new(Item::new_from_element(DashPattern::from(lengths))),
Self::BoxCorners(values) => Arc::new(Item::new_from_element(BoxCorners::from(values))),
Self::Color(color) => Arc::new(Item::new_from_element(color)),
Self::Gradient(stops) => Arc::new(Item::new_from_element(stops)),
Self::GradientRamp(ramp) => Arc::new(Item::new_from_element(Gradient::from(ramp))),
Self::BrushStrokes(strokes) => Arc::new(core_types::list::Item::new_from_element(BrushTrace::from(strokes))),
// =======================
// AUTO-GENERATED VARIANTS
@@ -301,8 +293,7 @@ macro_rules! tagged_value {
Self::F64Array(_) => list!(f64),
Self::DashPattern(_) => item!(DashPattern),
Self::BoxCorners(_) => item!(BoxCorners),
Self::Color(_) => item!(Color),
Self::Gradient(_) => item!(Gradient),
Self::GradientRamp(_) => item!(Gradient),
Self::BrushStrokes(_) => item!(BrushTrace),
// =======================
// AUTO-GENERATED VARIANTS
@@ -340,10 +331,8 @@ macro_rules! tagged_value {
x if x == TypeId::of::<Item<DashPattern>>() => Ok(TaggedValue::DashPattern(downcast::<Item<DashPattern>>(input).unwrap().into_element().0.iter_element_values().copied().collect())),
x if x == TypeId::of::<BoxCorners>() => Ok(TaggedValue::BoxCorners(downcast::<BoxCorners>(input).unwrap().0.iter_element_values().copied().collect())),
x if x == TypeId::of::<Item<BoxCorners>>() => Ok(TaggedValue::BoxCorners(downcast::<Item<BoxCorners>>(input).unwrap().into_element().0.iter_element_values().copied().collect())),
x if x == TypeId::of::<Color>() => Ok(TaggedValue::Color(*downcast(input).unwrap())),
x if x == TypeId::of::<Item<Color>>() => Ok(TaggedValue::Color(downcast::<Item<Color>>(input).unwrap().into_element())),
x if x == TypeId::of::<Gradient>() => Ok(TaggedValue::Gradient(*downcast(input).unwrap())),
x if x == TypeId::of::<Item<Gradient>>() => Ok(TaggedValue::Gradient(downcast::<Item<Gradient>>(input).unwrap().into_element())),
x if x == TypeId::of::<Gradient>() => Ok(TaggedValue::GradientRamp(GradientRamp::from(*downcast::<Gradient>(input).unwrap()))),
x if x == TypeId::of::<Item<Gradient>>() => Ok(TaggedValue::GradientRamp(GradientRamp::from(downcast::<Item<Gradient>>(input).unwrap().into_element()))),
x if x == TypeId::of::<Vec<BrushStroke>>() => Ok(TaggedValue::BrushStrokes(*downcast(input).unwrap())),
x if x == TypeId::of::<Item<BrushTrace>>() => Ok(TaggedValue::BrushStrokes(downcast::<Item<BrushTrace>>(input).unwrap().into_element().0.iter_element_values().cloned().collect())),
// =======================
@@ -376,10 +365,8 @@ macro_rules! tagged_value {
x if x == TypeId::of::<Item<DashPattern>>() => Ok(TaggedValue::DashPattern(input.downcast_ref::<Item<DashPattern>>().unwrap().element().0.iter_element_values().copied().collect())),
x if x == TypeId::of::<BoxCorners>() => Ok(TaggedValue::BoxCorners(input.downcast_ref::<BoxCorners>().unwrap().0.iter_element_values().copied().collect())),
x if x == TypeId::of::<Item<BoxCorners>>() => Ok(TaggedValue::BoxCorners(input.downcast_ref::<Item<BoxCorners>>().unwrap().element().0.iter_element_values().copied().collect())),
x if x == TypeId::of::<Color>() => Ok(TaggedValue::Color(*input.downcast_ref::<Color>().unwrap())),
x if x == TypeId::of::<Item<Color>>() => Ok(TaggedValue::Color(*input.downcast_ref::<Item<Color>>().unwrap().element())),
x if x == TypeId::of::<Gradient>() => Ok(TaggedValue::Gradient(input.downcast_ref::<Gradient>().unwrap().clone())),
x if x == TypeId::of::<Item<Gradient>>() => Ok(TaggedValue::Gradient(input.downcast_ref::<Item<Gradient>>().unwrap().element().clone())),
x if x == TypeId::of::<Gradient>() => Ok(TaggedValue::GradientRamp(GradientRamp::from(input.downcast_ref::<Gradient>().unwrap()))),
x if x == TypeId::of::<Item<Gradient>>() => Ok(TaggedValue::GradientRamp(GradientRamp::from(input.downcast_ref::<Item<Gradient>>().unwrap().element()))),
x if x == TypeId::of::<Vec<BrushStroke>>() => Ok(TaggedValue::BrushStrokes(input.downcast_ref::<Vec<BrushStroke>>().unwrap().clone())),
x if x == TypeId::of::<Item<BrushTrace>>() => Ok(TaggedValue::BrushStrokes(input.downcast_ref::<Item<BrushTrace>>().unwrap().element().0.iter_element_values().cloned().collect())),
// =======================
@@ -406,8 +393,7 @@ macro_rules! tagged_value {
// TODO: Add default implementations for types such as TaggedValue::Subpaths, and use the defaults here and in document_node_types
// Tries using the default for the tagged value type. If it not implemented, then uses the default used in document_node_types. If it is not used there, then TaggedValue::None is returned.
if name == std::any::type_name::<()>() { return Some(TaggedValue::None) }
if name == std::any::type_name::<Color>() { return Some(TaggedValue::Color(Color::default())) }
if name == std::any::type_name::<Gradient>() { return Some(TaggedValue::Gradient(Gradient::default())) }
if name == std::any::type_name::<Gradient>() { return Some(TaggedValue::GradientRamp(GradientRamp::default())) }
if name == std::any::type_name::<DashPattern>() { return Some(TaggedValue::DashPattern(Vec::new())) }
if name == std::any::type_name::<BoxCorners>() { return Some(TaggedValue::BoxCorners(Vec::new())) }
$( if name == std::any::type_name::<$ty>() { return Some(TaggedValue::$identifier(Default::default())) } )*
@@ -463,8 +449,7 @@ macro_rules! tagged_value {
Self::F64Array(values) => format!("F64Array({values:?})"),
Self::DashPattern(lengths) => format!("DashPattern({lengths:?})"),
Self::BoxCorners(values) => format!("BoxCorners({values:?})"),
Self::Color(color) => format!("Color({color:?})"),
Self::Gradient(stops) => format!("Gradient({stops:?})"),
Self::GradientRamp(ramp) => format!("GradientRamp({ramp:?})"),
Self::BrushStrokes(strokes) => format!("BrushStrokes({strokes:?})"),
// =======================
// AUTO-GENERATED VARIANTS
@@ -519,6 +504,11 @@ tagged_value! {
DVec2(DVec2),
#[serde(alias = "Affine2")]
DAffine2(DAffine2),
/// A plain, always-present color. Aliases recover legacy on-disk shapes; a legacy `null` payload (the old "no color")
/// is routed to [`TaggedValue::no_paint`] by `deserialize_tagged_value_with_legacy_migration`.
#[serde(deserialize_with = "core_types::misc::migrate_to_color")] // TODO: Eventually remove this document upgrade code
#[serde(alias = "ColorTable", alias = "OptionalColor", alias = "ColorNotInTable")]
Color(Color),
Font(Font),
Footprint(Footprint),
VectorModification(Box<VectorModification>),
@@ -690,7 +680,7 @@ impl TaggedValue {
() if ty == TypeId::of::<Color>() => to_color(string).map(TaggedValue::Color)?,
// The Fill/Stroke paint wires carry `Graphic` or `Gradient` elements, so a paint default parses through the element recursion as a color or gradient literal
() if ty == TypeId::of::<Graphic>() => to_color(string).map(TaggedValue::Color)?,
() if ty == TypeId::of::<Gradient>() => to_gradient(string).map(TaggedValue::Gradient)?,
() if ty == TypeId::of::<Gradient>() => to_gradient(string).map(|gradient| TaggedValue::GradientRamp(gradient.into()))?,
() if ty == TypeId::of::<ReferencePoint>() => to_reference_point(string).map(TaggedValue::ReferencePoint)?,
() if ty == TypeId::of::<DashPattern>() => TaggedValue::DashPattern(core_types::misc::parse_f64_list(string)),
() if ty == TypeId::of::<BoxCorners>() => TaggedValue::BoxCorners(core_types::misc::parse_f64_list(string)),
@@ -736,8 +726,8 @@ impl TaggedValue {
/// - `Vector` (or alias `VectorData`):
/// - non-empty → `TaggedValue::VectorModification(<built from first element>)` (the document_migration's Path pass disambiguates this between SVG-import legacy and a discardable modern baked value via the input's `exposed` flag)
/// - empty → `TaggedValue::TypeDefault(list!(Vector))`
/// - `FillChoice` → `TaggedValue::Color` (solid), `TaggedValue::Gradient` (gradient), or `TaggedValue::no_paint()` (none)
/// - `Gradient` (or alias `GradientTable`/`GradientPositions`/`GradientStops`) → `TaggedValue::LegacyGradient` (ancient full struct) or `TaggedValue::Gradient` (stops shapes, unwrapped from the legacy table form)
/// - `FillChoice` → `TaggedValue::Color` (solid), `TaggedValue::GradientRamp` (gradient), or `TaggedValue::no_paint()` (none)
/// - `Gradient` (or alias `GradientTable`/`GradientPositions`/`GradientStops`) → `TaggedValue::LegacyGradient` (ancient full struct) or `TaggedValue::GradientRamp` (ramp and legacy stops shapes, unwrapped from the legacy table form)
/// - `TypeDefault` with the old bare-`TypeDescriptor` payload → the same variant wrapping a `Type` (name-encoded `List` normalized to structural)
///
/// All other tags (including ones with the modern shape) fall through to the standard derived `Deserialize` for `TaggedValue`.
@@ -799,14 +789,14 @@ pub fn deserialize_tagged_value_with_legacy_migration<'de, D: serde::Deserialize
return Ok(MemoHash::new(TaggedValue::Color(color)));
}
if let Some(gradient) = payload.get("Gradient") {
let gradient: Gradient = serde_json::from_value(gradient.clone()).map_err(serde::de::Error::custom)?;
return Ok(MemoHash::new(TaggedValue::Gradient(gradient)));
let ramp = graphic_types::migrations::migrate_to_gradient_ramp(gradient.clone()).map_err(serde::de::Error::custom)?;
return Ok(MemoHash::new(TaggedValue::GradientRamp(ramp)));
}
}
return Ok(MemoHash::new(TaggedValue::no_paint()));
}
// The gradient tags carried several shapes over time, disambiguated here: the ancient full struct (`start`/`end` keys) becomes `LegacyGradient`,
// while the current stops struct, the old tuple list, and the legacy one-element table wrapper all parse as the stops value directly
// while the current ramp, the flat stops struct, the old tuple list, and the legacy one-element table wrapper all parse as the ramp value directly
"Gradient" | "GradientTable" | "GradientPositions" | "GradientStops" => {
let table_element = content
.as_object()
@@ -817,7 +807,7 @@ pub fn deserialize_tagged_value_with_legacy_migration<'de, D: serde::Deserialize
if let Some(array) = table_element
&& array.is_empty()
{
return Ok(MemoHash::new(TaggedValue::Gradient(Gradient::default())));
return Ok(MemoHash::new(TaggedValue::GradientRamp(GradientRamp::default())));
}
let payload = table_element.and_then(|array| array.first()).unwrap_or(content);
@@ -827,8 +817,8 @@ pub fn deserialize_tagged_value_with_legacy_migration<'de, D: serde::Deserialize
return Ok(MemoHash::new(TaggedValue::LegacyGradient(gradient)));
}
let gradient: Gradient = serde_json::from_value(payload.clone()).map_err(serde::de::Error::custom)?;
return Ok(MemoHash::new(TaggedValue::Gradient(gradient)));
let ramp = graphic_types::migrations::migrate_to_gradient_ramp(payload.clone()).map_err(serde::de::Error::custom)?;
return Ok(MemoHash::new(TaggedValue::GradientRamp(ramp)));
}
_ => {}
}
@@ -1028,3 +1018,74 @@ mod paint_default_parsing {
}
}
}
#[cfg(test)]
mod gradient_shape_migration {
use super::*;
fn load(payload: serde_json::Value) -> TaggedValue {
deserialize_tagged_value_with_legacy_migration(payload)
.expect("The gradient payload should deserialize")
.into_inner()
.as_ref()
.clone()
}
fn white() -> serde_json::Value {
serde_json::to_value(Color::WHITE).unwrap()
}
#[test]
fn modern_ramp_payload_round_trips() {
let mut gradient = Gradient::from(vec![Color::BLACK, Color::WHITE]);
gradient.set_positions(&[0.2, 0.9]);
let value = TaggedValue::GradientRamp(GradientRamp::from(gradient));
let json = serde_json::to_value(&value).unwrap();
assert!(json.get("GradientRamp").and_then(|payload| payload.get("stops")).is_some(), "the payload should nest its stops: {json}");
assert_eq!(load(json), value);
}
// TODO: Eventually remove this document upgrade code
#[test]
fn legacy_flat_stops_parse_faithfully() {
let json = serde_json::json!({ "Gradient": { "color": [white(), white()], "position": [0., 0.25], "midpoint": [0.5, 0.5] } });
let TaggedValue::GradientRamp(ramp) = load(json) else {
panic!("the flat stops should become a gradient ramp value")
};
let gradient = Gradient::from(ramp);
assert_eq!(gradient.positions(), vec![0., 0.25]);
assert!(gradient.has_midpoint_attribute(), "the flat form must parse faithfully");
}
// TODO: Eventually remove this document upgrade code
#[test]
fn legacy_tuple_stops_parse_with_defaults_elided() {
let json = serde_json::json!({ "Gradient": [[0., white()], [1., white()]] });
let TaggedValue::GradientRamp(ramp) = load(json) else {
panic!("the tuple stops should become a gradient ramp value")
};
let gradient = Gradient::from(ramp);
assert_eq!(gradient.positions(), vec![0., 1.]);
assert!(!gradient.has_position_attribute(), "even legacy tuple positions should elide");
}
// TODO: Eventually remove this document upgrade code
#[test]
fn empty_legacy_gradient_table_degrades_to_the_default() {
let json = serde_json::json!({ "GradientTable": { "element": [] } });
assert_eq!(load(json), TaggedValue::GradientRamp(GradientRamp::default()));
}
// TODO: Eventually remove this document upgrade code
#[test]
fn ancient_full_struct_routes_to_legacy_gradient() {
let json = serde_json::json!({ "Gradient": { "stops": [[0., white()], [1., white()]], "gradient_type": "Linear", "start": [0., 0.], "end": [1., 0.] } });
let TaggedValue::LegacyGradient(legacy) = load(json) else {
panic!("the ancient full struct should become a legacy gradient value")
};
assert_eq!(Gradient::from(legacy.stops).positions(), vec![0., 1.], "the nested tuple stops should parse through the field adapter");
}
}

View File

@@ -1059,7 +1059,7 @@ mod test {
// If this assert fails: These NodeIds seem to be changing when you modify TaggedValue, just update them.
assert_eq!(
ids,
vec![NodeId(12331852515109999872), NodeId(5084548161767585362), NodeId(14635346976242256925), NodeId(16015195863711239715)]
vec![NodeId(8464972237805743576), NodeId(3528778906331798968), NodeId(1126597937993520391), NodeId(17582929706900579130)]
);
}