Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions node-graph/graph-craft/src/document/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use super::DocumentNode;
use crate::application_io::PlatformEditorApi;
use crate::application_io::resource::Resource;
use crate::proto::{Any as DAny, FutureAny};
use brush_nodes::Stroke;
use brush_nodes::brush_stroke::{BrushStroke, BrushTrace};
use brush_nodes::{BrushCache, Stroke};
use core_types::color::SRGBA8;
use core_types::list::{Item, List, NodeIdPath};
use core_types::transform::Footprint;
Expand Down Expand Up @@ -99,6 +99,7 @@ macro_rules! tagged_value {
#[serde(alias = "BrushStrokeTable")]
BrushStrokes(Vec<BrushStroke>),
Strokes(Vec<Stroke>),
BrushCache(BrushCache),
// =======================
// AUTO-GENERATED VARIANTS
// =======================
Expand Down Expand Up @@ -143,6 +144,7 @@ macro_rules! tagged_value {
Self::GradientRamp(ramp) => ramp.cache_hash(state),
Self::BrushStrokes(strokes) => strokes.cache_hash(state),
Self::Strokes(strokes) => strokes.cache_hash(state),
Self::BrushCache(cache) => cache.cache_hash(state),
// =======================
// NON-SERIALIZED VARIANTS
// =======================
Expand Down Expand Up @@ -210,6 +212,7 @@ macro_rules! tagged_value {
let list: List<Stroke> = strokes.into_iter().map(core_types::list::Item::new_from_element).collect();
Box::new(list)
}
Self::BrushCache(cache) => Box::new(Item::new_from_element(cache)),
// =======================
// AUTO-GENERATED VARIANTS
// =======================
Expand Down Expand Up @@ -277,6 +280,7 @@ macro_rules! tagged_value {
let list: List<Stroke> = strokes.into_iter().map(core_types::list::Item::new_from_element).collect();
Arc::new(list)
}
Self::BrushCache(cache) => Arc::new(Item::new_from_element(cache)),
// =======================
// AUTO-GENERATED VARIANTS
// =======================
Expand All @@ -300,13 +304,14 @@ macro_rules! tagged_value {
// MANUAL VARIANTS
// ===============
Self::None => concrete!(()),
Self::TypeDefault(td) => td.clone(),
Self::TypeDefault(td) => td.clone(),
Self::F64Array(_) => list!(f64),
Self::DashPattern(_) => item!(DashPattern),
Self::BoxCorners(_) => item!(BoxCorners),
Self::GradientRamp(_) => item!(Gradient),
Self::BrushStrokes(_) => item!(BrushTrace),
Self::Strokes(_) => list!(Stroke),
Self::BrushCache(_) => item!(BrushCache),
// =======================
// AUTO-GENERATED VARIANTS
// =======================
Expand Down Expand Up @@ -348,6 +353,7 @@ macro_rules! tagged_value {
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())),
x if x == TypeId::of::<List<Stroke>>() => Ok(TaggedValue::Strokes(downcast::<List<Stroke>>(input).unwrap().into_iter().map(Item::into_element).collect())),
x if x == TypeId::of::<Item<BrushCache>>() => Ok(TaggedValue::BrushCache(downcast::<Item<BrushCache>>(input).unwrap().into_element())),
// =======================
// AUTO-GENERATED VARIANTS
// =======================
Expand Down Expand Up @@ -383,6 +389,7 @@ macro_rules! tagged_value {
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())),
x if x == TypeId::of::<List<Stroke>>() => Ok(TaggedValue::Strokes(input.downcast_ref::<List<Stroke>>().unwrap().iter_element_values().cloned().collect())),
x if x == TypeId::of::<Item<BrushCache>>() => Ok(TaggedValue::BrushCache(input.downcast_ref::<Item<BrushCache>>().unwrap().element().clone())),
// =======================
// AUTO-GENERATED VARIANTS
// =======================
Expand Down Expand Up @@ -412,6 +419,7 @@ macro_rules! tagged_value {
$( if name == std::any::type_name::<$ty>() { return Some(TaggedValue::$identifier(Default::default())) } )*
if name == std::any::type_name::<BrushTrace>() { return Some(TaggedValue::BrushStrokes(Vec::new())) }
if name == std::any::type_name::<List<Stroke>>() { return Some(TaggedValue::Strokes(Vec::new())) }
if name == std::any::type_name::<BrushCache>() { return Some(TaggedValue::BrushCache(Default::default())) }
// Unranked types without a variant route through `TypeDefault`, with `to_dynany`/`to_any` constructing the actual default at execution time
macro_rules! check_bare {
($type_default:ty) => {
Expand Down Expand Up @@ -469,6 +477,7 @@ macro_rules! tagged_value {
Self::GradientRamp(ramp) => format!("GradientRamp({ramp:?})"),
Self::BrushStrokes(strokes) => format!("BrushStrokes({strokes:?})"),
Self::Strokes(strokes) => format!("Strokes({strokes:?})"),
Self::BrushCache(cache) => format!("{cache:?}"),
// =======================
// AUTO-GENERATED VARIANTS
// =======================
Expand Down Expand Up @@ -739,7 +748,6 @@ impl TaggedValue {
///
/// Routes legacy variant names into modern variants, in typed Rust. Each legacy name is also matched against the historical `#[serde(alias = "...")]` spellings the deleted variant accepted, so old-shape inner payloads are caught:
///
/// - `BrushCache` → `TaggedValue::None` (purely runtime cache; no payload to preserve)
/// - `Graphic` (or alias `GraphicGroup`/`Group`) → `TaggedValue::TypeDefault(list!(Graphic))`
/// - `Artboard` (or alias `ArtboardGroup`) → `TaggedValue::TypeDefault(list!(Artboard))`
/// - `Raster` (or alias `ImageFrame`/`RasterData`/`Image`):
Expand All @@ -764,7 +772,6 @@ pub fn deserialize_tagged_value_with_legacy_migration<'de, D: serde::Deserialize
&& let Some((tag, content)) = map.iter().next()
{
match tag.as_str() {
"BrushCache" => return Ok(MemoHash::new(TaggedValue::None)),
"Graphic" | "GraphicGroup" | "Group" => return Ok(MemoHash::new(TaggedValue::TypeDefault(list!(Graphic)))),
"Artboard" | "ArtboardGroup" => return Ok(MemoHash::new(TaggedValue::TypeDefault(list!(Artboard)))),
"Raster" | "ImageFrame" | "RasterData" | "Image" => {
Expand Down
2 changes: 1 addition & 1 deletion node-graph/graph-craft/src/proto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(9617677014563055585), NodeId(3306304180790283913), NodeId(4482673701109291121), NodeId(1535890178157254933)]
);
}

Expand Down
250 changes: 250 additions & 0 deletions node-graph/libraries/brush-types/src/cache.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,250 @@
//! Opaque render state cached per footprint.
//!
//! ```ignore
//! let state: SomeState = cache.take(ctx.footprint()).unwrap_or_default();
//! // ...render, freely mutating the state
//! cache.store(ctx.footprint(), state);
//! ```

use core_types::transform::Footprint;
use glam::DMat2;
use std::sync::{Arc, Mutex};

const STALE_EPOCHS: u64 = 2;
const MAX_VIEWS: usize = 3;

#[derive(Clone)]
pub struct BrushCache {
state: Arc<Mutex<State>>,
nonce: u64, // Avoid deduplication of cache entries across different brush nodes.
}

impl Default for BrushCache {
fn default() -> Self {
Self {
state: Default::default(),
nonce: core_types::uuid::generate_uuid(),
}
}
}

impl BrushCache {
pub fn take<S: std::any::Any + Send + Sync>(&self, footprint: &Footprint) -> Option<S> {
let mut guard = self.state.lock().unwrap();
let state = guard.take(footprint)?;
match state.downcast() {
Ok(state) => Some(*state),
Err(state) => {
guard.store(footprint, state);
None
}
}
}

pub fn store<S: std::any::Any + Send + Sync>(&self, footprint: &Footprint, state: S) {
self.state.lock().unwrap().store(footprint, Box::new(state));
}
}

impl PartialEq for BrushCache {
fn eq(&self, _: &Self) -> bool {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Different brush caches compare equal here even though cache_hash deliberately distinguishes their per-node nonces, so derived TaggedValue equality can collapse distinct cache identities. Compare the nonces so equality reflects the identity used for deduplication.

(Based on your team's feedback about meaningful equality and consistent hashing.) .

View Feedback

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/libraries/brush-types/src/cache.rs, line 43:

<comment>Different brush caches compare equal here even though `cache_hash` deliberately distinguishes their per-node nonces, so derived `TaggedValue` equality can collapse distinct cache identities. Compare the nonces so equality reflects the identity used for deduplication.

(Based on your team's feedback about meaningful equality and consistent hashing.) .</comment>

<file context>
@@ -0,0 +1,243 @@
+}
+
+impl PartialEq for BrushCache {
+	fn eq(&self, _: &Self) -> bool {
+		true
+	}
</file context>

true
}
}

impl std::fmt::Debug for BrushCache {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("BrushCache").field("slots", &self.state.lock().unwrap().slots.len()).finish()
}
}

impl core_types::CacheHash for BrushCache {
fn cache_hash<H: core::hash::Hasher>(&self, state: &mut H) {
state.write_u64(self.nonce);
}
}

unsafe impl dyn_any::StaticType for BrushCache {
type Static = BrushCache;
}

#[cfg(feature = "serde")]
impl serde::Serialize for BrushCache {
fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
serializer.serialize_unit()
}
}

#[cfg(feature = "serde")]
impl<'de> serde::Deserialize<'de> for BrushCache {
fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
serde::de::IgnoredAny::deserialize(deserializer)?;
Ok(Self::default())
}
}

type BoxedData = Box<dyn std::any::Any + Send + Sync>;

#[derive(Default)]
struct State {
epoch: u64,
slots: Vec<Slot>,
}

struct Slot {
footprint: Footprint,
epoch: u64,
data: BoxedData,
}

impl Slot {
fn view(&self) -> DMat2 {
self.footprint.transform.matrix2
}
}

impl State {
fn take(&mut self, footprint: &Footprint) -> Option<BoxedData> {
self.touch(footprint.transform.matrix2);
let index = self.slots.iter().position(|slot| slot.footprint == *footprint);
let hit = index.map(|index| {
let slot = self.slots.remove(index);
if slot.epoch == self.epoch {
self.epoch += 1;
}
slot.data
});
self.retire();
hit
}

fn store(&mut self, footprint: &Footprint, data: BoxedData) {
self.touch(footprint.transform.matrix2);
self.slots.retain(|slot| slot.footprint != *footprint);
self.slots.push(Slot {
footprint: *footprint,
epoch: self.epoch,
data,
});
self.retire();
}

fn touch(&mut self, view: DMat2) {
self.slots.sort_by_key(|slot| slot.view() == view);
}

fn retire(&mut self) {
let epoch = self.epoch;
self.slots.retain(|slot| epoch - slot.epoch < STALE_EPOCHS);
while self.slots.chunk_by(|a, b| a.view() == b.view()).count() > MAX_VIEWS {
let front = self.slots[0].view();
let group = self.slots.iter().take_while(|slot| slot.view() == front).count();
self.slots.drain(..group.max(1));
}
}
}

#[cfg(test)]
mod tests {
use super::*;
use core_types::transform::RenderQuality;
use glam::{DAffine2, DVec2, UVec2};

struct Dummy;

fn view(zoom: f64, rotation: f64, pan: DVec2) -> Footprint {
Footprint {
transform: DAffine2::from_scale_angle_translation(DVec2::splat(zoom), rotation, pan),
resolution: UVec2::new(1920, 1080),
quality: RenderQuality::Full,
}
}

fn thumbnail(zoom: f64) -> Footprint {
Footprint {
resolution: UVec2::new(150, 150),
..view(zoom, 0., DVec2::ZERO)
}
}

fn live(cache: &BrushCache) -> usize {
cache.state.lock().unwrap().slots.len()
}

fn render(cache: &BrushCache, footprint: &Footprint) -> bool {
let hit = cache.take::<Dummy>(footprint).is_some();
cache.store(footprint, Dummy);
hit
}

#[test]
fn continuous_zoom_is_bounded_by_views() {
let cache = BrushCache::default();
for step in 0..100 {
render(&cache, &view(1. + step as f64 * 0.01, 0., DVec2::ZERO));
}
assert!(live(&cache) <= MAX_VIEWS);
}

#[test]
fn continuous_rotation_is_bounded_by_views() {
let cache = BrushCache::default();
for step in 0..100 {
render(&cache, &view(2., step as f64 * 0.01, DVec2::ZERO));
}
assert!(live(&cache) <= MAX_VIEWS);
}

#[test]
fn zooming_reclaims_pan_slots() {
let cache = BrushCache::default();
for step in 0..30 {
render(&cache, &view(1., 0., DVec2::splat(step as f64 * 100.)));
}
for step in 1..=3 {
render(&cache, &view(1. + step as f64, 0., DVec2::ZERO));
}
assert_eq!(live(&cache), 3);
}

#[test]
fn frames_may_hold_many_footprints_per_view() {
let cache = BrushCache::default();
let footprints: Vec<_> = (0..5).map(|step| view(1., 0., DVec2::splat(step as f64 * 100.))).collect();
for frame in 0..10 {
for footprint in &footprints {
assert_eq!(render(&cache, footprint), frame > 0, "footprint evicted while its frame still renders it");
}
}
assert_eq!(live(&cache), 5);
}

#[test]
fn thumbnail_drift_is_bounded_and_keeps_the_view() {
let cache = BrushCache::default();
for step in 0..100 {
render(&cache, &thumbnail(1. + step as f64 * 0.001));
}
assert!(live(&cache) <= MAX_VIEWS);

let viewport = view(2., 0., DVec2::ZERO);
render(&cache, &viewport);
for step in 0..50 {
render(&cache, &thumbnail(2. + step as f64 * 0.001));
assert!(render(&cache, &viewport), "thumbnail churn evicted the viewport slot");
}
}

#[test]
fn settled_view_retires_stale_slots() {
let cache = BrushCache::default();
for step in 0..3 {
render(&cache, &view(1. + step as f64, 0., DVec2::ZERO));
}
assert_eq!(live(&cache), 3);
for _ in 0..STALE_EPOCHS {
render(&cache, &view(1., 0., DVec2::ZERO));
}
assert_eq!(live(&cache), 1);
}
}
3 changes: 3 additions & 0 deletions node-graph/libraries/brush-types/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
pub mod cache;
pub use cache::BrushCache;

use core_types::CacheHash;
use core_types::bounds::{BoundingBox, RenderBoundingBox};
use core_types::render_complexity::RenderComplexity;
Expand Down
Loading