Poll: GPU type refactor and bevy_gpu split

[kurtkuehnert]

I am working on splitting up the bevy_render and bevy_gpu code.
This change will affect most of the existing code and render plugins, which is why I want to make sure that you are on board with this refactor.

There are two open design decisions, that need to be answered.
Please take a look at issue #7157 and tell me which combination of naming and exporting strategies you prefer.

// Bevy Main
use bevy::render::{renderer::RenderDevice, settings::WgpuSettings, render_resource::{Buffer, RenderPipeline}};

// Gpu Prefix & Root Export
use bevy::gpu::{GpuDevice, GpuSettings, GpuBuffer, GpuRenderPipeline};

// Prefixless & Root Export
use bevy::gpu::{Device, Settings, Buffer, RenderPipeline};

// Hybrid & Root Export
use bevy::gpu::{GpuDevice, GpuSettings, Buffer, RenderPipeline};

// Gpu Prefix & Module Export
use bevy::gpu::{GpuDevice, GpuSettings, gpu_resource::{GpuBuffer, GpuRenderPipeline}};

// Prefixless & Module Export
use bevy::gpu::{Device, Settings, gpu_resource::{Buffer, RenderPipeline}};

// Hybrid & Module Export
use bevy::gpu::{GpuDevice, GpuSettings, gpu_resource::{Buffer, RenderPipeline}};

[superdump]

Not to sway anyone's opinions, rather to document the nuances and reasoning I have...:

• I don't like the hybrid options because of the inconsistency in naming. Or I could put it as if we're going to restructure/rename then I'd prefer more consistency.
• I was surprised when @kurtkuehnert reported that the prefixless options worked without collisions and my concern there was that there would be collisions but it seems that as these are all used in rendering code, their context is clear and collisions haven't happened yet. Even if they did happen, one can always import the module and qualify (gpu::Buffer or so) instead so maybe not a big deal.
• Still, I feel like I like the consistency of Gpu prefix everywhere.
• I don't have a strong opinion about root vs module export.

[kurtkuehnert]

I totally agree.
The Gpu prefix sounds nice, but I think that it will be quit repetitive.
We will have to type 3/4 characters more for every gpu variable/type, because autocomplete can not distinguish between them based on the prefix.

Also the missing context for e.g. Settings may be a bit confusing.

The problem with both Hybrid and Module Export is that it will be hard to decide what to put where.
It is pretty easy for Buffer, BindGroup, etc.
But wgpu has got many additional types, like RenderPassDepthStencilAttachment or BufferBindingType. Do we really prefix all of them? Where do we draw the line. And are they gpu_resources, or should we export them at the root?

[robtfm]

i definitely prefer prefixless - there's actually a (pedantic) clippy lint specifically for "don't start all your struct names with your module name" - https://rust-lang.github.io/rust-clippy/master/#module_name_repetitions

on root vs module export, there's a level of abstraction difference between the root and the render_resources, i think there's some value to having the more friendly stuff in the top level and the lower level stuff behind a curtain.
i also find that once i start using something out of render_resource i often need 50 things, so i use render_resource::* a lot. i don't necessarily want everything from the outer module in scope.

[kurtkuehnert]

What even is a gpu_resource?
Buffer, Texture, etc. for sure. Also their descriptors? And what about types like PipelineStatisticsTypes or Extent3d are they really gpu_resources?
If we fragment the modules too much importing will become very painful.

[kurtkuehnert]

IMO we should follow the wgpu route of having one large root module instead of many small ones.

[robtfm]

agree that the current split may not be ideal, but i think having "Bevy" things that can be used without worrying about wgpu, like Camera, RenderLayers, Mesh, etc at a higher level than the low-level details of BindGroupLayoutEntry and DynamicUniformBuffer is useful. IMO render_resource should be wgpu wrappers and helpers for wgpu types.

[kurtkuehnert]

I agree, but the question is: If we split bevy_render and bevy_gpu do we still need to subdivide bevy_gpu?

bevy_render will retain its current structure, regardless of the export strategy we chose for bevy_gpu.

Our GPU wrappers (e.g. Buffer, RenderPipeline) are pretty lightweight for the most part.
Only Shader and PipelineCache, as well as UniformBuffer, etc. are significant abstractions on top of wgpu.
Can/Should we export them all at once, or are there useful submodules to be found? If so which ones should we add?

[robtfm]

ah! i misunderstood, i thought this was a wholesale rename of bevy_render. if we're keeping bevy_render with the higher level types then i agree that the distinction is not useful.

[soqb]

i think Buffer is too general and is likely to cause conflicts (which are of course avoidable) but GpuBuffer makes sense. but GpuRenderPipeline is too specific because Render*** implies Gpu***.

i'm not a gpu dev so my opinion might not be as important here but surely there are contexts where you might want wgpu::Device and bevy_gpu::Device in the same scope?

[kurtkuehnert]

Yes, I do agree that Buffer is pretty vague. On the other hand, we have been using this naming scheme for all of our "GPU resources" for over a year already and I have never heard anybody complain about it, so we might be fine.

Actually, we might need the raw wgpu types, for interop with external crates build on top of wgpu.
Currently, we expose some of the types that we overload using a Raw prefix (e.g. VertexState as RawVertexState.
I think this pattern is great. Alternatively, we can simply prefix them with wgpu:: as you said.

[CptPotato]

i think Buffer is too general and is likely to cause conflicts (which are of course avoidable) but GpuBuffer makes sense. but GpuRenderPipeline is too specific because Render*** implies Gpu***.

This might be just me but I actually prefer Buffer.

Within the module bevy::gpu it's pretty clear that it refers to a buffer on the Gpu in my opinion. Using types like this in a "mixed" context (where it's not obvious) I tend to import the module and qualify the type name to be more specific - i.e. gpu::Buffer or io::Result.
Not sure if this is idiomatic rust, but I personally like it.

[kurtkuehnert]

Yes, that is exactly my opinion as well 😄.
It seems that most people are on board with this refactor. 👍