Bug 42125 - bufferData and bufferSubData generate wrong error when null buffer is bound
Summary: bufferData and bufferSubData generate wrong error when null buffer is bound
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Zhenyao Mo
URL:
Keywords:
Depends on: 41884
Blocks:
  Show dependency treegraph
 
Reported: 2010-07-12 18:19 PDT by Kenneth Russell
Modified: 2010-07-16 16:40 PDT (History)
11 users (show)

See Also:


Attachments
patch (12.17 KB, patch)
2010-07-13 16:39 PDT, Zhenyao Mo
zmo: commit-queue-
Details | Formatted Diff | Diff
revised patch (10.86 KB, patch)
2010-07-15 14:34 PDT, Zhenyao Mo
japhet: review+
zmo: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Russell 2010-07-12 18:19:15 PDT
When no buffer is bound to the ARRAY_BUFFER or ELEMENT_ARRAY_BUFFER binding point and bufferData or bufferSubData are called, the WebGLRenderingContext will synthesize INVALID_ENUM. In this case it should synthesize INVALID_OPERATION.
Comment 1 Zhenyao Mo 2010-07-13 16:39:22 PDT
Created attachment 61432 [details]
patch
Comment 2 Zhenyao Mo 2010-07-15 14:34:53 PDT
Created attachment 61723 [details]
revised patch

Sync the test with khronos conformance tests using kbr's script.  No other changes.
Comment 3 Kenneth Russell 2010-07-15 15:11:50 PDT
Comment on attachment 61723 [details]
revised patch

Looks nice.
Comment 4 Nate Chapin 2010-07-15 16:03:25 PDT
Comment on attachment 61723 [details]
revised patch

Ok.
Comment 5 David Levin 2010-07-15 16:10:30 PDT
It looks like you change the behavior so that when usage is not a valid enum, it will set INVALID_ENUM but it did not do this before from what I can tell.

Would you add a test for that as well?
Comment 6 Zhenyao Mo 2010-07-15 18:32:31 PDT
(In reply to comment #5)
> It looks like you change the behavior so that when usage is not a valid enum, it will set INVALID_ENUM but it did not do this before from what I can tell.
> 
> Would you add a test for that as well?

The test case was added by an earlier patch to invalid-passed-params.html.

See https://bugs.webkit.org/show_bug.cgi?id=41574
Comment 7 Zhenyao Mo 2010-07-15 19:20:30 PDT
Seems like there is a bug in webkit-patch land.  Changeset message got mixed with a previous patch, and it didn't automatically close this bug.

Close it manually: http://trac.webkit.org/changeset/63505
Comment 8 Zhenyao Mo 2010-07-15 19:36:41 PDT
(In reply to comment #7)
> Seems like there is a bug in webkit-patch land.  Changeset message got mixed with a previous patch, and it didn't automatically close this bug.
> 
> Close it manually: http://trac.webkit.org/changeset/63505

Seems like the cause is the automatic rebasing of WebCore/Changelog went wrong: a latest patch's log wasn't pushed on top; it was inserted second place.  Manually fix this issue in the ChangeLog: http://trac.webkit.org/changeset/63506
Comment 9 Eric Seidel (no email) 2010-07-16 07:53:42 PDT
@abarth:  Could this be the new retry code failing?  Or did svn-apply somehow apply this (seeminly correctly created diff) wrong?
Comment 10 Adam Barth 2010-07-16 15:23:28 PDT
> Could this be the new retry code failing? 

I doubt it.  That's for commit-queue only.

> Or did svn-apply somehow apply this (seeminly correctly created diff) wrong?

I be the committer ran update-webkit can got a clean merge so resolve-ChangeLogs didn't move the ChangeLog entry to the top.  We just need to error out of webkit-patch land if the diff to the ChangeLog isn't at the top.
Comment 11 Eric Seidel (no email) 2010-07-16 16:40:00 PDT
Oh!  I was confused.  I thought this was landed by the cq.  Yes, if you landed it manually, git will often mangle your ChangeLogs like this.

We need to put some detection code in webkit-patch land to abort hte commit if your ChangeLog got git-mangled.