Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require a compatible surface for request_adapter() on WebGL2 #5901

Merged
merged 2 commits into from
Jul 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,19 @@ Platform support:

By @atlv24 in [#5383](https://github.com/gfx-rs/wgpu/pull/5383)

#### A compatible surface is now required for `request_adapter()` on WebGL2 + `enumerate_adapters()` is now native only.

When targeting WebGL2, it has always been the case that a surface had to be created before calling `request_adapter()`.
We now make this requirement explicit.

Validation was also added to prevent configuring the surface with a device that doesn't share the same underlying
WebGL2 context since this has never worked.

Calling `enumerate_adapters()` when targeting WebGPU used to return an empty `Vec` and since we now require users
to pass a compatible surface when targeting WebGL2, having `enumerate_adapters()` doesn't make sense.

By @teoxoy in [#5901](https://github.com/gfx-rs/wgpu/pull/5901)

### New features
#### Vulkan

Expand Down
21 changes: 14 additions & 7 deletions tests/src/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,17 @@ pub fn initialize_instance() -> Instance {
pub async fn initialize_adapter(adapter_index: usize) -> (Instance, Adapter, Option<SurfaceGuard>) {
let instance = initialize_instance();
#[allow(unused_variables)]
let _surface: wgpu::Surface;
let surface: Option<wgpu::Surface>;
let surface_guard: Option<SurfaceGuard>;

// Create a canvas iff we need a WebGL2RenderingContext to have a working device.
#[allow(unused_assignments)]
// Create a canvas if we need a WebGL2RenderingContext to have a working device.
#[cfg(not(all(
target_arch = "wasm32",
any(target_os = "emscripten", feature = "webgl")
)))]
{
surface = None;
surface_guard = None;
}
#[cfg(all(
Expand All @@ -60,23 +62,28 @@ pub async fn initialize_adapter(adapter_index: usize) -> (Instance, Adapter, Opt
// On wasm, append a canvas to the document body for initializing the adapter
let canvas = initialize_html_canvas();

_surface = instance
.create_surface(wgpu::SurfaceTarget::Canvas(canvas.clone()))
.expect("could not create surface from canvas");
surface = Some(
instance
.create_surface(wgpu::SurfaceTarget::Canvas(canvas.clone()))
.expect("could not create surface from canvas"),
);

surface_guard = Some(SurfaceGuard { canvas });
}

cfg_if::cfg_if! {
if #[cfg(any(not(target_arch = "wasm32"), feature = "webgl"))] {
if #[cfg(not(target_arch = "wasm32"))] {
let adapter_iter = instance.enumerate_adapters(wgpu::Backends::all());
let adapter_count = adapter_iter.len();
let adapter = adapter_iter.into_iter()
.nth(adapter_index)
.unwrap_or_else(|| panic!("Tried to get index {adapter_index} adapter, but adapter list was only {adapter_count} long. Is .gpuconfig out of date?"));
} else {
assert_eq!(adapter_index, 0);
let adapter = instance.request_adapter(&wgpu::RequestAdapterOptions::default()).await.unwrap();
let adapter = instance.request_adapter(&wgpu::RequestAdapterOptions {
compatible_surface: surface.as_ref(),
..Default::default()
}).await.unwrap();
}
}

Expand Down
6 changes: 4 additions & 2 deletions wgpu-core/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -712,7 +712,7 @@ impl Global {
profiling::scope!("enumerating", &*format!("{:?}", A::VARIANT));
let hub = HalApi::hub(self);

let hal_adapters = unsafe { inst.enumerate_adapters() };
let hal_adapters = unsafe { inst.enumerate_adapters(None) };
for raw in hal_adapters {
let adapter = Adapter::new(raw);
log::info!("Adapter {:?} {:?}", A::VARIANT, adapter.raw.info);
Expand Down Expand Up @@ -791,7 +791,9 @@ impl Global {
let id = inputs.find(A::VARIANT);
match (id, instance) {
(Some(id), Some(inst)) => {
let mut adapters = unsafe { inst.enumerate_adapters() };
let compatible_hal_surface =
compatible_surface.and_then(|surface| A::surface_as_hal(surface));
let mut adapters = unsafe { inst.enumerate_adapters(compatible_hal_surface) };
if force_software {
adapters.retain(|exposed| exposed.info.device_type == wgt::DeviceType::Cpu);
}
Expand Down
2 changes: 1 addition & 1 deletion wgpu-hal/examples/halmark/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ impl<A: hal::Api> Example<A> {
};

let (adapter, capabilities) = unsafe {
let mut adapters = instance.enumerate_adapters();
let mut adapters = instance.enumerate_adapters(Some(&surface));
if adapters.is_empty() {
return Err("no adapters found".into());
}
Expand Down
2 changes: 1 addition & 1 deletion wgpu-hal/examples/ray-traced-triangle/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ impl<A: hal::Api> Example<A> {
};

let (adapter, features) = unsafe {
let mut adapters = instance.enumerate_adapters();
let mut adapters = instance.enumerate_adapters(Some(&surface));
if adapters.is_empty() {
panic!("No adapters found");
}
Expand Down
5 changes: 4 additions & 1 deletion wgpu-hal/src/dx12/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,10 @@ impl crate::Instance for super::Instance {
// just drop
}

unsafe fn enumerate_adapters(&self) -> Vec<crate::ExposedAdapter<super::Api>> {
unsafe fn enumerate_adapters(
&self,
_surface_hint: Option<&super::Surface>,
) -> Vec<crate::ExposedAdapter<super::Api>> {
let adapters = auxil::dxgi::factory::enumerate_adapters(self.factory.clone());

adapters
Expand Down
5 changes: 4 additions & 1 deletion wgpu-hal/src/empty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ impl crate::Instance for Context {
Ok(Context)
}
unsafe fn destroy_surface(&self, surface: Context) {}
unsafe fn enumerate_adapters(&self) -> Vec<crate::ExposedAdapter<Api>> {
unsafe fn enumerate_adapters(
&self,
_surface_hint: Option<&Context>,
) -> Vec<crate::ExposedAdapter<Api>> {
Vec::new()
}
}
Expand Down
5 changes: 5 additions & 0 deletions wgpu-hal/src/gles/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1152,6 +1152,11 @@ impl crate::Adapter for super::Adapter {
&self,
surface: &super::Surface,
) -> Option<crate::SurfaceCapabilities> {
#[cfg(webgl)]
if self.shared.context.webgl2_context != surface.webgl2_context {
return None;
}

if surface.presentable {
let mut formats = vec![
wgt::TextureFormat::Rgba8Unorm,
Expand Down
5 changes: 4 additions & 1 deletion wgpu-hal/src/gles/egl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1001,7 +1001,10 @@ impl crate::Instance for Instance {

unsafe fn destroy_surface(&self, _surface: Surface) {}

unsafe fn enumerate_adapters(&self) -> Vec<crate::ExposedAdapter<super::Api>> {
unsafe fn enumerate_adapters(
&self,
_surface_hint: Option<&Surface>,
) -> Vec<crate::ExposedAdapter<super::Api>> {
let inner = self.inner.lock();
inner.egl.make_current();

Expand Down
48 changes: 20 additions & 28 deletions wgpu-hal/src/gles/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use super::TextureFormatDesc;
/// with the `AdapterContext` API from the EGL implementation.
pub struct AdapterContext {
pub glow_context: glow::Context,
pub webgl2_context: web_sys::WebGl2RenderingContext,
}

impl AdapterContext {
Expand All @@ -24,10 +25,7 @@ impl AdapterContext {
}

#[derive(Debug)]
pub struct Instance {
/// Set when a canvas is provided, and used to implement [`Instance::enumerate_adapters()`].
webgl2_context: Mutex<Option<web_sys::WebGl2RenderingContext>>,
}
pub struct Instance;

impl Instance {
pub fn create_surface_from_canvas(
Expand Down Expand Up @@ -85,10 +83,6 @@ impl Instance {
.dyn_into()
.expect("canvas context is not a WebGl2RenderingContext");

// It is not inconsistent to overwrite an existing context, because the only thing that
// `self.webgl2_context` is used for is producing the response to `enumerate_adapters()`.
*self.webgl2_context.lock() = Some(webgl2_context.clone());

Ok(Surface {
canvas,
webgl2_context,
Expand Down Expand Up @@ -121,21 +115,27 @@ impl crate::Instance for Instance {

unsafe fn init(_desc: &crate::InstanceDescriptor) -> Result<Self, crate::InstanceError> {
profiling::scope!("Init OpenGL (WebGL) Backend");
Ok(Instance {
webgl2_context: Mutex::new(None),
})
Ok(Instance)
}

unsafe fn enumerate_adapters(&self) -> Vec<crate::ExposedAdapter<super::Api>> {
let context_guard = self.webgl2_context.lock();
let gl = match *context_guard {
Some(ref webgl2_context) => glow::Context::from_webgl2_context(webgl2_context.clone()),
None => return Vec::new(),
};
unsafe fn enumerate_adapters(
&self,
surface_hint: Option<&Surface>,
) -> Vec<crate::ExposedAdapter<super::Api>> {
if let Some(surface_hint) = surface_hint {
let gl = glow::Context::from_webgl2_context(surface_hint.webgl2_context.clone());

unsafe { super::Adapter::expose(AdapterContext { glow_context: gl }) }
unsafe {
super::Adapter::expose(AdapterContext {
glow_context: gl,
webgl2_context: surface_hint.webgl2_context.clone(),
})
}
.into_iter()
.collect()
} else {
Vec::new()
}
}

unsafe fn create_surface(
Expand Down Expand Up @@ -172,21 +172,13 @@ impl crate::Instance for Instance {
self.create_surface_from_canvas(canvas)
}

unsafe fn destroy_surface(&self, surface: Surface) {
let mut context_option_ref = self.webgl2_context.lock();

if let Some(context) = context_option_ref.as_ref() {
if context == &surface.webgl2_context {
*context_option_ref = None;
}
}
}
unsafe fn destroy_surface(&self, _surface: Surface) {}
}

#[derive(Debug)]
pub struct Surface {
canvas: Canvas,
webgl2_context: web_sys::WebGl2RenderingContext,
pub(super) webgl2_context: web_sys::WebGl2RenderingContext,
pub(super) swapchain: RwLock<Option<Swapchain>>,
texture: Mutex<Option<glow::Texture>>,
pub(super) presentable: bool,
Expand Down
5 changes: 4 additions & 1 deletion wgpu-hal/src/gles/wgl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,10 @@ impl crate::Instance for Instance {
}
unsafe fn destroy_surface(&self, _surface: Surface) {}

unsafe fn enumerate_adapters(&self) -> Vec<crate::ExposedAdapter<super::Api>> {
unsafe fn enumerate_adapters(
&self,
_surface_hint: Option<&Surface>,
) -> Vec<crate::ExposedAdapter<super::Api>> {
unsafe {
super::Adapter::expose(AdapterContext {
inner: self.inner.clone(),
Expand Down
6 changes: 5 additions & 1 deletion wgpu-hal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,11 @@ pub trait Instance: Sized + WasmNotSendSync {
window_handle: raw_window_handle::RawWindowHandle,
) -> Result<<Self::A as Api>::Surface, InstanceError>;
unsafe fn destroy_surface(&self, surface: <Self::A as Api>::Surface);
unsafe fn enumerate_adapters(&self) -> Vec<ExposedAdapter<Self::A>>;
/// `surface_hint` is only used by the GLES backend targeting WebGL2
unsafe fn enumerate_adapters(
&self,
surface_hint: Option<&<Self::A as Api>::Surface>,
) -> Vec<ExposedAdapter<Self::A>>;
}

pub trait Surface: WasmNotSendSync {
Expand Down
5 changes: 4 additions & 1 deletion wgpu-hal/src/metal/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,10 @@ impl crate::Instance for Instance {
unsafe { surface.dispose() };
}

unsafe fn enumerate_adapters(&self) -> Vec<crate::ExposedAdapter<Api>> {
unsafe fn enumerate_adapters(
&self,
_surface_hint: Option<&Surface>,
) -> Vec<crate::ExposedAdapter<Api>> {
let devices = metal::Device::all();
let mut adapters: Vec<crate::ExposedAdapter<Api>> = devices
.into_iter()
Expand Down
5 changes: 4 additions & 1 deletion wgpu-hal/src/vulkan/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -884,7 +884,10 @@ impl crate::Instance for super::Instance {
unsafe { surface.functor.destroy_surface(surface.raw, None) };
}

unsafe fn enumerate_adapters(&self) -> Vec<crate::ExposedAdapter<super::Api>> {
unsafe fn enumerate_adapters(
&self,
_surface_hint: Option<&super::Surface>,
) -> Vec<crate::ExposedAdapter<super::Api>> {
use crate::auxil::db;

let raw_devices = match unsafe { self.shared.raw.enumerate_physical_devices() } {
Expand Down
1 change: 1 addition & 0 deletions wgpu/src/backend/wgpu_core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ impl ContextWgpuCore {
Self(unsafe { wgc::global::Global::from_instance(core_instance) })
}

#[cfg(native)]
pub fn enumerate_adapters(&self, backends: wgt::Backends) -> Vec<wgc::id::AdapterId> {
self.0
.enumerate_adapters(wgc::instance::AdapterInputs::Mask(backends, |_| None))
Expand Down
9 changes: 4 additions & 5 deletions wgpu/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2369,13 +2369,10 @@ impl Instance {

/// Retrieves all available [`Adapter`]s that match the given [`Backends`].
///
/// Always returns an empty vector if the instance decided upon creation to
/// target WebGPU since adapter creation is always async on WebGPU.
///
/// # Arguments
///
/// - `backends` - Backends from which to enumerate adapters.
#[cfg(wgpu_core)]
#[cfg(native)]
pub fn enumerate_adapters(&self, backends: Backends) -> Vec<Adapter> {
let context = Arc::clone(&self.context);
self.context
Expand All @@ -2391,14 +2388,16 @@ impl Instance {
})
.collect()
})
.unwrap_or_default()
.unwrap()
}

/// Retrieves an [`Adapter`] which matches the given [`RequestAdapterOptions`].
///
/// Some options are "soft", so treated as non-mandatory. Others are "hard".
///
/// If no adapters are found that suffice all the "hard" options, `None` is returned.
///
/// A `compatible_surface` is required when targeting WebGL2.
pub fn request_adapter(
&self,
options: &RequestAdapterOptions<'_, '_>,
Expand Down
Loading