Skip to content

Commit a40de58

Browse files
mTvare6Keavon
andauthored
Fix brush bounding boxes by making BrushCacheImpl's hash not shared between different instances (#2845)
* fix: add cache to each layer * fix: warning * fix: tests * Clean up code --------- Co-authored-by: Keavon Chambers <keavon@keavon.com>
1 parent 00236c8 commit a40de58

File tree

4 files changed

+45
-49
lines changed

4 files changed

+45
-49
lines changed

editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -946,7 +946,7 @@ fn static_nodes() -> Vec<DocumentNodeDefinition> {
946946
inputs: vec![
947947
NodeInput::value(TaggedValue::RasterData(RasterDataTable::default()), true),
948948
NodeInput::value(TaggedValue::BrushStrokes(Vec::new()), false),
949-
NodeInput::value(TaggedValue::BrushCache(BrushCache::new_proto()), false),
949+
NodeInput::value(TaggedValue::BrushCache(BrushCache::default()), false),
950950
],
951951
..Default::default()
952952
},

editor/src/messages/tool/tool_messages/brush_tool.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use super::tool_prelude::*;
22
use crate::consts::DEFAULT_BRUSH_SIZE;
3-
use crate::messages::portfolio::document::graph_operation::transform_utils::{get_current_normalized_pivot, get_current_transform};
3+
use crate::messages::portfolio::document::graph_operation::transform_utils::get_current_transform;
44
use crate::messages::portfolio::document::node_graph::document_node_definitions::resolve_document_node_type;
55
use crate::messages::portfolio::document::utility_types::document_metadata::LayerNodeIdentifier;
66
use crate::messages::portfolio::document::utility_types::network_interface::FlowType;
@@ -287,9 +287,7 @@ impl BrushToolData {
287287
}
288288

289289
if *reference == Some("Transform".to_string()) {
290-
let upstream = document.metadata().upstream_transform(node_id);
291-
let pivot = DAffine2::from_translation(upstream.transform_point2(get_current_normalized_pivot(&node.inputs)));
292-
self.transform = pivot * get_current_transform(&node.inputs) * pivot.inverse() * self.transform;
290+
self.transform = get_current_transform(&node.inputs) * self.transform;
293291
}
294292
}
295293

node-graph/gbrush/src/brush.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ mod test {
403403
blend_mode: BlendMode::Normal,
404404
},
405405
}],
406-
BrushCache::new_proto(),
406+
BrushCache::default(),
407407
)
408408
.await;
409409
assert_eq!(image.instance_ref_iter().next().unwrap().instance.width, 20);

node-graph/gbrush/src/brush_cache.rs

Lines changed: 41 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,16 @@ use graphene_core::raster_types::CPU;
66
use graphene_core::raster_types::Raster;
77
use std::collections::HashMap;
88
use std::hash::Hash;
9-
use std::sync::Arc;
10-
use std::sync::Mutex;
9+
use std::hash::Hasher;
10+
use std::sync::atomic::{AtomicU64, Ordering};
11+
use std::sync::{Arc, Mutex};
1112

12-
#[derive(Clone, Debug, PartialEq, DynAny, Default, serde::Serialize, serde::Deserialize)]
13+
// TODO: This is a temporary hack, be sure to not reuse this when the brush is being rewritten.
14+
static NEXT_BRUSH_CACHE_IMPL_ID: AtomicU64 = AtomicU64::new(0);
15+
16+
#[derive(Clone, Debug, DynAny, serde::Serialize, serde::Deserialize)]
1317
struct BrushCacheImpl {
18+
unique_id: u64,
1419
// The full previous input that was cached.
1520
prev_input: Vec<BrushStroke>,
1621

@@ -90,9 +95,29 @@ impl BrushCacheImpl {
9095
}
9196
}
9297

98+
impl Default for BrushCacheImpl {
99+
fn default() -> Self {
100+
Self {
101+
unique_id: NEXT_BRUSH_CACHE_IMPL_ID.fetch_add(1, Ordering::SeqCst),
102+
prev_input: Vec::new(),
103+
background: Default::default(),
104+
blended_image: Default::default(),
105+
last_stroke_texture: Default::default(),
106+
brush_texture_cache: HashMap::new(),
107+
}
108+
}
109+
}
110+
111+
impl PartialEq for BrushCacheImpl {
112+
fn eq(&self, other: &Self) -> bool {
113+
self.unique_id == other.unique_id
114+
}
115+
}
116+
93117
impl Hash for BrushCacheImpl {
94-
// Zero hash.
95-
fn hash<H: std::hash::Hasher>(&self, _state: &mut H) {}
118+
fn hash<H: Hasher>(&self, state: &mut H) {
119+
self.unique_id.hash(state);
120+
}
96121
}
97122

98123
#[derive(Clone, Debug, Default)]
@@ -103,82 +128,55 @@ pub struct BrushPlan {
103128
pub first_stroke_point_skip: usize,
104129
}
105130

106-
#[derive(Debug, DynAny, serde::Serialize, serde::Deserialize)]
107-
pub struct BrushCache {
108-
inner: Arc<Mutex<BrushCacheImpl>>,
109-
proto: bool,
110-
}
111-
112-
impl Default for BrushCache {
113-
fn default() -> Self {
114-
Self::new_proto()
115-
}
116-
}
131+
#[derive(Debug, Default, DynAny, serde::Serialize, serde::Deserialize)]
132+
pub struct BrushCache(Arc<Mutex<BrushCacheImpl>>);
117133

118134
// A bit of a cursed implementation to work around the current node system.
119135
// The original object is a 'prototype' that when cloned gives you a independent
120136
// new object. Any further clones however are all the same underlying cache object.
121137
impl Clone for BrushCache {
122138
fn clone(&self) -> Self {
123-
if self.proto {
124-
let inner_val = self.inner.lock().unwrap();
125-
Self {
126-
inner: Arc::new(Mutex::new(inner_val.clone())),
127-
proto: false,
128-
}
129-
} else {
130-
Self {
131-
inner: Arc::clone(&self.inner),
132-
proto: false,
133-
}
134-
}
139+
Self(Arc::new(Mutex::new(self.0.lock().unwrap().clone())))
135140
}
136141
}
137142

138143
impl PartialEq for BrushCache {
139144
fn eq(&self, other: &Self) -> bool {
140-
if Arc::ptr_eq(&self.inner, &other.inner) {
145+
if Arc::ptr_eq(&self.0, &other.0) {
141146
return true;
142147
}
143148

144-
let s = self.inner.lock().unwrap();
145-
let o = other.inner.lock().unwrap();
149+
let s = self.0.lock().unwrap();
150+
let o = other.0.lock().unwrap();
146151

147152
*s == *o
148153
}
149154
}
150155

151156
impl Hash for BrushCache {
152157
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
153-
self.inner.lock().unwrap().hash(state);
158+
self.0.lock().unwrap().hash(state);
154159
}
155160
}
156161

157162
impl BrushCache {
158-
pub fn new_proto() -> Self {
159-
Self {
160-
inner: Default::default(),
161-
proto: true,
162-
}
163-
}
164-
165163
pub fn compute_brush_plan(&self, background: Instance<Raster<CPU>>, input: &[BrushStroke]) -> BrushPlan {
166-
let mut inner = self.inner.lock().unwrap();
164+
let mut inner = self.0.lock().unwrap();
167165
inner.compute_brush_plan(background, input)
168166
}
169167

170168
pub fn cache_results(&self, input: Vec<BrushStroke>, blended_image: Instance<Raster<CPU>>, last_stroke_texture: Instance<Raster<CPU>>) {
171-
let mut inner = self.inner.lock().unwrap();
169+
let mut inner = self.0.lock().unwrap();
172170
inner.cache_results(input, blended_image, last_stroke_texture)
173171
}
174172

175173
pub fn get_cached_brush(&self, style: &BrushStyle) -> Option<Raster<CPU>> {
176-
let inner = self.inner.lock().unwrap();
174+
let inner = self.0.lock().unwrap();
177175
inner.brush_texture_cache.get(style).cloned()
178176
}
179177

180178
pub fn store_brush(&self, style: BrushStyle, brush: Raster<CPU>) {
181-
let mut inner = self.inner.lock().unwrap();
179+
let mut inner = self.0.lock().unwrap();
182180
inner.brush_texture_cache.insert(style, brush);
183181
}
184182
}

0 commit comments

Comments
 (0)