Bug 74783 – [QT] WebGL can not make the frame buffer with the stencil buffer.

• Component: WebGL
• Status: RESOLVED FIXED

[Dongseong Hwang]

GraphicsContext3DQt was able to make the frame buffer with the depth buffer and the depth-stencil buffer, not with stencil buffer.
In the case QT_OPENGL_ES_2 is defined, GraphicsContext3DQt was able to make only the frame buffer with the depth buffer.

                                            Depth                Depth && Stencil                  Stencil
QT                                           O                              O                                 X
QT_OPENGL_ES_2                    O                              X                                  X

[Dongseong Hwang]

Created attachment 119718 [details]
patch

This makes GraphicsContext3DQt make the frame buffer with any combination of the depth buffer and the stencil buffer.

[Dongseong Hwang]

Created attachment 119720 [details]
patch

[Noam Rosenthal]

Comment on attachment 119720 [details]
patch

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

Good patch, see my usual changelog nitpicks

> Source/WebCore/ChangeLog:11
> +        GraphicsContext3DQt was able to make the frame buffer with the depth buffer and
> +        the depth-stencil buffer, not with the stencil buffer.
> +        In the case QT_OPENGL_ES_2 is defined, GraphicsContext3DQt was able to make only
> +        the frame buffer with the depth buffer.
> +        This makes GraphicsContext3DQt make the frame buffer with any combination of the
> +        depth buffer and the stencil buffer.

Proposing alternative wording for the Changelog:

When initializing a framebuffer in OpenGL ES 2, we need to initialize the depth and stencil buffers separately, as opposed to the combined depth-stencil buffer we initialize for desktop GL.

> Source/WebCore/ChangeLog:14
> +

You should mention that existing WebGL tests cover this.

[Dongseong Hwang]

Created attachment 119965 [details]
patch

[Dongseong Hwang]

Comment on attachment 119965 [details]
patch

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

> Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:-321
> -#if defined(QT_OPENGL_ES_2)

I amended here, also.

> Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:483
>          glTexImage2D(GraphicsContext3D::TEXTURE_2D, /* level */ 0, GraphicsContext3D::RGB, width, height, /* border */ 0, GraphicsContext3D::RGB, GraphicsContext3D::UNSIGNED_BYTE, /* data */ 0);

Actually, GraphicsContext3DQt::reshape never be called in Desktop Qt. Instead, GraphicsContext3DOpenGL::reshape would be called. I don't know how qmake to choose the right symbol from both GraphicsContext3DQt.o and GraphicsContext3DOpenGL.o.

[Dongseong Hwang]

Created attachment 119968 [details]
proposed layout test.

I made new layout test for verifying WebGL to initialize the depth and stencil buffers separately, because fast/canvas/webgl/context-attributes-depth-stencil-combination.html is not enough.
However, the most ports fail this test because those turn on the depth buffer if the stencil buffer is enabled.

The previous bug patch can not make Qt Desktop pass this test, because Qt Desktop calls GraphicsContext3DOpenGL::reshape instead of GraphicsContext3DQt::reshape, strangely.
I think this layout test can not be used.

[WebKit Review Bot]

Comment on attachment 119965 [details]
patch

Clearing flags on attachment: 119965

Committed r103301: <http://trac.webkit.org/changeset/103301>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.