Bug 126944

Summary: [WebGL2] Vertex Array Objects
Product: WebKit Reporter: Dean Jackson <dino>
Component: WebGLAssignee: Dean Jackson <dino>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, buildbot, clopez, commit-queue, dino, esprehn+autocc, gyuyoung.kim, kondapallykalyan, noam, ossy, peavo, rniwa, roger_fong
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 126404    
Attachments:
Description Flags
patch
none
patch
none
patch
buildbot: commit-queue-
Archive of layout-test-results from ews100 for mac-mavericks
none
Archive of layout-test-results from ews105 for mac-mavericks-wk2
none
patch dino: review+

Description Dean Jackson 2014-01-13 15:48:33 PST
/* Vertex Array Objects */
  WebGLVertexArrayObject? createVertexArray();
  void deleteVertexArray(WebGLVertexArrayObject? vertexArray);
  [WebGLHandlesContextLoss] GLboolean isVertexArray(WebGLVertexArrayObject? vertexArray);
  void bindVertexArray(WebGLVertexArrayObject? array);
Comment 1 Dean Jackson 2014-01-13 15:48:59 PST
<rdar://problem/15002455>
Comment 2 Roger Fong 2015-03-15 21:05:22 PDT
Created attachment 248702 [details]
patch
Comment 3 WebKit Commit Bot 2015-03-15 21:08:30 PDT
Attachment 248702 [details] did not pass style-queue:


ERROR: Source/WebCore/html/canvas/WebGLGetInfo.h:74:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 1 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Roger Fong 2015-03-15 21:19:22 PDT
Created attachment 248703 [details]
patch
Comment 5 WebKit Commit Bot 2015-03-15 21:21:54 PDT
Attachment 248703 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:38:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
ERROR: Source/WebCore/html/canvas/WebGLGetInfo.h:74:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 2 in 23 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Roger Fong 2015-03-15 21:25:59 PDT
Created attachment 248704 [details]
patch
Comment 7 WebKit Commit Bot 2015-03-15 21:29:02 PDT
Attachment 248704 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:38:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
ERROR: Source/WebCore/html/canvas/WebGLGetInfo.h:74:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 2 in 23 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Build Bot 2015-03-15 21:50:24 PDT
Comment on attachment 248704 [details]
patch

Attachment 248704 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5816189607477248

New failing tests:
webgl/1.0.2/conformance/context/context-lost-restored.html
fast/canvas/webgl/context-lost-restored.html
Comment 9 Build Bot 2015-03-15 21:50:27 PDT
Created attachment 248706 [details]
Archive of layout-test-results from ews100 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 10 Build Bot 2015-03-15 21:56:47 PDT
Comment on attachment 248704 [details]
patch

Attachment 248704 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4664811417763840

New failing tests:
webgl/1.0.2/conformance/context/context-lost-restored.html
fast/canvas/webgl/context-lost-restored.html
Comment 11 Build Bot 2015-03-15 21:56:51 PDT
Created attachment 248708 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 12 Roger Fong 2015-03-15 22:08:31 PDT
Created attachment 248709 [details]
patch
Comment 13 WebKit Commit Bot 2015-03-15 22:09:53 PDT
Attachment 248709 [details] did not pass style-queue:


ERROR: Source/WebCore/html/canvas/WebGLGetInfo.h:74:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 1 in 23 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 14 Dean Jackson 2015-03-16 11:40:52 PDT
Comment on attachment 248709 [details]
patch

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

> Source/WebCore/ChangeLog:12
> +        This test will be landed along with other modified extension conformance tests
> +        once approval from Khronos is received.

I'm not sure what this means. Do you mean that we plan to contribute this back to Khronos?

> Source/WebCore/html/canvas/WebGLVertexArrayObjectBase.h:40
> +        VaoTypeDefault,
> +        VaoTypeUser,

Can be enum class, and use VAO rather than Vao

> Source/WebCore/html/canvas/WebGLVertexArrayObjectOES.cpp:54
> -
> +    

Oops.
Comment 15 Roger Fong 2015-03-16 11:43:56 PDT
(In reply to comment #14)
> Comment on attachment 248709 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=248709&action=review
> 
> > Source/WebCore/ChangeLog:12
> > +        This test will be landed along with other modified extension conformance tests
> > +        once approval from Khronos is received.
> 
> I'm not sure what this means. Do you mean that we plan to contribute this
> back to Kronos?


That's the hope. I have a bunch of conformance tests from the test suite that were extension tests but I changed all the calls to treat the extension stuff as core to WebGL2. Hoping to contribute that back to Khronos since their WebGL2 tests right now look like they need some work (looks like they were mostly just copy pasted from a previous suite and then a WebGL2 context was queried...)
 
> > Source/WebCore/html/canvas/WebGLVertexArrayObjectBase.h:40
> > +        VaoTypeDefault,
> > +        VaoTypeUser,
> 
> Can be enum class, and use VAO rather than Vao
> 
> > Source/WebCore/html/canvas/WebGLVertexArrayObjectOES.cpp:54
> > -
> > +    
> 
> Oops.
Comment 16 Roger Fong 2015-03-16 12:33:31 PDT
http://trac.webkit.org/changeset/181571
Comment 17 Csaba Osztrogonác 2015-03-16 15:39:42 PDT
(In reply to comment #16)
> http://trac.webkit.org/changeset/181571

It broke the Apple Windows build as the EWS noticed it in time.
Comment 19 Alex Christensen 2015-03-16 17:15:12 PDT
http://trac.webkit.org/changeset/181589 will cause problems.  glBindVertexArray et al. are in GLESv3.  I carefully made gl2softlinking.h to have everything and only everything that is in GLESv2.  It should crash every time it looks for those functions. This change should be reverted and replaced by proper separation of WebGL and WebGL2.
Comment 20 Carlos Alberto Lopez Perez 2015-05-18 17:58:08 PDT
(In reply to comment #19)
> http://trac.webkit.org/changeset/181589 will cause problems. 
> glBindVertexArray et al. are in GLESv3.  I carefully made gl2softlinking.h
> to have everything and only everything that is in GLESv2.  It should crash
> every time it looks for those functions. This change should be reverted and
> replaced by proper separation of WebGL and WebGL2.

Reported on bug 145156 a related regression.