Consistently use normal object access notation for GLctx

[sbc100]

We were already using the normal notation in most cases anyway:

$ git grep "GLctx\[" | wc -l
74
$ git grep "GLctx\." | wc -l
505

IIUC we have tests that verify this doesn't break with --closure=1.

I automated this change in vim using %s/GLctx\['\(\w*\)'\]/GLctx\.\1/c

[juj]

IIUC we have tests that verify this doesn't break with --closure=1.

In order to test that this change would not break, we would have to have tests that actually exercise calling these functions against a WebGL 2 context. Otherwise an incorrect minification would not be detected.

The original reason for [' '] access in here was that WebGL 2 support was added before Closure gained built-in support for WebGL 2.

It is likely that by now Closure does have built-in WebGL 2 awareness. However, it is pretty much guaranteed that Closure will not be aware of the different WebGL extension functions that exist, e.g. drawBuffersWebGL added by https://registry.khronos.org/webgl/extensions/WEBGL_draw_buffers/, or multiDraw* added by https://registry.khronos.org/webgl/extensions/WEBGL_multi_draw/ .

I don't think there is much functional benefit in using an unified form here? Closure will transform GLctx['asdf'] to GLctx.asdf anyways. (this will help non-Closure users though, which I agree is nice)

In order to land this, it would be best to do so by auditing against the Closure externs file at https://github.com/google/closure-compiler/blob/master/externs/browser/webgl2.js to verify that the entry points do exist there, since I don't think we have functional test coverage against most of these functions.

[juj]

Scanning through, all of the API changes here do read like core WebGL 2 functions, so I think Closure externs should be covering these 👍

[sbc100]

Scanning through, all of the API changes here do read like core WebGL 2 functions, so I think Closure externs should be covering these +1

I guess I could write a script that verifies that all the strings in question do not appear minified in the --closure=1 output? Do you think its worth it to do that?

[kripken]

lgtm with some verification, either to scan the output as suggested, or perhaps to force closure to be on in all the WebGL2 tests and verify those manually locally.

[sbc100]

lgtm with some verification, either to scan the output as suggested, or perhaps to force closure to be on in all the WebGL2 tests and verify those manually locally.

I think I will write a test that can verify that absence of minification for a set of symbols.

[sbc100]

I wrote a test that verifies the lack of minification. It is slightly worrying is we lack test coverage for all of these functions though. How much of our GL layer is completely untested I wonder? Would be good to have coverage information.

[juj]

I guess I could write a script that verifies that all the strings in question do not appear minified in the --closure=1 output? Do you think its worth it to do that?

To me it would be enough search for each symbol in the upstream Closure WebGL2 externs file. I doubt that they would have missed a core WebGL 2 API entry point. (but I'm pretty confident they don't have the extension entry points covered)

[juj]

It is slightly worrying is we lack test coverage for all of these functions though. How much of our GL layer is completely untested I wonder? Would be good to have coverage information.

It would be good to have complete test coverage. I would expect writing such would be a few man years of work, the GL API surface area is massive.

At some point in history I remember there was conversation with Google to compile the deqp GLES2 conformance test suite with Emscripten, but the project did not progress back then, since it was expected to require large code changes to the suite due to WebGL's implicit swap and sync vs async computation nature.

Overall I think that would still be the best way to proceed to get "complete" coverage.