Bug 141318

Summary:

WebGL 2: Texture call format, internal format, and type validation

Product:

WebKit

Reporter:

Roger Fong <roger_fong>

Component:

WebGL

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, commit-queue, darin, dino, esprehn+autocc, gyuyoung.kim, kondapallykalyan, roger_fong

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
patch	none
patch	bfulgham: review+

Roger Fong

Reported 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

Attachments
patch (117.74 KB, patch) 2015-02-07 15:58 PST, Roger Fong	no flags	Details Formatted Diff Diff
patch (117.74 KB, patch) 2015-02-07 16:06 PST, Roger Fong	bfulgham: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Roger Fong

Comment 1 2015-02-07 15:58:33 PST

Created attachment 246223 [details] patch

WebKit Commit Bot

Comment 2 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.

Roger Fong

Comment 3 2015-02-07 16:06:44 PST

Created attachment 246225 [details] patch

WebKit Commit Bot

Comment 4 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.

Darin Adler

Comment 5 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?

Darin Adler

Comment 6 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.

Roger Fong

Comment 7 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.

Roger Fong

Comment 8 2015-02-09 11:39:33 PST

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

Brent Fulgham

Comment 9 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!

Roger Fong

Comment 10 2015-02-09 13:15:41 PST

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

Note You need to log in before you can comment on or make changes to this bug.