| Summary: | Unity RubysAdventure demo does not render properly if WebGL2 is enabled. | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | jujjyl | ||||||||
| Component: | WebGL | Assignee: | 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
jujjyl
2020-08-26 02:06:53 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. 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. 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. 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. 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. 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. 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. 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. 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. Created attachment 411179 [details]
Patch
Looks like the fix doesn't affect any conformance tests :( I will write a new one tomorrow. 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+
Created attachment 411381 [details]
Patch
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 on attachment 411381 [details]
Patch
Looks good to me again!
Created attachment 411387 [details]
Patch
Committed r268502: <https://trac.webkit.org/changeset/268502> All reviewed patches have been landed. Closing bug and clearing flags on attachment 411387 [details]. *** Bug 218327 has been marked as a duplicate of this bug. *** |