Bug 164111

Summary: [WebGL2] Implement getBufferSubData()
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: New BugsAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, dino, esprehn+autocc, graouts, gyuyoung.kim, kondapallykalyan, noam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
WIP
none
Patch
none
Patch
none
Patch
dino: review+
Patch for committing
none
Patch for committing
mmaxfield: commit-queue-
Patch for committing none

Description Myles C. Maxfield 2016-10-27 23:54:27 PDT 
[WebGL2] Implement getBufferSubData()
Comment 1 Myles C. Maxfield 2016-10-28 00:00:28 PDT 
Created attachment 293119 [details]
Patch
Comment 2 WebKit Commit Bot 2016-10-28 00:02:37 PDT 
Attachment 293119 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Myles C. Maxfield 2016-10-28 00:37:35 PDT 
Created attachment 293124 [details]
WIP
Comment 4 WebKit Commit Bot 2016-10-28 00:39:05 PDT 
Attachment 293124 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/GraphicsContext3D.h:715:  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:718:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 3 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Myles C. Maxfield 2016-10-28 17:15:00 PDT 
Created attachment 293251 [details]
Patch
Comment 6 WebKit Commit Bot 2016-10-28 17:17:12 PDT 
Attachment 293251 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/GraphicsContext3D.h:715:  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:718:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 2 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Myles C. Maxfield 2016-10-28 17:17:26 PDT 
*** Bug 164024 has been marked as a duplicate of this bug. ***
Comment 8 Myles C. Maxfield 2016-10-28 17:32:09 PDT 
Created attachment 293256 [details]
Patch
Comment 9 WebKit Commit Bot 2016-10-28 17:34:36 PDT 
Attachment 293256 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/GraphicsContext3D.h:715:  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:718:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Alex Christensen 2016-10-28 17:40:22 PDT 
Comment on attachment 293256 [details]
Patch

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

> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:576
> +    return ::glMapBufferRange(target, offset, length, access);

It looks like these need to be inside #if ENABLE(WEBGL2) or something.  It doesn't exist in GLESv2 headers
Comment 11 Myles C. Maxfield 2016-10-28 22:34:53 PDT 
Created attachment 293277 [details]
Patch
Comment 12 WebKit Commit Bot 2016-10-28 22:37:01 PDT 
Attachment 293277 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/GraphicsContext3D.h:715:  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:718:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 13 Dean Jackson 2016-10-31 13:42:38 PDT 
Comment on attachment 293277 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        The call exists in OpenGL 3.2 but not OpenGL ES 3. On OpenGL
> +        ES, we need to call glMapBufferRange() instead.
> +
> +        OpenGL 3.2 also has glMapBufferRange, so perhaps this
> +        implementation should be used on both systems.

I agree.

> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:203
> +        synthesizeGLError(GraphicsContext3D::INVALID_OPERATION, "getBufferSubData", "dstOffset is too high");

Rather than "too high" in all the error messages, I think we should be more explicit. e.g.

dstOffset is larger than the length/size of the destination buffer.

> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:232
> +        synthesizeGLError(GraphicsContext3D::INVALID_OPERATION, "getBufferSubData", "Reading too far in the source array");

e.g. "Parameters would read outside the bounds of the source buffer."

> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:237
> +        synthesizeGLError(GraphicsContext3D::INVALID_OPERATION, "getBufferSubData", "WebGL buffer is too small");

e.g. "something more explicit here :)"
Comment 14 Myles C. Maxfield 2016-11-03 16:14:47 PDT 
Created attachment 293816 [details]
Patch for committing
Comment 15 Myles C. Maxfield 2016-11-03 16:34:34 PDT 
Created attachment 293826 [details]
Patch for committing
Comment 16 WebKit Commit Bot 2016-11-03 16:39:42 PDT 
Attachment 293826 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/GraphicsContext3D.h:715:  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:718:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 17 Myles C. Maxfield 2016-11-03 17:25:24 PDT 
Created attachment 293833 [details]
Patch for committing
Comment 18 WebKit Commit Bot 2016-11-03 17:27:07 PDT 
Attachment 293833 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/GraphicsContext3D.h:715:  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:718:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 19 WebKit Commit Bot 2016-11-03 19:00:15 PDT 
Comment on attachment 293833 [details]
Patch for committing

Clearing flags on attachment: 293833

Committed r208363: <http://trac.webkit.org/changeset/208363>