Bug 141318

Summary: WebGL 2: Texture call format, internal format, and type validation
Product: WebKit Reporter: Roger Fong <roger_fong>
Component: WebGLAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, darin, dino, esprehn+autocc, gyuyoung.kim, kondapallykalyan, roger_fong
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
none
patch bfulgham: review+

Description Roger Fong 2015-02-05 18:57:37 PST 
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
Comment 1 Roger Fong 2015-02-07 15:58:33 PST 
Created attachment 246223 [details]
patch
Comment 2 WebKit Commit Bot 2015-02-07 16:01:12 PST 
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.
Comment 3 Roger Fong 2015-02-07 16:06:44 PST 
Created attachment 246225 [details]
patch
Comment 4 WebKit Commit Bot 2015-02-07 16:09:23 PST 
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.
Comment 5 Darin Adler 2015-02-07 17:33:45 PST 
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?
Comment 6 Darin Adler 2015-02-07 17:34:32 PST 
I mean specifically leaks related to 3D Canvas, I guess they are not WebGL, but that still might be the thing to do.
Comment 7 Roger Fong 2015-02-09 10:40:50 PST 
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.
Comment 8 Roger Fong 2015-02-09 11:39:33 PST 
Bug to keep track of leaks investigation here: https://bugs.webkit.org/show_bug.cgi?id=141394
Comment 9 Brent Fulgham 2015-02-09 12:26:19 PST 
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!
Comment 10 Roger Fong 2015-02-09 13:15:41 PST 
Committed: http://trac.webkit.org/changeset/179842