Bug 208875 - [WebGL2] Complete new texture upload entry points in WebGL2RenderingContext
Summary: [WebGL2] Complete new texture upload entry points in WebGL2RenderingContext
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kenneth Russell
URL:
Keywords: InRadar
Depends on: 210766
Blocks: 209510 209513 209516 211484 211602
  Show dependency treegraph
 
Reported: 2020-03-10 12:31 PDT by Justin Fan
Modified: 2020-07-24 14:42 PDT (History)
11 users (show)

See Also:


Attachments
Patch (3.87 MB, patch)
2020-05-05 20:36 PDT, Kenneth Russell
no flags Details | Formatted Diff | Diff
Patch (3.87 MB, patch)
2020-05-06 11:16 PDT, Kenneth Russell
no flags Details | Formatted Diff | Diff
Patch (3.88 MB, patch)
2020-05-06 13:10 PDT, Kenneth Russell
no flags Details | Formatted Diff | Diff
Patch (3.88 MB, patch)
2020-05-06 13:53 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 Justin Fan 2020-03-10 12:31:40 PDT
[WebGL2] Introduce 3D textures to WebGL2RenderingContext
Comment 1 Kenneth Russell 2020-04-20 13:51:27 PDT
Once the DOM-to-texture upload paths have been refactored in Bug 210766, it's feasible to expose the 3D texture upload entry points as they'll just activate code paths already fully implemented underneath.
Comment 2 Kenneth Russell 2020-04-20 13:55:59 PDT
Changing this bug to capture addition of the new pixelStorei parameters, which are needed both for 3D texture support as well as sub-rectangle texture uploads new in WebGL 2.0.
Comment 3 Kenneth Russell 2020-04-29 22:27:38 PDT
Handling of WebGL 2.0's pixelStorei parameters, and uploading of DOM content to 3D textures, had to be folded in to the fix for Bug 210766 in order to be able to validate that fix by passing a large number of additional WebGL 2.0 conformance tests.

Changing this bug's synopsis to cover completion of the remaining new texture upload entry points in WebGL2RenderingContext. This is a separable piece of work.
Comment 4 Kenneth Russell 2020-05-04 10:18:14 PDT
Justin, let me take this bug from you. Examining the current texture-related code, there are several missing pieces here and there that aren't that easily separable, and will be best found via a code audit.
Comment 5 Kenneth Russell 2020-05-05 20:36:51 PDT
Created attachment 398581 [details]
Patch
Comment 6 Kenneth Russell 2020-05-06 11:16:01 PDT
Created attachment 398640 [details]
Patch
Comment 7 Kenneth Russell 2020-05-06 13:10:20 PDT
Created attachment 398648 [details]
Patch
Comment 8 Kenneth Russell 2020-05-06 13:53:28 PDT
Created attachment 398652 [details]
Patch
Comment 9 Kenneth Russell 2020-05-06 14:46:24 PDT
The current patch looks like it will pass the EWS. Per the ChangeLogs, it roughly does the following:

- Supports the TEXTURE_3D and TEXTURE_2D_ARRAY binding points.
- Supports the PIXEL_UNPACK_BUFFER binding point.
- Implements all of the 3D texture uploading entry points, hooking them up to the lower levels refactored in Bug 210766.

In support of these changes, it also:

- Refactors the WebGL 2.0-specific binding points into WebGL2RenderingContext, where they were previously mixed into WebGLRenderingContextBase.
- Refactors getParameter, some other get* calls, and associated validation into WebGLRenderingContextBase, overridden as appropriate in WebGL2RenderingContext.
- Other necessary changes along these call trees.

This patch makes all of the 3D texture tests - the remaining failures in the webgl/2.0.0/conformance2/textures/canvas_sub_rectangle/ and webgl/2.0.0/conformance2/textures/image_data/ directories - pass completely. It also progresses several other WebGL conformance tests, making some also pass completely.
Comment 10 Dean Jackson 2020-05-08 13:43:18 PDT
Comment on attachment 398652 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=398652&action=review

> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:964
> +    case GraphicsContextGL::TEXTURE_BASE_LEVEL:
> +        FALLTHROUGH;
> +    case GraphicsContextGL::TEXTURE_MAX_LEVEL:
> +        FALLTHROUGH;
> +    case GraphicsContextGL::TEXTURE_COMPARE_FUNC:
> +        FALLTHROUGH;
> +    case GraphicsContextGL::TEXTURE_COMPARE_MODE:
> +        FALLTHROUGH;
> +    case GraphicsContextGL::TEXTURE_IMMUTABLE_LEVELS:
> +        FALLTHROUGH;

I thought you only had to put FALLTHROUGH if you also had content in the case block?

> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:976
> +    case GraphicsContextGL::TEXTURE_MIN_LOD:
> +        FALLTHROUGH;

Ditto.
Comment 11 Kenneth Russell 2020-05-08 13:45:48 PDT
Comment on attachment 398652 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=398652&action=review

>> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:964
>> +        FALLTHROUGH;
> 
> I thought you only had to put FALLTHROUGH if you also had content in the case block?

Ah, thanks - was wondering why leaving this out didn't cause compile/style failures. Will clean up later.
Comment 12 EWS 2020-05-08 13:48:53 PDT
Committed r261413: <https://trac.webkit.org/changeset/261413>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 398652 [details].
Comment 13 Radar WebKit Bug Importer 2020-05-08 13:49:23 PDT
<rdar://problem/63034628>