Bug 215844 - WebGL goes in a bad state where glContext.createProgram() returns null
Summary: WebGL goes in a bad state where glContext.createProgram() returns null
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: ANGLE (show other bugs)
Version: Safari Technology Preview
Hardware: Mac macOS 10.15
: P2 Normal
Assignee: James Darpinian
URL:
Keywords: InRadar
Depends on: 215973
Blocks: 214640
  Show dependency treegraph
 
Reported: 2020-08-26 01:42 PDT by jujjyl
Modified: 2020-09-09 11:13 PDT (History)
9 users (show)

See Also:


Attachments
Patch (1.38 KB, patch)
2020-08-26 16:58 PDT, James Darpinian
no flags Details | Formatted Diff | Diff
Patch (9.61 KB, patch)
2020-08-28 17:06 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 01:42:10 PDT
Running STP through emunittest suite, some of the tests fail with an error

TypeError: null is not an object (evaluating 'program.name=id')

in Emscripten WebGL library function

  glCreateProgram: function() {
    var id = GL.getNewId(GL.programs);
    var program = GLctx.createProgram();
    program.name = id;
    GL.programs[id] = program;
    return id;
  },

Here GLctx is WebGLRenderingContext, and the return of GLctx.createProgram() comes out as null, causing the next line after that to fail.

The issue can be reproduced on project http://clb.confined.space/emunittest_unity/Tanks_20191004_152744_wasm_release_profiling.zip
Comment 1 Radar WebKit Bug Importer 2020-08-26 11:17:56 PDT
<rdar://problem/67817170>
Comment 2 James Darpinian 2020-08-26 16:49:27 PDT
I'll take this one.
Comment 3 James Darpinian 2020-08-26 16:51:48 PDT
The context is lost. Looks like some texture uploads are triggering some GL errors in ANGLE (probably a different bug), and then GraphicsContextGLOpenGL::reshape() is bailing out because it assumes the GL error was caused by something it did. Easy fix, we can call moveErrorsToSyntheticErrorList() at the top of reshape(). Patch forthcoming.
Comment 4 James Darpinian 2020-08-26 16:58:12 PDT
Created attachment 407358 [details]
Patch
Comment 5 James Darpinian 2020-08-26 16:59:04 PDT
Comment on attachment 407358 [details]
Patch

This fixes it. Need to write a test though.
Comment 6 Kenneth Russell 2020-08-26 17:45:18 PDT
Comment on attachment 407358 [details]
Patch

The fix looks good! Thanks in advance for writing a WebGL conformance test. r+
Comment 7 James Darpinian 2020-08-28 17:06:14 PDT
Created attachment 407518 [details]
Patch
Comment 8 James Darpinian 2020-08-28 17:07:55 PDT
Comment on attachment 407518 [details]
Patch

Added a test.
Comment 9 James Darpinian 2020-08-28 23:52:03 PDT
Also I found and fixed the texture upload error issue in bug 215973. Seems like  the Unity Tanks demo assumes that S3TC support implies S3TC sRGB support, which is technically wrong but probably should be true these days.
Comment 10 Kenneth Russell 2020-08-31 11:11:43 PDT
Comment on attachment 407518 [details]
Patch

Great diagnosis, fix and test James!

One slight concern - if WebKit's WebGL conformance suite snapshot is rolled forward again using the LayoutTests/webgl/generate-webgl-tests.py script, your changes to LayoutTests/webgl/2.0.0/resources/webgl_test_files/conformance/canvas/render-after-resize-test.html will be overwritten, since that script simply copies the files into the WebKit repository, ignoring whether any changes were made locally.

Now, any future rolls of this test snapshot will likely be small, since it's not under active development. Could you still think a little bit about this and decide whether it's worth preventing accidents in this area? Thanks.
Comment 11 James Darpinian 2020-08-31 11:24:00 PDT
True. Maybe I can just update the test in the 2.0.0 snapshot upstream.
Comment 12 James Darpinian 2020-08-31 11:26:44 PDT
Actually it looks like this test isn't included in the upstream 2.0.0 snapshot, so it won't be overwritten by the script.
Comment 13 EWS 2020-08-31 11:28:38 PDT
Committed r266362: <https://trac.webkit.org/changeset/266362>

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