Bug 89865 – [Qt] Avoid a deep copy of QImage in GraphicsContext3D::getImageData.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Dongseong Hwang]

[Qt] Avoid a deep copy of QImage in GraphicsContext3D::getImageData.

[Dongseong Hwang]

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);
}

[Noam Rosenthal]

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.

[Dongseong Hwang]

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

[Noam Rosenthal]

Comment on attachment 149270 [details]
patch

There is an additional optimization to do, but we should do it in a different patch.

[WebKit Review Bot]

Comment on attachment 149270 [details]
patch

Clearing flags on attachment: 149270

Committed r121209: <http://trac.webkit.org/changeset/121209>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Noam Rosenthal]

Created a new bug for the proposed additional optimization, https://bugs.webkit.org/show_bug.cgi?id=89937