Add AdapterInfo to WgpuOptions

[Weasy666]

Objective

Make it easy to access adapter info in a bevy program. Can be useful to display such information in a dev ui or to send it with an error report.

Solution

Add AdapterInfo to WgpuOptions after an adapter is set up. <- chanded this to its own "view" resource RendererInfo

[mockersf]

I think WgpuOptions should be split between the options used to setup wgpu, and the result of this setup.

Does it ever make sense for the user to set a custom value for the adapter_info? If not, it would be better to have it in a private field with a public method to access the value, instead of leaving it open.

[Weasy666]

I went with a public field, because backends: Option<Backends> does the same and i was under the impression that backends is also nothing a user would/should ever set as it comes from wgpu. It at least looked that way to me (if my editor wasn't lying to me 😅)

[mockersf]

backends can be used by a user to force using OpenGL or Vulkan or... or set to None to run in headless.

I think features and limits can be overwritten from what the user specified in some case, I would rather have a difference between the values set by the user and the values actually used for wgpu

[Weasy666]

So....does the default value of backends (Some(VULKAN | METAL | DX12 | BROWSER_WEBGPU | PRIMARY)) mean that wgpu tries to use them in that order?
I am not against making AdapterInfo private and use a getter function. Another option (like u suggested) would be to add a new struct/resource RendererInfo that holds all info of the currently active renderer.
Which way do you prefer? Personally, i am leaning towards the extra struct/resource, because it is cleaner, but has the downside that its fields need to stay in sync with changes to WgpuOptions.

[mockersf]

So....does the default value of backends (Some(VULKAN | METAL | DX12 | BROWSER_WEBGPU | PRIMARY)) mean that wgpu tries to use them in that order?

No, it means wgpu is allowed to pick between those backends based on the hardware available and the other limits. There are no notion of order.

I am not against making AdapterInfo private and use a getter function. Another option (like u suggested) would be to add a new struct/resource RendererInfo that holds all info of the currently active renderer.
Which way do you prefer? Personally, i am leaning towards the extra struct/resource, because it is cleaner, but has the downside that its fields need to stay in sync with changes to WgpuOptions.

I would prefer a new resource. There's no need to keep in sync with WgpuOptions as I'm pretty sure updating that resource during runtime has zero effects.

[Weasy666]

No, it means wgpu is allowed to pick between those backends based on the hardware available and the other limits. There are no notion of order.

Ah…ok, thanks for explaining.

I would prefer a new resource. There's no need to keep in sync with WgpuOptions as I'm pretty sure updating that resource during runtime has zero effects.

Then a new resource it will be, will do that tomorrow after i got my beauty sleep. 🙃😴

[Weasy666]

OK...i've added RendererInfo with all fields private and getter functions. It would be best if some native speaker takes a look at the doc strings.
I made RendererInfo a part of the return tuple of initialize_renderer, because if the initialization of the renderer somehow goes wrong, we don't have a renderer and don't have any info on it, so only on success do we have any reliable info which will be added as a resource.

[alice-i-cecile]

This is very, very useful for debugging. Can we link this from the module level docs of bevy_render?

[Weasy666]

Sure, can you point me to the module level docs? I can't seem to find them, bevy_render/lib.rs doesn't seem to have any? 😅

[alice-i-cecile]

Based on what's showing up on docs.rs, they appear to be found at

bevy/crates/bevy_internal/src/lib.rs

Line 115 in 130953c

//! Cameras, meshes, textures, shaders, and pipelines.

Tossing in another line under there should work fine for this PR.

[Weasy666]

Taking the way through doc.rs didn't come to my mind, thanks for pointing that out.
Something like this?
Information about the current renderer can be accessed through the resource [renderer::RendererInfo].

[alice-i-cecile]

Native English speaker: the doc strings are perfect :) This design is very useful, and should be highlighted in our teaching materials.

[superdump]

@Weasy666 oh! I’d missed this PR. I was literally doing the same thing - splitting out the configuration and the result. Chatting with @cart about it he asked the question “why can’t we use RenderDevice for this?” I had missed that it had limits and features methods and is already present in both the main and render worlds so inserting another resource with a copy of it is unnecessary duplication and it’s probably better that the wgpu Device be the sole source of truth. Then the missing information about which GPU and backend are used is the AdapterInfo, so that just needs inserting into both worlds. As such, I think #3931 is probably the preferred solution but as I made that PR, I’ll defer to @cart or someone else to make the judgement. Sorry for missing this PR and duplicating your effort! If it’s preferred then I can close that PR and we can perhaps update this one?

[Weasy666]

I initially did nearly the same thing as you did in your PR. But i changed it to its own resource, to make clear, that it is only intended to be a "view" for diagnostics, or some such, on the active renderer and its configuration, and nothing to set up or configure it.
So...yeah...let's wait and see what the preferred approach is.

[cart]

I think RenderDevice is the right place to query "actual wgpu limits and features". There should only be one place, and I think making RenderDevice that the one place makes the most sense because it is (1) guaranteed to be in sync with the state of wgpu (2) consistent across main and render apps (3) the same pattern wgpu uses, which increases our "conceptual compatibility" with that ecosystem.

WgpuOptions should not contain any dynamic information. It should be the place for "wgpu configuration requested at startup". I also think we should rename it to WgpuSettings for consistency with the "plugin settings as resources" pattern used elsewhere in Bevy crates.

That leaves AdapterInfo. Given that wgpu already has a type for that (and separates it conceptually from Devices), I think it makes the most sense to just insert that as an ECS resource.

[Weasy666]

Ok, then we can close this PR in favor of #3931, as that PR does tick nearly all boxes (except the rename).