Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate renderpass arc resolve & renderpass consume on end #5794

Merged
merged 4 commits into from
Jun 25, 2024

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Jun 10, 2024

Connections

Description

As before with ComputePass, we now convert all incoming RenderCommand first to ArcRenderCommand and then consume only those upon pass ending.
Unlike with ComputePass, we already had ArcRenderCommand around (because of render bundles), I "only" had to fill in the gaps here.

Testing
Existing tests pass. Similar to how to compute passes, more are to come for the actual lifetime removal

Checklist

  • Run cargo fmt.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
    • --target wasm32-unknown-emscripten
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.

@Wumpf Wumpf requested a review from a team as a code owner June 10, 2024 11:39
@ErichDonGubler ErichDonGubler self-requested a review June 10, 2024 18:17
@ErichDonGubler ErichDonGubler self-assigned this Jun 10, 2024
@ErichDonGubler ErichDonGubler added the kind: refactor Making existing function faster or nicer label Jun 10, 2024
@ErichDonGubler
Copy link
Member

I'll take on review for this.

@Wumpf Wumpf force-pushed the separate-renderpass-arc-resolve branch from 90fb38c to 973201f Compare June 16, 2024 21:12
@Wumpf Wumpf force-pushed the separate-renderpass-arc-resolve branch from 973201f to 0e85ae2 Compare June 23, 2024 19:36
@Wumpf
Copy link
Member Author

Wumpf commented Jun 23, 2024

@ErichDonGubler bump? :)
Just did a fairly painful rebase after Teo did some great improvements to some error handling and avoidance of arc clone, but naturally it conflicted a lot with this and even more with the follow-up branch I have almost done for a while now.

@teoxoy
Copy link
Member

teoxoy commented Jun 24, 2024

I got some more commits coming in that touch this code so I asked Erich if I can look at this PR to minimize fixing conflicts.

@teoxoy teoxoy requested review from teoxoy and removed request for ErichDonGubler June 24, 2024 17:00
@teoxoy teoxoy assigned teoxoy and unassigned ErichDonGubler Jun 24, 2024
Copy link
Member

@teoxoy teoxoy left a comment

Choose a reason for hiding this comment

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

Left a few comments about the new DrawKind. Looks good otherwise!

wgpu-core/src/command/bundle.rs Outdated Show resolved Hide resolved
wgpu-core/src/command/bundle.rs Outdated Show resolved Hide resolved
wgpu-core/src/command/render.rs Outdated Show resolved Hide resolved
wgpu-core/src/command/render_command.rs Outdated Show resolved Hide resolved
@Wumpf
Copy link
Member Author

Wumpf commented Jun 24, 2024

great catch, thank you!

@Wumpf Wumpf requested a review from teoxoy June 24, 2024 17:43
@Wumpf Wumpf force-pushed the separate-renderpass-arc-resolve branch from 307c84f to 6ec5d34 Compare June 24, 2024 17:45
@teoxoy teoxoy merged commit b4c7987 into gfx-rs:trunk Jun 25, 2024
25 checks passed
@Wumpf Wumpf deleted the separate-renderpass-arc-resolve branch June 25, 2024 08:06
@Wumpf Wumpf mentioned this pull request Jun 26, 2024
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: refactor Making existing function faster or nicer
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants