[webkit-reviews] review denied: [Bug 37070] [Qt] WebGL is not visible when QGLWidget viewport is used : [Attachment 53319] Removed UNUSED_PARAM from create method added by mistake

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has denied Jarkko Sakkinen
<jarkko.j.sakkinen at gmail.com>'s request for review:
Bug 37070: [Qt] WebGL is not visible when QGLWidget viewport is used
https://bugs.webkit.org/show_bug.cgi?id=37070

Attachment 53319: Removed UNUSED_PARAM from create method added by mistake
https://bugs.webkit.org/attachment.cgi?id=53319&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>


>  typedef void* PlatformGraphicsContext3D;
>  const  PlatformGraphicsContext3D NullPlatformGraphicsContext3D = 0;
> -typedef int Platform3DObject;
> +typedef GLuint Platform3DObject;

The ChangeLog does not explain why this is needed.

> -    m_attrs.alpha = true;
> -    m_attrs.depth = true;
> -    m_attrs.stencil = false;
> -    m_attrs.antialias = false;
> -    m_attrs.premultipliedAlpha = true;
> +    QWebPageClient* webPageClient = hostWindow->platformPageClient();
> +    QGLWidget* ownerGLWidget  = getOwnerGLWidget(webPageClient);
>  
> -    QGLFormat format;
> +    if (ownerGLWidget) 
> +	   m_glWidget = new QGLWidget(0, ownerGLWidget);
> +    else {
> +	   QGLFormat format;
> +	   format.setDepth(true);
> +	   format.setSampleBuffers(true);
> +	   format.setStencil(false);
>  
> -    format.setDepth(true);
> -    format.setSampleBuffers(true);
> -    format.setStencil(false);
> +	   m_glWidget = new QGLWidget(format);
> +    }

I would like to see the removal of QGLWidget altogether if possible. Not in
this patch though :)


> +QGLWidget* GraphicsContext3DInternal::getOwnerGLWidget(QWebPageClient*
webPageClient)
> +{
> +    QGraphicsView* ownerGraphicsView =
qobject_cast<QGraphicsView*>(webPageClient->ownerWidget());
> +
> +    if (ownerGraphicsView) 
> +	   return qobject_cast<QGLWidget*>(ownerGraphicsView->viewport());
> +
> +    return 0;

viewport() is not a function of QGraphicsView but its base-class
QAbstractScrollArea. So the qobject_cast should cast towards
QAbstractScrollArea instead of QGraphicsView.

I suggest something along these lines:

QWidget* viewport = webPageClient->ownerWidget();
if (QAbstractScrollArea* scrollArea =
qobject_cast<QAbstractScrollArea*>(viewport))
    viewport = scrollArea->viewport();

return qobject_cast<QGLWidget*>(viewport);