Fix vello related GPU out of memory issues - #4446
Conversation
There was a problem hiding this comment.
1 issue found across 3 files
Confidence score: 3/5
- In
node-graph/libraries/rendering/src/renderer.rs, blob IDs are derived from cumulativeresource_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; |
There was a problem hiding this comment.
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>
27a7343 to
002d5d9
Compare
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.