Bug 141318 – WebGL 2: Texture call format, internal format, and type validation

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Roger Fong]

Validate internal format, format, and type for texture calls (CopyTexImage2D, TexImage2D, TexSubImage2D) using the GLES3 spec.
This will aid in implementing the new texture calls introduced in the WebGL2 spec later.

It also will help with promoting the SRGB extension to core.

rdar://problem/19733828

[Roger Fong]

Created attachment 246223 [details]
patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/platform/graphics/GraphicsContext3D.h:511:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContextBase.h:128:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContextBase.h:129:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContextBase.h:291:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContextBase.h:293:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContextBase.h:295:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContextBase.h:297:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContextBase.h:300:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContext.cpp:401:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContext.h:47:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContext.h:48:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContext.h:50:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContext.h:52:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContext.h:54:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContext.h:57:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:1025:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 16 in 8 files


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

[Roger Fong]

Created attachment 246225 [details]
patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/platform/graphics/GraphicsContext3D.h:511:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContextBase.h:128:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContextBase.h:129:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContextBase.h:291:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContextBase.h:293:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContextBase.h:295:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContextBase.h:297:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContextBase.h:300:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContext.cpp:401:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContext.h:47:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContext.h:48:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContext.h:50:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContext.h:52:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContext.h:54:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGLRenderingContext.h:57:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:1025:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 16 in 8 files


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

[Darin Adler]

I think that before we make this change, we should fix the major leaks we are seeing on the leaks bot. This might introduce more leaks. How would we know?

[Darin Adler]

I mean specifically leaks related to 3D Canvas, I guess they are not WebGL, but that still might be the thing to do.

[Roger Fong]

The majority of this patch is either copy pasting or just conditional checks for validation. I don't allocate anything new. I think this is very low risk in terms of introducing more leaks and it is blocking me from completing other important WebGL2 work, for which I have very little time to complete in the first place.

I will investigate the leaks separately but I think that we should go ahead and land this so I can at least continue to make progress on the WebGL2 implementation.

[Roger Fong]

Bug to keep track of leaks investigation here: https://bugs.webkit.org/show_bug.cgi?id=141394

[Brent Fulgham]

Comment on attachment 246225 [details]
patch

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

r=me. It would be nice if the function declarations could be more inline with our coding style, though I know much of this is code being moved from one file to another.

> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:1014
> +void WebGL2RenderingContext::texSubImage2D(GC3Denum target, GC3Dint level, GC3Dint xoffset, GC3Dint yoffset,

It's weird that this is broken into multiple lines, but the other functions are on the same line.

> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:1125
> +    GC3Denum format, GC3Denum type)

Ditto. So many lines!

> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:498
> +    GC3Denum format, GC3Denum type)

So many lines for this function declaration!

[Roger Fong]

Committed: http://trac.webkit.org/changeset/179842