Test both alpha:true and alpha:false in webgl_canvas tests. #3071
Conversation
Remove explicit use of Promises in the inner loop, and change to use async/await, to provide better control of the execution order of the test cases. Verified in Chrome, Firefox and Safari on macOS. ToT of all browsers pass the new tests; Firefox has a couple of preexisting failures with the RGBA10_A2/RGBA/UNSIGNED_INT_2_10_10_10_REV combination.
|
@jdashg and perhaps @jdarpinian or @shrekshao could you please review? This does increase the run time of this subset of the conformance2/ tests, but it's a fairly small subset, so shouldn't have a large impact. Note that these new tests actually pass in all current WebGL 2.0 implementations. It could be argued that they don't catch any new bugs, but on the other hand, it's probably a good idea to increase testing coverage of alpha:false. |
| @@ -67,7 +69,7 @@ function generateTest(internalFormat, pixelFormat, pixelType, prologue, resource | |||
| runTest(); | |||
There was a problem hiding this comment.
Good point and good question. I just tested both with and without the await and the tests seem to behave the same. The Promise returned by init() is explicitly waited on in the anonymous function returned at the bottom of the file.
One behavioral difference with this change is that the intermediate results aren't displayed any more. I tried changing "await wtu.dispatchPromise(...)" to "await wtu.dispatchRAFPromise(...)", and doing a requestAnimationFrame inside it. Still no difference - the web page only displays the final results, not the intermediate ones.
I'm not an expert in Promises. Can you think of any improvements to make here?
There was a problem hiding this comment.
Figured it out - needed to await wtu.dispatchPromise() after every test case. Thanks for your catch; please re-review.
|
Update: these new alpha:false tests actually do catch a bug in ToT WebKit on iOS! Will appreciate your reviews to get these in. Thanks. |
|
@grorg FYI please see above comment! :) |
| let visibleCtx = wtu.create3DContext(null, { preserveDrawingBuffer:true, alpha:alpha }); | ||
| if (!visibleCtx) { | ||
| testFailed("context does not exist"); | ||
| finishTest(); |
There was a problem hiding this comment.
I believe when returned it would run finishTest anyway. So it's duplicate here?
There was a problem hiding this comment.
Yes, thanks, that and another finishTest() can be removed.
There was a problem hiding this comment.
Removed in the current version.
|
New tests have caught bugs, so merging now. |
Remove explicit use of Promises in the inner loop, and change to use
async/await, to provide better control of the execution order of the
test cases.
Verified in Chrome, Firefox and Safari on macOS. ToT of all browsers
pass the new tests; Firefox has a couple of preexisting failures with
the RGBA10_A2/RGBA/UNSIGNED_INT_2_10_10_10_REV combination.