Changed QPixmap VG backend to use CFbsBitmap on Symbian.

Similarly to QS60PixmapData, that is used on raster, QVGPixmapData
is also backed by a CFbsBitmap from now on (at least when it makes
sense to do so). This allows copy-less bitmap handle duplication in
from- and toSymbianCFbsBitmap() in case of certain image formats,
reduces local heap usage and improves performance with the s60 style
due to a reduced number of pixel data copies.

Task-number: QT-2505

Reviewed-by: Jason Barron
Reviewed-by: Jani Hautakangas

4 files changed, 113 insertions, 134 deletions

diff --git a/src/openvg/qpaintengine_vg.cpp b/src/openvg/qpaintengine_vg.cpp
index 22cbbf5..f0f198f 100644
--- a/src/openvg/qpaintengine_vg.cpp
+++ b/src/openvg/qpaintengine_vg.cpp
@@ -3134,9 +3134,13 @@ void QVGPaintEngine::drawPixmap(const QRectF &r, const QPixmap &pm, const QRectF
         if (pm.size().width() <= screenSize.width()
             && pm.size().height() <= screenSize.height())
             vgpd->failedToAlloc = false;
-    }
 
-    drawImage(r, *(pd->buffer()), sr, Qt::AutoColor);
+        vgpd->source.beginDataAccess();
+        drawImage(r, vgpd->source.imageRef(), sr, Qt::AutoColor);
+        vgpd->source.endDataAccess(true);
+    } else {
+        drawImage(r, *(pd->buffer()), sr, Qt::AutoColor);
+    }
 }
 
 void QVGPaintEngine::drawPixmap(const QPointF &pos, const QPixmap &pm)
@@ -3162,9 +3166,13 @@ void QVGPaintEngine::drawPixmap(const QPointF &pos, const QPixmap &pm)
         if (pm.size().width() <= screenSize.width()
             && pm.size().height() <= screenSize.height())
             vgpd->failedToAlloc = false;
-    }
 
-    drawImage(pos, *(pd->buffer()));
+        vgpd->source.beginDataAccess();
+        drawImage(pos, vgpd->source.imageRef());
+        vgpd->source.endDataAccess();
+    } else {
+        drawImage(pos, *(pd->buffer()));
+    }
 }
 
 void QVGPaintEngine::drawImage
diff --git a/src/openvg/qpixmapdata_vg.cpp b/src/openvg/qpixmapdata_vg.cpp
index 732b484..c5da115 100644
--- a/src/openvg/qpixmapdata_vg.cpp
+++ b/src/openvg/qpixmapdata_vg.cpp
@@ -70,7 +70,7 @@ QVGPixmapData::QVGPixmapData(PixelType type)
     context = 0;
     qt_vg_register_pixmap(this);
 #endif
-    setSerialNumber(++qt_vg_pixmap_serial);
+    updateSerial();
 }
 
 QVGPixmapData::~QVGPixmapData()
@@ -138,25 +138,31 @@ bool QVGPixmapData::isValid() const
     return (w > 0 && h > 0);
 }
 
+void QVGPixmapData::updateSerial()
+{
+    setSerialNumber(++qt_vg_pixmap_serial);
+}
+
 void QVGPixmapData::resize(int wid, int ht)
 {
-    if (w == wid && h == ht)
+    if (w == wid && h == ht) {
+        updateSerial();
         return;
+    }
 
     w = wid;
     h = ht;
     d = 32; // We always use ARGB_Premultiplied for VG pixmaps.
     is_null = (w <= 0 || h <= 0);
-    source = QImage();
+    source = QVolatileImage();
     recreate = true;
 
-    setSerialNumber(++qt_vg_pixmap_serial);
+    updateSerial();
 }
 
-void QVGPixmapData::fromImage
-        (const QImage &image, Qt::ImageConversionFlags flags)
+void QVGPixmapData::fromImage(const QImage &image, Qt::ImageConversionFlags flags)
 {
-    if(image.isNull())
+    if (image.isNull())
         return;
 
     QImage img = image;
@@ -200,33 +206,34 @@ bool QVGPixmapData::fromData(const uchar *buffer, uint len, const char *format,
     return !isNull();
 }
 
-/*!
-    out-of-place conversion (inPlace == false) will always detach()
- */
-void QVGPixmapData::createPixmapForImage(QImage &image, Qt::ImageConversionFlags flags, bool inPlace)
+QImage::Format QVGPixmapData::idealFormat(QImage *image, Qt::ImageConversionFlags flags) const
 {
-    if (image.size() == QSize(w, h))
-        setSerialNumber(++qt_vg_pixmap_serial);
-    else
-        resize(image.width(), image.height());
-
     QImage::Format format = sourceFormat();
-    int d = image.depth();
-    if (d == 1 || d == 16 || d == 24 || (d == 32 && !image.hasAlphaChannel()))
+    int d = image->depth();
+    if (d == 1 || d == 16 || d == 24 || (d == 32 && !image->hasAlphaChannel()))
         format = QImage::Format_RGB32;
-    else if (!(flags & Qt::NoOpaqueDetection) && const_cast<QImage &>(image).data_ptr()->checkForAlphaPixels())
+    else if (!(flags & Qt::NoOpaqueDetection) && image->data_ptr()->checkForAlphaPixels())
         format = sourceFormat();
     else
-        format = image.hasAlphaChannel() ? sourceFormat() : QImage::Format_RGB32;
+        format = image->hasAlphaChannel() ? sourceFormat() : QImage::Format_RGB32;
+    return format;
+}
+
+void QVGPixmapData::createPixmapForImage(QImage &image, Qt::ImageConversionFlags flags, bool inPlace)
+{
+    resize(image.width(), image.height());
+
+    QImage::Format format = idealFormat(&image, flags);
 
     if (inPlace && image.data_ptr()->convertInPlace(format, flags)) {
-        source = image;
+        source = QVolatileImage(image);
     } else {
-        source = image.convertToFormat(format);
-
-        // convertToFormat won't detach the image if format stays the same.
-        if (image.format() == format)
-            source.detach();
+        QImage convertedImage = image.convertToFormat(format);
+        // convertToFormat won't detach the image if format stays the
+        // same. Detaching is needed to prevent issues with painting
+        // onto this QPixmap later on.
+        convertedImage.detach();
+        source = QVolatileImage(convertedImage);
     }
 
     recreate = true;
@@ -238,12 +245,13 @@ void QVGPixmapData::fill(const QColor &color)
         return;
 
     if (source.isNull())
-        source = QImage(w, h, sourceFormat());
+        source = QVolatileImage(w, h, sourceFormat());
 
     if (source.depth() == 1) {
         // Pick the best approximate color in the image's colortable.
         int gray = qGray(color.rgba());
-        if (qAbs(qGray(source.color(0)) - gray) < qAbs(qGray(source.color(1)) - gray))
+        if (qAbs(qGray(source.imageRef().color(0)) - gray)
+            < qAbs(qGray(source.imageRef().color(1)) - gray))
             source.fill(0);
         else
             source.fill(1);
@@ -266,7 +274,7 @@ bool QVGPixmapData::hasAlphaChannel() const
 void QVGPixmapData::setAlphaChannel(const QPixmap &alphaChannel)
 {
     forceToImage();
-    source.setAlphaChannel(alphaChannel.toImage());
+    source.setAlphaChannel(alphaChannel);
 }
 
 QImage QVGPixmapData::toImage() const
@@ -275,17 +283,41 @@ QImage QVGPixmapData::toImage() const
         return QImage();
 
     if (source.isNull()) {
-        source = QImage(w, h, sourceFormat());
+        source = QVolatileImage(w, h, sourceFormat());
         recreate = true;
     }
 
-    return source;
+    return source.toImage();
+}
+
+void QVGPixmapData::copy(const QPixmapData *data, const QRect &rect)
+{
+    // toImage() is potentially expensive with QVolatileImage so provide a
+    // more efficient implementation of copy() that does not rely on it.
+    if (!data) {
+        return;
+    }
+    if (data->classId() != OpenVGClass) {
+        fromImage(data->toImage(rect), Qt::NoOpaqueDetection);
+        return;
+    }
+    const QVGPixmapData *pd = static_cast<const QVGPixmapData *>(data);
+    QRect r = rect;
+    if (r.isNull() || r.contains(QRect(0, 0, pd->w, pd->h))) {
+        r = QRect(0, 0, pd->w, pd->h);
+    }
+    resize(r.width(), r.height());
+    recreate = true;
+    if (!pd->source.isNull()) {
+        source = QVolatileImage(r.width(), r.height(), pd->source.format());
+        source.copyFrom(&pd->source, r);
+    }
 }
 
 QImage *QVGPixmapData::buffer()
 {
-    forceToImage();
-    return &source;
+    // Cannot be safely implemented and QVGPixmapData is not (must not be) RasterClass anyway.
+    return 0;
 }
 
 QPaintEngine* QVGPixmapData::paintEngine() const
@@ -329,10 +361,12 @@ VGImage QVGPixmapData::toVGImage()
     }
 
     if (!source.isNull() && recreate) {
+        source.beginDataAccess();
         vgImageSubData
             (vgImage,
              source.constBits(), source.bytesPerLine(),
              qt_vg_image_to_vg_format(source.format()), 0, 0, w, h);
+        source.endDataAccess(true);
     }
 
     recreate = false;
@@ -442,14 +476,14 @@ int QVGPixmapData::metric(QPaintDevice::PaintDeviceMetric metric) const
     }
 }
 
-// Force the pixmap data to be in QImage format.
+// Force the pixmap data to be backed by some valid data.
 void QVGPixmapData::forceToImage()
 {
     if (!isValid())
         return;
 
     if (source.isNull())
-        source = QImage(w, h, sourceFormat());
+        source = QVolatileImage(w, h, sourceFormat());
 
     recreate = true;
 }
diff --git a/src/openvg/qpixmapdata_vg_p.h b/src/openvg/qpixmapdata_vg_p.h
index 58e69db..c4fd47f 100644
--- a/src/openvg/qpixmapdata_vg_p.h
+++ b/src/openvg/qpixmapdata_vg_p.h
@@ -54,6 +54,7 @@
 //
 
 #include <QtGui/private/qpixmap_raster_p.h>
+#include <QtGui/private/qvolatileimage_p.h>
 #include <private/qvg_p.h>
 
 #if defined(Q_OS_SYMBIAN)
@@ -99,6 +100,7 @@ public:
     bool hasAlphaChannel() const;
     void setAlphaChannel(const QPixmap &alphaChannel);
     QImage toImage() const;
+    void copy(const QPixmapData *data, const QRect &rect);
     QImage *buffer();
     QPaintEngine* paintEngine() const;
 
@@ -161,7 +163,7 @@ protected:
     VGImage vgImage;
     VGImage vgImageOpacity;
     qreal cachedOpacity;
-    mutable QImage source;
+    mutable QVolatileImage source;
     mutable bool recreate;
     bool inImagePool;
 #if !defined(QT_NO_EGL)
@@ -170,6 +172,8 @@ protected:
 
     void forceToImage();
     QImage::Format sourceFormat() const;
+    QImage::Format idealFormat(QImage *image, Qt::ImageConversionFlags flags) const;
+    void updateSerial();
 
     void destroyImageAndContext();
     void destroyImages();
diff --git a/src/openvg/qvg_symbian.cpp b/src/openvg/qvg_symbian.cpp
index 2ea38c1..22cbb3c 100644
--- a/src/openvg/qvg_symbian.cpp
+++ b/src/openvg/qvg_symbian.cpp
@@ -44,7 +44,6 @@
 #include <private/qt_s60_p.h>
 
 #include <fbs.h>
-#include <bitdev.h>
 
 #ifdef QT_SYMBIAN_SUPPORTS_SGIMAGE
 #  include <sgresource/sgimage.h>
@@ -75,33 +74,6 @@ VGImage QVG::vgCreateEGLImageTargetKHR(VGeglImageKHR eglImage)
 
 extern int qt_vg_pixmap_serial;
 
-static CFbsBitmap* createBlitCopy(CFbsBitmap* bitmap)
-{
-    CFbsBitmap *copy = q_check_ptr(new CFbsBitmap);
-    if(!copy)
-        return 0;
-
-    if (copy->Create(bitmap->SizeInPixels(), bitmap->DisplayMode()) != KErrNone) {
-        delete copy;
-        copy = 0;
-
-        return 0;
-    }
-
-    CFbsBitmapDevice* bitmapDevice = 0;
-    CFbsBitGc *bitmapGc = 0;
-    QT_TRAP_THROWING(bitmapDevice = CFbsBitmapDevice::NewL(copy));
-    QT_TRAP_THROWING(bitmapGc = CFbsBitGc::NewL());
-    bitmapGc->Activate(bitmapDevice);
-
-    bitmapGc->BitBlt(TPoint(), bitmap);
-
-    delete bitmapGc;
-    delete bitmapDevice;
-
-    return copy;
-}
-
 #ifdef QT_SYMBIAN_SUPPORTS_SGIMAGE
 static VGImage sgImageToVGImage(QEglContext *context, const RSgImage &sgImage)
 {
@@ -136,7 +108,7 @@ void QVGPixmapData::cleanup()
 {
     is_null = w = h = 0;
     recreate = false;
-    source = QImage();
+    source = QVolatileImage();
 }
 
 void QVGPixmapData::fromNativeType(void* pixmap, NativeType type)
@@ -155,55 +127,28 @@ void QVGPixmapData::fromNativeType(void* pixmap, NativeType type)
         }
 
         is_null = (w <= 0 || h <= 0);
-        source = QImage(); // vgGetImageSubData() some day?
+        source = QVolatileImage(); // vgGetImageSubData() some day?
         recreate = false;
         prevSize = QSize(w, h);
-        //setSerialNumber(++qt_vg_pixmap_serial);
+        updateSerial();
 #endif
-    } else if (type == QPixmapData::FbsBitmap) {
-        CFbsBitmap *bitmap = reinterpret_cast<CFbsBitmap*>(pixmap);
-
-        bool deleteSourceBitmap = false;
-#ifdef Q_SYMBIAN_HAS_EXTENDED_BITMAP_TYPE
-
-        // Rasterize extended bitmaps
-
-        TUid extendedBitmapType = bitmap->ExtendedBitmapType();
-        if (extendedBitmapType != KNullUid) {
-            bitmap = createBlitCopy(bitmap);
-            deleteSourceBitmap = true;
-        }
-#endif
-
-        if (bitmap->IsCompressedInRAM()) {
-            bitmap = createBlitCopy(bitmap);
-            deleteSourceBitmap = true;
-        }
-
-        TDisplayMode displayMode = bitmap->DisplayMode();
-        QImage::Format format = qt_TDisplayMode2Format(displayMode);
-
-        TSize size = bitmap->SizeInPixels();
-        int bytesPerLine = bitmap->ScanLineLength(size.iWidth, displayMode);
-
-        bitmap->BeginDataAccess();
-        uchar *bytes = (uchar*)bitmap->DataAddress();
-        QImage img = QImage(bytes, size.iWidth, size.iHeight, bytesPerLine, format);
-        img = img.copy();
-        bitmap->EndDataAccess();
-
-        if(displayMode == EGray2) {
-            //Symbian thinks set pixels are white/transparent, Qt thinks they are foreground/solid
-            //So invert mono bitmaps so that masks work correctly.
-            img.invertPixels();
-        } else if(displayMode == EColor16M) {
-            img = img.rgbSwapped(); // EColor16M is BGR
-        }
-
-        fromImage(img, Qt::AutoColor);
-
-        if(deleteSourceBitmap)
-            delete bitmap;
+    } else if (type == QPixmapData::FbsBitmap && pixmap) {
+        CFbsBitmap *bitmap = reinterpret_cast<CFbsBitmap *>(pixmap);
+        QSize size(bitmap->SizeInPixels().iWidth, bitmap->SizeInPixels().iHeight);
+        resize(size.width(), size.height());
+        source = QVolatileImage(bitmap); // duplicates only, if possible
+        if (source.isNull())
+            return;
+        // Here we may need to copy if the formats do not match.
+        // (e.g. for display modes other than EColor16MAP and EColor16MU)
+        source.ensureFormat(idealFormat(&source.imageRef(), Qt::AutoColor));
+        recreate = true;
+    } else if (type == QPixmapData::VolatileImage && pixmap) {
+        QVolatileImage *img = static_cast<QVolatileImage *>(pixmap);
+        resize(img->width(), img->height());
+        source = *img;
+        source.ensureFormat(idealFormat(&source.imageRef(), Qt::AutoColor));
+        recreate = true;
     }
 }
 
@@ -270,26 +215,14 @@ void* QVGPixmapData::toNativeType(NativeType type)
         driver.Close();
         return reinterpret_cast<void*>(sgImage.take());
 #endif
-    } else if (type == QPixmapData::FbsBitmap) {
-                CFbsBitmap *bitmap = q_check_ptr(new CFbsBitmap);
-
-        if (bitmap) {
-            if (bitmap->Create(TSize(source.width(), source.height()),
-                              EColor16MAP) == KErrNone) {
-                const uchar *sptr = const_cast<const QImage&>(source).bits();
-                bitmap->BeginDataAccess();
-
-                uchar *dptr = (uchar*)bitmap->DataAddress();
-                Mem::Copy(dptr, sptr, source.byteCount());
-
-                bitmap->EndDataAccess();
-            } else {
-                delete bitmap;
-                bitmap = 0;
-            }
+    } else if (type == QPixmapData::FbsBitmap && isValid()) {
+        if (source.isNull()) {
+            source = QVolatileImage(w, h, sourceFormat());
         }
-
-        return reinterpret_cast<void*>(bitmap);
+        // Just duplicate the bitmap handle, no data copying happens.
+        return source.duplicateNativeImage();
+    } else if (type == QPixmapData::VolatileImage) {
+        return &source;
     }
     return 0;
 }