Bug 215846

Summary: Unity RubysAdventure demo does not render properly if WebGL2 is enabled.
Product: WebKit Reporter: jujjyl
Component: WebGLAssignee: James Darpinian <jdarpinian>
Status: RESOLVED FIXED    
Severity: Major CC: brendanduncan, cdumez, changseok, dino, esprehn+autocc, ews-watchlist, graouts, gyuyoung.kim, jdarpinian, jonlee, kbr, kimmo.t.kinnunen, kkinnunen, kondapallykalyan, msokalski, webkit-bug-importer
Priority: P1 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Mac   
OS: macOS 10.15   
Bug Depends on:    
Bug Blocks: 126404, 218327    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

jujjyl
Reported 2020-08-26 02:06:53 PDT
The Unity emunittest demo RubysAdventure does not render in Safari STP if WebGL 2 experimental feature is enabled. It does work properly in WebGL 1 mode. RubysAdventure build can be downloaded from http://clb.confined.space/emunittest_unity/RubysAdventure_20190722_201255_wasm_release_profiling.zip The source Unity project can be downloaded from https://assetstore.unity.com/packages/essentials/tutorial-projects/2d-beginner-complete-project-140253 . The above build has been exported from this Unity project unmodified.
Attachments
Patch (12.75 KB, patch)
2020-10-12 15:44 PDT, James Darpinian
no flags
Patch (29.56 KB, patch)
2020-10-14 15:53 PDT, James Darpinian
no flags
Patch (29.56 KB, patch)
2020-10-14 16:51 PDT, James Darpinian
no flags
Radar WebKit Bug Importer
Comment 1 2020-08-26 11:17:41 PDT
Kenneth Russell
Comment 2 2020-10-08 13:35:23 PDT
https://tiny.vision/ also renders all black in Safari Technology Preview Release 114 (Safari 14.1, WebKit 15611.1.1.3) . It was broken in Release 113 too. jdarpinian@ said he's actively investigating this, so reassigning. Raising to P1, Major - this is key content we've been aiming to enable with the WebGL 2.0 upgrade in Safari.
James Darpinian
Comment 3 2020-10-09 16:53:34 PDT
So far I haven't found anything wrong with the rendering itself. What I have found using spector.js is that Unity is setting a bunch of uniforms to 0, which are set to 1 normally in Chrome. I'm looking at one of the first draw calls and there are uniforms _Color, _Flip, and _RendererColor which have all their values set to 0, when in Chrome they are all 1. At least, that's what spector.js sees.
James Darpinian
Comment 4 2020-10-09 16:57:35 PDT
I haven't yet ruled out a bug in WebKit that's causing those uniforms to be set to 0 incorrectly, but it's also possible that Unity is setting them wrong. I will investigate more on Monday.
Brendan Duncan
Comment 5 2020-10-09 22:18:08 PDT
Digging into the GL functions Unity is using, I found the WebGL2 variant of glUniform4fv, gl.uniform4fv(location, data, srcOffset, srcLength), is failing on Safari. When I changed that call to the WebGL1 variant, the test started working on Safari. I haven't verified yet if other functions are failing as well.
Brendan Duncan
Comment 6 2020-10-10 17:01:45 PDT
Looks like there are problems with the WebGL2 variants for all of the gl.uniform*v functions. When I disabled the WebGL2 variants for those functions in the RubysAdventure build Jukka provided, it started working. I did this by editing the framework.unityweb file in the Build folder, and replacing the code `if(GL.currentContext.supportsWebGL2EntryPoints)` in the `_glUniform*v` functions with `if(false)`, forcing those functions to use the WebGL1 variants.
Brendan Duncan
Comment 7 2020-10-12 12:03:19 PDT
I built WebKit from source to figure out why the glUniform* functions were failing. Focusing on glUniform4fv, I found ANGLE was generating the error in ValidateUniformCommonBase (ANGLE/src/libANGLE/validationES.cpp). Specifically, count > 1 && !uniform.isArray(). The uniform it doesn't think was an array is `uniform vec4 _MainTex_ST`. This is the translate/scale transform for the screen blit UVs, so being considered invalid and not getting set, the transform has a 0 scale, and hence the black screen. The real bug then is why ANGLE doesn't think that vec4 uniform is an array.
Brendan Duncan
Comment 8 2020-10-12 12:47:36 PDT
The WebGL1 variant calls validate with a count of 1, that's why it didn't have an issue with isArray being false. So the real bug is with Webkit's validateUniformParameters using a count size of 4 instead of 1 for the vec4 uniform.
Brendan Duncan
Comment 9 2020-10-12 13:11:39 PDT
Here's the bug: Source/WebCore/html/canvas/WebGL2RenderingContext.cpp, WebGL2RenderingContext::uniform4fv: m_context->uniform4fv(location->location(), data.data(), srcOffset, srcLength ? srcLength : (data.length() - srcOffset) / 4); should be: m_context->uniform4fv(location->location(), data.data(), srcOffset, srcLength ? srcLength / 4 : (data.length() - srcOffset) / 4); And likewise for the other unform*v functions.
James Darpinian
Comment 10 2020-10-12 15:27:09 PDT
Thanks for the help! This is definitely the issue. Confirmed that this fixes *all* of the emunittest demos on http://clb.confined.space/emunittest/ including RubysAdventure. I'm currently investigating whether one of the known test failures covers this or whether we just lack test coverage. If the latter, I will write a new WebGL conformance test.
James Darpinian
Comment 11 2020-10-12 15:44:43 PDT
James Darpinian
Comment 12 2020-10-12 17:09:52 PDT
Looks like the fix doesn't affect any conformance tests :( I will write a new one tomorrow.
Kenneth Russell
Comment 13 2020-10-12 22:21:44 PDT
Comment on attachment 411179 [details] Patch Thank you Brendan for tracking down the root problem and James for fixing this! Yes, please do add a new WebGL 2.0 conformance test for all of these entry points. Looking at Chrome's implementation, I'm surprised it doesn't have the same bug. r+
James Darpinian
Comment 14 2020-10-14 15:53:19 PDT
James Darpinian
Comment 15 2020-10-14 15:55:32 PDT
I found that there is actually a test covering this, but it is not enabled in the 2.0.0 webgl conformance snapshot. I guess in addition to passing 2.0.0, before release we should try a 2.0.1 run and examine the results to see if there are any other serious issues hiding in there.
Kenneth Russell
Comment 16 2020-10-14 16:13:35 PDT
Comment on attachment 411381 [details] Patch Looks good to me again!
James Darpinian
Comment 17 2020-10-14 16:51:53 PDT
EWS
Comment 18 2020-10-14 17:24:10 PDT
Committed r268502: <https://trac.webkit.org/changeset/268502> All reviewed patches have been landed. Closing bug and clearing flags on attachment 411387 [details].
Kenneth Russell
Comment 19 2020-10-30 08:30:46 PDT
*** Bug 218327 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.