Bug 209098 – Use ANGLE_robust_client_memory to replace framebuffer/texture validation

• Component: New Bugs
• Status: RESOLVED FIXED

[James Darpinian]

Use ANGLE_robust_client_memory to replace framebuffer/texture validation

[James Darpinian]

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

[EWS Watchlist]

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

[James Darpinian]

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.

[Kenneth Russell]

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

[Kenneth Russell]

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.

[Kenneth Russell]

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

[Kenneth Russell]

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.

[Kenneth Russell]

Created attachment 394468 [details]
Patch

[Kenneth Russell]

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.

[Kenneth Russell]

Comment on attachment 394468 [details]
Patch

Will revise the non-ANGLE code path.

[Kenneth Russell]

Created attachment 394503 [details]
Patch

[Kenneth Russell]

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.

[Kenneth Russell]

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.

[Kenneth Russell]

Created attachment 394577 [details]
Patch

[Kenneth Russell]

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.

[Kenneth Russell]

Created attachment 394582 [details]
Patch

[Kenneth Russell]

Created attachment 394672 [details]
Patch

[Kenneth Russell]

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.

[Kenneth Russell]

Created attachment 394688 [details]
Patch

[Kenneth Russell]

More fixes for the method prototypes for ANGLE_robust_client_memory.

[Kenneth Russell]

Created attachment 394698 [details]
Patch

[Kenneth Russell]

More WinCairo build fixes.

[Kenneth Russell]

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.

[Dean Jackson]

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.

[Kenneth Russell]

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.

[EWS]

Committed r259139: <https://trac.webkit.org/changeset/259139>

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

[Radar WebKit Bug Importer]

<rdar://problem/60984228>

[Fujii Hironori]

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

[Kenneth Russell]

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