Bug 219139

<rdar://problem/71742593>

Created attachment 417930 [details] Patch

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.

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

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.

Created attachment 417997 [details] Patch

EWS is green. CQ'ing.

Committed r271679: <https://trac.webkit.org/changeset/271679> All reviewed patches have been landed. Closing bug and clearing flags on attachment 417997 [details].