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

wgpu-core: Only produce StageError::InputNotConsumed on DX11/DX12 #4116

Merged
merged 3 commits into from
Sep 21, 2023

Conversation

Aaron1011
Copy link
Contributor

Checklist

  • Run cargo clippy.
  • Run cargo clippy --target wasm32-unknown-unknown if applicable.
  • Add change to CHANGELOG.md. See simple instructions inside file.

Connections

Description

This error only exists due to an issue with naga's HLSL support:
The WGPU spec itself allows vertex shader outputs that are not consumed by the fragment shader.

Until the issue is fixed, we can allow unconsumed outputs on all platforms other than DX11/DX12.

Testing
Run a program with an unconsumed shader output on a non-DX backend, and note the lack of error.

@Aaron1011 Aaron1011 force-pushed the consume-output-dx-only branch from 8b5f328 to 6cfd34e Compare September 5, 2023 01:45
@cwfitzgerald
Copy link
Member

This is a pretty big portability hazard. If we wanted to do this, we would need to surface it as a feature that users can enable, knowing they are opting into behavior that is not portable to D3D. I would personally prefer if this was just fixed in naga, but a feature would be an acceptable interim solution.

@nical nical marked this pull request as draft September 12, 2023 08:55
@nical
Copy link
Contributor

nical commented Sep 12, 2023

Marked the PR as draft to reflect that Connor requested some changes to be made before it can be merged.

@Aaron1011 Aaron1011 marked this pull request as ready for review September 13, 2023 23:43
wgpu-hal/src/gles/adapter.rs Outdated Show resolved Hide resolved
@nical
Copy link
Contributor

nical commented Sep 20, 2023

Looks ready once the merge conflict in wgpu-types/src/lib.rs is resolved.

This error only exists due to an issue with naga's HLSL support:
gfx-rs/naga#1945
The WGPU spec itself allows vertex shader outputs that are
not consumed by the fragment shader.

Until the issue is fixed, we can allow unconsumed outputs on
all platforms other than DX11/DX12.
@Aaron1011 Aaron1011 force-pushed the consume-output-dx-only branch from b002299 to c31d0bc Compare September 20, 2023 23:40
@Aaron1011
Copy link
Contributor Author

I fixed the merge conflict - I had to pick a different id for the feature

@cwfitzgerald cwfitzgerald merged commit 2b4a8b3 into gfx-rs:trunk Sep 21, 2023
20 checks passed
Aaron1011 added a commit to ruffle-rs/wgpu that referenced this pull request Sep 30, 2023
…x-rs#4116)

* wgpu-core: Only produce StageError::InputNotConsumed on DX11/DX12

This error only exists due to an issue with naga's HLSL support:
gfx-rs/naga#1945
The WGPU spec itself allows vertex shader outputs that are
not consumed by the fragment shader.

Until the issue is fixed, we can allow unconsumed outputs on
all platforms other than DX11/DX12.

* Add Features::SHADER_UNUSED_VERTEX_OUTPUT to allow disabling check

* Pick an unused feature id
torokati44 pushed a commit to torokati44/wgpu that referenced this pull request Oct 9, 2023
…x-rs#4116)

* wgpu-core: Only produce StageError::InputNotConsumed on DX11/DX12

This error only exists due to an issue with naga's HLSL support:
gfx-rs/naga#1945
The WGPU spec itself allows vertex shader outputs that are
not consumed by the fragment shader.

Until the issue is fixed, we can allow unconsumed outputs on
all platforms other than DX11/DX12.

* Add Features::SHADER_UNUSED_VERTEX_OUTPUT to allow disabling check

* Pick an unused feature id
@torokati44
Copy link
Contributor

Would it be possible to get this backported to the v0.17 branch, please? And perhaps get a new release with it in somewhat soon-ish?
We're in a similar situation as #3817 (comment).

torokati44 pushed a commit to torokati44/wgpu that referenced this pull request Oct 9, 2023
…x-rs#4116)

* wgpu-core: Only produce StageError::InputNotConsumed on DX11/DX12

This error only exists due to an issue with naga's HLSL support:
gfx-rs/naga#1945
The WGPU spec itself allows vertex shader outputs that are
not consumed by the fragment shader.

Until the issue is fixed, we can allow unconsumed outputs on
all platforms other than DX11/DX12.

* Add Features::SHADER_UNUSED_VERTEX_OUTPUT to allow disabling check

* Pick an unused feature id
/// - Metal
/// - OpenGL
const SHADER_UNUSED_VERTEX_OUTPUT = 1 << 54;

// 54..59 available
Copy link
Contributor

Choose a reason for hiding this comment

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

Just noticed, this line did not get updated.

torokati44 pushed a commit to torokati44/wgpu that referenced this pull request Oct 10, 2023
…x-rs#4116)

* wgpu-core: Only produce StageError::InputNotConsumed on DX11/DX12

This error only exists due to an issue with naga's HLSL support:
gfx-rs/naga#1945
The WGPU spec itself allows vertex shader outputs that are
not consumed by the fragment shader.

Until the issue is fixed, we can allow unconsumed outputs on
all platforms other than DX11/DX12.

* Add Features::SHADER_UNUSED_VERTEX_OUTPUT to allow disabling check

* Pick an unused feature id
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants