Skip to content

Allow separate read-only state for depth and stencil aspects #4331

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.

Sign up for GitHub

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

Jump to bottom
Merged
kainino0x merged 1 commit into from
Oct 31, 2023
Merged

Allow separate read-only state for depth and stencil aspects #4331

kainino0x merged 1 commit into from
Oct 31, 2023

Conversation

@kainino0x
Copy link
Contributor

@kainino0x kainino0x commented Oct 13, 2023 •
edited
Loading

Draft PR while the change is still under deliberation.

Fixes #3036

@kainino0x kainino0x added proposal needs-cts-issue This change requires tests (or would need tests if accepted), but may not have a CTS issue filed yet labels Oct 13, 2023
@kainino0x kainino0x added this to the Milestone 1 milestone Oct 13, 2023
@kainino0x kainino0x requested a review from toji October 13, 2023 20:27
@github-actions
Copy link
Contributor

github-actions bot commented Oct 13, 2023

Previews, as seen when this build job started (6295be0):
WebGPU webgpu.idl | Explainer | Correspondence Reference
WGSL grammar.js | wgsl.lalr.txt

toji
toji reviewed Oct 13, 2023
View reviewed changes
Copy link
Member

@toji toji left a comment

Choose a reason for hiding this comment

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

Spec changes look good, assuming that the change is approved by the group.

@Kangz
Copy link
Contributor

Kangz commented Oct 31, 2023

In the last WebGPU meeting all participants agreed to landing this, so I think this PR should be good to go?

@kainino0x kainino0x marked this pull request as ready for review October 31, 2023 16:18
@kainino0x kainino0x merged commit 3243b58 into gpuweb:main Oct 31, 2023
@kainino0x kainino0x deleted the separate-rods branch October 31, 2023 16:34
@teoxoy
Copy link
Member

teoxoy commented Nov 2, 2023

I thought we are still waiting on #3036 (comment)?

@Kangz
Copy link
Contributor

Kangz commented Nov 2, 2023

I did end up validating it with at least KHR_maintenance2. Without it we technically have a race that the VVL catch (unless we use STORE_OP_NONE) but that's ok, we can require maintenance2. I have tests for various cases that work on all target APIs (D3D11/12, Metal, OpenGL ES and Vulkan).

@teoxoy
Copy link
Member

teoxoy commented Nov 2, 2023

Sounds good to me but we should update #1069.

teoxoy added a commit to kainino0x/gpuinfo-vulkan-query that referenced this pull request Nov 2, 2023
@teoxoy
require VK_KHR_maintenance2
fa6e22a 
see gpuweb/gpuweb#4331 (comment)
@teoxoy teoxoy mentioned this pull request Nov 2, 2023
Merged
@Kangz
Copy link
Contributor

Kangz commented Nov 2, 2023

Done

kainino0x pushed a commit to kainino0x/gpuinfo-vulkan-query that referenced this pull request Nov 3, 2023
@teoxoy
require VK_KHR_maintenance2 (#10)
a7089aa 
see gpuweb/gpuweb#4331 (comment)
@Kangz Kangz mentioned this pull request Jan 4, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@toji toji toji approved these changes

Assignees

No one assigned

Labels

needs-cts-issue This change requires tests (or would need tests if accepted), but may not have a CTS issue filed yet

Projects

None yet

Milestone

Milestone 1

Development

Successfully merging this pull request may close these issues.

Separate read-only state for stencil and depth aspects.

4 participants

@kainino0x @Kangz @teoxoy @toji