Bug 61743 – [Qt] LayoutTests/fast/canvas/webgl/renderbuffer-initialization.html fails for Qt based webkit

• Component: WebGL
• Status: RESOLVED WONTFIX

[Idrees]

Created attachment 95365 [details]
Test Case

We are not initializing the color, depth and stensil buffer upon the resizing of webgl canvas.

As a result the test case is failing.

[Idrees]

Created attachment 95367 [details]
Patch for the fix.

This patch will initialize the buffers upon resizing.

[Andreas Kling]

Comment on attachment 95367 [details]
Patch for the fix.

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

> Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:554
> +

One newline is enough.

> Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:596
> +    if (m_attrs.depth) {
> +            glClearDepth(clearDepth);
> +            glDepthMask(depthMask);
> +        }
> +        if (m_attrs.stencil) {
> +            glClearStencil(clearStencil);
> +            stencilMaskSeparate(GraphicsContext3D::FRONT, stencilMask);
> +        }
> +        if (isScissorEnabled)
> +            glEnable(GraphicsContext3D::SCISSOR_TEST);
> +        if (isDitherEnabled)
> +            glEnable(GraphicsContext3D::DITHER);
> +

Wrong indentation.

[Idrees]

Created attachment 95448 [details]
Another patch with indentation fixed.

[Simon Hausmann]

I wish we were using GraphicsContext3DOpenGL.cpp with Qt and get rid of this duplicated code :(

[Idrees]

(In reply to comment #4)
> I wish we were using GraphicsContext3DOpenGL.cpp with Qt and get rid of this duplicated code :(

I second your wish. What is stopping us from doing so?

[Simon Hausmann]

(In reply to comment #5)
> (In reply to comment #4)
> > I wish we were using GraphicsContext3DOpenGL.cpp with Qt and get rid of this duplicated code :(
> 
> I second your wish. What is stopping us from doing so?

Nothing conceptually AFAIK. Do you want to give it a try? :)

[Idrees]

(In reply to comment #6)
> 
> Nothing conceptually AFAIK. Do you want to give it a try? :)

I will but am not sure when.

[Idrees]

Getting issues resolved with qtwebkit webgl should be more important. We can do the restructuring of the code later on. Using GraphicsContext3DOpenGL.cpp wont fix all the issues with qtwebkit webgl.

[Alexis Menard (darktears)]

Comment on attachment 95448 [details]
Another patch with indentation fixed.

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

> Source/WebCore/ChangeLog:8
> +        LayoutTests/fast/canvas/webgl/renderbuffer-initialization.html

The test description goes after the explanation of the change.

[Idrees]

Created attachment 97335 [details]
Patch for the fix.

Fixing the position of texts in changelog.

[Robert Hogan]

Comment on attachment 97335 [details]
Patch for the fix.

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

This is shared code with GraphicsContext3DOpenGL. Seems fair enough. Let me know what you think of the comment below and we'll bug one of the reviewers!

> Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:593
> +    if (isScissorEnabled)
> +        glEnable(GraphicsContext3D::SCISSOR_TEST);
> +    if (isDitherEnabled)
> +        glEnable(GraphicsContext3D::DITHER);
> +

No need for the glDisable() equivalents if false?

[James Robinson]

(In reply to comment #11)
> (From update of attachment 97335 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=97335&action=review
> 
> This is shared code with GraphicsContext3DOpenGL. Seems fair enough. Let me know what you think of the comment below and we'll bug one of the reviewers!
> 

By shared, do you mean copy-pasted?

[Robert Hogan]

(In reply to comment #12)
> By shared, do you mean copy-pasted?

Yes.

[Kenneth Russell]

Comment on attachment 97335 [details]
Patch for the fix.

This patch is out of date. GraphicsContext3DInternal was renamed to GraphicsContext3DPrivate in https://bugs.webkit.org/show_bug.cgi?id=67172 . Further, if the Qt port uses GraphicsContext3DOpenGL.cpp at all, then it should be able to share the implementation of GraphicsContext3D::reshape in that file. Please both update the patch and try to reuse this code.

[Brent Fulgham]

The QT port is no longer part of WebKit.