Bug 85117 - vertexAttribPointer needs to reject large negative offsets
: vertexAttribPointer needs to reject large negative offsets
Status: RESOLVED FIXED
: WebKit
WebGL
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
: 85722
:
  Show dependency treegraph
 
Reported: 2012-04-27 18:39 PST by
Modified: 2012-05-08 15:38 PST (History)


Attachments
Patch (16.18 KB, patch)
2012-05-04 16:23 PST, Zhenyao Mo
no flags Review Patch | Details | Formatted Diff | Diff
Patch (21.00 KB, patch)
2012-05-07 09:36 PST, Zhenyao Mo
kbr: review+
Review Patch | Details | Formatted Diff | Diff


Note

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


Description From 2012-04-27 18:39:14 PST
The WebGL conformance test conformance/buffers/index-validation.html is now failing because the test was updated to reflect the spec, which is that negative offsets cause an INVALID_VALUE error to be generated.

The test was previously expecting that the large negative offset would be truncated to 32 bits and show up as a positive value.

I am not even sure that WebKit's IDL and binding generators can support long long values passed from JavaScript to C++ right now. That might be the bulk of this fix.
------- Comment #1 From 2012-05-04 15:16:26 PST -------
Taking this one.  It's the last bug fix to make chrome webgl 1.0.1 conformant.
------- Comment #2 From 2012-05-04 16:23:46 PST -------
Created an attachment (id=140358) [details]
Patch
------- Comment #3 From 2012-05-04 16:25:33 PST -------
Tested in chromium and webkit on Mac.  The test is synced from khronos.  Please have a look.

(I used long long in place of GLsizeiptr and GLintptr to be consistent with the types used in the bindings)
------- Comment #4 From 2012-05-04 16:33:05 PST -------
(From update of attachment 140358 [details])
Looks good. r=me

Let's wait for it to clear the EWS before committing though.
------- Comment #5 From 2012-05-05 07:50:58 PST -------
(From update of attachment 140358 [details])
Clearing flags on attachment: 140358

Committed r116221: <http://trac.webkit.org/changeset/116221>
------- Comment #6 From 2012-05-05 07:51:03 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #7 From 2012-05-05 13:15:02 PST -------
This breaks the build on platforms that compile with -Wshorten-64-to-32, since there are now several call sites that pass a long long to a function that expects a GC3Dintptr.
------- Comment #8 From 2012-05-05 15:52:23 PST -------
Reopening because of the breakage that Andy is talking about.
------- Comment #9 From 2012-05-05 15:55:41 PST -------
Sorry, but I have to roll this out. :-(  Please let me know if you need assistance debugging the build errors we're seeing.
------- Comment #10 From 2012-05-07 09:36:28 PST -------
Created an attachment (id=140538) [details]
Patch
------- Comment #11 From 2012-05-07 09:51:04 PST -------
(From update of attachment 140538 [details])
Ken, I added explicit converting from long long to GC3D types to avoid compiling failures on 32.

Please have another look.
------- Comment #12 From 2012-05-07 11:48:03 PST -------
The new patch builds for me on the same machine that couldn't build the last patch.
------- Comment #13 From 2012-05-07 12:57:18 PST -------
Thanks Andy for testing this out.
------- Comment #14 From 2012-05-07 13:03:40 PST -------
(From update of attachment 140538 [details])
New patch looks OK as long as it compiles. It's a little unfortunate that we lose the ability to change the signatures here with one typedef.
------- Comment #15 From 2012-05-07 16:56:01 PST -------
Committed r116374: <http://trac.webkit.org/changeset/116374>
------- Comment #16 From 2012-05-08 15:38:49 PST -------
*** Bug 85528 has been marked as a duplicate of this bug. ***