Bug 38044

Summary: QWebFrame::render() clips content right of 640 pixels wide when rendering to a QImage
Product: WebKit Reporter: Will Stokes <wstokes>
Component: WebKit QtAssignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Normal CC: benjamin, hausmann, kenneth, laszlo.gombos, manyoso, ostap73, robert, sriram.yadavalli, tonikitoo
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: Mac (Intel)   
OS: OS X 10.6   
URL: http://bugreports.qt.nokia.com/browse/QTBUG-9906
Attachments:
Description Flags
screenshot of bug
none
example program that produces bug (.cpp file)
none
example program that produces bug (.h file) none

Description Will Stokes 2010-04-23 06:03:33 PDT
If you try to render content in a QWebFrame to an image that is wider than 640 pixels, the content in the QWebFrame gets clipped right of the 640 pixel column. Attached I've provided a small test case that produces the problem.
Comment 1 Will Stokes 2010-04-23 06:04:37 PDT
Created attachment 54152 [details]
screenshot of bug
Comment 2 Will Stokes 2010-04-23 06:05:06 PDT
Created attachment 54153 [details]
example program that produces bug (.cpp file)
Comment 3 Will Stokes 2010-04-23 06:05:29 PDT
Created attachment 54154 [details]
example program that produces bug (.h file)
Comment 4 Sriram 2010-06-08 10:54:01 PDT
Seems like a bad bug for use cases where Webkit is embedded.
Comment 5 Robert Hogan 2010-07-13 13:52:47 PDT
(In reply to comment #4)
> Seems like a bad bug for use cases where Webkit is embedded.

QWebFrame::render() clips to the frameRect() of the frame. In the absence of content to update the size of the rect you will need to update it first with the expected size of the image. So in your example you need to do:

  webView.page()->setViewportSize(QSize(maxWidth, 480));

QWebFrame::render() could remove the need for this step and check the viewport() of the painter and adjust the frame->view() as follows:

        WebCore::FrameView* view = frame->view();
        view->setFrameRect(QRect(QPoint(0, 0), painter.viewport()));
        view->adjustViewSize();

Simon/Kenneth, what do you think?
Comment 6 Kenneth Rohde Christiansen 2010-07-13 14:16:21 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > Seems like a bad bug for use cases where Webkit is embedded.
> 
> QWebFrame::render() clips to the frameRect() of the frame. In the absence of content to update the size of the rect you will need to update it first with the expected size of the image. So in your example you need to do:
> 
>   webView.page()->setViewportSize(QSize(maxWidth, 480));
> 
> QWebFrame::render() could remove the need for this step and check the viewport() of the painter and adjust the frame->view() as follows:
> 
>         WebCore::FrameView* view = frame->view();
>         view->setFrameRect(QRect(QPoint(0, 0), painter.viewport()));
>         view->adjustViewSize();
> 
> Simon/Kenneth, what do you think?

you might be getting the QWebElement from an existing view, so in that case you do not want to change the viewport, unless you would somewhat clone the element.
Comment 7 Robert Hogan 2010-07-13 14:19:28 PDT
(In reply to comment #6)
> 
> you might be getting the QWebElement from an existing view, so in that case you do not want to change the viewport, unless you would somewhat clone the element.

So do you think the bug is invalid, or just hard to fix? ;-)
Comment 8 Antonio Gomes 2010-07-13 14:31:12 PDT
(In reply to comment #7)
> (In reply to comment #6)
> > 
> > you might be getting the QWebElement from an existing view, so in that case you do not want to change the viewport, unless you would somewhat clone the element.
> 
> So do you think the bug is invalid, or just hard to fix? ;-)

invalid to me.
Comment 9 Kenneth Rohde Christiansen 2010-07-13 14:42:22 PDT
(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > 
> > > you might be getting the QWebElement from an existing view, so in that case you do not want to change the viewport, unless you would somewhat clone the element.
> > 
> > So do you think the bug is invalid, or just hard to fix? ;-)
> 
> invalid to me.

I'm not sure, he is called setPreferredContentSize(), so I'm not sure it should be clipped to the frameRect, especially not if the fixedLayoutSize is larger.

Adam Treat, can you give some input here?
Comment 10 Viatcheslav Ostapenko 2010-11-12 15:25:38 PST
(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > 
> > > you might be getting the QWebElement from an existing view, so in that case you do not want to change the viewport, unless you would somewhat clone the element.
> > 
> > So do you think the bug is invalid, or just hard to fix? ;-)
> 
> invalid to me.

No, I don't think it's invalid.
Generally, there is now no _right_ way to render contents without clipping to viewport. Suggested QWebElement::render is not very usable in this case, because it doesn't have any clipping paramter and will traverse the whole webkit render tree if only part of the page (different from viewport) been rendered.

I think QWebFramePrivate::renderRelativeCoords unnecessarily clips rendered content to viewport (framerect). It used in 3 places (variants of QWebFrame::render method) and those places all default to framerect, if clipping was not specified or empty. IMHO, if clipping _was_ specified, than it should be used as it is (well, may be clipped to contents rect). 

So far clipping in QWebFramePrivate::renderRelativeCoords contradicts to the code written in QWebFrame::render(QPainter* painter, RenderLayer layer, const QRegion& clip) .
Comment 11 Benjamin Poulain 2010-11-19 06:47:44 PST
As said in comments, invalid bug.
Comment 12 Viatcheslav Ostapenko 2010-11-19 08:02:26 PST
(In reply to comment #11)
> As said in comments, invalid bug.

Ok!
1. Here is use case:
I need to create full page snapshot, but because of limited RAM I have to create it in chunks.
2. qtwebkit doc doesn't say, that QWebFrame::render clips to viewport, so current behavior doesn't match documentation.
Comment 13 Benjamin Poulain 2010-11-19 08:19:01 PST
(In reply to comment #12)

What you suggest is basically extending the API of QWebElement::render(). If you plan to do that, you should create a new bug report.

If you want to change the API of QWebFrame::render(), this is behavior breakage of public APIs, and you need to start a thread on the mailing list about that.

In both cases, this bug report is invalid. The bug is "QWebFrame::render() clips content right of 640 pixels wide when rendering to a QImage" which is simply how the API is defined.
Comment 14 Antonio Gomes 2010-11-19 08:22:52 PST
Maybe patching the docs?
Comment 15 Viatcheslav Ostapenko 2010-11-19 08:44:57 PST
(In reply to comment #13)
> (In reply to comment #12)
> 
> If you want to change the API of QWebFrame::render(), this is behavior breakage of public APIs, and you need to start a thread on the mailing list about that.

Ok.
Return QWebFrame::paintContents ?

(In reply to comment #14)
> Maybe patching the docs?

Yes.

Also, renderRelativeCoords clips every rect from the clip region in a cycle. I think it should clip the clip region outside of cycle - this will reduce possible number of regions and eliminate situation, when view->paintContents is called with an empty rect (it happens rarely).
Comment 16 Viatcheslav Ostapenko 2010-11-19 08:47:53 PST
"reduce possible number of regions" = "reduce possible number of rects"