Skip to content

Commit

Permalink
Remove surface extent validation (and thus fix the annoying `Requeste…
Browse files Browse the repository at this point in the history
…d size ... is outside of the supported range` warning) (#4796)

* Remove surface extent validation

* silence pnext vulkan validation warning which can happen on surface resize

* remove old VUID-VkSwapchainCreateInfoKHR-imageExtent-01689 validation warning ignore

* Validate surface against max texture size
  • Loading branch information
Wumpf authored Nov 29, 2023
1 parent 4184932 commit 8da4925
Show file tree
Hide file tree
Showing 10 changed files with 24 additions and 62 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ Previously, `DeviceExt::create_texture_with_data` only allowed data to be provid

#### General
- Added `DownlevelFlags::VERTEX_AND_INSTANCE_INDEX_RESPECTS_RESPECTIVE_FIRST_VALUE_IN_INDIRECT_DRAW` to know if `@builtin(vertex_index)` and `@builtin(instance_index)` will respect the `first_vertex` / `first_instance` in indirect calls. If this is not present, both will always start counting from 0. Currently enabled on all backends except DX12. By @cwfitzgerald in [#4722](https://github.com/gfx-rs/wgpu/pull/4722)
- No longer validate surfaces against their allowed extent range on configure. This caused warnings that were almost impossible to avoid. As before, the resulting behavior depends on the compositor. By @wumpf in [#????](https://github.com/gfx-rs/wgpu/pull/????)

#### OpenGL
- `@builtin(instance_index)` now properly reflects the range provided in the draw call instead of always counting from 0. By @cwfitzgerald in [#4722](https://github.com/gfx-rs/wgpu/pull/4722).
Expand Down
22 changes: 12 additions & 10 deletions wgpu-core/src/device/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1809,21 +1809,19 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
fn validate_surface_configuration(
config: &mut hal::SurfaceConfiguration,
caps: &hal::SurfaceCapabilities,
max_texture_dimension_2d: u32,
) -> Result<(), E> {
let width = config.extent.width;
let height = config.extent.height;
if width < caps.extents.start().width
|| width > caps.extents.end().width
|| height < caps.extents.start().height
|| height > caps.extents.end().height
{
log::warn!(
"Requested size {}x{} is outside of the supported range: {:?}",

if width > max_texture_dimension_2d || height > max_texture_dimension_2d {
return Err(E::TooLarge {
width,
height,
caps.extents
);
max_texture_dimension_2d,
});
}

if !caps.present_modes.contains(&config.present_mode) {
let new_mode = 'b: loop {
// Automatic present mode checks.
Expand Down Expand Up @@ -1997,7 +1995,11 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
view_formats: hal_view_formats,
};

if let Err(error) = validate_surface_configuration(&mut hal_config, &caps) {
if let Err(error) = validate_surface_configuration(
&mut hal_config,
&caps,
device.limits.max_texture_dimension_2d,
) {
break error;
}

Expand Down
6 changes: 6 additions & 0 deletions wgpu-core/src/present.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,12 @@ pub enum ConfigureSurfaceError {
PreviousOutputExists,
#[error("Both `Surface` width and height must be non-zero. Wait to recreate the `Surface` until the window has non-zero area.")]
ZeroArea,
#[error("`Surface` width and height must be within the maximum supported texture size. Requested was ({width}, height), maximum extent is {max_texture_dimension_2d}.")]
TooLarge {
width: u32,
height: u32,
max_texture_dimension_2d: u32,
},
#[error("Surface does not support the adapter's queue family")]
UnsupportedQueueFamily,
#[error("Requested format {requested:?} is not in list of supported formats: {available:?}")]
Expand Down
10 changes: 0 additions & 10 deletions wgpu-hal/src/dx12/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -626,16 +626,6 @@ impl crate::Adapter<super::Api> for super::Adapter {
// we currently use a flip effect which supports 2..=16 buffers
swap_chain_sizes: 2..=16,
current_extent,
// TODO: figure out the exact bounds
extents: wgt::Extent3d {
width: 16,
height: 16,
depth_or_array_layers: 1,
}..=wgt::Extent3d {
width: 4096,
height: 4096,
depth_or_array_layers: 1,
},
usage: crate::TextureUses::COLOR_TARGET
| crate::TextureUses::COPY_SRC
| crate::TextureUses::COPY_DST,
Expand Down
10 changes: 0 additions & 10 deletions wgpu-hal/src/gles/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,6 @@ impl super::Adapter {
workarounds,
features,
shading_language_version,
max_texture_size,
next_shader_id: Default::default(),
program_cache: Default::default(),
es: es_ver.is_some(),
Expand Down Expand Up @@ -1145,15 +1144,6 @@ impl crate::Adapter<super::Api> for super::Adapter {
composite_alpha_modes: vec![wgt::CompositeAlphaMode::Opaque], //TODO
swap_chain_sizes: 2..=2,
current_extent: None,
extents: wgt::Extent3d {
width: 4,
height: 4,
depth_or_array_layers: 1,
}..=wgt::Extent3d {
width: self.shared.max_texture_size,
height: self.shared.max_texture_size,
depth_or_array_layers: 1,
},
usage: crate::TextureUses::COLOR_TARGET,
})
} else {
Expand Down
1 change: 0 additions & 1 deletion wgpu-hal/src/gles/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,6 @@ struct AdapterShared {
features: wgt::Features,
workarounds: Workarounds,
shading_language_version: naga::back::glsl::Version,
max_texture_size: u32,
next_shader_id: AtomicU32,
program_cache: Mutex<ProgramCache>,
es: bool,
Expand Down
5 changes: 0 additions & 5 deletions wgpu-hal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -882,11 +882,6 @@ pub struct SurfaceCapabilities {
/// Current extent of the surface, if known.
pub current_extent: Option<wgt::Extent3d>,

/// Range of supported extents.
///
/// `current_extent` must be inside this range.
pub extents: RangeInclusive<wgt::Extent3d>,

/// Supported texture usage flags.
///
/// Must have at least `TextureUses::COLOR_TARGET`
Expand Down
9 changes: 0 additions & 9 deletions wgpu-hal/src/metal/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,15 +339,6 @@ impl crate::Adapter<super::Api> for super::Adapter {
],

current_extent,
extents: wgt::Extent3d {
width: 4,
height: 4,
depth_or_array_layers: 1,
}..=wgt::Extent3d {
width: pc.max_texture_size as u32,
height: pc.max_texture_size as u32,
depth_or_array_layers: 1,
},
usage: crate::TextureUses::COLOR_TARGET | crate::TextureUses::COPY_DST, //TODO: expose more
})
}
Expand Down
13 changes: 0 additions & 13 deletions wgpu-hal/src/vulkan/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1659,18 +1659,6 @@ impl crate::Adapter<super::Api> for super::Adapter {
None
};

let min_extent = wgt::Extent3d {
width: caps.min_image_extent.width,
height: caps.min_image_extent.height,
depth_or_array_layers: 1,
};

let max_extent = wgt::Extent3d {
width: caps.max_image_extent.width,
height: caps.max_image_extent.height,
depth_or_array_layers: caps.max_image_array_layers,
};

let raw_present_modes = {
profiling::scope!("vkGetPhysicalDeviceSurfacePresentModesKHR");
match unsafe {
Expand Down Expand Up @@ -1709,7 +1697,6 @@ impl crate::Adapter<super::Api> for super::Adapter {
formats,
swap_chain_sizes: caps.min_image_count..=max_image_count,
current_extent,
extents: min_extent..=max_extent,
usage: conv::map_vk_image_usage(caps.supported_usage_flags),
present_modes: raw_present_modes
.into_iter()
Expand Down
9 changes: 5 additions & 4 deletions wgpu-hal/src/vulkan/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,11 @@ unsafe extern "system" fn debug_utils_messenger_callback(
}
}

// Silence Vulkan Validation error "VUID-VkSwapchainCreateInfoKHR-imageExtent-01274"
// - it's a false positive due to the inherent racy-ness of surface resizing
const VUID_VKSWAPCHAINCREATEINFOKHR_IMAGEEXTENT_01274: i32 = 0x7cd0911d;
if cd.message_id_number == VUID_VKSWAPCHAINCREATEINFOKHR_IMAGEEXTENT_01274 {
// Silence Vulkan Validation error "VUID-VkSwapchainCreateInfoKHR-pNext-07781"
// This happens when a surface is configured with a size outside the allowed extent.
// It's s false positive due to the inherent racy-ness of surface resizing.
const VUID_VKSWAPCHAINCREATEINFOKHR_PNEXT_07781: i32 = 0x4c8929c1;
if cd.message_id_number == VUID_VKSWAPCHAINCREATEINFOKHR_PNEXT_07781 {
return vk::FALSE;
}

Expand Down

0 comments on commit 8da4925

Please sign in to comment.