Bug 39855 - Map GLsizei to long instead of unsigned long in WebGLRenderingContext and GraphicsContext3D
Summary: Map GLsizei to long instead of unsigned long in WebGLRenderingContext and Gra...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Zhenyao Mo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-27 16:02 PDT by Zhenyao Mo
Modified: 2010-12-28 13:27 PST (History)
4 users (show)

See Also:


Attachments
Patch (20.93 KB, patch)
2010-12-23 15:13 PST, Zhenyao Mo
no flags Details | Formatted Diff | Diff
Patch (27.86 KB, patch)
2010-12-28 11:10 PST, Zhenyao Mo
kbr: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zhenyao Mo 2010-05-27 16:02:27 PDT
Currently GLsizei is mapped to unsigned, so instead of generating an INVALID_VALUE error, negative level/border/xoffset/yoffset/width/height are mapped to a very large positive number, which causes buffer size validation to fail (once we have them).

Also, some functions map enum to unsigned, and some to unsigned long.
Comment 1 Zhenyao Mo 2010-12-23 15:13:11 PST
Created attachment 77374 [details]
Patch
Comment 2 Zhenyao Mo 2010-12-23 15:16:18 PST
There are a bunch of wrong types in GraphicsContext3D.  It's less important, and it's much more than the violations in WebGLRenderingContext, so I don't include the fix in the same patch.
Comment 3 David Levin 2010-12-27 09:32:04 PST
Comment on attachment 77374 [details]
Patch

The code changes looks fine, but it should have some tests.

It seems like you could call these method with negative values and verify that you get a failure.
Comment 4 Kenneth Russell 2010-12-27 20:41:13 PST
Comment on attachment 77374 [details]
Patch

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

I'd like to point out that this patch just brings WebKit's implementation closer to spec compliance; GLsizei is typedef'ed to "long" in the WebGL spec, but up until this point we've had the wrong mapping in the associated WebKit IDL. Mo would know better than me how practical it is to test all of the entry points affected by this change.

> WebCore/ChangeLog:8
> +        No new tests. (OOPS!)

This OOPS needs to either be fixed (by adding a new test) or removed.
Comment 5 Zhenyao Mo 2010-12-28 11:10:29 PST
Created attachment 77563 [details]
Patch
Comment 6 Zhenyao Mo 2010-12-28 11:11:32 PST
(In reply to comment #5)
> Created an attachment (id=77563) [details]
> Patch

This patch only added test cases to cover the affected functions.  The OOPS is removed.

Will sync with khronos once this is reviewed.
Comment 7 Kenneth Russell 2010-12-28 11:15:13 PST
Comment on attachment 77563 [details]
Patch

Looks excellent. Thanks for handling this and for the added test cases.
Comment 8 Zhenyao Mo 2010-12-28 13:27:23 PST
Committed r74719: <http://trac.webkit.org/changeset/74719>