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

Add BGRA8UNORM_STORAGE extension #3634

Closed
wants to merge 3 commits into from
Closed
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
7 changes: 7 additions & 0 deletions deno_webgpu/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,9 @@ fn deserialize_features(features: &wgpu_types::Features) -> Vec<&'static str> {
if features.contains(wgpu_types::Features::RG11B10UFLOAT_RENDERABLE) {
return_features.push("rg11b10ufloat-renderable");
}
if features.contains(wgpu_types::Features::BGRA8UNORM_STORAGE) {
return_features.push("bgra8unorm-storage");
}

// extended from spec

Expand Down Expand Up @@ -491,6 +494,10 @@ impl From<GpuRequiredFeatures> for wgpu_types::Features {
wgpu_types::Features::RG11B10UFLOAT_RENDERABLE,
required_features.0.contains("rg11b10ufloat-renderable"),
);
features.set(
wgpu_types::Features::BGRA8UNORM_STORAGE,
required_features.0.contains("bgra8unorm-storage"),
);

// extended from spec

Expand Down
1 change: 1 addition & 0 deletions deno_webgpu/webgpu.idl
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ enum GPUFeatureName {
// shader
"shader-f16",
"rg11b10ufloat-renderable",
"bgra8unorm-storage",

// extended from spec

Expand Down
2 changes: 1 addition & 1 deletion player/tests/data/buffer-copy.ron
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
(
features: 0x1_0000,
features: 0x2_0000,
expectations: [
(
name: "basic",
Expand Down
2 changes: 1 addition & 1 deletion player/tests/data/clear-buffer-texture.ron
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
(
features: 0x0000_0004_0001_0000,
features: 0x0000_0004_0002_0000,
expectations: [
(
name: "Quad",
Expand Down
2 changes: 1 addition & 1 deletion player/tests/data/zero-init-buffer.ron
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
(
features: 0x1_0000,
features: 0x2_0000,
expectations: [
// Ensuring that mapping zero-inits buffers.
(
Expand Down
11 changes: 11 additions & 0 deletions wgpu-core/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1017,6 +1017,17 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
adapter_guard
.get(adapter_id)
.map(|adapter| adapter.raw.features)
.map(|mut features| {
// SHADER_F16 is not supported in naga yet (https://github.com/gfx-rs/naga/issues/1884)
if features.contains(wgt::Features::SHADER_F16) {
features.remove(wgt::Features::SHADER_F16);
}
// BGRA8UNORM_STORAGE is not supported in naga yet (https://github.com/gfx-rs/naga/issues/2195)
if features.contains(wgt::Features::BGRA8UNORM_STORAGE) {
features.remove(wgt::Features::BGRA8UNORM_STORAGE);
}
features
})
.map_err(|_| InvalidAdapter)
}

Expand Down
23 changes: 22 additions & 1 deletion wgpu-hal/src/dx12/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ use crate::{
};
use std::{mem, ptr, sync::Arc, thread};
use winapi::{
shared::{dxgi, dxgi1_2, minwindef::DWORD, windef, winerror},
shared::{
dxgi, dxgi1_2, dxgiformat::DXGI_FORMAT_B8G8R8A8_UNORM, minwindef::DWORD, windef, winerror,
},
um::{d3d12 as d3d12_ty, d3d12sdklayers, winuser},
};

Expand Down Expand Up @@ -274,6 +276,25 @@ impl super::Adapter {
shader_model_support.HighestShaderModel >= d3d12_ty::D3D_SHADER_MODEL_5_1,
);

let bgra8unorm_storage_supported = {
let mut bgra8unorm_info: d3d12_ty::D3D12_FEATURE_DATA_FORMAT_SUPPORT =
unsafe { mem::zeroed() };
bgra8unorm_info.Format = DXGI_FORMAT_B8G8R8A8_UNORM;
let hr = unsafe {
device.CheckFeatureSupport(
d3d12_ty::D3D12_FEATURE_FORMAT_SUPPORT,
&mut bgra8unorm_info as *mut _ as *mut _,
mem::size_of::<d3d12_ty::D3D12_FEATURE_DATA_FORMAT_SUPPORT>() as _,
)
};
hr == 0
&& (bgra8unorm_info.Support2 & d3d12_ty::D3D12_FORMAT_SUPPORT2_UAV_TYPED_STORE != 0)
};
features.set(
wgt::Features::BGRA8UNORM_STORAGE,
bgra8unorm_storage_supported,
);

// TODO: Determine if IPresentationManager is supported
let presentation_timer = auxil::dxgi::time::PresentationTimer::new_dxgi();

Expand Down
3 changes: 2 additions & 1 deletion wgpu-hal/src/metal/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,8 @@ impl super::PrivateCapabilities {
| F::TEXTURE_FORMAT_16BIT_NORM
| F::SHADER_F16
| F::DEPTH32FLOAT_STENCIL8
| F::MULTI_DRAW_INDIRECT;
| F::MULTI_DRAW_INDIRECT
| F::BGRA8UNORM_STORAGE;

features.set(
F::TIMESTAMP_QUERY,
Expand Down
8 changes: 8 additions & 0 deletions wgpu-hal/src/vulkan/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,14 @@ impl PhysicalDeviceFeatures {
);
features.set(F::RG11B10UFLOAT_RENDERABLE, rg11b10ufloat_renderable);

let bgra8unorm_storage = supports_format(
instance,
phd,
vk::Format::B8G8R8A8_UNORM,
vk::ImageTiling::OPTIMAL,
vk::FormatFeatureFlags::STORAGE_IMAGE,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the shader side we need to use Unknown since bgra8unorm is not in this table.

So, we need to check for VK_FORMAT_FEATURE_2_STORAGE_WRITE_WITHOUT_FORMAT_BIT instead.

See KhronosGroup/Vulkan-Docs#2027 (comment) for more background.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, I don't know how to do the check of VK_FORMAT_FEATURE_2_STORAGE_WRITE_WITHOUT_FORMAT_BIT . Check for FORMAT_FEATURE_2 requires obtaining FormatProperties3 first, but no relevant method was found in ash.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but no way to get it is provided, onlyget_physical_device_format_properties and get_physical_device_format_properties to get FormatProperties FormatProperties2.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, how I think this should work is we call get_physical_device_format_properties2 and pass it a FormatProperties2 with a FormatProperties3 in it's pNext chain.

This is quite convoluted as it requires extensions or specific vulkan versions.

VK_KHR_get_physical_device_properties2 or VK1.1 for the query function and FormatProperties2 structure +
VK_KHR_format_feature_flags2 or VK1.3 for FormatProperties3

);
features.set(F::BGRA8UNORM_STORAGE, bgra8unorm_storage);
(features, dl_flags)
}

Expand Down
21 changes: 19 additions & 2 deletions wgpu-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,19 @@ bitflags::bitflags! {
//
// ? const FORMATS_TIER_1 = 1 << 14; (https://github.com/gpuweb/gpuweb/issues/3837)
// ? const RW_STORAGE_TEXTURE_TIER_1 = 1 << 15; (https://github.com/gpuweb/gpuweb/issues/3838)
// TODO const BGRA8UNORM_STORAGE = 1 << 16;

/// Allows the [`wgpu::TextureUsages::STORAGE_BINDING`] usage on textures with format [`TextureFormat::Bgra8unorm`]
///
/// Note: this is not supported in naga yet.
///
/// Supported Platforms:
/// - Vulkan
/// - DX12
/// - Metal
///
/// This is a web and native feature.
const BGRA8UNORM_STORAGE = 1 << 16;

// ? const NORM16_FILTERABLE = 1 << 17; (https://github.com/gpuweb/gpuweb/issues/3839)
// ? const NORM16_RESOLVE = 1 << 18; (https://github.com/gpuweb/gpuweb/issues/3839)
// TODO const FLOAT32_FILTERABLE = 1 << 19;
Expand Down Expand Up @@ -2857,6 +2869,11 @@ impl TextureFormat {
} else {
basic
};
let bgra8unorm = if device_features.contains(Features::BGRA8UNORM_STORAGE) {
attachment | TextureUsages::STORAGE_BINDING
} else {
attachment
};

#[rustfmt::skip] // lets make a nice table
let (
Expand Down Expand Up @@ -2885,7 +2902,7 @@ impl TextureFormat {
Self::Rgba8Snorm => ( noaa, storage),
Self::Rgba8Uint => ( msaa, all_flags),
Self::Rgba8Sint => ( msaa, all_flags),
Self::Bgra8Unorm => (msaa_resolve, attachment),
Self::Bgra8Unorm => (msaa_resolve, bgra8unorm),
Self::Bgra8UnormSrgb => (msaa_resolve, attachment),
Self::Rgb10a2Unorm => (msaa_resolve, attachment),
Self::Rg11b10Float => ( msaa, rg11b10f),
Expand Down
6 changes: 5 additions & 1 deletion wgpu/src/backend/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ fn map_map_mode(mode: crate::MapMode) -> u32 {
}
}

const FEATURES_MAPPING: [(wgt::Features, web_sys::GpuFeatureName); 9] = [
const FEATURES_MAPPING: [(wgt::Features, web_sys::GpuFeatureName); 10] = [
//TODO: update the name
(
wgt::Features::DEPTH_CLIP_CONTROL,
Expand Down Expand Up @@ -673,6 +673,10 @@ const FEATURES_MAPPING: [(wgt::Features, web_sys::GpuFeatureName); 9] = [
wgt::Features::RG11B10UFLOAT_RENDERABLE,
web_sys::GpuFeatureName::Rg11b10ufloatRenderable,
),
(
wgt::Features::BGRA8UNORM_STORAGE,
web_sys::GpuFeatureName::Bgra8unormStorage,
),
];

fn map_wgt_features(supported_features: web_sys::GpuSupportedFeatures) -> wgt::Features {
Expand Down