Bug 215846 - Unity RubysAdventure demo does not render properly if WebGL2 is enabled.
Summary: Unity RubysAdventure demo does not render properly if WebGL2 is enabled.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: Safari Technology Preview
Hardware: Mac macOS 10.15
: P1 Major
Assignee: James Darpinian
URL:
Keywords: InRadar
: 218327 (view as bug list)
Depends on:
Blocks: 126404 218327
  Show dependency treegraph
 
Reported: 2020-08-26 02:06 PDT by jujjyl
Modified: 2020-10-30 08:30 PDT (History)
16 users (show)

See Also:


Attachments
Patch (12.75 KB, patch)
2020-10-12 15:44 PDT, James Darpinian
no flags Details | Formatted Diff | Diff
Patch (29.56 KB, patch)
2020-10-14 15:53 PDT, James Darpinian
no flags Details | Formatted Diff | Diff
Patch (29.56 KB, patch)
2020-10-14 16:51 PDT, James Darpinian
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description jujjyl 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.
Comment 1 Radar WebKit Bug Importer 2020-08-26 11:17:41 PDT
<rdar://problem/67817161>
Comment 2 Kenneth Russell 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.
Comment 3 James Darpinian 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.
Comment 4 James Darpinian 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.
Comment 5 Brendan Duncan 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.
Comment 6 Brendan Duncan 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.
Comment 7 Brendan Duncan 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.
Comment 8 Brendan Duncan 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.
Comment 9 Brendan Duncan 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.
Comment 10 James Darpinian 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.
Comment 11 James Darpinian 2020-10-12 15:44:43 PDT
Created attachment 411179 [details]
Patch
Comment 12 James Darpinian 2020-10-12 17:09:52 PDT
Looks like the fix doesn't affect any conformance tests :( I will write a new one tomorrow.
Comment 13 Kenneth Russell 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+
Comment 14 James Darpinian 2020-10-14 15:53:19 PDT
Created attachment 411381 [details]
Patch
Comment 15 James Darpinian 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.
Comment 16 Kenneth Russell 2020-10-14 16:13:35 PDT
Comment on attachment 411381 [details]
Patch

Looks good to me again!
Comment 17 James Darpinian 2020-10-14 16:51:53 PDT
Created attachment 411387 [details]
Patch
Comment 18 EWS 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].
Comment 19 Kenneth Russell 2020-10-30 08:30:46 PDT
*** Bug 218327 has been marked as a duplicate of this bug. ***