Bug 39855 – Map GLsizei to long instead of unsigned long in WebGLRenderingContext and GraphicsContext3D

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Zhenyao Mo]

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.

[Zhenyao Mo]

Created attachment 77374 [details]
Patch

[Zhenyao Mo]

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.

[David Levin]

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.

[Kenneth Russell]

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.

[Zhenyao Mo]

Created attachment 77563 [details]
Patch

[Zhenyao Mo]

(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.

[Kenneth Russell]

Comment on attachment 77563 [details]
Patch

Looks excellent. Thanks for handling this and for the added test cases.

[Zhenyao Mo]

Committed r74719: <http://trac.webkit.org/changeset/74719>