Skip to content

Fix vello related GPU out of memory issues - #4446

Open
timon-schelling wants to merge 3 commits into
pen-events-web-prfrom
fix-vello-texture-leaks-pr
Open

Fix vello related GPU out of memory issues#4446
timon-schelling wants to merge 3 commits into
pen-events-web-prfrom
fix-vello-texture-leaks-pr

Conversation

@timon-schelling

Copy link
Copy Markdown
Member

Updates vello to include this workaround that I upstreamed

And makes blob ids passed to vello stable across frames, allowing vello to reuse texture atlas slots.

@cubic-dev-ai cubic-dev-ai Bot left a comment

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.

1 issue found across 3 files

Confidence score: 3/5

  • In node-graph/libraries/rendering/src/renderer.rs, blob IDs are derived from cumulative resource_overrides.len() plus dimensions, so scene insertions/removals/reordering can change IDs frame-to-frame and misidentify previously rendered images; this creates concrete rendering/cache regression risk when scene composition changes—switch to a stable per-resource key (order-independent) for blob ID generation.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="node-graph/libraries/rendering/src/renderer.rs">

<violation number="1" location="node-graph/libraries/rendering/src/renderer.rs:2196">
P2: The blob id is keyed on the cumulative `resource_overrides.len()` index plus width/height, so it is not actually stable across frames whenever the scene changes. Adding, removing, or reordering any image rendered before this raster shifts its index and changes the id, so vello allocates a fresh atlas slot instead of reusing one — which is exactly the growth the PR is meant to stop (e.g. while editing live). The id also carries no identity of the actual texture content, so two different rasters with the same `(index, width, height)` collapse to one vello atlas id and are treated as a single image in the cross-frame cache. Consider deriving the id from a per-raster identity (e.g. the texture handle/NodeId) rather than a transient scene-order index.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic


let resource_override_index = context.resource_overrides.len();
// Stable across frames so vello reuses the atlas slot; high bit avoids Blob::new counter ids.
let blob_id = (resource_override_index as u64) << 40 | (width as u64) << 20 | height as u64 | 1 << 63;

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: The blob id is keyed on the cumulative resource_overrides.len() index plus width/height, so it is not actually stable across frames whenever the scene changes. Adding, removing, or reordering any image rendered before this raster shifts its index and changes the id, so vello allocates a fresh atlas slot instead of reusing one — which is exactly the growth the PR is meant to stop (e.g. while editing live). The id also carries no identity of the actual texture content, so two different rasters with the same (index, width, height) collapse to one vello atlas id and are treated as a single image in the cross-frame cache. Consider deriving the id from a per-raster identity (e.g. the texture handle/NodeId) rather than a transient scene-order index.

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

<comment>The blob id is keyed on the cumulative `resource_overrides.len()` index plus width/height, so it is not actually stable across frames whenever the scene changes. Adding, removing, or reordering any image rendered before this raster shifts its index and changes the id, so vello allocates a fresh atlas slot instead of reusing one — which is exactly the growth the PR is meant to stop (e.g. while editing live). The id also carries no identity of the actual texture content, so two different rasters with the same `(index, width, height)` collapse to one vello atlas id and are treated as a single image in the cross-frame cache. Consider deriving the id from a per-raster identity (e.g. the texture handle/NodeId) rather than a transient scene-order index.</comment>

<file context>
@@ -2190,8 +2190,13 @@ impl Render for List<Raster<GPU>> {
+
+			let resource_override_index = context.resource_overrides.len();
+			// Stable across frames so vello reuses the atlas slot; high bit avoids Blob::new counter ids.
+			let blob_id = (resource_override_index as u64) << 40 | (width as u64) << 20 | height as u64 | 1 << 63;
+			let blob = peniko::Blob::from_raw_parts(LAZY_ARC_VEC_ZERO_U8.deref().clone(), blob_id);
 			let image = peniko::ImageBrush::new(peniko::ImageData {
</file context>

@timon-schelling
timon-schelling force-pushed the fix-vello-texture-leaks-pr branch from 27a7343 to 002d5d9 Compare August 17, 2026 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants