Bug 164493 – [WebGL2] Implement texStorage2D()

• Component: New Bugs
• Status: RESOLVED FIXED

[Myles C. Maxfield]

[WebGL2] Implement texStorage{2D,3D}()

[Myles C. Maxfield]

Created attachment 294097 [details]
WIP

[Myles C. Maxfield]

Created attachment 294141 [details]
WIP

[Myles C. Maxfield]

Created attachment 294901 [details]
Needs tests

[Myles C. Maxfield]

Created attachment 294904 [details]
Needs tests

[WebKit Commit Bot]

Attachment 294904 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Myles C. Maxfield]

Created attachment 295189 [details]
Patch

[Dean Jackson]

Comment on attachment 295189 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        Create a new validation functionw which only accepts sized internalFormats.

functionw

> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:317
> +    if (width < 0 || height < 0) {

Is == 0 valid?

> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:429
> +    {

Why do you do this?

> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:448
> +        Vector<char> data(size);
> +        memset(data.data(), 0, size);

I didn't check the answer to this last time, but can we use a Uint8 array here (which I think is already zero-filled).

> Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:4043
> +            if (textureInternalFormat == GraphicsContext3D::RGB
> +                || textureInternalFormat == GraphicsContext3D::RGBA
> +                || textureInternalFormat == GraphicsContext3D::RGB8
> +                || textureInternalFormat == GraphicsContext3D::RGBA8
> +                || !texture->isValid(target, level)) {

Maybe a static helper isRGBFormat(textureInternalFormat)?

> Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:4078
> +            if (textureInternalFormat == GraphicsContext3D::RGB
> +                || textureInternalFormat == GraphicsContext3D::RGBA
> +                || textureInternalFormat == GraphicsContext3D::RGB8
> +                || textureInternalFormat == GraphicsContext3D::RGBA8

... because you do it here too.

[Myles C. Maxfield]

Comment on attachment 295189 [details]
Patch

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

>> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:317
>> +    if (width < 0 || height < 0) {
> 
> Is == 0 valid?

I don't see anything in either spec that disallows it.

>> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:429
>> +    {
> 
> Why do you do this?

Work around potentially-buggy drivers. We want to guarantee that the contents are zero-filled.

>> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:448
>> +        memset(data.data(), 0, size);
> 
> I didn't check the answer to this last time, but can we use a Uint8 array here (which I think is already zero-filled).

Are you referring to the JSC type, or the C type? I don't know why the JSC type would be valuable to use, and the C type would put it on the stack which is a bad idea.

>> Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:4043
>> +                || !texture->isValid(target, level)) {
> 
> Maybe a static helper isRGBFormat(textureInternalFormat)?

Good idea.

[Myles C. Maxfield]

Created attachment 295194 [details]
Patch for committing

[Myles C. Maxfield]

Created attachment 295199 [details]
Patch for committing

[Build Bot]

Comment on attachment 295189 [details]
Patch

Attachment 295189 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2538830

New failing tests:
transitions/default-timing-function.html

[Build Bot]

Created attachment 295201 [details]
Archive of layout-test-results from ews117 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[WebKit Commit Bot]

Comment on attachment 295199 [details]
Patch for committing

Clearing flags on attachment: 295199

Committed r208910: <http://trac.webkit.org/changeset/208910>