Bug 200901 - Pass conformance/attribs WebGL conformance tests
Summary: Pass conformance/attribs WebGL conformance tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kai Ninomiya
URL:
Keywords: InRadar
Depends on:
Blocks: 198948
  Show dependency treegraph
 
Reported: 2019-08-19 16:20 PDT by Kenneth Russell
Modified: 2019-08-23 15:56 PDT (History)
12 users (show)

See Also:


Attachments
Patch (4.96 KB, patch)
2019-08-20 17:08 PDT, Kai Ninomiya
no flags Details | Formatted Diff | Diff
Patch (273.10 KB, patch)
2019-08-21 13:44 PDT, Kai Ninomiya
no flags Details | Formatted Diff | Diff
Patch (274.86 KB, patch)
2019-08-22 07:30 PDT, Kai Ninomiya
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Russell 2019-08-19 16:20:41 PDT
Currently WebKit's WebGL implementation fails a few of the conformance/attribs WebGL conformance tests, specifically:

https://www.khronos.org/registry/webgl/sdk/tests/conformance/attribs/gl-disabled-vertex-attrib-update.html?webglVersion=1&quiet=0&quick=1
https://www.khronos.org/registry/webgl/sdk/tests/conformance/attribs/gl-vertexattribpointer.html?webglVersion=1&quiet=0&quick=1
https://www.khronos.org/registry/webgl/sdk/tests/conformance/attribs/gl-vertex-attrib-unconsumed-out-of-bounds.html?webglVersion=1&quiet=0&quick=1

The reason appears to be that the spec was changed recently to allow vertexAttribPointer to be called with a null WebGLBuffer bound, but only if the associated offset is 0, in order to reset the vertex attribute's state. This one fix will likely fix all of these tests.
Comment 1 Kenneth Russell 2019-08-19 16:29:48 PDT
Kai, may I ask you to take this bug?
Comment 2 Kai Ninomiya 2019-08-20 17:08:10 PDT
Created attachment 376829 [details]
Patch
Comment 3 Alex Christensen 2019-08-21 08:33:14 PDT
Please update the LayoutTests/...-expected.txt of the newly passing tests.  You can do this automatically by using run-webkit-tests <test name> --reset
Comment 4 Kai Ninomiya 2019-08-21 11:58:30 PDT
Ah, thanks for the pointer. I was trying to find where I could update the expectations but couldn't find anything because I just grepped for the filename.
Comment 5 Kai Ninomiya 2019-08-21 13:44:55 PDT
Created attachment 376914 [details]
Patch
Comment 6 Kai Ninomiya 2019-08-21 13:59:26 PDT
I can't find the test logs for the first patch anymore (they seem to have vanished between this morning and reloading the page while working on the second patch?), so actually see what failed.
AFAICT without that, out of the 3 tests Ken listed, only 1 actually exists (in several places) in LayoutTests:

- webgl/2.0.0/conformance/attribs/gl-vertexattribpointer.html: updated results
- webgl/1.0.2/conformance/attribs/gl-vertexattribpointer.html: old snapshot, already passing
- fast/canvas/webgl/gl-vertexattribpointer.html: old snapshot, already passing
Comment 7 Alex Christensen 2019-08-21 14:52:17 PDT
Click "Show Obsolete", click the red bubble, then "view layout test results"
Comment 8 Kenneth Russell 2019-08-21 16:56:44 PDT
New patch LGTM though Alex should review and approve it.

I also only see the style and win bots having run against the now-obsolete patch.
Comment 9 Kai Ninomiya 2019-08-21 17:40:57 PDT
Yup, only "style" and "win" buttons; all of the other test jobs seem to disappear gradually as each one completes.

Is there a permission necessary to view the complete results?
Comment 10 Alex Christensen 2019-08-21 17:41:50 PDT
EWS indicates this patch still makes tests not match their expectations. Run this command:
Tools/Scripts/run-webkit-tests webgl/2.0.0/conformance/attribs/gl-vertexattribpointer.html webgl/1.0.2/conformance/attribs/gl-vertexattribpointer.html fast/canvas/webgl/gl-vertexattribpointer.html --reset
Comment 11 Kai Ninomiya 2019-08-21 17:44:47 PDT
Found that if I click "style" then "All EWS Queues", I can see the 4 other queues I could see before. But they are all jsc-* and all of them say "Patch was not relevant", so I guess that's why they disappeared.

FWIW, all of that looks exactly the same if I'm logged out (in incognito).
Comment 12 Kai Ninomiya 2019-08-21 17:48:49 PDT
Thanks for providing that command. Unfortunately those are the same files I tried to rerun earlier. Running them does not change the -expected.txt files, and if I look at the 3 "Writing new expected result" files manually, they already say everything is passing.
Comment 13 Alex Christensen 2019-08-21 18:04:27 PDT
Hmm, I just verified the tests pass.  Aakash, is something wrong with EWS?
Comment 14 Aakash Jain 2019-08-21 18:13:13 PDT
(In reply to Alex Christensen from comment #13)
> Hmm, I just verified the tests pass.  Aakash, is something wrong with EWS?
I don't see anything visibly wrong in EWS. EWS shows exact same failure on 4 different queues (4 different bots). Also, those failures were reported only on this patch, not on any other patch recently (so can't blame those tests as flaky).

Maybe the failures are specific to the OS/configuration. The bots are running High-Sierra.
Comment 15 Kai Ninomiya 2019-08-21 18:20:19 PDT
Thanks for looking into it and for the suggestion. Is it possible to get a link to or copy of the test logs that I can view to try to figure out the issue?
Comment 16 Aakash Jain 2019-08-21 18:26:36 PDT
(In reply to Kai Ninomiya from comment #15)
> Thanks for looking into it and for the suggestion. Is it possible to get a
> link to or copy of the test logs that I can view to try to figure out the issue?
Please click the red bubbles (e.g.: ios-wk2 bubble), it would take you to another (buildbot) page, in that click on "view layout test results" to see the test results.

e.g.: 
https://ews-build.webkit.org/results/iOS-12-Simulator-WK2-Tests-EWS/r376914-2216/results.html
https://ews-build.webkit.org/results/macOS-High-Sierra-Release-WK1-Tests-EWS/r376914-1304/results.html
https://ews-build.webkit.org/results/macOS-High-Sierra-Release-WK2-Tests-EWS/r376914-1627/results.html
https://ews-build.webkit.org/results/macOS-High-Sierra-Debug-WK1-Tests-EWS/r376914-157/results.html
Comment 17 Kai Ninomiya 2019-08-22 00:16:24 PDT
Thank you for the links! I will take a look at the results tomorrow.

As Ken and I pointed out in #c8 #c9 though, I am still having the issue where there are no red bubbles to click on, and I can only see 6 queues. Here are some screenshots:
https://drive.google.com/drive/folders/1cdIErzConMbGdY3yJTjgeDu4spM_32mH?usp=sharing
Comment 18 Kai Ninomiya 2019-08-22 07:30:37 PDT
Created attachment 377003 [details]
Patch
Comment 19 Kai Ninomiya 2019-08-22 07:31:13 PDT
Oh, the problem was just that this change also fixed conformance/more/functions/vertexAttribPointerBadArgs.html and I didn't know that.

Unfortunately, the 1.0.2 snapshot of this test is not compatible with 1.0.3+ and 2.0.0+, and it is expected to fail. (It fails in Chrome and Firefox as well.)

PTAL!
Comment 20 Alex Christensen 2019-08-22 07:48:25 PDT
Comment on attachment 377003 [details]
Patch

r=me

Aakash, I can also not see the bubbles from home, but I could see them from all browsers logged in or not from work.  Maybe they're only accessible from within Apple?  "curl 'https://ews.webkit.org/status-bubble/376829/' -v" returns 404 for me at home.
Comment 21 WebKit Commit Bot 2019-08-22 08:34:14 PDT
Comment on attachment 377003 [details]
Patch

Clearing flags on attachment: 377003

Committed r249007: <https://trac.webkit.org/changeset/249007>
Comment 22 WebKit Commit Bot 2019-08-22 08:34:15 PDT
All reviewed patches have been landed.  Closing bug.
Comment 23 Radar WebKit Bug Importer 2019-08-22 08:35:20 PDT
<rdar://problem/54599744>
Comment 24 Kenneth Russell 2019-08-22 10:54:02 PDT
Great work Kai getting to the bottom of this!
Comment 25 Aakash Jain 2019-08-22 11:20:44 PDT
(In reply to Kai Ninomiya from comment #17)
> Thank you for the links! I will take a look at the results tomorrow.
> 
> As Ken and I pointed out in #c8 #c9 though, I am still having the issue
> where there are no red bubbles to click on, and I can only see 6 queues.
> Here are some screenshots:
> https://drive.google.com/drive/folders/
> 1cdIErzConMbGdY3yJTjgeDu4spM_32mH?usp=sharing
Sorry, I missed reading your earlier comments, and was looking into the test failures.
There were some DNS changes yesterday, and because of that EWS bubbles are not loading for some of the users. We are looking into it.

https://lists.webkit.org/pipermail/webkit-dev/2019-August/030761.html
Comment 26 Kai Ninomiya 2019-08-23 15:56:49 PDT
No problem, there were 2 issues to solve, that one was just blocking the other :)

FYI, the bubbles have been working again recently!