Bug 209098 - Use ANGLE_robust_client_memory to replace framebuffer/texture validation
Summary: Use ANGLE_robust_client_memory to replace framebuffer/texture validation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kenneth Russell
URL:
Keywords: InRadar
Depends on: 209416
Blocks: 126404 209740 126447 126448 208188 209510
  Show dependency treegraph
 
Reported: 2020-03-13 18:00 PDT by James Darpinian
Modified: 2020-07-23 09:00 PDT (History)
14 users (show)

See Also:


Attachments
WIP: Replace framebuffer and texture validation with ANGLE_robust_client_memory (147.32 KB, patch)
2020-03-13 18:02 PDT, James Darpinian
no flags Details | Formatted Diff | Diff
Work-in-progress #2: before rebase (178.94 KB, patch)
2020-03-24 14:20 PDT, Kenneth Russell
no flags Details | Formatted Diff | Diff
Patch (184.64 KB, patch)
2020-03-24 21:15 PDT, Kenneth Russell
no flags Details | Formatted Diff | Diff
Patch (196.99 KB, patch)
2020-03-25 09:09 PDT, Kenneth Russell
no flags Details | Formatted Diff | Diff
Patch (1.07 MB, patch)
2020-03-25 22:26 PDT, Kenneth Russell
no flags Details | Formatted Diff | Diff
Patch (1.07 MB, patch)
2020-03-25 23:25 PDT, Kenneth Russell
no flags Details | Formatted Diff | Diff
Patch (1.07 MB, patch)
2020-03-26 16:26 PDT, Kenneth Russell
no flags Details | Formatted Diff | Diff
Patch (1.07 MB, patch)
2020-03-26 18:41 PDT, Kenneth Russell
no flags Details | Formatted Diff | Diff
Patch (1.07 MB, patch)
2020-03-26 20:09 PDT, Kenneth Russell
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James Darpinian 2020-03-13 18:00:46 PDT
Use ANGLE_robust_client_memory to replace framebuffer/texture validation
Comment 1 James Darpinian 2020-03-13 18:02:20 PDT
Created attachment 393564 [details]
WIP: Replace framebuffer and texture validation with ANGLE_robust_client_memory
Comment 2 EWS Watchlist 2020-03-13 18:02:52 PDT
Note that there are important steps to take when updating ANGLE. See http://trac.webkit.org/wiki/UpdatingANGLE
Comment 3 James Darpinian 2020-03-13 18:14:56 PDT
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.
Comment 4 Kenneth Russell 2020-03-16 13:08:02 PDT
Great work and thanks James for getting this work this far. I'll take this bug and get it to completion.
Comment 5 Kenneth Russell 2020-03-19 18:01:48 PDT
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.
Comment 6 Kenneth Russell 2020-03-24 14:20:49 PDT
Created attachment 394411 [details]
Work-in-progress #2: before rebase
Comment 7 Kenneth Russell 2020-03-24 14:24:56 PDT
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.
Comment 8 Kenneth Russell 2020-03-24 21:15:09 PDT
Created attachment 394468 [details]
Patch
Comment 9 Kenneth Russell 2020-03-24 21:30:43 PDT
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 10 Kenneth Russell 2020-03-24 21:31:56 PDT
Comment on attachment 394468 [details]
Patch

Will revise the non-ANGLE code path.
Comment 11 Kenneth Russell 2020-03-25 09:09:04 PDT
Created attachment 394503 [details]
Patch
Comment 12 Kenneth Russell 2020-03-25 09:11:28 PDT
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 13 Kenneth Russell 2020-03-25 10:39:49 PDT
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.
Comment 14 Kenneth Russell 2020-03-25 22:26:26 PDT
Created attachment 394577 [details]
Patch
Comment 15 Kenneth Russell 2020-03-25 22:29:52 PDT
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.
Comment 16 Kenneth Russell 2020-03-25 23:25:01 PDT
Created attachment 394582 [details]
Patch
Comment 17 Kenneth Russell 2020-03-26 16:26:19 PDT
Created attachment 394672 [details]
Patch
Comment 18 Kenneth Russell 2020-03-26 16:27:47 PDT
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.
Comment 19 Kenneth Russell 2020-03-26 18:41:28 PDT
Created attachment 394688 [details]
Patch
Comment 20 Kenneth Russell 2020-03-26 18:41:55 PDT
More fixes for the method prototypes for ANGLE_robust_client_memory.
Comment 21 Kenneth Russell 2020-03-26 20:09:21 PDT
Created attachment 394698 [details]
Patch
Comment 22 Kenneth Russell 2020-03-26 20:09:40 PDT
More WinCairo build fixes.
Comment 23 Kenneth Russell 2020-03-27 08:59:10 PDT
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 24 Dean Jackson 2020-03-27 11:53:10 PDT
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 25 Kenneth Russell 2020-03-27 13:16:48 PDT
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.
Comment 26 EWS 2020-03-27 14:08:33 PDT
Committed r259139: <https://trac.webkit.org/changeset/259139>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 394698 [details].
Comment 27 Radar WebKit Bug Importer 2020-03-27 14:09:17 PDT
<rdar://problem/60984228>
Comment 28 Fujii Hironori 2020-03-28 15:15:22 PDT
Committed r259164: <https://trac.webkit.org/changeset/259164>
Comment 29 Kenneth Russell 2020-03-30 10:26:21 PDT
Sorry for the WinCairo debug breakage and thanks Fujii for fixing it up!