From 5e305015602699680d0387a331089f69898648bf Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Wed, 9 Sep 2026 20:52:02 +0000 Subject: [PATCH] Cascade the appearance through the paint reach and both flatten paths The appearance cascade rides PaintReach beside the legacy paint push, with the opposite arbitration: a lane's own declared appearance wins wholesale and an undeclared lane inherits the nearest ancestor's, at any depth, crossing the group boundaries the paint push resets at. A fresh render entry starts bare, which is what clears the cascade for a pattern's own content. Both flatten paths push a parent's declared appearance into undeclared children without composing, pinned against each other by a shared test. The appearance and paint markers move to hand-written impls whose stored form is the bare value the appearance writers use, collapsing an absent and an empty cell to the same undeclared read at every seam. Co-Authored-By: Claude Fable 5 --- .../graphic-types/src/graphic/glue.rs | 22 ++--- .../graphic-types/src/graphic/mod.rs | 38 ++++++++ .../graphic-types/src/graphic/paint.rs | 94 +++++++++++++++++-- .../graphic-types/src/graphic/walk.rs | 54 ++++++++++- .../libraries/graphic-types/src/markers.rs | 86 +++++++++++++++-- 5 files changed, 263 insertions(+), 31 deletions(-) diff --git a/node-graph/libraries/graphic-types/src/graphic/glue.rs b/node-graph/libraries/graphic-types/src/graphic/glue.rs index 04004de574..f5145294db 100644 --- a/node-graph/libraries/graphic-types/src/graphic/glue.rs +++ b/node-graph/libraries/graphic-types/src/graphic/glue.rs @@ -383,28 +383,28 @@ fn map_appearance_groups_to_resident(appearance: &mut Appearance, arena: &core_t Some(()) } -/// The deep copy-out for appearance field values (the appearance marker's owned -/// form): content groups leave any paint column in their owned form. Declines -/// (`None`) for group-free content, which already owns everything. +/// The deep copy-out for appearance field values (the appearance marker's bare +/// owned form): content groups leave any paint column in their owned form. +/// Declines (`None`) for group-free content, which already owns everything. fn deep_clone_appearance(value: &dyn core_types::list::AnyAttributeValue) -> Option> { - let appearance = value.as_any().downcast_ref::>().expect("an appearance field deep-copies at its own type"); - let appearance = appearance.as_ref().filter(|appearance| appearance_contains_groups(appearance))?; + let appearance = value.as_any().downcast_ref::().expect("an appearance field deep-copies at its own type"); + appearance_contains_groups(appearance).then_some(())?; let mut appearance = appearance.clone(); map_appearance_groups_to_owned(&mut appearance); - Some(Box::new(Some(appearance))) + Some(Box::new(appearance)) } /// The deep replay for appearance field values: owned content groups replay /// into the serving arena before the field re-parks. `Some(None)` declines for /// group-free content; `None` reports arena exhaustion. fn deep_repark_appearance(value: &dyn core_types::list::AnyAttributeValue, arena: &core_types::arena::Arena) -> Option>> { - let appearance = value.as_any().downcast_ref::>().expect("an appearance field replays at its own type"); - let Some(appearance) = appearance.as_ref().filter(|appearance| appearance_contains_groups(appearance)) else { + let appearance = value.as_any().downcast_ref::().expect("an appearance field replays at its own type"); + if !appearance_contains_groups(appearance) { return Some(None); - }; + } let mut appearance = appearance.clone(); map_appearance_groups_to_resident(&mut appearance, arena)?; - Some(Some(Box::new(Some(appearance)))) + Some(Some(Box::new(appearance))) } /// Every group held in an appearance's paint columns, promoted into the @@ -465,7 +465,7 @@ const _: () = { core_types::record::register_deep_field_value::>>(deep_clone_graphic_list, deep_repark_graphic_list); core_types::record::register_field_promote::>>>(promote_graphic_list); core_types::record::register_element_promote::(promote_graphic); - core_types::record::register_deep_field_value::>(deep_clone_appearance, deep_repark_appearance); + core_types::record::register_deep_field_value::(deep_clone_appearance, deep_repark_appearance); core_types::record::register_field_promote::>(promote_appearance); core_types::record::register_retained_heap::(|value| value.downcast_ref::().map_or(0, appearance_retained_heap)); core_types::record::register_retained_heap::(|value| value.downcast_ref::().map_or(0, graphic_retained_heap)); diff --git a/node-graph/libraries/graphic-types/src/graphic/mod.rs b/node-graph/libraries/graphic-types/src/graphic/mod.rs index 62ae5e3db4..2de8cd5856 100644 --- a/node-graph/libraries/graphic-types/src/graphic/mod.rs +++ b/node-graph/libraries/graphic-types/src/graphic/mod.rs @@ -14,6 +14,8 @@ pub use paint::{ pub use walk::{GraphicLevel, GraphicLevelColumn, RowStep, VectorRow, direct_vector_len, flatten_vector_rows, group_is_empty, lane_attributes, run_lane_attributes, walk_vector_rows}; use walk::{group_all_clipped, group_bounding_box, group_is_fully_transparent, group_is_opaque, group_render_complexity}; +use crate::appearance::Appearance; +use crate::markers::ATTR_APPEARANCE; use core_types::bounds::{BoundingBox, RenderBoundingBox}; use core_types::graphene_hash::CacheHash; use core_types::list::{Item, List}; @@ -176,6 +178,7 @@ fn flatten_graphic_list(content: List, extract_variant: fn(Graphic) let current_opacity: f64 = current_graphic_item.attribute_cloned_or(ATTR_OPACITY, 1.); let current_fill: f64 = current_graphic_item.attribute_cloned_or(ATTR_OPACITY_FILL, 1.); let lane_layer_path: Option> = current_graphic_item.attribute::>(ATTR_EDITOR_LAYER_PATH).cloned(); + let parent_appearance = current_graphic_item.attribute::(ATTR_APPEARANCE).and_then(Appearance::declared).cloned(); let (element, attributes) = current_graphic_item.into_parts(); match element { @@ -197,6 +200,14 @@ fn flatten_graphic_list(content: List, extract_variant: fn(Graphic) *v *= current_fill; } } + // Appearance cascades into each child whose own is undeclared, since a declared child wins wholesale + if let Some(appearance) = &parent_appearance { + for v in sub_list.iter_attribute_values_mut_or_default::(ATTR_APPEARANCE) { + if v.is_empty() { + *v = appearance.clone(); + } + } + } flatten_recursive(output, sub_list, extract_variant, lane_layer_path.as_deref()); } @@ -599,6 +610,33 @@ mod tests { let flattened: List = group.into_flattened_list(); assert_eq!(flattened.attribute_cloned_or_default::(ATTR_OPACITY, 0), 0.5); } + + // A padded (empty) appearance cell is undeclared, so the parent's appearance cascades into it while a declared sibling keeps its own + #[test] + fn flatten_cascades_into_padded_empty_appearance_items() { + use crate::appearance::Coverage; + + let single = |color: Color| Appearance::new_single(Coverage::new_fill(), Graphic::Color(color)); + + // Declaring an appearance on item 0 forces the attribute, padding item 1 with the empty appearance + let mut inner = List::new(); + inner.push(Item::new_from_element(vector_graphic())); + inner.push(Item::new_from_element(vector_graphic())); + inner.set_attribute(ATTR_APPEARANCE, 0, single(Color::BLACK)); + + let mut outer = List::new_from_element(Graphic::Graphic(inner)); + outer.set_attribute(ATTR_APPEARANCE, 0, single(Color::WHITE)); + + let flattened: List = outer.into_flattened_list(); + let color_of = |index: usize| { + let appearance = flattened.attribute::(ATTR_APPEARANCE, index)?; + let Graphic::Color(color) = appearance.paint_at(0)? else { return None }; + Some(*color) + }; + + assert_eq!(color_of(0), Some(Color::BLACK), "a declared item should keep its own appearance"); + assert_eq!(color_of(1), Some(Color::WHITE), "a padded item should inherit the parent appearance"); + } } #[cfg(test)] diff --git a/node-graph/libraries/graphic-types/src/graphic/paint.rs b/node-graph/libraries/graphic-types/src/graphic/paint.rs index c84349716d..1200a69a2d 100644 --- a/node-graph/libraries/graphic-types/src/graphic/paint.rs +++ b/node-graph/libraries/graphic-types/src/graphic/paint.rs @@ -1,7 +1,8 @@ //! The paint column level: fill and stroke read as lane columns and threaded down to the elements they reach. use super::{Graphic, IntoGraphicList}; -use crate::markers::{ATTR_FILL, ATTR_STROKE, Fill, Stroke}; +use crate::appearance::Appearance; +use crate::markers::{ATTR_FILL, ATTR_STROKE, Appearance as AppearanceMarker, Fill, Stroke}; use core_types::ATTR_TRANSFORM; use core_types::attribute::{Attribute, Opacity}; use core_types::lane::{LaneColumn, LaneSource}; @@ -69,10 +70,11 @@ impl<'a> LanePaint<'a> { } } -/// A source's fill and stroke columns, resolved once for per-lane reads. +/// A source's fill, stroke, and appearance columns, resolved once for per-lane reads. pub struct PaintColumns<'a, S: LaneSource + 'a> { fill: S::Column<'a, Fill>, stroke: S::Column<'a, Stroke>, + appearance: S::Column<'a, AppearanceMarker>, } impl<'a, S: LaneSource> PaintColumns<'a, S> { @@ -80,6 +82,7 @@ impl<'a, S: LaneSource> PaintColumns<'a, S> { Self { fill: source.column::(), stroke: source.column::(), + appearance: source.column::(), } } @@ -91,27 +94,50 @@ impl<'a, S: LaneSource> PaintColumns<'a, S> { stroke: present(self.stroke.try_get(lane)), } } + + /// The lane's own declared appearance; an absent or empty cell is undeclared. + pub fn read_appearance(&self, lane: usize) -> Option<&'a Appearance> { + self.appearance.try_get(lane).flatten().and_then(Appearance::declared) + } } /// How far a lane's paint reaches into the element beneath it, mirroring the /// legacy conversion's paint push: vector interiors directly and vector /// children of a nested graphic list, one level deep. +/// +/// The appearance cascade rides beside the paint push with its own rule: a +/// lane's own declared appearance wins wholesale, an undeclared lane inherits +/// the nearest ancestor's, at any depth. Only a fresh entry (a pattern's own +/// render, or any standalone render root) starts without an inherited one. #[derive(Clone, Copy)] pub struct PaintReach<'a> { pub paint: LanePaint<'a>, + /// The cascade's resolved appearance: the nearest declared one at or above this lane. + pub appearance: Option<&'a Appearance>, hops: u8, } impl<'a> PaintReach<'a> { - pub const NONE: Self = Self { paint: LanePaint::NONE, hops: 0 }; + pub const NONE: Self = Self { + paint: LanePaint::NONE, + appearance: None, + hops: 0, + }; /// The lane's effective reach: an inherited paint stays authoritative /// (lane paint below a push's origin is inert in the legacy model), an - /// absent one reads the lane's own paint. + /// absent one reads the lane's own paint. The appearance arbitrates the + /// opposite way: the lane's own declared appearance wins over the + /// inherited one. pub fn for_lane(self, columns: &PaintColumns<'a, S>, index: usize) -> Self { + let appearance = Appearance::cascade(columns.read_appearance(index), self.appearance); match self.paint.is_present() { - true => self, - false => Self { paint: columns.read(index), hops: 2 }, + true => Self { appearance, ..self }, + false => Self { + paint: columns.read(index), + appearance, + hops: 2, + }, } } @@ -119,20 +145,25 @@ impl<'a> PaintReach<'a> { self.hops > 0 && self.paint.is_present() } - /// The reach one graphic nesting level further down. + /// The reach one graphic nesting level further down. The appearance + /// cascade is not hop-limited, so it passes through unchanged. pub fn nested(self) -> Self { Self { - paint: self.paint, hops: self.hops.saturating_sub(1), + ..self } } /// The reach entering a group's own graphic run: a spent or absent reach - /// resets so the group's own lane paint applies at its own boundary. + /// resets so the group's own lane paint applies at its own boundary, + /// while the appearance cascades through the boundary. pub fn into_group_graphics(self) -> Self { match self.applies() { true => self.nested(), - false => Self::NONE, + false => Self { + appearance: self.appearance, + ..Self::NONE + }, } } } @@ -269,4 +300,47 @@ mod run_tests { let legacy = run_to_legacy_list::(&item).expect("the run lowers to a legacy vector list"); assert_eq!(paint_graphics::(&legacy, 0), paint_graphics::(&run, 0)); } + + #[test] + fn reach_cascades_the_appearance_with_own_wins_arbitration() { + use crate::appearance::Coverage; + use crate::markers::ATTR_APPEARANCE; + + let own = Appearance::new_single(Coverage::new_fill(), Graphic::Color(Color::BLACK)); + let inherited = Appearance::new_single(Coverage::new_fill(), Graphic::Color(Color::WHITE)); + + // Lane 0 declares its own appearance, lane 1 is padded with the empty (undeclared) one + let mut list: List> = List::new_from_element(Graphic::Vector(Vector::default())); + list.push(core_types::list::Item::new_from_element(Graphic::Vector(Vector::default()))); + list.set_attribute(ATTR_APPEARANCE, 0, own.clone()); + + let columns = PaintColumns::new(&list); + let ancestor = PaintReach { + appearance: Some(&inherited), + ..PaintReach::NONE + }; + + assert_eq!(ancestor.for_lane(&columns, 0).appearance, Some(&own), "a declared lane wins over the inherited appearance"); + assert_eq!(ancestor.for_lane(&columns, 1).appearance, Some(&inherited), "a padded lane inherits"); + assert_eq!(PaintReach::NONE.for_lane(&columns, 1).appearance, None, "no ancestor leaves an undeclared lane bare"); + } + + #[test] + fn reach_carries_the_appearance_through_nesting_and_group_boundaries() { + use crate::appearance::Coverage; + + let inherited = Appearance::new_single(Coverage::new_fill(), Graphic::Color(Color::WHITE)); + let reach = PaintReach { + appearance: Some(&inherited), + ..PaintReach::NONE + }; + + assert_eq!(reach.nested().appearance, Some(&inherited), "nesting does not hop-limit the cascade"); + assert_eq!( + reach.into_group_graphics().appearance, + Some(&inherited), + "the cascade crosses a group boundary the paint push resets at" + ); + assert_eq!(PaintReach::NONE.into_group_graphics().appearance, None); + } } diff --git a/node-graph/libraries/graphic-types/src/graphic/walk.rs b/node-graph/libraries/graphic-types/src/graphic/walk.rs index b28436fb13..419ad665df 100644 --- a/node-graph/libraries/graphic-types/src/graphic/walk.rs +++ b/node-graph/libraries/graphic-types/src/graphic/walk.rs @@ -2,7 +2,8 @@ use super::Graphic; use super::paint::{LanePaint, PaintColumns, PaintReach, is_paint_present, paint_graphics, set_paint_attribute_at}; -use crate::markers::{ATTR_FILL, ATTR_STROKE, Fill}; +use crate::appearance::Appearance; +use crate::markers::{ATTR_APPEARANCE, ATTR_FILL, ATTR_STROKE, Fill}; use core_types::attribute::{Attribute, ClippingMask, EditorLayerPath, Opacity, OpacityFill, Transform}; use core_types::bounds::{BoundingBox, RenderBoundingBox}; use core_types::lane::LaneSource; @@ -252,6 +253,7 @@ pub struct VectorRow<'w> { scale: FlattenScale, layer_path: Option<&'w [NodeId]>, paint: LanePaint<'w>, + appearance: Option<&'w Appearance>, } enum RowSourceRef<'w> { @@ -316,10 +318,23 @@ impl VectorRow<'_> { if let Some(layer_path) = self.layer_path { out.set_attribute(ATTR_EDITOR_LAYER_PATH, index, layer_path.to_vec()); } + // The cascade's resolved appearance lands on a row whose own is undeclared, since a declared row wins wholesale + if let Some(appearance) = self.appearance + && out.attribute::(ATTR_APPEARANCE, index).and_then(Appearance::declared).is_none() + { + out.set_attribute(ATTR_APPEARANCE, index, appearance.clone()); + } } } -fn walk_rows_of_run(item: &core_types::record::GroupItem, scale: FlattenScale, layer_path: Option<&[NodeId]>, paint: LanePaint<'_>, visit: &mut dyn FnMut(VectorRow<'_>) -> RowStep) -> RowStep { +fn walk_rows_of_run( + item: &core_types::record::GroupItem, + scale: FlattenScale, + layer_path: Option<&[NodeId]>, + paint: LanePaint<'_>, + appearance: Option<&Appearance>, + visit: &mut dyn FnMut(VectorRow<'_>) -> RowStep, +) -> RowStep { let Some(run) = core_types::record::RunView::::new(item) else { return RowStep::Continue; }; @@ -329,6 +344,7 @@ fn walk_rows_of_run(item: &core_types::record::GroupItem, scale: FlattenScale, l scale, layer_path, paint, + appearance, }) { return RowStep::Stop; } @@ -359,7 +375,7 @@ fn walk_vector_rows_impl<'a>( true => inherited.paint, false => LanePaint::NONE, }; - return walk_rows_of_run(item, scale, parent_layer_path, paint, visit); + return walk_rows_of_run(item, scale, parent_layer_path, paint, inherited.appearance, visit); } } let columns = PaintColumns::new(&level); @@ -376,6 +392,7 @@ fn walk_vector_rows_impl<'a>( scale, layer_path: parent_layer_path, paint: row_paint, + appearance: reach.appearance, }), Graphic::Graphic(children) => walk_vector_rows_impl( GraphicLevel::Legacy(children), @@ -387,7 +404,7 @@ fn walk_vector_rows_impl<'a>( Graphic::Group(group) => { let item = &group.content; if item.typed_lanes::().is_some() { - walk_rows_of_run(item, scale.composed(&level, index), level.try_attr::(index), row_paint, visit) + walk_rows_of_run(item, scale.composed(&level, index), level.try_attr::(index), row_paint, reach.appearance, visit) } else if item.typed_lanes::().is_some() { walk_vector_rows_impl( GraphicLevel::Run(item), @@ -569,4 +586,33 @@ mod run_tests { assert_eq!(run.thumbnail_bounding_box(outer, include_stroke), legacy.thumbnail_bounding_box(outer, include_stroke)); } } + + #[test] + fn the_walk_cascades_appearance_like_the_legacy_flatten() { + use crate::appearance::Coverage; + + let single = |color: Color| Appearance::new_single(Coverage::new_fill(), Graphic::Color(color)); + + let mut inner = List::new(); + inner.push(Item::new_from_element(Graphic::Vector(unit_square_at(DVec2::ZERO)))); + inner.push(Item::new_from_element(Graphic::Vector(unit_square_at(DVec2::ONE)))); + inner.set_attribute(ATTR_APPEARANCE, 0, single(Color::BLACK)); + + let mut top = List::new_from_element(Graphic::Graphic(inner)); + top.set_attribute(ATTR_APPEARANCE, 0, single(Color::WHITE)); + + let walked = flatten_vector_rows(GraphicLevel::Legacy(&top)); + let legacy: List = top.clone().into_flattened_list(); + + let color_of = |list: &List, index: usize| { + let appearance = list.attribute::(ATTR_APPEARANCE, index)?; + let Graphic::Color(color) = appearance.paint_at(0)? else { return None }; + Some(*color) + }; + + for list in [&walked, &legacy] { + assert_eq!(color_of(list, 0), Some(Color::BLACK), "a declared row keeps its own appearance"); + assert_eq!(color_of(list, 1), Some(Color::WHITE), "an undeclared row inherits the level's appearance"); + } + } } diff --git a/node-graph/libraries/graphic-types/src/markers.rs b/node-graph/libraries/graphic-types/src/markers.rs index e0bfdae910..b8473c0d9b 100644 --- a/node-graph/libraries/graphic-types/src/markers.rs +++ b/node-graph/libraries/graphic-types/src/markers.rs @@ -20,14 +20,88 @@ core_types::attribute! { /// Rasterize, etc.), so the editor can still surface click targets for the original child /// layers after their content has been collapsed. pub EditorMergedLayers("editor:merged_layers"): Option<&List>>; - /// The item's ordered list of paint passes. An absent or empty value is the undeclared - /// state that inherits the nearest ancestor's appearance through the cascade. - pub Appearance("appearance"): Option<&crate::appearance::Appearance>; - /// One coverage row's paint, a bare graphic riding the coverage list as a column. - /// Absent when the coverage paints nothing. - pub Paint("paint"): Option<&Graphic<'static>>; } +/// The item's ordered list of paint passes. An absent or empty value is the undeclared +/// state that inherits the nearest ancestor's appearance through the cascade, so both +/// read as `None`. The stored form is the bare [`crate::appearance::Appearance`], the +/// shape the appearance writers use, which the `attribute!` macro's optional-reference +/// arm cannot express. +pub struct Appearance; + +// SAFETY: `read_erased` produces the bare owned appearance `from_stored` reads, `REPARK` +// re-parks that same form, and the empty appearance collapses to the `None` default at +// every read seam. +unsafe impl Attribute for Appearance { + const NAME: &'static str = "appearance"; + type Value<'e> = Option<&'e crate::appearance::Appearance>; + + fn from_stored<'a>(stored: &'a dyn std::any::Any) -> Option> { + stored.downcast_ref::().map(crate::appearance::Appearance::declared) + } + + unsafe fn read_erased(ptr: *const u8) -> Box { + Box::new(unsafe { ptr.cast::>().read() }.cloned().unwrap_or_default()) + } + + const REPARK: Option = { + unsafe fn repark(value: &dyn core_types::list::AnyAttributeValue, dst: *mut u8, arena: &core_types::arena::Arena) -> Option<()> { + let owned: &crate::appearance::Appearance = value.as_any().downcast_ref().expect("an appearance attribute replays its bare owned clone"); + let parked = match owned.is_empty() { + true => None, + false => { + let (parked, _) = arena.alloc(owned.clone())?; + Some(&*parked) + } + }; + // SAFETY: the slot is a live field of this marker's value type. + unsafe { dst.cast::>().write(parked) }; + Some(()) + } + Some(repark) + }; +} + +core_types::attribute!(@register Appearance); + +/// One coverage row's paint, a bare graphic riding the coverage list as a column. Absent +/// or empty paint draws nothing, so both read as `None`; the stored form is the bare +/// [`Graphic`], the shape [`crate::appearance`]'s row writers use. +pub struct Paint; + +// SAFETY: as for `Appearance`, at the bare `Graphic` stored form. +unsafe impl Attribute for Paint { + const NAME: &'static str = "paint"; + type Value<'e> = Option<&'e Graphic<'static>>; + + fn from_stored<'a>(stored: &'a dyn std::any::Any) -> Option> { + stored.downcast_ref::>().map(|paint| (!paint.is_empty()).then_some(paint)) + } + + unsafe fn read_erased(ptr: *const u8) -> Box { + Box::new(unsafe { ptr.cast::>>().read() }.cloned().unwrap_or_default()) + } + + const REPARK: Option = { + unsafe fn repark(value: &dyn core_types::list::AnyAttributeValue, dst: *mut u8, arena: &core_types::arena::Arena) -> Option<()> { + let owned: &Graphic<'static> = value.as_any().downcast_ref().expect("a paint attribute replays its bare owned clone"); + let parked = match owned.is_empty() { + true => None, + false => { + let (parked, _) = arena.alloc(owned.clone())?; + Some(&*parked) + } + }; + // SAFETY: the slot is a live field of this marker's value type. + unsafe { dst.cast::>>().write(parked) }; + Some(()) + } + Some(repark) + }; +} + +core_types::attribute!(@register Paint); + pub const ATTR_FILL: &str = Fill::NAME; pub const ATTR_STROKE: &str = Stroke::NAME; pub const ATTR_EDITOR_MERGED_LAYERS: &str = EditorMergedLayers::NAME;