[Webkit-unassigned] [Bug 64879] [Qt] Implement WebGL antialiasing (part 3)

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=64879


Noam Rosenthal <noam.rosenthal at nokia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #103241|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #11 from Noam Rosenthal <noam.rosenthal at nokia.com>  2011-08-08 07:42:31 PST ---
(From update of attachment 103241)
View in context: https://bugs.webkit.org/attachment.cgi?id=103241&action=review

> Source/WebCore/ChangeLog:8
> +        Existing WebGL layout tests.

Do you mean "Existing WebGL layout tests cover this"?

> Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:155
> +    if (m_context->m_attrs.antialias) {
> +        // Switch to FBOs context for resolving multisamples, then restore.
> +        const QGLContext* currentContext = QGLContext::currentContext();
> +        const QGLContext* widgetContext = m_glWidget->context();
> +        if (currentContext != widgetContext)
> +            m_glWidget->makeCurrent();
> +        blitMultisampleFramebuffer();
> +        if (currentContext) {
> +            if (currentContext != widgetContext)
> +                const_cast<QGLContext*>(currentContext)->makeCurrent();
> +        } else
> +            m_glWidget->doneCurrent();
> +    }
> +

I don't like this inside paintToTextureMapper. Maybe put this in a new function like blitMultisampleFramebufferAndRestoreContext?
If we do that you don't actually need the comment and the code describes itself.

> Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:389
>      if (m_internal->m_glWidget->isValid()) {
>          glDeleteTextures(1, &m_texture);
> -        glDeleteRenderbuffers(1, &m_depthStencilBuffer);
> +        if (m_attrs.antialias) {
> +            glDeleteRenderbuffers(1, &m_multisampleColorBuffer);
> +            if (m_attrs.stencil || m_attrs.depth)
> +                glDeleteRenderbuffers(1, &m_multisampleDepthStencilBuffer);
> +            glDeleteFramebuffers(1, &m_multisampleFBO);
> +        } else if (m_attrs.stencil || m_attrs.depth)
> +            glDeleteRenderbuffers(1, &m_depthStencilBuffer);
>          glDeleteFramebuffers(1, &m_fbo);
>      }
>  }

This is becoming rather unreadable... can we refactor to use early returns? Looks to me that with early returns this can shrink to a single condition level from 3.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.