Skip to content

Commit 42440c0

Browse files
Fix rasterize node document leakage with hashmap and eq check (#3550)
* fix: rasterize node document leakage with hashmap and eq check * Use single Hashmap and ignore source id * use or_insert_with instead of Entry match --------- Co-authored-by: Dennis Kobert <dennis@kobert.dev>
1 parent ee2e61f commit 42440c0

4 files changed

Lines changed: 27 additions & 17 deletions

File tree

node-graph/libraries/no-std-types/src/color/color_types.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ impl Pixel for Luma {}
216216
/// The other components (RGB) are stored as `f32` that range from `0.0` up to `f32::MAX`,
217217
/// the values encode the brightness of each channel proportional to the light intensity in cd/m² (nits) in HDR, and `0.0` (black) to `1.0` (white) in SDR color.
218218
#[repr(C)]
219-
#[derive(Debug, Default, Clone, Copy, PartialEq, Pod, Zeroable, BufferStruct)]
219+
#[derive(Debug, Default, Clone, Copy, Pod, Zeroable, BufferStruct)]
220220
#[cfg_attr(feature = "std", derive(dyn_any::DynAny, specta::Type, serde::Serialize, serde::Deserialize))]
221221
pub struct Color {
222222
red: f32,
@@ -225,6 +225,14 @@ pub struct Color {
225225
alpha: f32,
226226
}
227227

228+
impl PartialEq for Color {
229+
fn eq(&self, other: &Self) -> bool {
230+
self.red == other.red && self.green == other.green && self.blue == other.blue && self.alpha == other.alpha
231+
}
232+
}
233+
234+
impl Eq for Color {}
235+
228236
#[allow(clippy::derived_hash_with_manual_eq)]
229237
impl Hash for Color {
230238
fn hash<H: core::hash::Hasher>(&self, state: &mut H) {

node-graph/libraries/raster-types/src/image.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ mod base64_serde {
3939
}
4040
}
4141

42-
#[derive(Clone, PartialEq, Default, specta::Type, serde::Serialize, serde::Deserialize)]
42+
#[derive(Clone, Eq, Default, specta::Type, serde::Serialize, serde::Deserialize)]
4343
pub struct Image<P: Pixel> {
4444
pub width: u32,
4545
pub height: u32,
@@ -53,6 +53,12 @@ pub struct Image<P: Pixel> {
5353
// TODO: Currently it is always anchored at the top left corner at (0, 0). The bottom right corner of the new origin field would correspond to (1, 1).
5454
}
5555

56+
impl<P: Pixel + PartialEq> PartialEq for Image<P> {
57+
fn eq(&self, other: &Self) -> bool {
58+
self.width == other.width && self.height == other.height && self.data == other.data
59+
}
60+
}
61+
5662
#[derive(Debug, Clone, dyn_any::DynAny, Default, PartialEq, serde::Serialize, serde::Deserialize, specta::Type)]
5763
pub struct TransformImage(pub DAffine2);
5864

node-graph/libraries/rendering/src/renderer.rs

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use kurbo::Shape;
2626
use num_traits::Zero;
2727
use std::collections::{HashMap, HashSet};
2828
use std::fmt::Write;
29-
use std::hash::{DefaultHasher, Hash, Hasher};
29+
use std::hash::{Hash, Hasher};
3030
use std::ops::Deref;
3131
use std::sync::{Arc, LazyLock};
3232
use vello::*;
@@ -59,7 +59,7 @@ pub struct SvgRender {
5959
pub svg: Vec<SvgSegment>,
6060
pub svg_defs: String,
6161
pub transform: DAffine2,
62-
pub image_data: Vec<(u64, Image<Color>)>,
62+
pub image_data: HashMap<Image<Color>, u64>,
6363
indent: usize,
6464
}
6565

@@ -69,7 +69,7 @@ impl SvgRender {
6969
svg: Vec::default(),
7070
svg_defs: String::new(),
7171
transform: DAffine2::IDENTITY,
72-
image_data: Vec::new(),
72+
image_data: HashMap::new(),
7373
indent: 0,
7474
}
7575
}
@@ -1239,23 +1239,18 @@ impl Render for Table<Raster<CPU>> {
12391239
fn render_svg(&self, render: &mut SvgRender, render_params: &RenderParams) {
12401240
for row in self.iter() {
12411241
let image = row.element;
1242+
12421243
let transform = *row.transform;
12431244

12441245
if image.data.is_empty() {
12451246
continue;
12461247
}
12471248

12481249
if render_params.to_canvas() {
1249-
let id = row.source_node_id.map(|x| x.0).unwrap_or_else(|| {
1250-
let mut state = DefaultHasher::new();
1251-
image.data().hash(&mut state);
1252-
state.finish()
1253-
});
1254-
if !render.image_data.iter().any(|(old_id, _)| *old_id == id) {
1255-
let mut image = image.data().clone();
1256-
image.map_pixels(|p| p.to_unassociated_alpha());
1257-
render.image_data.push((id, image));
1258-
}
1250+
let mut image_copy = image.clone();
1251+
image_copy.data_mut().map_pixels(|p| p.to_unassociated_alpha());
1252+
let id = *render.image_data.entry(image_copy.into_data()).or_insert_with(generate_uuid);
1253+
12591254
render.parent_tag(
12601255
"foreignObject",
12611256
|attributes| {

node-graph/nodes/gstd/src/render_node.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,13 @@ use graphic_types::raster_types::Image;
1313
use graphic_types::raster_types::{CPU, Raster};
1414
use rendering::{Render, RenderOutputType as RenderOutputTypeRequest, RenderParams, RenderSvgSegmentList, SvgRender, format_transform_matrix};
1515
use rendering::{RenderMetadata, SvgSegment};
16+
use std::collections::HashMap;
1617
use std::sync::Arc;
1718
use vector_types::GradientStops;
1819
use wgpu_executor::RenderContext;
1920

2021
/// List of (canvas id, image data) pairs for embedding images as canvases in the final SVG string.
21-
type ImageData = Vec<(u64, Image<Color>)>;
22+
type ImageData = HashMap<Image<Color>, u64>;
2223

2324
#[derive(Clone, dyn_any::DynAny)]
2425
pub enum RenderIntermediateType {
@@ -162,7 +163,7 @@ async fn render<'a: 'n>(ctx: impl Ctx + ExtractFootprint + ExtractVarArgs, edito
162163
rendering.wrap_with_transform(footprint.transform, Some(logical_resolution));
163164
RenderOutputType::Svg {
164165
svg: rendering.svg.to_svg_string(),
165-
image_data: rendering.image_data,
166+
image_data: rendering.image_data.into_iter().map(|(image, id)| (id, image)).collect(),
166167
}
167168
}
168169
(RenderOutputTypeRequest::Vello, RenderIntermediateType::Vello(vello_data)) => {

0 commit comments

Comments
 (0)