Bug 106975

Summary: Simplify validation and data copying in WebGLBuffer
Product: WebKit Reporter: Kenneth Russell <kbr>
Component: WebGLAssignee: Kenneth Russell <kbr>
Status: RESOLVED FIXED    
Severity: Normal CC: bajones, cmarrin, dino, gman, leviw, ojan.autocc, webkit.review.bot, zmo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 107185    
Attachments:
Description Flags
Patch none

Description Kenneth Russell 2013-01-15 19:38:49 PST
The WebGLBuffer class helps validate calls to bufferData and bufferSubData, and holds on to copies of indices for ELEMENT_ARRAY_BUFFER type buffers. Currently, its lowest level validation is written in terms of ArrayBuffers, which makes the validation checks it does much more complicated than they should be. Typed array instances, in particular subarrays, are already verified during construction, and some of the checks being done in WebGLBuffer are redundant. Additionally, it looks like they may even be incomplete because of the complexity of the code.

Changing the base validation routines to operate on a (void*, GC3Dsizeiptr) pair simplifies them considerably.
Comment 1 Kenneth Russell 2013-01-15 19:44:10 PST
Created attachment 182908 [details]
Patch
Comment 2 WebKit Review Bot 2013-01-16 13:10:33 PST
Comment on attachment 182908 [details]
Patch

Clearing flags on attachment: 182908

Committed r139914: <http://trac.webkit.org/changeset/139914>
Comment 3 WebKit Review Bot 2013-01-16 13:10:36 PST
All reviewed patches have been landed.  Closing bug.
Comment 4 Kenneth Russell 2013-01-16 14:07:22 PST
Reverted r139914 for reason:

Caused crashes in compositing/visibility/visibility-simple-webgl-layer.html

Committed r139923: <http://trac.webkit.org/changeset/139923>
Comment 5 Kenneth Russell 2013-01-16 15:05:03 PST
I've attempted to reproduce locally the crash seen in the flakiness dashboard with both Debug and Release builds of DRT but can not. I suspect that the crash is some rare preexisting bug. Subsequent runs on the flakiness dashboard which still contained my patch were clean. I'm going to re-land it.
Comment 6 Kenneth Russell 2013-01-16 15:10:32 PST
Committed r139928: <http://trac.webkit.org/changeset/139928>
Comment 7 Kenneth Russell 2013-01-16 15:16:24 PST
For the record, here was the failing build:

http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.8/builds/4201

and the layout test results from that run:

http://build.chromium.org/f/chromium/layout_test_results/WebKit_Mac10_8/177217/layout-test-results.zip