Bug 23211 – Add QWebFrame::renderContents to API

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Qt

[Adam Treat]

Hi,

Currently QWebFrame::render is clipped internally by WebCore to the visibleContentRect.  This makes it impossible to render parts of a QWebFrame that are not visible in the frame's viewport.  There are a few use cases where this becomes problematic.  One such instance is printing of internal iframe's.  Currently, the Qt API provides no method for doing so as QWebFrame::render will clip to the internal iframe's viewport.  Another use case is a client which wants to render parts of the frame outside of the viewport.

The attached patch adds a new method to QWebFrame to allow such use cases.  The essential difference is that QWebFrame::renderContents will call FrameView::paintContents rather than FrameView::paint.  This results in no internal clipping to the viewport and also will not draw the scrollbars of the frame.

Cheers,
Adam

[Adam Treat]

Created attachment 26567 [details]
Implements new API

[Nikolas Zimmermann]

Comment on attachment 26567 [details]
Implements new API

> diff --git a/WebKit/qt/Api/qwebframe.cpp b/WebKit/qt/Api/qwebframe.cpp
> index 2ec76cf..1de9496 100644
> --- a/WebKit/qt/Api/qwebframe.cpp
> +++ b/WebKit/qt/Api/qwebframe.cpp
> @@ -781,6 +781,33 @@ void QWebFrame::render(QPainter *painter)
>  }
>  
>  /*!
> +  \since 4.6
> +  Render the frame's \a contents into \a painter while clipping to \a contents.
> +*/
> +void QWebFrame::renderContents(QPainter *painter, const QRegion &contents)
> +{
> +    if (!d->frame->view() || !d->frame->contentRenderer())
> +        return;
> +
> +    d->frame->view()->layoutIfNeededRecursive();
> +
> +    GraphicsContext ctx(painter);
I'd name it 'context', as we dislike abbrevations in WebCore.

> +    QVector<QRect> vector = contents.rects();
> +    WebCore::FrameView* view = d->frame->view();
> +    for (int i = 0; i < vector.size(); ++i) {
> +        if (i > 0) {
> +            painter->save();
> +            painter->setClipRect(vector.at(i), Qt::IntersectClip);
> +        }
> +
> +        view->paintContents(&ctx, vector.at(i));
> +
> +        if (i > 0)
> +            painter->restore();
> +    }
> +}

I'd rewrite the function as follows, to save some cycles:
    QVector<QRect> vector = contents.rects();
    if (vector.isEmpty())
        return;

    WebCore::FrameView* view = d->frame->view();
    view->paintContents(&context, vector.first());

    for (int i = 0; i < vector.size(); ++i) {
        const QRect& clipRect = vector.at(i);
        painter->save();
        painter->setClipRect(clipRect, Qt::IntersectClip);
        view->paintContents(&context, clipRect);
        painter->restore();
    }
}

r=me, with those changes.

[Adam Treat]

Your rewrite will cause a duplicate paint of the first rect in the vector by my reading.  I don't think that will save any cycles :)  Instead you could take the first rect out of the vector.

Also, shall I change the implementation in QWebFrame::render while I'm at it?  It has the same exact implementation as shown here complete with GraphicsContext named ctx :)

[Adam Treat]

Add Niko to the CC list so he'll see my last comment :)

[Nikolas Zimmermann]

(In reply to comment #3)
> Your rewrite will cause a duplicate paint of the first rect in the vector by my
> reading.  I don't think that will save any cycles :)  Instead you could take
> the first rect out of the vector.
Oops, it should read "for (int i = 1") :-)
> 
> Also, shall I change the implementation in QWebFrame::render while I'm at it? 
> It has the same exact implementation as shown here complete with
> GraphicsContext named ctx :)
>
Sure, why not? Maybe even refactor the code in a static helper function, if it's identical?

Cheers,
Niko

[Adam Treat]

Created attachment 26993 [details]
New version incorporating comments

This patch adds the refactoring suggested.

[Nikolas Zimmermann]

Comment on attachment 26993 [details]
New version incorporating comments

Looks good, r=me.

[Adam Treat]

Committed with r40218