diff --git a/node-graph/libraries/graphic-types/src/graphic/glue.rs b/node-graph/libraries/graphic-types/src/graphic/glue.rs index 83ca9a9a83..9083ccbd14 100644 --- a/node-graph/libraries/graphic-types/src/graphic/glue.rs +++ b/node-graph/libraries/graphic-types/src/graphic/glue.rs @@ -109,12 +109,18 @@ fn attribute_keys(list: &List) -> Vec { pub(crate) fn map_attribute_groups_to_owned(list: &mut List>) { for key in attribute_keys(list) { // A column of a type that cannot hold groups is skipped whole. - let Some(values) = list.iter_attribute_values_mut::>>(&key) else { continue }; - for value in values.flatten() { - for element in value.iter_element_values_mut() { - *element = map_groups_to_owned(element); + if let Some(values) = list.iter_attribute_values_mut::>>(&key) { + for value in values.flatten() { + for element in value.iter_element_values_mut() { + *element = map_groups_to_owned(element); + } + map_attribute_groups_to_owned(value); } - map_attribute_groups_to_owned(value); + continue; + } + let Some(appearances) = list.iter_attribute_values_mut::(&key) else { continue }; + for appearance in appearances { + map_appearance_groups_to_owned(appearance); } } } @@ -124,15 +130,21 @@ pub(crate) fn map_attribute_groups_to_owned(list: &mut List>) { pub(crate) fn map_attribute_groups_to_resident(list: &mut List>, arena: &core_types::arena::Arena) -> Option<()> { for key in attribute_keys(list) { // A column of a type that cannot hold groups is skipped whole. - let Some(values) = list.iter_attribute_values_mut::>>(&key) else { continue }; - for value in values.flatten() { - for element in value.iter_element_values_mut() { - let resident = map_groups_to_resident(element, arena)?; - // SAFETY: the attribute store is erased, and the replay serves as - // long as `arena`, which the store's reader outlives. - *element = unsafe { core_types::record::erase_static(resident) }; + if let Some(values) = list.iter_attribute_values_mut::>>(&key) { + for value in values.flatten() { + for element in value.iter_element_values_mut() { + let resident = map_groups_to_resident(element, arena)?; + // SAFETY: the attribute store is erased, and the replay serves as + // long as `arena`, which the store's reader outlives. + *element = unsafe { core_types::record::erase_static(resident) }; + } + map_attribute_groups_to_resident(value, arena)?; } - map_attribute_groups_to_resident(value, arena)?; + continue; + } + let Some(appearances) = list.iter_attribute_values_mut::(&key) else { continue }; + for appearance in appearances { + map_appearance_groups_to_resident(appearance, arena)?; } } Some(()) @@ -144,15 +156,21 @@ pub(crate) fn map_attribute_groups_to_resident(list: &mut List>, are pub(crate) fn map_attribute_groups_to_persistent(list: &mut List>, promotion: &core_types::record::Promotion<'_>) -> Option<()> { for key in attribute_keys(list) { // A column of a type that cannot hold groups is skipped whole. - let Some(values) = list.iter_attribute_values_mut::>>(&key) else { continue }; - for value in values.flatten() { - for element in value.iter_element_values_mut() { - let promoted = map_groups_to_persistent(element, promotion)?; - // SAFETY: the attribute store is erased, and persistent content - // outlives the evaluation. - *element = unsafe { core_types::record::erase_static(promoted) }; + if let Some(values) = list.iter_attribute_values_mut::>>(&key) { + for value in values.flatten() { + for element in value.iter_element_values_mut() { + let promoted = map_groups_to_persistent(element, promotion)?; + // SAFETY: the attribute store is erased, and persistent content + // outlives the evaluation. + *element = unsafe { core_types::record::erase_static(promoted) }; + } + map_attribute_groups_to_persistent(value, promotion)?; } - map_attribute_groups_to_persistent(value, promotion)?; + continue; + } + let Some(appearances) = list.iter_attribute_values_mut::(&key) else { continue }; + for appearance in appearances { + map_appearance_groups_to_persistent(appearance, promotion)?; } } Some(()) @@ -231,13 +249,14 @@ pub(crate) fn list_contains_groups(list: &List) -> bool { /// Whether any group hides in the list's item attribute values. Only the /// group-capable columns are scanned: those are the value types the deep field -/// glue is registered for, today `Option>` alone. A list with no -/// attribute columns costs nothing, and a column of any other type is decided -/// by its one downcast rather than per value. +/// glue is registered for, `Option>` and `Appearance`. A list +/// with no attribute columns costs nothing, and a column of any other type is +/// decided by its one downcast rather than per value. fn attribute_values_contain_groups(list: &List) -> bool { list.attribute_keys().any(|key| { list.iter_attribute_values::>>(key) .is_some_and(|mut values| values.any(|value| value.as_ref().is_some_and(list_contains_groups))) + || list.iter_attribute_values::(key).is_some_and(|mut values| values.any(appearance_contains_groups)) }) } @@ -969,4 +988,75 @@ mod run_tests { "the stroke parameters survive the move" ); } + + #[test] + fn a_paint_field_whose_appearance_column_holds_groups_never_moves() { + let inner_vector = unit_square_at(DVec2::ZERO); + let mut transient = core_types::arena::Arena::new(1 << 16).unwrap(); + let persistent = core_types::arena::Arena::new(1 << 16).unwrap(); + + // Group-free elements, with the resident group hidden in an item's appearance paint cell. + let native = native_group_paint(&inner_vector, &transient); + let Some(native_group) = native.element(0) else { panic!("the paint carries a group") }; + let expected = map_groups_to_legacy(native_group); + // SAFETY: the erased native group serves only while `transient` is live; + // the promote under test replaces its borrows. + let paint_cell = unsafe { core_types::record::erase_static(native_group.clone()) }; + let mut paint = List::new_from_element(Graphic::Vector(unit_square_at(DVec2::new(4., 4.)))); + paint.set_attribute(crate::markers::ATTR_APPEARANCE, 0, Appearance::new_single(Coverage::new_fill(), paint_cell)); + let (paint, _) = transient.alloc_sized_keyed(paint, 0).unwrap(); + + let (layout, span, _frames) = promote_paint_field(Some(paint), 1, &transient, &persistent); + let served = promoted_paint(&span, &layout, 0, &persistent); + let moved = std::ptr::eq(std::ptr::from_ref(served).cast::(), std::ptr::from_ref(paint).cast::()); + assert!(!moved, "an appearance-held group denies the move, so the promote parks a header of its own"); + + transient.reset(); + let served = promoted_paint(&span, &layout, 0, &persistent); + let held = served.attribute::(crate::markers::ATTR_APPEARANCE, 0).expect("the appearance rides the promoted list"); + let held = held.paint_at(0).expect("the fill coverage keeps its paint"); + assert_eq!(map_groups_to_legacy(held), expected, "the appearance-held group serves from persistent storage after the reset"); + } + + #[test] + fn an_owned_record_deep_copies_appearance_column_groups() { + let inner_vector = unit_square_at(DVec2::ZERO); + let source = core_types::arena::Arena::new(1 << 16).unwrap(); + let native = native_group_paint(&inner_vector, &source); + let Some(native_group) = native.element(0) else { panic!("the paint carries a group") }; + let expected = map_groups_to_legacy(native_group); + + // The field's elements are group-free; the group rides an item's appearance + // paint cell, which the shallow read alone would leave borrowing `source`. + // SAFETY: the erased native group serves only while `source` is live; the + // deep glue under test replaces its borrows at the copy-out seam. + let paint_cell = unsafe { core_types::record::erase_static(native_group.clone()) }; + let mut paint = List::new_from_element(Graphic::Vector(unit_square_at(DVec2::new(2., 2.)))); + paint.set_attribute(crate::markers::ATTR_APPEARANCE, 0, Appearance::new_single(Coverage::new_fill(), paint_cell)); + + let vector = unit_square_at(DVec2::new(4., 4.)); + let mut builder = RunBuilder::new(&source, element_write_hashed::(), &[FieldWrite::of::(0)], 1).unwrap(); + let lane = builder.push(vector.clone()).unwrap(); + builder.attr::(lane, Some(&paint)); + let item = builder.finish(); + let layout = item.layout().clone(); + let offset = layout.offset_of(EditorMergedLayers::NAME, 0).unwrap(); + // SAFETY: the item's lane is a live record of `layout`. + let owned = unsafe { core_types::record::OwnedRecord::copy_out(&layout, item.lanes().get(0).rec()) }; + drop(item); + drop(paint); + drop(source); + + let arena = core_types::arena::Arena::new(1 << 16).unwrap(); + let frames = core_types::record::test_frames(layout.frame_bytes()); + let mut slot = frames.claim(&layout); + owned.replay_into(&mut slot, &arena).expect("the arena holds the replay"); + // SAFETY: the replay completes the record in the claimed frame. + let value = unsafe { slot.finish() }; + // SAFETY: the replay wrote a record of `layout`. + let served = unsafe { layout.rec(&value).read::>>(offset) }.expect("the field replays present"); + let held = served.attribute::(crate::markers::ATTR_APPEARANCE, 0).expect("the appearance rides the replayed list"); + let held = held.paint_at(0).expect("the fill coverage keeps its paint"); + assert_eq!(map_groups_to_legacy(held), expected, "the appearance-held group replayed into the serving arena"); + } }