Expose shader validation by daxpedda · Pull Request #4811 · gfx-rs/wgpu

daxpedda · 2023-11-30T14:55:24Z

I was trying to pre-compile shaders while still running the exact same validation that wgpu does.
Would it be acceptable to expose this module for that purpose?

Wumpf · 2023-12-02T09:25:08Z

I think the better way to go about this is to call https://docs.rs/naga/0.14.1/naga/valid/struct.Validator.html#method.validate
which is most of what wgpu does. Is there a reason this would miss something in your case?

let module =
    naga::front::wgsl::parse_str(&shader.source).expect("should parse :O");

let mut validator = naga::valid::Validator::new(
    naga::valid::ValidationFlags::all(),
    naga::valid::Capabilities::all(),
);

validator.validate(&module).expect("should be valid!");

The nice thing about this is that you can then at least skip the parsing on native wgpu since you're allowed to pass in a naga module for creating the shader module https://github.com/gfx-rs/wgpu/blob/trunk/wgpu/src/lib.rs#L576

daxpedda · 2023-12-02T10:41:12Z

What you are describing is a different use-case, one which I've used pretty successfully so far!

I'm talking specifically about passing raw SPIRV modules, which I would generate externally first. Wgpu currently does a bunch of validation, including using the Naga Validator, but that's not all it does. E.g. Interface::check_stage() does a lot of checking against the render pipeline, which is completely skipped when providing a raw SPIRV shader.

The goal is to replicate exactly what Wgpu does internally, most of this stuff is exposed and easy to re-use. But the whole validation going on in wgpu_core::validation will require me to almost copy the whole file. Which, alternatively, I can do as well.

I was told before that large parts of the wgpu-core and wgpu-hal API are not exactly considered stable, so I thought exposing this isn't an issue.

Cc #3103, which would make this pointless as well.

Wumpf · 2023-12-02T11:58:42Z

ah I see, thanks for elaborating, makes sense! I'd be in favor of exposing then as well, but I'd like to hear what @cwfitzgerald says

cwfitzgerald

I don't love it, but can't argue against it.

wgpu-core/src/lib.rs

Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>

…eb3c29ef785. r=webgpu-reviewers,supply-chain-reviewers,jimb # Changelog * #4807 Bump wasm-bindgen-test from 0.3.38 to 0.3.39 By dependabot[bot] in gfx-rs/wgpu#4807 * #4830 Use Display instead of Debug to log errors. By nical in gfx-rs/wgpu#4830 * #4755 [naga wgsl-in] Automatic conversions for `var` initializers. By jimblandy in gfx-rs/wgpu#4755 * #4755 [naga wgsl-in] Automatic conversions for `var` initializers. By jimblandy in gfx-rs/wgpu#4755 * #4755 [naga wgsl-in] Automatic conversions for `var` initializers. By jimblandy in gfx-rs/wgpu#4755 * #4832 vulkan: fix multi-planar texture creation By xiaopengli89 in gfx-rs/wgpu#4832 * #4758 [vk] remove (old) unused blocklist for dual source blending By teoxoy in gfx-rs/wgpu#4758 * #4828 Remove DX11 backend By valaphee in gfx-rs/wgpu#4828 * #4836 [gl] add support for line and point polygon modes By valaphee in gfx-rs/wgpu#4836 * #4820 Bump futures-lite from 2.0.1 to 2.1.0 By dependabot[bot] in gfx-rs/wgpu#4820 * #4811 Expose shader validation By daxpedda in gfx-rs/wgpu#4811 * #3507 [wgpu-hal] Inline RayQuery Support By daniel-keitel in gfx-rs/wgpu#3507 * #4726 Fix Javascript exception on repeated `BufferSlice::get_mapped_range` calls By DouglasDwyer in gfx-rs/wgpu#4726 * #4841 Remove `expose-ids` Feature By cwfitzgerald in gfx-rs/wgpu#4841 * #4843 Some Minor `wgpu-core` Cleanups By cwfitzgerald in gfx-rs/wgpu#4843 * #4844 Work around cbindgen issue By nical in gfx-rs/wgpu#4844 Differential Revision: https://phabricator.services.mozilla.com/D195735

…eb3c29ef785. r=webgpu-reviewers,supply-chain-reviewers,jimb # Changelog * #4807 Bump wasm-bindgen-test from 0.3.38 to 0.3.39 By dependabot[bot] in gfx-rs/wgpu#4807 * #4830 Use Display instead of Debug to log errors. By nical in gfx-rs/wgpu#4830 * #4755 [naga wgsl-in] Automatic conversions for `var` initializers. By jimblandy in gfx-rs/wgpu#4755 * #4755 [naga wgsl-in] Automatic conversions for `var` initializers. By jimblandy in gfx-rs/wgpu#4755 * #4755 [naga wgsl-in] Automatic conversions for `var` initializers. By jimblandy in gfx-rs/wgpu#4755 * #4832 vulkan: fix multi-planar texture creation By xiaopengli89 in gfx-rs/wgpu#4832 * #4758 [vk] remove (old) unused blocklist for dual source blending By teoxoy in gfx-rs/wgpu#4758 * #4828 Remove DX11 backend By valaphee in gfx-rs/wgpu#4828 * #4836 [gl] add support for line and point polygon modes By valaphee in gfx-rs/wgpu#4836 * #4820 Bump futures-lite from 2.0.1 to 2.1.0 By dependabot[bot] in gfx-rs/wgpu#4820 * #4811 Expose shader validation By daxpedda in gfx-rs/wgpu#4811 * #3507 [wgpu-hal] Inline RayQuery Support By daniel-keitel in gfx-rs/wgpu#3507 * #4726 Fix Javascript exception on repeated `BufferSlice::get_mapped_range` calls By DouglasDwyer in gfx-rs/wgpu#4726 * #4841 Remove `expose-ids` Feature By cwfitzgerald in gfx-rs/wgpu#4841 * #4843 Some Minor `wgpu-core` Cleanups By cwfitzgerald in gfx-rs/wgpu#4843 * #4844 Work around cbindgen issue By nical in gfx-rs/wgpu#4844 Differential Revision: https://phabricator.services.mozilla.com/D195735 UltraBlame original commit: 0dfeaf41028d13f9dd98b106e262c1deb255398d

daxpedda requested a review from a team as a code owner November 30, 2023 14:55

cwfitzgerald requested changes Dec 6, 2023

View reviewed changes

wgpu-core/src/lib.rs Show resolved Hide resolved

Expose shader validation

378e8fe

daxpedda force-pushed the patch-1 branch from acd5912 to 378e8fe Compare December 6, 2023 20:26

daxpedda commented Dec 6, 2023

View reviewed changes

wgpu-core/src/lib.rs Show resolved Hide resolved

cwfitzgerald reviewed Dec 6, 2023

View reviewed changes

wgpu-core/src/lib.rs Outdated Show resolved Hide resolved

Update wgpu-core/src/lib.rs

b19ddb4

cwfitzgerald approved these changes Dec 6, 2023

View reviewed changes

cwfitzgerald enabled auto-merge (squash) December 6, 2023 20:43

cwfitzgerald merged commit 3e0fb2c into gfx-rs:trunk Dec 6, 2023

bradwerth pushed a commit to bradwerth/wgpu that referenced this pull request Dec 8, 2023

Expose shader validation (gfx-rs#4811)

fa172e1

Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose shader validation#4811

Expose shader validation#4811
cwfitzgerald merged 2 commits intogfx-rs:trunkfrom
daxpedda:patch-1

daxpedda commented Nov 30, 2023

Uh oh!

Wumpf commented Dec 2, 2023 •

edited

Loading

Uh oh!

daxpedda commented Dec 2, 2023

Uh oh!

Wumpf commented Dec 2, 2023

Uh oh!

cwfitzgerald left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

daxpedda commented Nov 30, 2023

Uh oh!

Wumpf commented Dec 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daxpedda commented Dec 2, 2023

Uh oh!

Wumpf commented Dec 2, 2023

Uh oh!

cwfitzgerald left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Wumpf commented Dec 2, 2023 •

edited

Loading