Bug 106975 - Simplify validation and data copying in WebGLBuffer
Summary: Simplify validation and data copying in WebGLBuffer
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Kenneth Russell
URL:
Keywords:
Depends on:
Blocks: 107185
  Show dependency treegraph
 
Reported: 2013-01-15 19:38 PST by Kenneth Russell
Modified: 2013-01-17 15:00 PST (History)
8 users (show)

See Also:


Attachments
Patch (7.71 KB, patch)
2013-01-15 19:44 PST, Kenneth Russell
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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