wgsl_to_wgpu icon indicating copy to clipboard operation
wgsl_to_wgpu copied to clipboard

Require initialization of RenderPass

Open AndrewBrownK opened this issue 3 years ago • 4 comments

Require initialization of the RenderPass

  • bind group
  • vertex buffers

I feel like there is a lot of potential along these lines (as wide as the RenderPass set of methods, almost). So we just have to decide how much the generated shader can tell us, and where we want to draw the line.

For example, we might be able to make a struct dedicated to each RenderPipeline so that a only a correctly typed RenderPipeline can be set on the EnhancedRenderPass. However we'd also need a way to reuse the inner RenderPass for multiple different RenderPipelines. It should be possible, but is not in this implementation yet.

AndrewBrownK avatar Apr 14 '23 22:04 AndrewBrownK

So we just have to decide how much the generated shader can tell us, and where we want to draw the line.

My goal with this project is to facilitate a shader driven workflow with changes in WGSL being reflected in Rust or at least giving a compile error. I also don't want to impose any unnecessary restrictions on what people are able to do. Tracking the bind groups and vertex buffers definitely sounds like something that would be nice to have and generate relatively little code.

Trying to completely validate the render pass could start looking more like library code. I'd like to avoid generating a duplicate rendering library for each shader if possible. It should be clearer after discussing a few issues whether this makes sense for wgsl_to_wgpu or if you could make it into its own project.

For example, we might be able to make a struct dedicated to each RenderPipeline so that a only a correctly typed RenderPipeline can be set on the EnhancedRenderPass.

All the state you listed is tied to the pipeline rather than the pass. This would require each pipeline to be a separate type. This looks very similar to vulkano's design. You could do something like builder.begin_pipeline(pipeline).set_vertex_buffer(...).set_bind_group0(...).... vulkano tries to prevent invalid API usage using a combination of compile time and runtime checks similar to wgpu. You would need to think through how to do things at compile time to avoid being redundant with wgpu.

The main issue is what the user does once all the state we can track is set. The wgpu::RenderPass API has a pretty large surface area. The only thing that comes to mind right now is to somehow own the render pass and only return the render pass in a build() method. This wouldn't prevent people from then setting the wrong bind groups after the fact.

ScanMountGoat avatar Apr 14 '23 23:04 ScanMountGoat

If you only need two states, you could try using const generics with booleans instead.

ScanMountGoat avatar Apr 14 '23 23:04 ScanMountGoat

how much the generated shader

I was not very precise here. I should have/meant to say the generated rust code for the shader. I follow the shader-driven workflow angle. My mistake communicating here.

Emphasizing the fact that the buffers and bind groups are associated with the RenderPipeline and not the RenderPass makes sense.

I'm also hearing you on not wanting to duplicate more or less an entire library for every generated rust module. You are probably right that returning the RenderPass in a final build-like method is a good default course of action in this regard. Doesn't replicate all of RenderPass's belongings too much.

With regards to two states and const generic booleans, I lean towards the named structs instead. This is because none of it affects runtime performance anyway, and the named structs are much more apparent in meaning to the programmer.

I'll keep playing with it a bit with these things in mind

AndrewBrownK avatar Apr 17 '23 20:04 AndrewBrownK

Emphasizing the fact that the buffers and bind groups are associated with the RenderPipeline and not the RenderPass makes sense.

We should be able to track whether the associated state has been set using generics like in your current implementation. The main challenge is making it intuitive to use and give a meaningful error when the user doesn't correctly specify all the required state. It's also worth mentioning that bind groups or vertex buffers can be set multiple times. It might not make sense for the render pass wrapper to be "consumed" when calling the method that returns the render pass.

In regards to vertex buffers, there currently isn't any way to track what type of data is in each buffer. Multiple types of data can use the same buffer at different offsets, so there isn't an obvious way of doing this. Just checking that some sort of buffer slice is set for each vertex input is probably fine. You seem to already be using slices, which handles the case where vertex inputs share the same buffer.

Different pipelines may have the same vertex inputs. Changing from solid to wireframe shading still requires a pipeline change, for example. I'm not sure if it's worth tracking buffer slots across pipelines. We can probably just assume each pipeline has different inputs and set the vertex state to uninitialized when changing pipelines.

Let me know if you make any progress or run into any issues. This would help resolve a lot of the limitations with the existing code if we can get it working.

ScanMountGoat avatar Apr 18 '23 20:04 ScanMountGoat

I'm going to close this due to inactivity. Feel free to reopen this or make a new pull request if you decide to work on this again or have any other ideas.

ScanMountGoat avatar Mar 16 '24 00:03 ScanMountGoat