Skip to content
This repository has been archived by the owner on Jun 18, 2021. It is now read-only.

[Blocked] Relax render/compute pass lifetimes #604

Closed
wants to merge 1 commit into from

Conversation

kvark
Copy link
Member

@kvark kvark commented Nov 4, 2020

Mostly reverts #168 given that wgpu-core is now properly handling error IDs.
Closes #603
Closes #188

Has a major positive usability impact 🚆 It is no longer required by the type system to keep resources alive for the duration of a render/compute pass encoding. It's still required by the runtime: dropping a pass will produce an error if some of the dependencies are dropped before it.

Looking at the error messages may be difficult until an approach similar to gfx-rs/wgpu#931 (comment) is rolled out in wgpu-rs (cc @scoopr). We'll make it a priority before releasing wgpu-0.7.

Edit: actually, the IDs here are not "error IDs" that wgpu-core handles. It will panic straight upon seeing such a stale ID...

@kvark kvark requested a review from cwfitzgerald November 4, 2020 22:46
Copy link

@monocodus monocodus bot left a comment

Choose a reason for hiding this comment

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

This is an autogenerated code review.

Checker summary (by rust_clippy):
The tool has found 10 warnings, 1 errors.

The .monocodus config not found in your repo. Default config is used.
Check config documentation here

@scoopr
Copy link
Contributor

scoopr commented Nov 5, 2020

I think the "stale IDs" will need to be handled more gracefully anyway. Consider, pushErrorScope, loadScene, popErrorScope, I would imagine should work without paniccing, no matter if some of the IDs are invalid on creation or dropped at the wrong time.

I'm so conflicted about this. I do love when the compiler tells me something is wrong, but in this particular case, the papercut is so such a big wound that I do hope good documentation and error messages will be enough.

@kvark kvark changed the title Relax render/compute pass lifetimes [Blocked] Relax render/compute pass lifetimes Nov 5, 2020
Copy link

@monocodus monocodus bot left a comment

Choose a reason for hiding this comment

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

This is an autogenerated code review.

Checker summary (by rust_clippy):
The tool has found 10 warnings, 1 errors.

The .monocodus config not found in your repo. Default config is used.
Check config documentation here

@ghost
Copy link

ghost commented Mar 29, 2021

Just curious, what is this blocked by? Deciding if I should wait for this.

@kvark
Copy link
Member Author

kvark commented Mar 29, 2021

It's blocked because of this:

Edit: actually, the IDs here are not "error IDs" that wgpu-core handles. It will panic straight upon seeing such a stale ID...

We don't want the panics for this, since WebGPU API doesn't allow panics there. At the same time, we don't want to add special handling of this in wgpu-core because it would have no way to tell if the ID was wrong to begin with (in which case we should panic, since this can't happen in WebGPU proper), or just got removed after it was used...

So @definitelynotrobot should probably not wait for this. If you are constrained by the lifetimes here, consider typed_arena to keep stuff that needs extended lifetimes.

@pythonesque
Copy link

I actually really like the lifetime discipline enforced here, I'd much rather get an error at compile time than runtime for stuff like this.

@kvark
Copy link
Member Author

kvark commented Jun 3, 2021

Project moved to wgpu

@kvark kvark closed this Jun 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Demonstrate TypedArena use in an example Lifetimes on RenderPass make it difficult to use.
3 participants