Skip to content

Commit 332cd03

Browse files
kpreidcwfitzgerald
kpreid
and
cwfitzgerald
authored
Add details to InstanceError and CreateSurfaceError. (#4066)
Co-authored-by: Connor Fitzgerald <[email protected]>
1 parent 41efabb commit 332cd03

File tree

17 files changed

+322
-149
lines changed

17 files changed

+322
-149
lines changed

‎CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,12 @@ By @Valaphee in [#3402](https://github.com/gfx-rs/wgpu/pull/3402)
7070

7171
### Changes
7272

73+
#### General
74+
7375
- Omit texture store bound checks since they are no-ops if out of bounds on all APIs. By @teoxoy in [#3975](https://github.com/gfx-rs/wgpu/pull/3975)
7476
- Validate `DownlevelFlags::READ_ONLY_DEPTH_STENCIL`. By @teoxoy in [#4031](https://github.com/gfx-rs/wgpu/pull/4031)
7577
- Add validation in accordance with WebGPU `setViewport` valid usage for `x`, `y` and `this.[[attachment_size]]`. By @James2022-rgb in [#4058](https://github.com/gfx-rs/wgpu/pull/4058)
78+
- `wgpu::CreateSurfaceError` now gives details of the failure, but no longer implements `PartialEq`. By @kpreid in [#4066](https://github.com/gfx-rs/wgpu/pull/4066)
7679
- Make `WGPU_POWER_PREF=none` a valid value. By @fornwall in [4076](https://github.com/gfx-rs/wgpu/pull/4076)
7780

7881
#### Vulkan

‎tests/src/lib.rs

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ pub fn initialize_test(parameters: TestParameters, test_function: impl FnOnce(Te
312312
if #[cfg(any(not(target_arch = "wasm32"), target_os = "emscripten"))] {
313313
let canary_set = wgpu::hal::VALIDATION_CANARY.get_and_reset();
314314
} else {
315-
let canary_set = _surface_guard.check_for_unreported_errors();
315+
let canary_set = _surface_guard.unwrap().check_for_unreported_errors();
316316
}
317317
);
318318

@@ -345,24 +345,18 @@ pub fn initialize_test(parameters: TestParameters, test_function: impl FnOnce(Te
345345
}
346346
}
347347

348-
fn initialize_adapter() -> (Adapter, SurfaceGuard) {
349-
let backends = wgpu::util::backend_bits_from_env().unwrap_or_else(Backends::all);
350-
let dx12_shader_compiler = wgpu::util::dx12_shader_compiler_from_env().unwrap_or_default();
351-
let gles_minor_version = wgpu::util::gles_minor_version_from_env().unwrap_or_default();
352-
let instance = Instance::new(wgpu::InstanceDescriptor {
353-
backends,
354-
dx12_shader_compiler,
355-
gles_minor_version,
356-
});
357-
let surface_guard;
348+
fn initialize_adapter() -> (Adapter, Option<SurfaceGuard>) {
349+
let instance = initialize_instance();
350+
let surface_guard: Option<SurfaceGuard>;
358351
let compatible_surface;
359352

353+
// Create a canvas iff we need a WebGL2RenderingContext to have a working device.
360354
#[cfg(not(all(
361355
target_arch = "wasm32",
362356
any(target_os = "emscripten", feature = "webgl")
363357
)))]
364358
{
365-
surface_guard = SurfaceGuard {};
359+
surface_guard = None;
366360
compatible_surface = None;
367361
}
368362
#[cfg(all(
@@ -398,7 +392,7 @@ fn initialize_adapter() -> (Adapter, SurfaceGuard) {
398392
.expect("could not create surface from canvas")
399393
};
400394

401-
surface_guard = SurfaceGuard { canvas };
395+
surface_guard = Some(SurfaceGuard { canvas });
402396

403397
compatible_surface = Some(surface);
404398
}
@@ -413,12 +407,21 @@ fn initialize_adapter() -> (Adapter, SurfaceGuard) {
413407
(adapter, surface_guard)
414408
}
415409

416-
struct SurfaceGuard {
417-
#[cfg(all(
418-
target_arch = "wasm32",
419-
any(target_os = "emscripten", feature = "webgl")
420-
))]
421-
canvas: web_sys::HtmlCanvasElement,
410+
pub fn initialize_instance() -> Instance {
411+
let backends = wgpu::util::backend_bits_from_env().unwrap_or_else(Backends::all);
412+
let dx12_shader_compiler = wgpu::util::dx12_shader_compiler_from_env().unwrap_or_default();
413+
let gles_minor_version = wgpu::util::gles_minor_version_from_env().unwrap_or_default();
414+
Instance::new(wgpu::InstanceDescriptor {
415+
backends,
416+
dx12_shader_compiler,
417+
gles_minor_version,
418+
})
419+
}
420+
421+
// Public because it is used by tests of interacting with canvas
422+
pub struct SurfaceGuard {
423+
#[cfg(target_arch = "wasm32")]
424+
pub canvas: web_sys::HtmlCanvasElement,
422425
}
423426

424427
impl SurfaceGuard {
@@ -452,11 +455,8 @@ impl Drop for SurfaceGuard {
452455
}
453456
}
454457

455-
#[cfg(all(
456-
target_arch = "wasm32",
457-
any(target_os = "emscripten", feature = "webgl")
458-
))]
459-
fn create_html_canvas() -> web_sys::HtmlCanvasElement {
458+
#[cfg(target_arch = "wasm32")]
459+
pub fn create_html_canvas() -> web_sys::HtmlCanvasElement {
460460
use wasm_bindgen::JsCast;
461461

462462
web_sys::window()

‎tests/tests/create_surface_error.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
//! Test that `create_surface_*()` accurately reports those errors we can provoke.
2+
3+
/// This test applies to those cfgs that have a `create_surface_from_canvas` method, which
4+
/// include WebGL and WebGPU, but *not* Emscripten GLES.
5+
#[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
6+
#[wasm_bindgen_test::wasm_bindgen_test]
7+
fn canvas_get_context_returned_null() {
8+
// Not using initialize_test() because that goes straight to creating the canvas for us.
9+
let instance = wgpu_test::initialize_instance();
10+
// Create canvas and cleanup on drop
11+
let canvas_g = wgpu_test::SurfaceGuard {
12+
canvas: wgpu_test::create_html_canvas(),
13+
};
14+
// Using a context id that is not "webgl2" or "webgpu" will render the canvas unusable by wgpu.
15+
canvas_g.canvas.get_context("2d").unwrap();
16+
17+
#[allow(clippy::redundant_clone)] // false positive — can't and shouldn't move out.
18+
let error = instance
19+
.create_surface_from_canvas(canvas_g.canvas.clone())
20+
.unwrap_err();
21+
22+
assert!(
23+
error
24+
.to_string()
25+
.contains("canvas.getContext() returned null"),
26+
"{error}"
27+
);
28+
}

‎tests/tests/root.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ mod buffer;
1010
mod buffer_copy;
1111
mod buffer_usages;
1212
mod clear_texture;
13+
mod create_surface_error;
1314
mod device;
1415
mod encoder;
1516
mod example_wgsl;

‎wgpu-hal/examples/halmark/main.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ struct Example<A: hal::Api> {
8686
}
8787

8888
impl<A: hal::Api> Example<A> {
89-
fn init(window: &winit::window::Window) -> Result<Self, hal::InstanceError> {
89+
fn init(window: &winit::window::Window) -> Result<Self, Box<dyn std::error::Error>> {
9090
let instance_desc = hal::InstanceDescriptor {
9191
name: "example",
9292
flags: if cfg!(debug_assertions) {
@@ -108,13 +108,13 @@ impl<A: hal::Api> Example<A> {
108108
let (adapter, capabilities) = unsafe {
109109
let mut adapters = instance.enumerate_adapters();
110110
if adapters.is_empty() {
111-
return Err(hal::InstanceError);
111+
return Err("no adapters found".into());
112112
}
113113
let exposed = adapters.swap_remove(0);
114114
(exposed.adapter, exposed.capabilities)
115115
};
116-
let surface_caps =
117-
unsafe { adapter.surface_capabilities(&surface) }.ok_or(hal::InstanceError)?;
116+
let surface_caps = unsafe { adapter.surface_capabilities(&surface) }
117+
.ok_or("failed to get surface capabilities")?;
118118
log::info!("Surface caps: {:#?}", surface_caps);
119119

120120
let hal::OpenDevice { device, mut queue } = unsafe {

‎wgpu-hal/src/auxil/dxgi/factory.rs

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,9 @@ pub fn create_factory(
9696
required_factory_type: DxgiFactoryType,
9797
instance_flags: crate::InstanceFlags,
9898
) -> Result<(d3d12::DxgiLib, d3d12::DxgiFactory), crate::InstanceError> {
99-
let lib_dxgi = d3d12::DxgiLib::new().map_err(|_| crate::InstanceError)?;
99+
let lib_dxgi = d3d12::DxgiLib::new().map_err(|e| {
100+
crate::InstanceError::with_source(String::from("failed to load dxgi.dll"), e)
101+
})?;
100102

101103
let mut factory_flags = d3d12::FactoryCreationFlags::empty();
102104

@@ -128,18 +130,22 @@ pub fn create_factory(
128130
Ok(factory) => Some(factory),
129131
// We hard error here as we _should have_ been able to make a factory4 but couldn't.
130132
Err(err) => {
131-
log::error!("Failed to create IDXGIFactory4: {}", err);
132-
return Err(crate::InstanceError);
133+
// err is a Cow<str>, not an Error implementor
134+
return Err(crate::InstanceError::new(format!(
135+
"failed to create IDXGIFactory4: {err:?}"
136+
)));
133137
}
134138
},
135139
// If we require factory4, hard error.
136140
Err(err) if required_factory_type == DxgiFactoryType::Factory4 => {
137-
log::error!("IDXGIFactory1 creation function not found: {:?}", err);
138-
return Err(crate::InstanceError);
141+
return Err(crate::InstanceError::with_source(
142+
String::from("IDXGIFactory1 creation function not found"),
143+
err,
144+
));
139145
}
140146
// If we don't print it to info as all win7 will hit this case.
141147
Err(err) => {
142-
log::info!("IDXGIFactory1 creation function not found: {:?}", err);
148+
log::info!("IDXGIFactory1 creation function not found: {err:?}");
143149
None
144150
}
145151
};
@@ -153,8 +159,10 @@ pub fn create_factory(
153159
}
154160
// If we require factory6, hard error.
155161
Err(err) if required_factory_type == DxgiFactoryType::Factory6 => {
156-
log::warn!("Failed to cast IDXGIFactory4 to IDXGIFactory6: {:?}", err);
157-
return Err(crate::InstanceError);
162+
// err is a Cow<str>, not an Error implementor
163+
return Err(crate::InstanceError::new(format!(
164+
"failed to cast IDXGIFactory4 to IDXGIFactory6: {err:?}"
165+
)));
158166
}
159167
// If we don't print it to info.
160168
Err(err) => {
@@ -169,14 +177,18 @@ pub fn create_factory(
169177
Ok(pair) => match pair.into_result() {
170178
Ok(factory) => factory,
171179
Err(err) => {
172-
log::error!("Failed to create IDXGIFactory1: {}", err);
173-
return Err(crate::InstanceError);
180+
// err is a Cow<str>, not an Error implementor
181+
return Err(crate::InstanceError::new(format!(
182+
"failed to create IDXGIFactory1: {err:?}"
183+
)));
174184
}
175185
},
176186
// We always require at least factory1, so hard error
177187
Err(err) => {
178-
log::error!("IDXGIFactory1 creation function not found: {:?}", err);
179-
return Err(crate::InstanceError);
188+
return Err(crate::InstanceError::with_source(
189+
String::from("IDXGIFactory1 creation function not found"),
190+
err,
191+
));
180192
}
181193
};
182194

@@ -188,8 +200,10 @@ pub fn create_factory(
188200
}
189201
// If we require factory2, hard error.
190202
Err(err) if required_factory_type == DxgiFactoryType::Factory2 => {
191-
log::warn!("Failed to cast IDXGIFactory1 to IDXGIFactory2: {:?}", err);
192-
return Err(crate::InstanceError);
203+
// err is a Cow<str>, not an Error implementor
204+
return Err(crate::InstanceError::new(format!(
205+
"failed to cast IDXGIFactory1 to IDXGIFactory2: {err:?}"
206+
)));
193207
}
194208
// If we don't print it to info.
195209
Err(err) => {

‎wgpu-hal/src/dx11/instance.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,13 @@ impl crate::Instance<super::Api> for super::Instance {
88
};
99

1010
if !enable_dx11 {
11-
return Err(crate::InstanceError);
11+
return Err(crate::InstanceError::new(String::from(
12+
"DX11 support is unstable; set WGPU_UNSTABLE_DX11_BACKEND=1 to enable anyway",
13+
)));
1214
}
1315

14-
let lib_d3d11 = super::library::D3D11Lib::new().ok_or(crate::InstanceError)?;
16+
let lib_d3d11 = super::library::D3D11Lib::new()
17+
.ok_or_else(|| crate::InstanceError::new(String::from("failed to load d3d11.dll")))?;
1518

1619
let (lib_dxgi, factory) = auxil::dxgi::factory::create_factory(
1720
auxil::dxgi::factory::DxgiFactoryType::Factory1,

‎wgpu-hal/src/dx12/instance.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ impl Drop for super::Instance {
1212

1313
impl crate::Instance<super::Api> for super::Instance {
1414
unsafe fn init(desc: &crate::InstanceDescriptor) -> Result<Self, crate::InstanceError> {
15-
let lib_main = d3d12::D3D12Lib::new().map_err(|_| crate::InstanceError)?;
15+
let lib_main = d3d12::D3D12Lib::new().map_err(|e| {
16+
crate::InstanceError::with_source(String::from("failed to load d3d12.dll"), e)
17+
})?;
1618

1719
if desc.flags.contains(crate::InstanceFlags::VALIDATION) {
1820
// Enable debug layer
@@ -95,7 +97,9 @@ impl crate::Instance<super::Api> for super::Instance {
9597
supports_allow_tearing: self.supports_allow_tearing,
9698
swap_chain: None,
9799
}),
98-
_ => Err(crate::InstanceError),
100+
_ => Err(crate::InstanceError::new(format!(
101+
"window handle {window_handle:?} is not a Win32 handle"
102+
))),
99103
}
100104
}
101105
unsafe fn destroy_surface(&self, _surface: super::Surface) {

‎wgpu-hal/src/gles/adapter.rs

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,9 @@ impl super::Adapter {
4343
src = &src[pos + es_sig.len()..];
4444
}
4545
None => {
46-
log::warn!("ES not found in '{}'", src);
47-
return Err(crate::InstanceError);
46+
return Err(crate::InstanceError::new(format!(
47+
"OpenGL version {src:?} does not contain 'ES'"
48+
)));
4849
}
4950
}
5051
};
@@ -86,10 +87,9 @@ impl super::Adapter {
8687
},
8788
minor,
8889
)),
89-
_ => {
90-
log::warn!("Unable to extract the version from '{}'", version);
91-
Err(crate::InstanceError)
92-
}
90+
_ => Err(crate::InstanceError::new(format!(
91+
"unable to extract OpenGL version from {version:?}"
92+
))),
9393
}
9494
}
9595

@@ -975,27 +975,30 @@ mod tests {
975975

976976
#[test]
977977
fn test_version_parse() {
978-
let error = Err(crate::InstanceError);
979-
assert_eq!(Adapter::parse_version("1"), error);
980-
assert_eq!(Adapter::parse_version("1."), error);
981-
assert_eq!(Adapter::parse_version("1 h3l1o. W0rld"), error);
982-
assert_eq!(Adapter::parse_version("1. h3l1o. W0rld"), error);
983-
assert_eq!(Adapter::parse_version("1.2.3"), error);
984-
assert_eq!(Adapter::parse_version("OpenGL ES 3.1"), Ok((3, 1)));
978+
Adapter::parse_version("1").unwrap_err();
979+
Adapter::parse_version("1.").unwrap_err();
980+
Adapter::parse_version("1 h3l1o. W0rld").unwrap_err();
981+
Adapter::parse_version("1. h3l1o. W0rld").unwrap_err();
982+
Adapter::parse_version("1.2.3").unwrap_err();
983+
984+
assert_eq!(Adapter::parse_version("OpenGL ES 3.1").unwrap(), (3, 1));
985+
assert_eq!(
986+
Adapter::parse_version("OpenGL ES 2.0 Google Nexus").unwrap(),
987+
(2, 0)
988+
);
989+
assert_eq!(Adapter::parse_version("GLSL ES 1.1").unwrap(), (1, 1));
985990
assert_eq!(
986-
Adapter::parse_version("OpenGL ES 2.0 Google Nexus"),
987-
Ok((2, 0))
991+
Adapter::parse_version("OpenGL ES GLSL ES 3.20").unwrap(),
992+
(3, 2)
988993
);
989-
assert_eq!(Adapter::parse_version("GLSL ES 1.1"), Ok((1, 1)));
990-
assert_eq!(Adapter::parse_version("OpenGL ES GLSL ES 3.20"), Ok((3, 2)));
991994
assert_eq!(
992995
// WebGL 2.0 should parse as OpenGL ES 3.0
993-
Adapter::parse_version("WebGL 2.0 (OpenGL ES 3.0 Chromium)"),
994-
Ok((3, 0))
996+
Adapter::parse_version("WebGL 2.0 (OpenGL ES 3.0 Chromium)").unwrap(),
997+
(3, 0)
995998
);
996999
assert_eq!(
997-
Adapter::parse_version("WebGL GLSL ES 3.00 (OpenGL ES GLSL ES 3.0 Chromium)"),
998-
Ok((3, 0))
1000+
Adapter::parse_version("WebGL GLSL ES 3.00 (OpenGL ES GLSL ES 3.0 Chromium)").unwrap(),
1001+
(3, 0)
9991002
);
10001003
}
10011004
}

0 commit comments

Comments
 (0)