Skip to content

GPU: add support for N-many input images and not just exactly one#4166

Open
Firestar99 wants to merge 1 commit into
masterfrom
fire/gpu-multi-input-images
Open

GPU: add support for N-many input images and not just exactly one#4166
Firestar99 wants to merge 1 commit into
masterfrom
fire/gpu-multi-input-images

Conversation

@Firestar99
Copy link
Copy Markdown
Collaborator

I'm sitting in the Eurostar from rustweek to Paris, running down the track at 300km/h, with a 30min delay of course... and finally found some time for something I wanted to add right after GSoC but never came around to it :D

GPU blend nodes
image

  • add support for N-many image inputs for nodes, not just one
    • enables blend GPU node with 2 inputs
      • I could not get the CPU-based blend node to work, it'll always error out, so can't compare it's outputs
    • Can you think of other nodes this could enable?
  • TODO support for 0 input textures
    • the output texture and format is determined from the first input image and format, matching behavior of blend node writing it's result into the first input texture
    • with no first input texture, there's no way to determine the output texture resolution
    • mandelbrot node's output image is created based on viewport and some weird code computing something?
  • Makes multiple assumptions on how raster images should behave. I feel like a universal definition on how raster textures should handle things would make this a lot easier:
    • size of the output texture? Assumption: based on first input texture
    • format of the output texture? Assumption: currently first input texture, and upload uses RGBA8 so every texture is that
      • changing away from RGBA8 requires a pass to convert ton RGBA8 before using it with vello
    • 2 input textures with mismatched input sizes: couldn't verify CPU impl, currently just places image in the top left and matches pixel by pixel, no scaling

@Firestar99 Firestar99 requested review from Keavon and TrueDoctor May 24, 2026 17:48
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

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 introduces support for multiple input images within the PerPixelAdjust shader runtime and macro. It updates the runtime to process a slice of texture lists, implements dynamic binding allocation using a new Counter utility, and synchronizes dispatch counts based on the shortest input list. Feedback focused on performance improvements, specifically recommending the use of Vec::with_capacity for the entries vectors during pipeline creation and within the dispatch loop to minimize re-allocations.

},
count: None,
let mut binding_alloc = Counter::default();
let mut entries = Vec::new();
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.

medium

Consider pre-allocating the entries vector with the known capacity to avoid potential re-allocations as elements are added during pipeline creation.

Suggested change
let mut entries = Vec::new();
let mut entries = Vec::with_capacity(info.input_images + info.has_uniform as usize);

let out = (0..dispatch_cnt)
.map(|dispatch_id| {
let mut binding_alloc = Counter::default();
let mut entries = Vec::new();
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.

medium

In the dispatch loop, entries is allocated for every image in the list. Pre-allocating with with_capacity based on the number of input images and the uniform buffer presence will improve performance, especially when processing long lists of images.

Suggested change
let mut entries = Vec::new();
let mut entries = Vec::with_capacity(self.input_images + self.has_uniform as usize);

Copy link
Copy Markdown
Contributor

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

Choose a reason for hiding this comment

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

No issues found across 3 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@TrueDoctor
Copy link
Copy Markdown
Member

The mandelbrot node computes only the which is going to be visible in the viewport and at the correct resolution, that is what that math is for

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