Skip to content

Fix cached textures lifetime#4333

Open
timon-schelling wants to merge 3 commits into
masterfrom
fix-cached-textures-lifetime
Open

Fix cached textures lifetime#4333
timon-schelling wants to merge 3 commits into
masterfrom
fix-cached-textures-lifetime

Conversation

@timon-schelling

Copy link
Copy Markdown
Member

Fixing

wgpu error: Validation Error

Caused by:
  In Queue::submit
    Texture with 'cached_texture_1954x1712' label has been destroyed

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request refactors the codebase to replace ImageTexture and raw Arc<wgpu::Texture> usages with a unified Texture type from raster-types. It updates signatures and dependencies across several crates, including editor, wgpu-executor, and gstd, and removes manual texture drops. The review feedback highlights a potential compilation failure when building without the wgpu feature due to a missing fallback Texture definition in raster-types. Additionally, it is recommended to use the idiomatic Raster::new_gpu helper function instead of manually constructing the GPU struct in per_pixel_adjust_runtime.rs.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread node-graph/libraries/raster-types/src/raster_types.rs
Comment thread node-graph/libraries/wgpu-executor/src/shader_runtime/per_pixel_adjust_runtime.rs Outdated

@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 18 files

Confidence score: 5/5

  • In node-graph/libraries/wgpu-executor/src/shader_runtime/per_pixel_adjust_runtime.rs, constructing the GPU path manually instead of using Raster::new_gpu(tex_out) mainly risks style drift and slightly higher maintenance overhead, not a functional regression; merging as-is is low risk, but switching to Raster::new_gpu(tex_out) would keep this code consistent with the rest of the codebase before or shortly after merge.
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/wgpu-executor/src/shader_runtime/per_pixel_adjust_runtime.rs">

<violation number="1" location="node-graph/libraries/wgpu-executor/src/shader_runtime/per_pixel_adjust_runtime.rs:237">
P3: `Raster::new_gpu(tex_out)` is a helper designed for exactly this purpose — it accepts `impl Into<Texture>` and constructs the `GPU` struct internally. Using it here would be more concise and consistent with the rest of the codebase (e.g., `runtime.rs` and `export.rs` both use `Raster::new_gpu`).</violation>
</file>

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

Re-trigger cubic

Comment thread node-graph/libraries/wgpu-executor/src/shader_runtime/per_pixel_adjust_runtime.rs Outdated
unpadded_bytes_per_row,
padded_bytes_per_row,
// Keep source texture alive
_source: data_gpu.texture.clone(),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would expect this to be the only actual fix right? Why is the diff so large?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, the Arc inside Raster is important, this case here is more theoretical. The rest is changes needed because of the different types and removal of the duplicated ImageTexture struct. No need to have multiple struct Type(Arc<wgpu::Texture>).

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