[webkit-reviews] review granted: [Bug 88785] [Qt] Change NativeImagePtr from QPixmap* to QImage* : [Attachment 152276] proposed patch

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has granted Zoltan Horvath
<zoltan at webkit.org>'s request for review:
Bug 88785: [Qt] Change NativeImagePtr from QPixmap* to QImage*
https://bugs.webkit.org/show_bug.cgi?id=88785

Attachment 152276: proposed patch
https://bugs.webkit.org/attachment.cgi?id=152276&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=152276&action=review


r=me

Land with care and watch the bots ;)

> Source/WebKit/qt/Api/qwebframe.cpp:1977
> -    return d->pixmap;
> +    return QPixmap::fromImage(d->image);

Is this a good move? QPixmap::fromImage will do a conversion every time, making
the pixmap() accessor a lot slower on repeated calls. Wouldn't it be simpler to
leave the pixmap as member? (under the assumption that we're not changing the
API)

Alternatively the conversion from QImage to QPixmap can be made _very_ cheap if
the QImage has the same format, so another option would be for example to make
the 'QImage image' member here hold a QImage respecting
QPixmap::defaultDepth().