Made QPixmap autotest pass with -graphicssystem opengl

The window surface has been modified to track widget deletion to make
sure it doesn't try to access the widget's context data after deletion.

QGLPixmapData now also uses GL_RGB instead of GL_RGBA when appropriate,
and hasAlphaChannel() has been modified in view of this.

A number of other issues have been fixed in QGLPixmapData, and the
autotest has been modified to use a more lenient pixmap compare function
due to off-by-one pixel errors here and there.

Reviewed-by: Trond

4 files changed, 82 insertions, 15 deletions

diff --git a/src/opengl/qpixmapdata_gl.cpp b/src/opengl/qpixmapdata_gl.cpp
index 98c406b..4c53c46 100644
--- a/src/opengl/qpixmapdata_gl.cpp
+++ b/src/opengl/qpixmapdata_gl.cpp
@@ -104,6 +104,7 @@ QGLPixmapData::QGLPixmapData(PixelType type)
     , m_ctx(0)
     , m_dirty(false)
     , m_hasFillColor(false)
+    , m_hasAlpha(false)
 {
     setSerialNumber(++qt_gl_pixmap_serial);
 }
@@ -136,6 +137,11 @@ void QGLPixmapData::resize(int width, int height)
     if (width == m_width && height == m_height)
         return;
 
+    if (width <= 0 || height <= 0) {
+        width = 0;
+        height = 0;
+    }
+
     m_width = width;
     m_height = height;
 
@@ -166,7 +172,8 @@ void QGLPixmapData::ensureCreated() const
     if (!m_textureId) {
         glGenTextures(1, &m_textureId);
         glBindTexture(target, m_textureId);
-        glTexImage2D(target, 0, GL_RGBA, m_width, m_height, 0,
+        GLenum format = m_hasAlpha ? GL_RGBA : GL_RGB;
+        glTexImage2D(target, 0, format, m_width, m_height, 0,
                 GL_RGBA, GL_UNSIGNED_BYTE, 0);
         glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
         glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
@@ -195,10 +202,20 @@ void QGLPixmapData::fromImage(const QImage &image,
     if (image.size() == QSize(m_width, m_height))
         setSerialNumber(++qt_gl_pixmap_serial);
     resize(image.width(), image.height());
-    m_source = image;
+
+    if (pixelType() == BitmapType) {
+        m_source = image.convertToFormat(QImage::Format_MonoLSB);
+    } else {
+        m_source = image.hasAlphaChannel()
+            ? image.convertToFormat(QImage::Format_ARGB32_Premultiplied)
+            : image.convertToFormat(QImage::Format_RGB32);
+    }
+
     m_dirty = true;
     m_hasFillColor = false;
 
+    m_hasAlpha = image.hasAlphaChannel();
+
     if (m_textureId) {
         QGLShareContextScope ctx(qt_gl_share_widget()->context());
         glDeleteTextures(1, &m_textureId);
@@ -230,24 +247,46 @@ void QGLPixmapData::fill(const QColor &color)
     if (!isValid())
         return;
 
+    if (!m_textureId)
+        m_hasAlpha = color.alpha() != 255;
+
     if (useFramebufferObjects()) {
         m_source = QImage();
         m_hasFillColor = true;
         m_fillColor = color;
-    } else if (!m_source.isNull()) {
-        m_source.fill(PREMUL(color.rgba()));
     } else {
-        // ## TODO: improve performance here
-        QImage img(m_width, m_height, QImage::Format_ARGB32_Premultiplied);
-        img.fill(PREMUL(color.rgba()));
-
-        fromImage(img, 0);
+        QImage image = fillImage(color);
+        fromImage(image, 0);
     }
 }
 
 bool QGLPixmapData::hasAlphaChannel() const
 {
-    return true;
+    return pixelType() == BitmapType || m_hasAlpha;
+}
+
+QImage QGLPixmapData::fillImage(const QColor &color) const
+{
+    QImage img;
+    if (pixelType() == BitmapType) {
+        img = QImage(m_width, m_height, QImage::Format_MonoLSB);
+        img.setNumColors(2);
+        img.setColor(0, QColor(Qt::color0).rgba());
+        img.setColor(1, QColor(Qt::color1).rgba());
+
+        int gray = qGray(color.rgba());
+        if (qAbs(255 - gray) < gray)
+            img.fill(0);
+        else
+            img.fill(1);
+    } else {
+        img = QImage(m_width, m_height,
+                m_hasAlpha
+                ? QImage::Format_ARGB32_Premultiplied
+                : QImage::Format_RGB32);
+        img.fill(PREMUL(color.rgba()));
+    }
+    return img;
 }
 
 QImage QGLPixmapData::toImage() const
@@ -260,10 +299,7 @@ QImage QGLPixmapData::toImage() const
     } else if (!m_source.isNull()) {
         return m_source;
     } else if (m_dirty || m_hasFillColor) {
-        QImage img(m_width, m_height, QImage::Format_ARGB32_Premultiplied);
-        if (m_hasFillColor)
-            img.fill(PREMUL(m_fillColor.rgba()));
-        return img;
+        return fillImage(m_fillColor);
     } else {
         ensureCreated();
     }
@@ -453,6 +489,9 @@ extern int qt_defaultDpiY();
 
 int QGLPixmapData::metric(QPaintDevice::PaintDeviceMetric metric) const
 {
+    if (m_width == 0)
+        return 0;
+
     switch (metric) {
     case QPaintDevice::PdmWidth:
         return m_width;
diff --git a/src/opengl/qpixmapdata_gl_p.h b/src/opengl/qpixmapdata_gl_p.h
index 1b6b7ae..af10f2c 100644
--- a/src/opengl/qpixmapdata_gl_p.h
+++ b/src/opengl/qpixmapdata_gl_p.h
@@ -115,6 +115,8 @@ private:
 
     static bool useFramebufferObjects();
 
+    QImage fillImage(const QColor &color) const;
+
     int m_width;
     int m_height;
 
@@ -131,6 +133,8 @@ private:
     // represented by a single fill color
     mutable QColor m_fillColor;
     mutable bool m_hasFillColor;
+
+    mutable bool m_hasAlpha;
 };
 
 QT_END_NAMESPACE
diff --git a/src/opengl/qwindowsurface_gl.cpp b/src/opengl/qwindowsurface_gl.cpp
index 965c7a5..01974a2 100644
--- a/src/opengl/qwindowsurface_gl.cpp
+++ b/src/opengl/qwindowsurface_gl.cpp
@@ -272,6 +272,24 @@ QGLWindowSurface::~QGLWindowSurface()
     delete d_ptr;
 }
 
+void QGLWindowSurface::deleted(QObject *object)
+{
+    QWidget *widget = qobject_cast<QWidget *>(object);
+    if (widget) {
+        QWidgetPrivate *widgetPrivate = widget->d_func();
+        if (widgetPrivate->extraData()) {
+            union { QGLContext **ctxPtr; void **voidPtr; };
+            voidPtr = &widgetPrivate->extraData()->glContext;
+            int index = d_ptr->contexts.indexOf(ctxPtr);
+            if (index != -1) {
+                delete *ctxPtr;
+                *ctxPtr = 0;
+                d_ptr->contexts.removeAt(index);
+            }
+        }
+    }
+}
+
 void QGLWindowSurface::hijackWindow(QWidget *widget)
 {
     QWidgetPrivate *widgetPrivate = widget->d_func();
@@ -288,6 +306,8 @@ void QGLWindowSurface::hijackWindow(QWidget *widget)
 
     union { QGLContext **ctxPtr; void **voidPtr; };
 
+    connect(widget, SIGNAL(destroyed(QObject *)), this, SLOT(deleted(QObject *)));
+
     voidPtr = &widgetPrivate->extraData()->glContext;
     d_ptr->contexts << ctxPtr;
     qDebug() << "hijackWindow() context created for" << widget << d_ptr->contexts.size();
diff --git a/src/opengl/qwindowsurface_gl_p.h b/src/opengl/qwindowsurface_gl_p.h
index d47e3e3..9efd1ae 100644
--- a/src/opengl/qwindowsurface_gl_p.h
+++ b/src/opengl/qwindowsurface_gl_p.h
@@ -65,8 +65,9 @@ class QRegion;
 class QWidget;
 struct QGLWindowSurfacePrivate;
 
-class QGLWindowSurface : public QWindowSurface, public QPaintDevice
+class QGLWindowSurface : public QObject, public QWindowSurface, public QPaintDevice
 {
+    Q_OBJECT
 public:
     QGLWindowSurface(QWidget *window);
     ~QGLWindowSurface();
@@ -91,6 +92,9 @@ public:
 protected:
     int metric(PaintDeviceMetric metric) const;
 
+private slots:
+    void deleted(QObject *object);
+
 private:
     void hijackWindow(QWidget *widget);