1226: Update gfx and naga to gfx-12 r=kvark a=kvark

**Connections**
Picks up https://github.com/gfx-rs/gfx/pull/3650
Fixes #1225

**Description**
Also fixes our playtests, interestingly. One of the changes here was that now `Analysis` is needed for everything, including the SPV-out backend of Naga. And we can only get it via validating the `naga::Module`.

What we used to was: if validation isn't enabled, we'd carry the module around, and then still try to produce a SPIR-V from it, if there is no original SPIR-V. This precise thing was happening in the tests. However, we had `dummy` backend depending on the `wgpu-core/cross` feature, which means the playtests were actually built and run with spirv-cross enabled!
Another factor here is the introduction of `ShaderModule::flags` instead of a single boolean, which was done in #1093. The problem here was that the playtest RON files weren't updated accordingly, they still had `experimental_translation: true` in them, which got ignored now.
So with this combination of events, the playtests ended up generating SPIR-V from Naga modules, without validation(!), and passing the SPIR-V to gfx-rs and SPIRV-Cross... Which still worked, as we'd expect, but definitely not want here.

So this PR fixes everything. It makes it required to have the validation for a module, and we force-validate if there is no SPV source to fall back on. And it disables "cross" feature implicitly enabled in testing.

**Testing**
Tested on wgpu-rs examples.

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
This commit is contained in:
bors[bot]
2021-02-21 16:36:54 +00:00
committed by GitHub
10 changed files with 74 additions and 80 deletions

20
Cargo.lock generated
View File

@@ -475,7 +475,7 @@ dependencies = [
[[package]]
name = "gfx-auxil"
version = "0.8.0"
source = "git+https://github.com/gfx-rs/gfx?rev=80655862ac4afc5efb6675eb9ac7c38e73544750#80655862ac4afc5efb6675eb9ac7c38e73544750"
source = "git+https://github.com/gfx-rs/gfx?rev=a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce#a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce"
dependencies = [
"fxhash",
"gfx-hal",
@@ -485,7 +485,7 @@ dependencies = [
[[package]]
name = "gfx-backend-dx11"
version = "0.7.0"
source = "git+https://github.com/gfx-rs/gfx?rev=80655862ac4afc5efb6675eb9ac7c38e73544750#80655862ac4afc5efb6675eb9ac7c38e73544750"
source = "git+https://github.com/gfx-rs/gfx?rev=a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce#a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce"
dependencies = [
"arrayvec",
"bitflags",
@@ -506,7 +506,7 @@ dependencies = [
[[package]]
name = "gfx-backend-dx12"
version = "0.7.0"
source = "git+https://github.com/gfx-rs/gfx?rev=80655862ac4afc5efb6675eb9ac7c38e73544750#80655862ac4afc5efb6675eb9ac7c38e73544750"
source = "git+https://github.com/gfx-rs/gfx?rev=a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce#a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce"
dependencies = [
"arrayvec",
"bit-set",
@@ -526,7 +526,7 @@ dependencies = [
[[package]]
name = "gfx-backend-empty"
version = "0.7.0"
source = "git+https://github.com/gfx-rs/gfx?rev=80655862ac4afc5efb6675eb9ac7c38e73544750#80655862ac4afc5efb6675eb9ac7c38e73544750"
source = "git+https://github.com/gfx-rs/gfx?rev=a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce#a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce"
dependencies = [
"gfx-hal",
"log",
@@ -536,7 +536,7 @@ dependencies = [
[[package]]
name = "gfx-backend-gl"
version = "0.7.0"
source = "git+https://github.com/gfx-rs/gfx?rev=80655862ac4afc5efb6675eb9ac7c38e73544750#80655862ac4afc5efb6675eb9ac7c38e73544750"
source = "git+https://github.com/gfx-rs/gfx?rev=a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce#a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce"
dependencies = [
"arrayvec",
"bitflags",
@@ -559,7 +559,7 @@ dependencies = [
[[package]]
name = "gfx-backend-metal"
version = "0.7.0"
source = "git+https://github.com/gfx-rs/gfx?rev=80655862ac4afc5efb6675eb9ac7c38e73544750#80655862ac4afc5efb6675eb9ac7c38e73544750"
source = "git+https://github.com/gfx-rs/gfx?rev=a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce#a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce"
dependencies = [
"arrayvec",
"bitflags",
@@ -584,7 +584,7 @@ dependencies = [
[[package]]
name = "gfx-backend-vulkan"
version = "0.7.0"
source = "git+https://github.com/gfx-rs/gfx?rev=80655862ac4afc5efb6675eb9ac7c38e73544750#80655862ac4afc5efb6675eb9ac7c38e73544750"
source = "git+https://github.com/gfx-rs/gfx?rev=a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce#a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce"
dependencies = [
"arrayvec",
"ash",
@@ -604,7 +604,7 @@ dependencies = [
[[package]]
name = "gfx-hal"
version = "0.7.0"
source = "git+https://github.com/gfx-rs/gfx?rev=80655862ac4afc5efb6675eb9ac7c38e73544750#80655862ac4afc5efb6675eb9ac7c38e73544750"
source = "git+https://github.com/gfx-rs/gfx?rev=a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce#a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce"
dependencies = [
"bitflags",
"naga",
@@ -946,7 +946,7 @@ dependencies = [
[[package]]
name = "naga"
version = "0.3.1"
source = "git+https://github.com/gfx-rs/naga?tag=gfx-11#539db1a317d62c8c50e21ff908cf952c731a05b6"
source = "git+https://github.com/gfx-rs/naga?tag=gfx-12#fa7d4d8b51d4eeffe9f648d285466637f733a4a1"
dependencies = [
"bit-set",
"bitflags",
@@ -1212,7 +1212,7 @@ dependencies = [
[[package]]
name = "range-alloc"
version = "0.1.2"
source = "git+https://github.com/gfx-rs/gfx?rev=80655862ac4afc5efb6675eb9ac7c38e73544750#80655862ac4afc5efb6675eb9ac7c38e73544750"
source = "git+https://github.com/gfx-rs/gfx?rev=a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce#a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce"
[[package]]
name = "raw-window-handle"

View File

@@ -12,4 +12,4 @@ publish = false
[dependencies.wgc]
path = "../wgpu-core"
package = "wgpu-core"
features = ["cross", "serial-pass", "trace"]
features = ["serial-pass", "trace"]

View File

@@ -13,6 +13,7 @@ license = "MPL-2.0"
publish = false
[features]
cross = ["wgc/cross"]
[dependencies]
env_logger = "0.8"

View File

@@ -11,7 +11,7 @@
id: Id(0, 1, Empty),
desc: (
label: None,
experimental_translation: true,
flags: (bits: 3),
),
data: "empty.wgsl",
),

View File

@@ -84,7 +84,7 @@
id: Id(0, 1, Empty),
desc: (
label: None,
experimental_translation: true,
flags: (bits: 3),
),
data: "buffer-zero-init-for-binding.wgsl",
),

View File

@@ -13,7 +13,7 @@
id: Id(0, 1, Empty),
desc: (
label: None,
experimental_translation: true,
flags: (bits: 3),
),
data: "quad.wgsl",
),

View File

@@ -38,28 +38,28 @@ thiserror = "1"
gpu-alloc = { version = "0.3", features = ["tracing"] }
gpu-descriptor = { version = "0.1", features = ["tracing"] }
hal = { package = "gfx-hal", git = "https://github.com/gfx-rs/gfx", rev = "80655862ac4afc5efb6675eb9ac7c38e73544750" }
gfx-backend-empty = { git = "https://github.com/gfx-rs/gfx", rev = "80655862ac4afc5efb6675eb9ac7c38e73544750" }
hal = { package = "gfx-hal", git = "https://github.com/gfx-rs/gfx", rev = "a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce" }
gfx-backend-empty = { git = "https://github.com/gfx-rs/gfx", rev = "a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce" }
[target.'cfg(all(not(target_arch = "wasm32"), all(unix, not(target_os = "ios"), not(target_os = "macos"))))'.dependencies]
gfx-backend-vulkan = { git = "https://github.com/gfx-rs/gfx", rev = "80655862ac4afc5efb6675eb9ac7c38e73544750", features = ["naga"] }
gfx-backend-gl = { git = "https://github.com/gfx-rs/gfx", rev = "80655862ac4afc5efb6675eb9ac7c38e73544750" }
gfx-backend-vulkan = { git = "https://github.com/gfx-rs/gfx", rev = "a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce", features = ["naga"] }
gfx-backend-gl = { git = "https://github.com/gfx-rs/gfx", rev = "a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce" }
[target.'cfg(all(not(target_arch = "wasm32"), any(target_os = "ios", target_os = "macos")))'.dependencies]
gfx-backend-metal = { git = "https://github.com/gfx-rs/gfx", rev = "80655862ac4afc5efb6675eb9ac7c38e73544750" }
gfx-backend-vulkan = { git = "https://github.com/gfx-rs/gfx", rev = "80655862ac4afc5efb6675eb9ac7c38e73544750", optional = true }
gfx-backend-metal = { git = "https://github.com/gfx-rs/gfx", rev = "a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce" }
gfx-backend-vulkan = { git = "https://github.com/gfx-rs/gfx", rev = "a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce", optional = true }
[target.'cfg(all(not(target_arch = "wasm32"), windows))'.dependencies]
gfx-backend-dx12 = { git = "https://github.com/gfx-rs/gfx", rev = "80655862ac4afc5efb6675eb9ac7c38e73544750" }
gfx-backend-dx11 = { git = "https://github.com/gfx-rs/gfx", rev = "80655862ac4afc5efb6675eb9ac7c38e73544750" }
gfx-backend-vulkan = { git = "https://github.com/gfx-rs/gfx", rev = "80655862ac4afc5efb6675eb9ac7c38e73544750", features = ["naga"] }
gfx-backend-dx12 = { git = "https://github.com/gfx-rs/gfx", rev = "a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce" }
gfx-backend-dx11 = { git = "https://github.com/gfx-rs/gfx", rev = "a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce" }
gfx-backend-vulkan = { git = "https://github.com/gfx-rs/gfx", rev = "a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce", features = ["naga"] }
[target.'cfg(target_arch = "wasm32")'.dependencies]
gfx-backend-gl = { git = "https://github.com/gfx-rs/gfx", rev = "80655862ac4afc5efb6675eb9ac7c38e73544750" }
gfx-backend-gl = { git = "https://github.com/gfx-rs/gfx", rev = "a76e68713ff46f0e5a3d0c31516e20d18bf0a3ce" }
[dependencies.naga]
git = "https://github.com/gfx-rs/naga"
tag = "gfx-11"
tag = "gfx-12"
features = ["spv-in", "spv-out", "wgsl-in"]
[dependencies.wgt]

View File

@@ -1008,7 +1008,7 @@ impl<B: GfxBackend> Device<B> {
let (naga_result, interface) = match module {
// If succeeded, then validate it and attempt to give it to gfx-hal directly.
Some(module) if desc.flags.contains(wgt::ShaderFlags::VALIDATION) => {
Some(module) if desc.flags.contains(wgt::ShaderFlags::VALIDATION) || spv.is_none() => {
let analysis = naga::proc::Validator::new().validate(&module)?;
if !self.features.contains(wgt::Features::PUSH_CONSTANTS)
&& module
@@ -1020,39 +1020,43 @@ impl<B: GfxBackend> Device<B> {
wgt::Features::PUSH_CONSTANTS,
));
}
let interface = validation::Interface::new(&module);
let interface = validation::Interface::new(&module, &analysis);
let shader = hal::device::NagaShader { module, analysis };
let naga_result = if desc
.flags
.contains(wgt::ShaderFlags::EXPERIMENTAL_TRANSLATION)
{
let shader = hal::device::NagaShader { module, analysis };
match unsafe { self.raw.create_shader_module_from_naga(shader) } {
Ok(raw) => Ok(raw),
Err((hal::device::ShaderError::CompilationFailed(msg), shader)) => {
tracing::warn!("Shader module compilation failed: {}", msg);
Err(Some(shader.module))
Err(Some(shader))
}
Err((_, shader)) => Err(Some(shader.module)),
Err((_, shader)) => Err(Some(shader)),
}
} else {
Err(Some(module))
Err(Some(shader))
};
(naga_result, Some(interface))
}
Some(module) => (Err(Some(module)), None),
_ => (Err(None), None),
};
// Otherwise, fall back to SPIR-V.
let spv_result = match naga_result {
Ok(raw) => Ok(raw),
Err(maybe_module) => {
Err(maybe_shader) => {
let spv = match spv {
Some(data) => Ok(data),
None => {
// Produce a SPIR-V from the Naga module
let module = maybe_module.unwrap();
naga::back::spv::write_vec(&module, &self.spv_options).map(Cow::Owned)
let shader = maybe_shader.unwrap();
naga::back::spv::write_vec(
&shader.module,
&shader.analysis,
&self.spv_options,
)
.map(Cow::Owned)
}
};
match spv {

View File

@@ -130,7 +130,7 @@ impl<B: GfxBackend> Adapter<B> {
span!(_guard, INFO, "Adapter::new");
let adapter_features = raw.physical_device.features();
let adapter_limits = raw.physical_device.limits();
let properties = raw.physical_device.properties();
let mut features = wgt::Features::default()
| wgt::Features::MAPPABLE_PRIMARY_BUFFERS
@@ -182,7 +182,7 @@ impl<B: GfxBackend> Adapter<B> {
);
features.set(
wgt::Features::TIMESTAMP_QUERY,
adapter_limits.timestamp_compute_and_graphics,
properties.limits.timestamp_compute_and_graphics,
);
features.set(
wgt::Features::PIPELINE_STATISTICS_QUERY,
@@ -215,40 +215,35 @@ impl<B: GfxBackend> Adapter<B> {
// All these casts to u32 are safe as the underlying vulkan types are u32s.
// If another backend provides larger limits than u32, we need to clamp them to u32::MAX.
// TODO: fix all gfx-hal backends to produce limits we care about, and remove .max
let desc_limits = &properties.limits.descriptor_limits;
let limits = wgt::Limits {
max_bind_groups: (adapter_limits.max_bound_descriptor_sets as u32)
max_bind_groups: (properties.limits.max_bound_descriptor_sets as u32)
.min(MAX_BIND_GROUPS as u32)
.max(default_limits.max_bind_groups),
max_dynamic_uniform_buffers_per_pipeline_layout: (adapter_limits
max_dynamic_uniform_buffers_per_pipeline_layout: desc_limits
.max_descriptor_set_uniform_buffers_dynamic
as u32)
.max(default_limits.max_dynamic_uniform_buffers_per_pipeline_layout),
max_dynamic_storage_buffers_per_pipeline_layout: (adapter_limits
max_dynamic_storage_buffers_per_pipeline_layout: desc_limits
.max_descriptor_set_storage_buffers_dynamic
as u32)
.max(default_limits.max_dynamic_storage_buffers_per_pipeline_layout),
max_sampled_textures_per_shader_stage: (adapter_limits
max_sampled_textures_per_shader_stage: desc_limits
.max_per_stage_descriptor_sampled_images
as u32)
.max(default_limits.max_sampled_textures_per_shader_stage),
max_samplers_per_shader_stage: (adapter_limits.max_per_stage_descriptor_samplers
as u32)
max_samplers_per_shader_stage: desc_limits
.max_per_stage_descriptor_samplers
.max(default_limits.max_samplers_per_shader_stage),
max_storage_buffers_per_shader_stage: (adapter_limits
max_storage_buffers_per_shader_stage: desc_limits
.max_per_stage_descriptor_storage_buffers
as u32)
.max(default_limits.max_storage_buffers_per_shader_stage),
max_storage_textures_per_shader_stage: (adapter_limits
max_storage_textures_per_shader_stage: desc_limits
.max_per_stage_descriptor_storage_images
as u32)
.max(default_limits.max_storage_textures_per_shader_stage),
max_uniform_buffers_per_shader_stage: (adapter_limits
max_uniform_buffers_per_shader_stage: desc_limits
.max_per_stage_descriptor_uniform_buffers
as u32)
.max(default_limits.max_uniform_buffers_per_shader_stage),
max_uniform_buffer_binding_size: (adapter_limits.max_uniform_buffer_range as u32)
max_uniform_buffer_binding_size: (properties.limits.max_uniform_buffer_range as u32)
.max(default_limits.max_uniform_buffer_binding_size),
max_push_constant_size: (adapter_limits.max_push_constants_size as u32)
max_push_constant_size: (properties.limits.max_push_constants_size as u32)
.max(MIN_PUSH_CONSTANT_SIZE), // As an extension, the default is always 0, so define a separate minimum.
};
@@ -465,7 +460,7 @@ impl<B: GfxBackend> Adapter<B> {
//TODO
}
let limits = phd.limits();
let limits = phd.properties().limits;
assert_eq!(
0,
BIND_BUFFER_ALIGNMENT % limits.min_storage_buffer_offset_alignment,

View File

@@ -3,6 +3,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
use crate::{binding_model::BindEntryMap, FastHashMap};
use naga::proc::analyzer::GlobalUse;
use std::collections::hash_map::Entry;
use thiserror::Error;
use wgt::{BindGroupLayoutEntry, BindingType};
@@ -60,7 +61,7 @@ struct SpecializationConstant {
struct EntryPoint {
inputs: Vec<Varying>,
outputs: Vec<Varying>,
resources: Vec<(naga::Handle<Resource>, naga::GlobalUse)>,
resources: Vec<(naga::Handle<Resource>, GlobalUse)>,
spec_constants: Vec<SpecializationConstant>,
}
@@ -117,7 +118,7 @@ pub enum BindingError {
#[error("visibility flags don't include the shader stage")]
Invisible,
#[error("load/store access flags {0:?} don't match the shader")]
WrongUsage(naga::GlobalUse),
WrongUsage(GlobalUse),
#[error("type on the shader side does not match the pipeline binding")]
WrongType,
#[error("buffer structure size {0}, added to one element of an unbound array, if it's the last field, ended up greater than the given `min_binding_size`")]
@@ -304,7 +305,7 @@ impl Resource {
fn check_binding_use(
&self,
entry: &BindGroupLayoutEntry,
shader_usage: naga::GlobalUse,
shader_usage: GlobalUse,
) -> Result<(), BindingError> {
let allowed_usage = match self.ty {
ResourceType::Buffer { size } => {
@@ -317,11 +318,9 @@ impl Resource {
let global_use = match ty {
wgt::BufferBindingType::Uniform
| wgt::BufferBindingType::Storage { read_only: true } => {
naga::GlobalUse::READ
}
wgt::BufferBindingType::Storage { read_only: _ } => {
naga::GlobalUse::all()
GlobalUse::READ
}
wgt::BufferBindingType::Storage { read_only: _ } => GlobalUse::all(),
};
(global_use, min_binding_size)
}
@@ -341,7 +340,7 @@ impl Resource {
comparison: cmp,
} => {
if cmp == comparison {
naga::GlobalUse::READ
GlobalUse::READ
} else {
return Err(BindingError::WrongSamplerComparison);
}
@@ -412,7 +411,7 @@ impl Resource {
},
wgt::TextureSampleType::Depth => naga::ImageClass::Depth,
};
(class, naga::GlobalUse::READ)
(class, GlobalUse::READ)
}
BindingType::StorageTexture {
access,
@@ -422,9 +421,9 @@ impl Resource {
let naga_format = map_storage_format_to_naga(format)
.ok_or(BindingError::BadStorageFormat(format))?;
let usage = match access {
wgt::StorageTextureAccess::ReadOnly => naga::GlobalUse::READ,
wgt::StorageTextureAccess::WriteOnly => naga::GlobalUse::WRITE,
wgt::StorageTextureAccess::ReadWrite => naga::GlobalUse::all(),
wgt::StorageTextureAccess::ReadOnly => GlobalUse::READ,
wgt::StorageTextureAccess::WriteOnly => GlobalUse::WRITE,
wgt::StorageTextureAccess::ReadWrite => GlobalUse::all(),
};
(naga::ImageClass::Storage(naga_format), usage)
}
@@ -447,16 +446,13 @@ impl Resource {
}
}
fn derive_binding_type(
&self,
shader_usage: naga::GlobalUse,
) -> Result<BindingType, BindingError> {
fn derive_binding_type(&self, shader_usage: GlobalUse) -> Result<BindingType, BindingError> {
Ok(match self.ty {
ResourceType::Buffer { size } => BindingType::Buffer {
ty: match self.class {
naga::StorageClass::Uniform => wgt::BufferBindingType::Uniform,
naga::StorageClass::Storage => wgt::BufferBindingType::Storage {
read_only: !shader_usage.contains(naga::GlobalUse::WRITE),
read_only: !shader_usage.contains(GlobalUse::WRITE),
},
_ => return Err(BindingError::WrongType),
},
@@ -499,7 +495,7 @@ impl Resource {
multisampled: false,
},
naga::ImageClass::Storage(format) => BindingType::StorageTexture {
access: if shader_usage.contains(naga::GlobalUse::WRITE) {
access: if shader_usage.contains(GlobalUse::WRITE) {
wgt::StorageTextureAccess::WriteOnly
} else {
wgt::StorageTextureAccess::ReadOnly
@@ -686,7 +682,7 @@ pub fn check_texture_format(format: wgt::TextureFormat, output: &NumericType) ->
pub type StageIo = FastHashMap<wgt::ShaderLocation, NumericType>;
impl Interface {
pub fn new(module: &naga::Module) -> Self {
pub fn new(module: &naga::Module, analysis: &naga::proc::analyzer::Analysis) -> Self {
let mut resources = naga::Arena::new();
let mut resource_mapping = FastHashMap::default();
for (var_handle, var) in module.global_variables.iter() {
@@ -731,13 +727,11 @@ impl Interface {
let mut entry_points = FastHashMap::default();
entry_points.reserve(module.entry_points.len());
for (&(stage, ref ep_name), entry_point) in module.entry_points.iter() {
for (&(stage, ref ep_name), _entry_point) in module.entry_points.iter() {
let info = analysis.get_entry_point(stage, ep_name);
let mut ep = EntryPoint::default();
for ((var_handle, var), &usage) in module
.global_variables
.iter()
.zip(&entry_point.function.global_usage)
{
for (var_handle, var) in module.global_variables.iter() {
let usage = info[var_handle];
if usage.is_empty() {
continue;
}