Bug 189625 – WebGL 2 Conformance: primitive restart and draw_primitive_restart WebGL2 sample

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Justin Fan]

WebGL 2 Conformance: primitive restart and draw_primitive_restart WebGL2 sample

[Justin Fan]

<rdar://problem/42882620>

[Justin Fan]

Patch is ready to go; waiting on updates to webgl testing framework before uploading.

[Justin Fan]

Thanks to the new tests, regressions: Unexpected text-only failures (6)
  webgl/2.0.0/conformance/extensions/angle-instanced-arrays-out-of-bounds.html [ Failure ]
  webgl/2.0.0/conformance/rendering/draw-elements-out-of-bounds.html [ Failure ]
  webgl/2.0.0/conformance2/rendering/instanced-arrays.html [ Failure ]
  webgl/2.0.0/conformance2/rendering/instanced-rendering-bug.html [ Failure ]
  webgl/2.0.0/conformance2/rendering/out-of-bounds-index-buffers-after-copying.html [ Failure ]
  webgl/2.0.0/conformance2/vertex_arrays/vertex-array-object.html [ Failure ]

[Justin Fan]

Regressions are fixed (on mac), but unfortunately there are no new passes within the webgl/conformance and conformance2 tests, only in the long-running deqp suite.

Whipping up a basic test case to verify primitive restart functionality.

[Justin Fan]

Created attachment 350780 [details]
Patch

[EWS Watchlist]

Attachment 350780 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/GraphicsContext3D.h:721:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/graphics/GraphicsContext3D.h:722:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/graphics/GraphicsContext3D.h:723:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 3 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dean Jackson]

Comment on attachment 350780 [details]
Patch

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

> Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:2009
> +unsigned WebGLRenderingContextBase::getMaxIndex(const RefPtr<JSC::ArrayBuffer> elementArrayBuffer, GC3Dintptr uoffset, GC3Dsizei n)

I think you should pass in numElements raw, and do the / sizeof(T) inside here, the same way you do with the uoffset.

> Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:2053
> +        maxIndex = getMaxIndex<GC3Duint>(buffer, offset, count);

... although I assume count here has already take the sizeof(T) into account?

> LayoutTests/webgl/webgl2-primitive-restart-expected.html:54
> +        if(!isWebGL2) {

Nit: if <space> (

[Justin Fan]

(In reply to Dean Jackson from comment #7)
> Comment on attachment 350780 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=350780&action=review
> 
> > Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:2009
> > +unsigned WebGLRenderingContextBase::getMaxIndex(const RefPtr<JSC::ArrayBuffer> elementArrayBuffer, GC3Dintptr uoffset, GC3Dsizei n)
> 
> I think you should pass in numElements raw, and do the / sizeof(T) inside
> here, the same way you do with the uoffset.
> 
> > Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:2053
> > +        maxIndex = getMaxIndex<GC3Duint>(buffer, offset, count);
> 
> ... although I assume count here has already take the sizeof(T) into account?

Yes, count as passed into the parent function is already the correct number.

Tryna figure out why the compilers aren't liking my function template.

[Justin Fan]

Created attachment 350802 [details]
Patch

[EWS Watchlist]

Attachment 350802 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/GraphicsContext3D.h:721:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/graphics/GraphicsContext3D.h:722:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/graphics/GraphicsContext3D.h:723:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 3 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Justin Fan]

Created attachment 350806 [details]
Patch

[EWS Watchlist]

Attachment 350806 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/GraphicsContext3D.h:721:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/graphics/GraphicsContext3D.h:722:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/graphics/GraphicsContext3D.h:723:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 3 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 350806 [details]
Patch

Clearing flags on attachment: 350806

Committed r236490: <https://trac.webkit.org/changeset/236490>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.