-
Notifications
You must be signed in to change notification settings - Fork 82
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
op: Add 'srcFactor' and 'dstFactor' to the parameters in 'blending,clamping' test #2107
base: main
Are you sure you want to change the base?
Conversation
Previews, as seen when this build job started (418a795): |
.filter(t => { | ||
return ( | ||
!( | ||
t.srcFactor === 'one-minus-src-alpha' || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I applied these factors that include 'alpha', the test failed. Do you know any clue why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I run this PR without the filter then I hit an unreachable
in calculateExpectedClampValue
. But fixing that it still fails. At least some of the failures are because we expect the color and alpha channels to always be the same, but that's not always true: https://gpuweb.github.io/gpuweb/#enumdef-gpublendfactor
"src-alpha-saturated" (min(Asrc, 1 - Adst), min(Asrc, 1 - Adst), min(Asrc, 1 - Adst), 1)
Note the last channel has different factors.
The other issue I see is that when the format is rg16float
, there is no alpha channel. Therefore dst-alpha won't have the initial value the code expects. I think missing channels default to ,0,0,1 so dst-alpha would be 1 (similar to this), but I'm not 100% sure about that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's ,0,0,1. Was confirmed in gpuweb/gpuweb#2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other issue I see is that when the format is rg16float, there is no alpha channel. Therefore dst-alpha won't have the initial value the code expects. I think missing channels default to ,0,0,1 so dst-alpha would be 1 (similar to this), but I'm not 100% sure about that.
yes, as you thought, the failures come from rg16float
format and src-alpha-saturated
factor. It's likely this PR should make the test use RGBA instead of using one value.
…amping' test This PR adds srcFactor and dstFactor to the parameters of 'blending,clamping' test in color_target_state.spec.ts. Issue: gpuweb#1835
418a795
to
dbaa9e3
Compare
Previews, as seen when this build job started (dbaa9e3): |
This PR adds srcFactor and dstFactor to the parameters of 'blending,clamping' test in color_target_state.spec.ts.
Issue: #1835
Requirements for PR author:
.unimplemented()
./** documented */
and new helper files are found inhelper_index.txt
.Requirements for reviewer sign-off:
When landing this PR, be sure to make any necessary issue status updates.