Bug 226865

Summary: WebGL2 demo doesn't work due to failing compilation to metal backend
Product: WebKit Reporter: Oliver Hunt <oliver>
Component: WebGLAssignee: Kyle Piddington <kpiddington>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, ews-watchlist, kbr, kkinnunen, kondapallykalyan, kpiddington, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
URL: https://zynaps.com/content/photon/
See Also: https://bugs.webkit.org/show_bug.cgi?id=229050
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Oliver Hunt 2021-06-09 21:47:22 PDT
The demo is at

https://zynaps.com/content/photon/

Checking for the error told me to file a bug :D
Comment 1 Kyle Piddington 2021-06-10 12:36:22 PDT
Thank you for filing! I'll have a look!
Comment 2 Radar WebKit Bug Importer 2021-06-11 12:45:27 PDT
<rdar://problem/79211560>
Comment 3 Kyle Piddington 2021-07-08 13:56:28 PDT
I suspect this was a duplicate of 
https://bugs.webkit.org/show_bug.cgi?id=226660, due to 'metal' being a pretty common struct member in most tracers. This appears to be working on current Top-of-Tree webkit, can you verify this on your end?

*** This bug has been marked as a duplicate of bug 226660 ***
Comment 4 Kyle Piddington 2021-07-08 15:18:18 PDT
I apologize, I closed this early. Reopening...
Comment 5 Kyle Piddington 2021-07-08 17:20:21 PDT
Created attachment 433189 [details]
Patch
Comment 6 EWS Watchlist 2021-07-08 17:21:27 PDT
Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE
Comment 7 Kenneth Russell 2021-07-08 17:39:09 PDT
Comment on attachment 433189 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433189&action=review

> LayoutTests/fast/canvas/webgl/shader-with-struct-array.html:36
> +        description("Tests that program compiling/linking with a reserved keyword.");

Remove "that" to make this grammatically correct?

> LayoutTests/fast/canvas/webgl/shader-with-struct-array.html:43
> +            runShaderTest(gl, vShaderSource, fshaderSource, "no error for using reserved keyword in struct")

Could you explain here what the reserved keyword is? This comment, and the description, don't seem to match the code change.
Comment 8 Kenneth Russell 2021-07-08 17:40:52 PDT
Note: I filed:
Add glsl3 regression test for arrays-of-structs bug in ANGLE's direct-to-Metal backend
https://github.com/KhronosGroup/WebGL/issues/3298

about porting this new test into the WebGL conformance suite.

Would be great if you could put up a pull request, but no worries if not.
Comment 9 Dean Jackson 2021-07-09 10:42:02 PDT
Comment on attachment 433189 [details]
Patch

+1 to Ken's comments.
Comment 10 Dean Jackson 2021-07-09 10:43:20 PDT
Comment on attachment 433189 [details]
Patch

Remember to provide the expected results for the test :)
Comment 11 Kyle Piddington 2021-07-09 13:54:11 PDT
Created attachment 433237 [details]
Patch
Comment 12 Kenneth Russell 2021-07-09 13:57:00 PDT
Comment on attachment 433237 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433237&action=review

> LayoutTests/fast/canvas/webgl/shader-with-struct-array.html:36
> +        description("Tests that program compiling/linking with a reserved keyword.");

Still have grammatical question about the description from the first patch.

> LayoutTests/fast/canvas/webgl/shader-with-struct-array.html:43
> +            runShaderTest(gl, vShaderSource, fshaderSource, "no error for using reserved keyword in struct")

Still have question about how accurate this comment is, relative to the code change. It looks more like a bug in handling of arrays of structs, than using reserved keywords.
Comment 13 Kyle Piddington 2021-07-09 14:07:20 PDT
Created attachment 433238 [details]
Patch
Comment 14 Kyle Piddington 2021-07-09 15:21:46 PDT
One more run, I forgot to change the result text.
Comment 15 Kyle Piddington 2021-07-14 19:08:38 PDT
Created attachment 433556 [details]
Patch
Comment 16 EWS 2021-07-15 15:57:04 PDT
Committed r279968 (239711@main): <https://commits.webkit.org/239711@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 433556 [details].