Bug 209098

Created attachment 393564 [details] WIP: Replace framebuffer and texture validation with ANGLE_robust_client_memory

Note that there are important steps to take when updating ANGLE. See http://trac.webkit.org/wiki/UpdatingANGLE

This is my current work in progress. I may not be able to make further progress before I go on paternity leave. The current patch adds ANGLE_robust_client_memory support and uses it to replace a bunch of framebuffer and texture related state tracking and validation code. With this patch we no longer track or validate the format or size of framebuffers, renderbuffers, and textures, letting ANGLE do that work instead. This will fix a slew of conformance tests. I think this is a promising direction, but texImage2D/texSubImage2D do some work before calling ANGLE and we need to verify if that code is safe when the parameter validation is removed. The patch builds and runs at r258027, I haven't rebased it yet. The patch also contains some code to implement read/draw framebuffers. It started out as a much smaller patch but I believe it's not possible to remove the validation in smaller chunks. Removing framebuffer validation would cause framebuffer state tracking to be wrong, so we have to remove the state tracking also. Removing framebuffer state tracking also requires removing texture state tracking because of copyTexImage2D.

Great work and thanks James for getting this work this far. I'll take this bug and get it to completion.

Reviewing the patch above, some of the validation code - in particular validateTexFuncData, validateTexFuncParameters, and validateTexFuncFormatAndType - can not be removed. Re-packing of the incoming texture data occurs along several texture upload code paths, and information like the texture's format and type have to be verified as legal before calling into that re-packing code. I'm going to figure out how to replace that validation while still removing the tracking and validation of textures' per-level information.

Created attachment 394411 [details] Work-in-progress #2: before rebase

The patch just attached is closer to passing all preexisting layout tests. Necessary validation has been re-added, and the fix for http://anglebug.com/4504 , a preexisting bug uncovered by this patch, has been incorporated. (The next ANGLE roll would have integrated it, too.) Some of the validation that was previously removed still needs to be revised to support WebGL 2.0. This will be handled in new sub-bugs of Bug 126404.

Created attachment 394468 [details] Patch

This is a large patch, but it removes a large amount of now-unnecessary and redundant validation logic and makes significant progress toward passing many more WebGL 2.0 conformance tests. It would be ideal if it could land in its entirety rather than being split up.

Comment on attachment 394468 [details] Patch Will revise the non-ANGLE code path.

Created attachment 394503 [details] Patch

I believe the current patch should compile with both ANGLE and non-ANGLE. The latest version has disabled the experimental WebGL 2.0 support for non-ANGLE backends, and added some assertions, because it's infeasible to continue to maintain GraphicsContextGLOpenGL and GraphicsContextGLOpenGLES assuming the relaxed constraints of OpenGL ES 3.0.

Comment on attachment 394503 [details] Patch Locally I only focused on fast/canvas/webgl; will look into the other webgl/ layout test changes / failures as well as the build failures on the other platforms.

Created attachment 394577 [details] Patch

The current patch rebaselines the webgl/1.0.3 and webgl/2.0.0 tests; many more of these tests are passing completely with James's patch. It fixes a couple of small additional issues found while examining these test results by hand. Almost all diffs are progressions. All of them will be eventually resolved as more of WebGL 2.0 is implemented. This patch also disables WebGL 2.0 for non-ANGLE backends, and skips all of the WebGL 2.0 tests on the WPE port. This has been communicated to the WPE team in Bug 208188.

Created attachment 394582 [details] Patch

Created attachment 394672 [details] Patch

The latest version of the patch hopefully fixes the build failures in the non-ANGLE code path, and also incorporates a bug fix for anglebug.com/4518 which James had included in his original patch and which was causing layout test differences for a few tests between dual-GPU MacBook Pros and the EWS bots, which are Mac Minis with Intel GPUs.

Created attachment 394688 [details] Patch

More fixes for the method prototypes for ANGLE_robust_client_memory.

Created attachment 394698 [details] Patch

More WinCairo build fixes.

Comment on attachment 394698 [details] Patch The ios-wk2 layout test failure fast/hidpi/image-srcset-relative-svg-canvas-2x.html is known, unrelated to this patch. Requesting review at this point. Thanks.

Comment on attachment 394698 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394698&action=review > Source/ThirdParty/ANGLE/ChangeLog:17 > + Incorporated fix from anglebug.com/4504 to make > + fast/canvas/webgl/uninitialized-test.html pass. > + > + Incorporated fix from anglebug.com/4518 to make: > + webgl/2.0.0/conformance2/renderbuffers/invalidate-framebuffer.html > + webgl/2.0.0/conformance2/rendering/blitframebuffer-test.html > + webgl/2.0.0/conformance2/rendering/rgb-format-support.html > + webgl/2.0.0/conformance2/state/gl-object-get-calls.html > + webgl/2.0.0/conformance2/textures/misc/tex-new-formats.html > + pass. I wonder what the best way to track ANGLE is if we start cherry-picking commits? The script that James wrote was also updating our data of which ANGLE revision we were taking, any local changes, etc.

Comment on attachment 394698 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394698&action=review >> Source/ThirdParty/ANGLE/ChangeLog:17 >> + pass. > > I wonder what the best way to track ANGLE is if we start cherry-picking commits? The script that James wrote was also updating our data of which ANGLE revision we were taking, any local changes, etc. They'll be auto-merged during the "git rebase" that James's script does, and will not show up in Source/ThirdParty/ANGLE/changes.diff.

Committed r259139: <https://trac.webkit.org/changeset/259139> All reviewed patches have been landed. Closing bug and clearing flags on attachment 394698 [details].

<rdar://problem/60984228>

Committed r259164: <https://trac.webkit.org/changeset/259164>

Sorry for the WinCairo debug breakage and thanks Fujii for fixing it up!