Summary: | [Qt] Avoid a deep copy of QImage in GraphicsContext3D::getImageData. | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dongseong Hwang <dongseong.hwang> | ||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | noam, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Dongseong Hwang
2012-06-25 04:07:15 PDT
Created attachment 149270 [details]
patch
This patch follows BitmapTextureGL::updateContents.
void BitmapTextureGL::updateContents(Image* image, const IntRect& targetRect, const IntPoint& offset)
{
if (!image)
return;
NativeImagePtr frameImage = image->nativeImageForCurrentFrame();
if (!frameImage)
return;
int bytesPerLine;
const char* imageData;
#if PLATFORM(QT)
QImage qtImage;
#if HAVE(QT5)
// With QPA, we can avoid a deep copy.
qtImage = *frameImage->handle()->buffer();
#else
// This might be a deep copy, depending on other references to the pixmap.
qtImage = frameImage->toImage();
#endif
imageData = reinterpret_cast<const char*>(qtImage.constBits());
bytesPerLine = qtImage.bytesPerLine();
#elif USE(CAIRO)
cairo_surface_t* surface = frameImage->surface();
imageData = reinterpret_cast<const char*>(cairo_image_surface_get_data(surface));
bytesPerLine = cairo_image_surface_get_stride(surface);
#endif
updateContents(imageData, targetRect, offset, bytesPerLine);
}
Comment on attachment 149270 [details]
patch
While we're at it, we should convert the format to Format_ARGB32_Premultiplied if premultiplied is requested, instead of changing to ARGB32 and then premultiplying as an additional step.
Would you mind explain more detail? I don't fully understand. As I understand, you means the two code stubs are different? 1. Original nativeImage = nativePixmap->toImage().convertToFormat(QImage::Format_ARGB32); 2. Changed (For Qt4) QImage qtImage; qtImage = nativePixmap->toImage(); nativeImage = qtImage.convertToFormat(QImage::Format_ARGB32); If the two is different and original stub is correct, can I change like this? - nativeImage = nativePixmap->toImage().convertToFormat(QImage::Format_ARGB32); +#if HAVE(QT5) + // With QPA, we can avoid a deep copy. + nativeImage = nativePixmap->handle()->buffer()->convertToFormat(QImage::Format_ARGB32); +#else + // This might be a deep copy, depending on other references to the pixmap. + nativeImage = nativePixmap->toImage().convertToFormat(QImage::Format_ARGB32); +#endif Comment on attachment 149270 [details]
patch
There is an additional optimization to do, but we should do it in a different patch.
Comment on attachment 149270 [details] patch Clearing flags on attachment: 149270 Committed r121209: <http://trac.webkit.org/changeset/121209> All reviewed patches have been landed. Closing bug. Created a new bug for the proposed additional optimization, https://bugs.webkit.org/show_bug.cgi?id=89937 |