-
Notifications
You must be signed in to change notification settings - Fork 84
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
Depth clip/clamp test #753
Conversation
Previews, as seen when this build job started (1d6a59c): |
4870d6f
to
ba512fb
Compare
ba512fb
to
8480dc5
Compare
8480dc5
to
bf076c7
Compare
Previews, as seen when this build job started (bf076c7): |
PTAL! @kvark @Jiawei-Shao since you've been following gpuweb/gpuweb#2100, perhaps you could both review these (rather nitpicky) tests? Please see if there's aspect I've missed testing. @Jiawei-Shao, perhaps additionally you could verify these tests against an implementation of the depth-clamping feature on D3D12, since I think you were working on that? These tests are passing in Chromium (except for depth16unorm which we don't implement yet) on Metal (with both depthClamp values) and D3D12 (with depthClamp:false). They pass on Vulkan with clampDepth:true but fail with clampDepth:false, because for consistency with D3D12/Metal, Tint will need to inject a clamp to the viewport bounds at the end of fragment stages that write These tests will need updating later because I think we need to change the spec from having a depthClamp toggle to having a depthClip toggle. |
Also I tried to write good comments, but please let me know if any of it can be clarified further. |
Thanks! |
See gpuweb/gpuweb#2100
In Chromium (except format we don't support):
Author checklist for test code/plans:
.unimplemented()
on a test./** doc comments */
and can be found viahelper_index.txt
.Reviewer sign-off for test code/plans: (Note: feel free to pull in other reviewers at any time for any reason.)
(This is critical for coverage. Assume anything without a TODO will be forgotten about forever.)