Bug 219139 – Support WEBGL_multi_draw extension

• Component: WebGL
• Status: RESOLVED FIXED

[Kenneth Russell]

https://www.khronos.org/registry/webgl/extensions/WEBGL_multi_draw/ has been moved to Community Approved status. It's shipping in Chromium and well supported by ANGLE.

[Radar WebKit Bug Importer]

<rdar://problem/71742593>

[Kenneth Russell]

Created attachment 417930 [details]
Patch

[Kimmo Kinnunen]

Comment on attachment 417930 [details]
Patch

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

looks consistent

> Source/WebCore/html/canvas/WebGLMultiDraw.cpp:136
> +    GCGLsizei drawcount)

So maybe the line breaking loses its meaning when the below code is anyway relying on user handling long lines? (The arg list here and in the header seems a bit unorthodox?)

> Source/WebCore/platform/graphics/ExtensionsGL.h:353
> +    // GL_ANGLE_multi_draw

I've been trying to move stuff out of the ExtensionsGL. If it fits your structure, these could also perhaps be of form GraphicsContextGL::multiDrawArrays(), etc.

[Dean Jackson]

Comment on attachment 417930 [details]
Patch

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

> Source/WebCore/html/canvas/WebGLMultiDraw.cpp:92
> +void WebGLMultiDraw::multiDrawArraysInstancedWEBGL(
> +    GCGLenum mode,
> +    Int32List firstsList,
> +    GCGLuint firstsOffset,
> +    Int32List countsList,
> +    GCGLuint countsOffset,
> +    Int32List instanceCountsList,
> +    GCGLuint instanceCountsOffset,
> +    GCGLsizei drawcount)
> +{

Nit: We do parameters on one line (same for elsewhere in the patch)

> Source/WebCore/html/canvas/WebGLMultiDraw.h:39
> +        using VariantType = Variant<RefPtr<TypedArray>, Vector<DataType>>;

Could we give this a better name?

>> Source/WebCore/platform/graphics/ExtensionsGL.h:353
>> +    // GL_ANGLE_multi_draw
> 
> I've been trying to move stuff out of the ExtensionsGL. If it fits your structure, these could also perhaps be of form GraphicsContextGL::multiDrawArrays(), etc.

+1

[Kenneth Russell]

Comment on attachment 417930 [details]
Patch

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

>> Source/WebCore/html/canvas/WebGLMultiDraw.cpp:92
>> +{
> 
> Nit: We do parameters on one line (same for elsewhere in the patch)

Sure thing. (I thought this would be easier to read.) Fixed.

>> Source/WebCore/html/canvas/WebGLMultiDraw.cpp:136
>> +    GCGLsizei drawcount)
> 
> So maybe the line breaking loses its meaning when the below code is anyway relying on user handling long lines? (The arg list here and in the header seems a bit unorthodox?)

Changed to follow WebKit's conventions.

>> Source/WebCore/html/canvas/WebGLMultiDraw.h:39
>> +        using VariantType = Variant<RefPtr<TypedArray>, Vector<DataType>>;
> 
> Could we give this a better name?

This was basically copied from WebGLRenderingContextBase.h (I couldn't think of a good way to share it between the headers) but will call it ListTypeOptions.

>>> Source/WebCore/platform/graphics/ExtensionsGL.h:353
>>> +    // GL_ANGLE_multi_draw
>> 
>> I've been trying to move stuff out of the ExtensionsGL. If it fits your structure, these could also perhaps be of form GraphicsContextGL::multiDrawArrays(), etc.
> 
> +1

Sure thing - done. Currently there still need to be calls to ExtensionsGL::supports and ExtensionsGL::ensureEnabled in WebGLMultiDraw.cpp. We could move those entry points over later.

[Kenneth Russell]

Created attachment 417997 [details]
Patch

[Kenneth Russell]

EWS is green. CQ'ing.

[EWS]

Committed r271679: <https://trac.webkit.org/changeset/271679>

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