From 6c4a3fc418ee60cb6fd5fc0d152b1153596a9564 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sun, 2 Jul 2023 20:01:36 -0500 Subject: [PATCH 1/3] Make RequestAdapterOptions.power_preference optional The WebGPU docs state that the `powerPreference` field is optional - it can be set to `undefined`, which provides no hint. Currently, wgpu only allows using an explicit LowPower or HighPerformance hint. This can cause a secondary backend to be chosen over a primary backend when the secondary backend is the only one matching the power preference - with no way to ensure that a primary backend takes priority. This commit changes `RequestAdapterOptions.power_preference` to store an `Option`, and removes the `Default` impl from `PowerPreference`. Under WebGPU, we leave `GPURequestAdapterOptions.powerPreference` when our value is `None`. Under native, we skip the backend preference logic. --- CHANGELOG.md | 1 + deno_webgpu/lib.rs | 2 +- examples/hello-triangle/src/main.rs | 2 +- player/src/bin/play.rs | 2 +- player/tests/test.rs | 2 +- tests/tests/instance.rs | 2 +- wgpu-core/src/instance.rs | 7 +++++-- wgpu-types/src/lib.rs | 10 +++++----- wgpu/src/backend/web.rs | 14 +++++++++----- wgpu/src/util/init.rs | 2 +- 10 files changed, 26 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf049ed786..667a6f691a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ Bottom level categories: - Change `AdapterInfo::{device,vendor}` to be `u32` instead of `usize`. By @ameknite in [#3760](https://github.com/gfx-rs/wgpu/pull/3760) - Remove the `backend_bits` parameter in `initialize_adapter_from_env` and `initialize_adapter_from_env_or_default` - use [InstanceDescriptor::backends](https://docs.rs/wgpu/latest/wgpu/struct.InstanceDescriptor.html#structfield.backends) instead. By @fornwall in [#3904](https://github.com/gfx-rs/wgpu/pull/3904) +- Make `RequestAdapterOptions.power_preference` optional. By @Aaron1011 in [#3903](https://github.com/gfx-rs/wgpu/pull/3903) #### Vulkan diff --git a/deno_webgpu/lib.rs b/deno_webgpu/lib.rs index f475723682..169287a297 100644 --- a/deno_webgpu/lib.rs +++ b/deno_webgpu/lib.rs @@ -412,7 +412,7 @@ pub async fn op_webgpu_request_adapter( }; let descriptor = wgpu_core::instance::RequestAdapterOptions { - power_preference: power_preference.unwrap_or_default(), + power_preference, force_fallback_adapter, compatible_surface: None, // windowless }; diff --git a/examples/hello-triangle/src/main.rs b/examples/hello-triangle/src/main.rs index 98abf5b8d5..7f27a2dcdd 100644 --- a/examples/hello-triangle/src/main.rs +++ b/examples/hello-triangle/src/main.rs @@ -13,7 +13,7 @@ async fn run(event_loop: EventLoop<()>, window: Window) { let surface = unsafe { instance.create_surface(&window) }.unwrap(); let adapter = instance .request_adapter(&wgpu::RequestAdapterOptions { - power_preference: wgpu::PowerPreference::default(), + power_preference: None, force_fallback_adapter: false, // Request an adapter which can render to our surface compatible_surface: Some(&surface), diff --git a/player/src/bin/play.rs b/player/src/bin/play.rs index cb596b4acc..1c4e1327e2 100644 --- a/player/src/bin/play.rs +++ b/player/src/bin/play.rs @@ -64,7 +64,7 @@ fn main() { let adapter = global .request_adapter( &wgc::instance::RequestAdapterOptions { - power_preference: wgt::PowerPreference::LowPower, + power_preference: None, force_fallback_adapter: false, #[cfg(feature = "winit")] compatible_surface: Some(surface), diff --git a/player/tests/test.rs b/player/tests/test.rs index cafb9c0ee5..c51b8e715b 100644 --- a/player/tests/test.rs +++ b/player/tests/test.rs @@ -193,7 +193,7 @@ impl Corpus { } let adapter = match global.request_adapter( &wgc::instance::RequestAdapterOptions { - power_preference: wgt::PowerPreference::LowPower, + power_preference: None, force_fallback_adapter: false, compatible_surface: None, }, diff --git a/tests/tests/instance.rs b/tests/tests/instance.rs index e9ff6afff0..fc53bb5d8f 100644 --- a/tests/tests/instance.rs +++ b/tests/tests/instance.rs @@ -16,7 +16,7 @@ fn request_adapter_inner(power: wgt::PowerPreference) { }); let _adapter = pollster::block_on(instance.request_adapter(&wgpu::RequestAdapterOptions { - power_preference: power, + power_preference: Some(power), force_fallback_adapter: false, compatible_surface: None, })) diff --git a/wgpu-core/src/instance.rs b/wgpu-core/src/instance.rs index b291de78cd..7595ca3383 100644 --- a/wgpu-core/src/instance.rs +++ b/wgpu-core/src/instance.rs @@ -894,8 +894,11 @@ impl Global { // This means that backends which do provide accurate device types // will be preferred if their device type indicates an actual // hardware GPU (integrated or discrete). - PowerPreference::LowPower => integrated.or(discrete).or(other).or(virt).or(cpu), - PowerPreference::HighPerformance => discrete.or(integrated).or(other).or(virt).or(cpu), + Some(PowerPreference::LowPower) => integrated.or(discrete).or(other).or(virt).or(cpu), + Some(PowerPreference::HighPerformance) => { + discrete.or(integrated).or(other).or(virt).or(cpu) + } + None => None, }; let mut selected = preferred_gpu.unwrap_or(0); diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index a5e838653b..b1c3b81436 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -130,13 +130,12 @@ impl Backend { /// Corresponds to [WebGPU `GPUPowerPreference`]( /// https://gpuweb.github.io/gpuweb/#enumdef-gpupowerpreference). #[repr(C)] -#[derive(Copy, Clone, Debug, Default, PartialEq, Eq, Hash)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] #[cfg_attr(feature = "trace", derive(Serialize))] #[cfg_attr(feature = "replay", derive(Deserialize))] #[cfg_attr(feature = "serde", serde(rename_all = "kebab-case"))] pub enum PowerPreference { /// Adapter that uses the least possible power. This is often an integrated GPU. - #[default] LowPower = 0, /// Adapter that has the highest performance. This is often a discrete GPU. HighPerformance = 1, @@ -192,8 +191,9 @@ impl From for Backends { #[cfg_attr(feature = "trace", derive(Serialize))] #[cfg_attr(feature = "replay", derive(Deserialize))] pub struct RequestAdapterOptions { - /// Power preference for the adapter. - pub power_preference: PowerPreference, + /// Power preference for the adapter. If `None`, then power usage + /// is not considered when choosing an adapter. + pub power_preference: Option, /// Indicates that only a fallback adapter can be returned. This is generally a "software" /// implementation on the system. pub force_fallback_adapter: bool, @@ -205,7 +205,7 @@ pub struct RequestAdapterOptions { impl Default for RequestAdapterOptions { fn default() -> Self { Self { - power_preference: PowerPreference::default(), + power_preference: None, force_fallback_adapter: false, compatible_surface: None, } diff --git a/wgpu/src/backend/web.rs b/wgpu/src/backend/web.rs index 1dc5c712ab..3cb85d86ef 100644 --- a/wgpu/src/backend/web.rs +++ b/wgpu/src/backend/web.rs @@ -985,11 +985,15 @@ impl crate::context::Context for Context { // and currently the Future here has no room for extra parameter `backends`. //assert!(backends.contains(wgt::Backends::BROWSER_WEBGPU)); let mut mapped_options = web_sys::GpuRequestAdapterOptions::new(); - let mapped_power_preference = match options.power_preference { - wgt::PowerPreference::LowPower => web_sys::GpuPowerPreference::LowPower, - wgt::PowerPreference::HighPerformance => web_sys::GpuPowerPreference::HighPerformance, - }; - mapped_options.power_preference(mapped_power_preference); + if let Some(power_preference) = options.power_preference { + let mapped_power_preference = match power_preference { + wgt::PowerPreference::LowPower => web_sys::GpuPowerPreference::LowPower, + wgt::PowerPreference::HighPerformance => { + web_sys::GpuPowerPreference::HighPerformance + } + }; + mapped_options.power_preference(mapped_power_preference); + } let adapter_promise = self.0.request_adapter_with_options(&mapped_options); MakeSendFuture::new( diff --git a/wgpu/src/util/init.rs b/wgpu/src/util/init.rs index e47a08a304..05135281fb 100644 --- a/wgpu/src/util/init.rs +++ b/wgpu/src/util/init.rs @@ -74,7 +74,7 @@ pub async fn initialize_adapter_from_env_or_default( None => { instance .request_adapter(&RequestAdapterOptions { - power_preference: power_preference_from_env().unwrap_or_default(), + power_preference: power_preference_from_env(), force_fallback_adapter: false, compatible_surface, }) From 2243859118b0b41bd3eb4a4120407ba8eff5a2d8 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sun, 9 Jul 2023 19:09:38 -0400 Subject: [PATCH 2/3] Address review comments --- deno_webgpu/lib.rs | 2 +- examples/hello-triangle/src/main.rs | 2 +- player/src/bin/play.rs | 2 +- player/tests/test.rs | 2 +- tests/tests/instance.rs | 2 +- wgpu-core/src/instance.rs | 17 +++++++++++++---- wgpu-types/src/lib.rs | 16 +++++++++------- wgpu/src/backend/web.rs | 17 +++++++++-------- wgpu/src/util/init.rs | 2 +- 9 files changed, 37 insertions(+), 25 deletions(-) diff --git a/deno_webgpu/lib.rs b/deno_webgpu/lib.rs index 169287a297..f475723682 100644 --- a/deno_webgpu/lib.rs +++ b/deno_webgpu/lib.rs @@ -412,7 +412,7 @@ pub async fn op_webgpu_request_adapter( }; let descriptor = wgpu_core::instance::RequestAdapterOptions { - power_preference, + power_preference: power_preference.unwrap_or_default(), force_fallback_adapter, compatible_surface: None, // windowless }; diff --git a/examples/hello-triangle/src/main.rs b/examples/hello-triangle/src/main.rs index 7f27a2dcdd..98abf5b8d5 100644 --- a/examples/hello-triangle/src/main.rs +++ b/examples/hello-triangle/src/main.rs @@ -13,7 +13,7 @@ async fn run(event_loop: EventLoop<()>, window: Window) { let surface = unsafe { instance.create_surface(&window) }.unwrap(); let adapter = instance .request_adapter(&wgpu::RequestAdapterOptions { - power_preference: None, + power_preference: wgpu::PowerPreference::default(), force_fallback_adapter: false, // Request an adapter which can render to our surface compatible_surface: Some(&surface), diff --git a/player/src/bin/play.rs b/player/src/bin/play.rs index 1c4e1327e2..a9a47ce5ff 100644 --- a/player/src/bin/play.rs +++ b/player/src/bin/play.rs @@ -64,7 +64,7 @@ fn main() { let adapter = global .request_adapter( &wgc::instance::RequestAdapterOptions { - power_preference: None, + power_preference: wgt::PowerPreference::None, force_fallback_adapter: false, #[cfg(feature = "winit")] compatible_surface: Some(surface), diff --git a/player/tests/test.rs b/player/tests/test.rs index c51b8e715b..bbaa66cc4e 100644 --- a/player/tests/test.rs +++ b/player/tests/test.rs @@ -193,7 +193,7 @@ impl Corpus { } let adapter = match global.request_adapter( &wgc::instance::RequestAdapterOptions { - power_preference: None, + power_preference: wgt::PowerPreference::None, force_fallback_adapter: false, compatible_surface: None, }, diff --git a/tests/tests/instance.rs b/tests/tests/instance.rs index fc53bb5d8f..e9ff6afff0 100644 --- a/tests/tests/instance.rs +++ b/tests/tests/instance.rs @@ -16,7 +16,7 @@ fn request_adapter_inner(power: wgt::PowerPreference) { }); let _adapter = pollster::block_on(instance.request_adapter(&wgpu::RequestAdapterOptions { - power_preference: Some(power), + power_preference: power, force_fallback_adapter: false, compatible_surface: None, })) diff --git a/wgpu-core/src/instance.rs b/wgpu-core/src/instance.rs index 7595ca3383..53113a6093 100644 --- a/wgpu-core/src/instance.rs +++ b/wgpu-core/src/instance.rs @@ -894,11 +894,20 @@ impl Global { // This means that backends which do provide accurate device types // will be preferred if their device type indicates an actual // hardware GPU (integrated or discrete). - Some(PowerPreference::LowPower) => integrated.or(discrete).or(other).or(virt).or(cpu), - Some(PowerPreference::HighPerformance) => { - discrete.or(integrated).or(other).or(virt).or(cpu) + PowerPreference::LowPower => integrated.or(discrete).or(other).or(virt).or(cpu), + PowerPreference::HighPerformance => discrete.or(integrated).or(other).or(virt).or(cpu), + PowerPreference::None => { + let option_min = |a: Option, b: Option| { + if let (Some(a), Some(b)) = (a, b) { + Some(a.min(b)) + } else { + a.or(b) + } + }; + // Pick the lowest id of these types (which will give us a `Backends::PRIMARY` + // adapter, if one exists). + option_min(option_min(discrete, integrated), other) } - None => None, }; let mut selected = preferred_gpu.unwrap_or(0); diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index b1c3b81436..d694735660 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -130,15 +130,18 @@ impl Backend { /// Corresponds to [WebGPU `GPUPowerPreference`]( /// https://gpuweb.github.io/gpuweb/#enumdef-gpupowerpreference). #[repr(C)] -#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, Default)] #[cfg_attr(feature = "trace", derive(Serialize))] #[cfg_attr(feature = "replay", derive(Deserialize))] #[cfg_attr(feature = "serde", serde(rename_all = "kebab-case"))] pub enum PowerPreference { + #[default] + /// Power usage is not considered when choosing an adapter. + None = 0, /// Adapter that uses the least possible power. This is often an integrated GPU. - LowPower = 0, + LowPower = 1, /// Adapter that has the highest performance. This is often a discrete GPU. - HighPerformance = 1, + HighPerformance = 2, } bitflags::bitflags! { @@ -191,9 +194,8 @@ impl From for Backends { #[cfg_attr(feature = "trace", derive(Serialize))] #[cfg_attr(feature = "replay", derive(Deserialize))] pub struct RequestAdapterOptions { - /// Power preference for the adapter. If `None`, then power usage - /// is not considered when choosing an adapter. - pub power_preference: Option, + /// Power preference for the adapter. + pub power_preference: PowerPreference, /// Indicates that only a fallback adapter can be returned. This is generally a "software" /// implementation on the system. pub force_fallback_adapter: bool, @@ -205,7 +207,7 @@ pub struct RequestAdapterOptions { impl Default for RequestAdapterOptions { fn default() -> Self { Self { - power_preference: None, + power_preference: PowerPreference::default(), force_fallback_adapter: false, compatible_surface: None, } diff --git a/wgpu/src/backend/web.rs b/wgpu/src/backend/web.rs index 3cb85d86ef..be5f798293 100644 --- a/wgpu/src/backend/web.rs +++ b/wgpu/src/backend/web.rs @@ -985,14 +985,15 @@ impl crate::context::Context for Context { // and currently the Future here has no room for extra parameter `backends`. //assert!(backends.contains(wgt::Backends::BROWSER_WEBGPU)); let mut mapped_options = web_sys::GpuRequestAdapterOptions::new(); - if let Some(power_preference) = options.power_preference { - let mapped_power_preference = match power_preference { - wgt::PowerPreference::LowPower => web_sys::GpuPowerPreference::LowPower, - wgt::PowerPreference::HighPerformance => { - web_sys::GpuPowerPreference::HighPerformance - } - }; - mapped_options.power_preference(mapped_power_preference); + let mapped_power_preference = match options.power_preference { + wgt::PowerPreference::None => None, + wgt::PowerPreference::LowPower => Some(web_sys::GpuPowerPreference::LowPower), + wgt::PowerPreference::HighPerformance => { + Some(web_sys::GpuPowerPreference::HighPerformance) + } + }; + if let Some(mapped_pref) = mapped_power_preference { + mapped_options.power_preference(mapped_pref); } let adapter_promise = self.0.request_adapter_with_options(&mapped_options); diff --git a/wgpu/src/util/init.rs b/wgpu/src/util/init.rs index 05135281fb..e47a08a304 100644 --- a/wgpu/src/util/init.rs +++ b/wgpu/src/util/init.rs @@ -74,7 +74,7 @@ pub async fn initialize_adapter_from_env_or_default( None => { instance .request_adapter(&RequestAdapterOptions { - power_preference: power_preference_from_env(), + power_preference: power_preference_from_env().unwrap_or_default(), force_fallback_adapter: false, compatible_surface, }) From 11685cabf0fb5438d73e7c9dbbd5f4831ab18bcb Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 19 Jul 2023 16:47:40 -0400 Subject: [PATCH 3/3] Fix comment --- wgpu-core/src/instance.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/wgpu-core/src/instance.rs b/wgpu-core/src/instance.rs index 53113a6093..45f01824b7 100644 --- a/wgpu-core/src/instance.rs +++ b/wgpu-core/src/instance.rs @@ -904,8 +904,7 @@ impl Global { a.or(b) } }; - // Pick the lowest id of these types (which will give us a `Backends::PRIMARY` - // adapter, if one exists). + // Pick the lowest id of these types option_min(option_min(discrete, integrated), other) } };