Bug 23211 - Add QWebFrame::renderContents to API
Summary: Add QWebFrame::renderContents to API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other All
: P2 Enhancement
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-01-09 12:18 PST by Adam Treat
Modified: 2009-07-20 08:58 PDT (History)
2 users (show)

See Also:


Attachments
Implements new API (2.12 KB, patch)
2009-01-09 12:19 PST, Adam Treat
zimmermann: review+
Details | Formatted Diff | Diff
New version incorporating comments (4.12 KB, patch)
2009-01-23 22:33 PST, Adam Treat
zimmermann: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Treat 2009-01-09 12:18:10 PST
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
Comment 1 Adam Treat 2009-01-09 12:19:15 PST
Created attachment 26567 [details]
Implements new API
Comment 2 Nikolas Zimmermann 2009-01-22 20:44:40 PST
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.
Comment 3 Adam Treat 2009-01-23 07:05:21 PST
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 :)
Comment 4 Adam Treat 2009-01-23 07:06:23 PST
Add Niko to the CC list so he'll see my last comment :)
Comment 5 Nikolas Zimmermann 2009-01-23 08:05:32 PST
(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
Comment 6 Adam Treat 2009-01-23 22:33:58 PST
Created attachment 26993 [details]
New version incorporating comments

This patch adds the refactoring suggested.
Comment 7 Nikolas Zimmermann 2009-01-24 05:08:24 PST
Comment on attachment 26993 [details]
New version incorporating comments

Looks good, r=me.
Comment 8 Adam Treat 2009-01-24 14:39:48 PST
Committed with r40218