[Webkit-unassigned] [Bug 28781] Add QWebFrame::renderElement to API

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Oct 14 15:31:42 PDT 2009


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





--- Comment #26 from Viatcheslav Ostapenko <ostapenko.viatcheslav at nokia.com>  2009-10-14 15:31:42 PDT ---
(In reply to comment #25)
> (From update of attachment 41045 [details])
> > +void QWebElement::render(QPainter *painter)
> Coding style, please place the * to the left :)

Fixed.
For a long time worked on another project, which has completely different
coding style ;)
BTW, webkit style check script doesn't detect this.

> > +{
> > +    WebCore::Element* e = m_element;
> > +    Document* doc = e->document();
> > +    if (!doc)
> > +        return;
> This is going to crash if the element is a null element. How about this?
> Document* doc = e ? e->document() : 0;

Fixed.

> > +    QImage resource(":/image.png");
> > +    QRect imageRect(0, 0, resource.width(), resource.height());
> > +
> > +    QImage testImage(resource.width(), resource.height(), QImage::Format_ARGB32);
> > +    QPainter painter0(&testImage);
> > +    painter0.fillRect(imageRect, Qt::white);
> > +    painter0.drawImage(0, 0, resource);
> > +    painter0.end();
> > +
> > +    QImage image1(resource.width(), resource.height(), QImage::Format_ARGB32);
> > +    QPainter painter1(&image1);
> > +    painter1.fillRect(imageRect, Qt::white);
> > +    imgs[0].render(&painter1);
> > +    painter1.end();
> > +
> > +    QCOMPARE(image1 == testImage, true);
> 
> It's easier to write
> QVERIFY(image1 == testImage)
> :-)

Fixed.

> > +    // render image 2nd time to make sure that cached rendering works fine
> > +    QImage image2(resource.width(), resource.height(), QImage::Format_ARGB32);
> > +    QPainter painter2(&image2);
> > +    painter2.fillRect(imageRect, Qt::white);
> > +    imgs[0].render(&painter2);
> > +    painter2.end();
> > +
> > +    QCOMPARE(image2 == testImage, true);
> > +
> > +    // compare table rendered through QWebElement::render to whole page table rendering
> > +    QRect tableRect(0, 0, 300, 300);
> > +    QList<QWebElement> tables = page.mainFrame()->findAllElements("table");
> > +    QCOMPARE(tables.count(), 1);
> > +
> > +    QImage image3(300, 300, QImage::Format_ARGB32);
> > +    QPainter painter3(&image3);
> > +    painter3.fillRect(tableRect, Qt::white);
> > +    tables[0].render(&painter3);
> > +    painter3.end();
> > +
> > +    QImage image4(300, 300, QImage::Format_ARGB32);
> > +    QPainter painter4(&image4);
> > +    page.mainFrame()->setClipRenderToViewport(false);
> > +    page.mainFrame()->render(&painter4, tableRect);
> > +    painter4.end();
> 
> It's probably a matter of preference, but instead of allocating QImage123 and
> QPainter123 you
> could also use scopes. Just an idea :)

Yes, that's would be better.
I was looking at another test cases and doing the same.
Now I prefer it to stay as it is, because if I start changing it a lot there is
a good chance to miss-place some spaces ;)

Thanks,
  Sl

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



More information about the webkit-unassigned mailing list