-
Notifications
You must be signed in to change notification settings - Fork 669
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
Test for having the same read and write buffer using a transform feedback #2349
base: main
Are you sure you want to change the base?
Test for having the same read and write buffer using a transform feedback #2349
Conversation
This is related to #2346 |
<!-- | ||
|
||
/* | ||
** Copyright (c) 2015 The Khronos Group Inc. |
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.
2017
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.
fixed
wtu.glErrorShouldBe(gl, gl.NO_ERROR, "linking transform feedback shader should not set an error"); | ||
|
||
const inLoc = 0; | ||
const outLoc = 0; |
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.
outLoc is not used.
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.
fixed
wtu.glErrorShouldBe(gl, gl.NO_ERROR, "there should be no errors"); | ||
|
||
const expected = [2, 4, 6]; | ||
gl.bindBuffer(gl.ARRAY_BUFFER, dstBuffer); |
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.
I am assuming this is where Chrome fails? According to the current spec restrictions, Chrome generates an INVALID_OPERATION here.
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.
this is one place where chrome fails if the "FIXME" lines are removed. Otherwise Chrome fails because it allows calling drawArrays
even though the same buffers are bound to a VAO and and TF.
wtu.glErrorShouldBe(gl, gl.INVALID_OPERATION, "reading and writing to same buffer"); | ||
|
||
gl.bindBuffer(gl.ARRAY_BUFFER, dstBuffer); | ||
wtu.checkFloatBuffer(gl, gl.ARRAY_BUFFER, expected, "should be the same as before as nothing has executed"); |
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.
Even if it's executed, the result is still the same expected because it's the same program? So this is a useless and misleading check.
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 incorrectly executed (as it currently is in chrome) the result is 4, 8, 12 because it's manipulating the result from the first * 2.0
operation.
That result is probably GPU dependent since the whole problem is results of having the same input and output buffer is undefined. In any case it shouldn't execute and the buffer should still have 2, 4, 6 in it. It doesn't in Chrome currently on my MBP. Seems like the test is valid
|
||
} | ||
|
||
function runFeedback(gl, prog, srcVAO, tf, dstBufferInfo) { |
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.
dstBufferInfo isn't used.
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.
fixed
I'd like to hold off merging this until Chrome's behavior has been updated per #2346 . |
…back note: there are 2 lines marked as "FIXME" that I believe should be removed assuming the spec changes so that `gl.bindBuffer` is less strict
8b75c1b
to
3ff07f5
Compare
lgtm |
Firefox passes, Chrome fails
note: there are 2 lines marked as "FIXME" that I believe should be removed
assuming the spec changes so that
gl.bindBuffer
is less strict