RESOLVED FIXED Bug 219139
Support WEBGL_multi_draw extension
https://bugs.webkit.org/show_bug.cgi?id=219139
Summary Support WEBGL_multi_draw extension
Kenneth Russell
Reported 2020-11-18 18:40:52 PST
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.
Attachments
Patch (87.93 KB, patch)
2021-01-19 17:27 PST, Kenneth Russell
no flags
Patch (86.13 KB, patch)
2021-01-20 14:34 PST, Kenneth Russell
no flags
Radar WebKit Bug Importer
Comment 1 2020-11-25 21:34:21 PST
Kenneth Russell
Comment 2 2021-01-19 17:27:41 PST
Kimmo Kinnunen
Comment 3 2021-01-20 11:08:41 PST
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 4 2021-01-20 11:55:36 PST
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 5 2021-01-20 14:25:39 PST
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
Comment 6 2021-01-20 14:34:11 PST
Kenneth Russell
Comment 7 2021-01-20 17:24:46 PST
EWS is green. CQ'ing.
EWS
Comment 8 2021-01-20 17:42:52 PST
Committed r271679: <https://trac.webkit.org/changeset/271679> All reviewed patches have been landed. Closing bug and clearing flags on attachment 417997 [details].
Note You need to log in before you can comment on or make changes to this bug.