WgpuOptionsPriority::Functionality overwrites app preferences with suboptimal configuration

[djeedai]

Bevy version

0.6.0

Operating system & version

Windows 10 (but issue is independent)

What you did

Run any app on Windows 10 NVIDIA GeForce RTX 2070 with Vulkan.

What you expected to happen

No warning.

What actually happened

WARN wgpu_core::instance: Feature MAPPABLE_PRIMARY_BUFFERS enabled on a discrete gpu. This is a massive performance footgun and likely not what you wanted

Additional information

I tried setting explicitly Features::MAPPABLE_PRIMARY_BUFFERS off in WgpuOptions, without success.

Eventually I ran under debugger and found that the default WgpuOptionsPriority value is Functionality, which in bevy_render while creating device will override any app's setting with those of the selected adapter:

bevy/crates/bevy_render/src/renderer/mod.rs

Lines 88 to 92 in 8e1f660

	if matches!(options.priority, WgpuOptionsPriority::Functionality) {
	options.features =
	adapter.features() | wgpu::Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES;
	options.limits = adapter.limits();
	}

While this makes sense for a lot of those settings, like the various hardware limits, this makes no sense for several flags which being reported by adapter.features() only inform the app that the GPU supports the feature, but in no way was intended to tell the app that this is an optimal feature to use. This is the case in particular for MAPPABLE_PRIMARY_BUFFERS which is supported on an RTX 2070 but as the warning shows is strongly discouraged. Yet the bevy_render code simply set options.features = adapter.features() indiscriminately.

[superdump]

Good catch. So we should:

1. Check for other such features that are not recommended to be used by default and disable them.
2. Respect any user-defined settings.

[superdump]

kvark from wgpu noted that enabling all supported features by default is a 'very bad practice'. I'm investigating what to do about this. (FYI @cart )

My initial feeling is that we could make a list of features that are safe to enable by default if supported and filter the adapter supported features by those.

In terms of respecting user-defined settings it becomes a bit more tricky. We currently only have the single Features member of WgpuOptions and we don't know if a flag was set or not by a user so we don't know if what is there is an explicit request. We could make it into an Option so if it is set then we use it, else we use what the adapter deems ok. A shortcoming of that is the inability to use the max (safe) supported features but disabling something that isn't good on one particular card or something. Another approach could be to have two members: enabled_features and disabled_features. Then we could use adapter or downlevel features | enabled_features to enable the specified features there, and then - disabled_features to disable those. I think this gives us more flexibility, though it's a bit more complicated.

EDIT: After some more discussion it sounds like the 'profiles' of Functionality/Compatibility/WebGL2 are ok if we think they are, but we should not auto-enable everything for Functionality as MAPPABLE_PRIMARY_BUFFERS can be supported but have the noted significant performance impact if resizable bar is not supported. So I think we should create some explicit allow-list of features to auto-enable when using the Functionality priority.

[superdump]

I just realised that as bevy’s core renderer only uses WebGL2 functionality at the moment pretty much, you could use the Compatibility priority without problem or loss of core renderer functionality, aside from the lower limits, but those could be overridden.

[superdump]

I have added this to the 0.6.1 milestone, but only for the "do not automatically enable mappable primary buffers" (and any other such 'risky' features to enable by default. The rest that is about how to improve the configuration of wgpu features and options should be for 0.7.

[superdump]

Some noted unexpected bad performance that for at least one case is probably due to this issue: #3687