Avoid creating a QPixmap on QBrush comparisons

We shouldn't create QPixmaps when comparing QBrushes that do not
contain a QPixmap.

This patch extends the comparison logic to comparing QImage cachekeys
if the brushes are QImage based.

Note the comparison still produces false negatives on equal content on
different pixmaps and images, but this is preserving existing behavior.

Task-number: QTBUG-43766
Change-Id: I001b4032172c1e568aad311f7df2eaae6aee8dc6
Reviewed-by: Gunnar Sletta <gunnar@sletta.org>
This commit is contained in:
Allan Sandfeld Jensen 2015-01-28 15:52:30 +01:00
parent a29374fc8e
commit 57469a8e10
3 changed files with 49 additions and 4 deletions

View File

@ -35,6 +35,7 @@
#include "qpixmap.h"
#include "qbitmap.h"
#include "qpixmapcache.h"
#include "qplatformpixmap.h"
#include "qdatastream.h"
#include "qvariant.h"
#include "qline.h"
@ -950,9 +951,34 @@ bool QBrush::operator==(const QBrush &b) const
switch (d->style) {
case Qt::TexturePattern:
{
const QPixmap &us = (static_cast<QTexturedBrushData *>(d.data()))->pixmap();
const QPixmap &them = (static_cast<QTexturedBrushData *>(b.d.data()))->pixmap();
return ((us.isNull() && them.isNull()) || us.cacheKey() == them.cacheKey());
// Note this produces false negatives if the textures have identical data,
// but does not share the same data in memory. Since equality is likely to
// be used to avoid iterating over the data for a texture update, this should
// still be better than doing an accurate comparison.
const QPixmap *us = 0, *them = 0;
qint64 cacheKey1, cacheKey2;
if (qHasPixmapTexture(*this)) {
us = (static_cast<QTexturedBrushData *>(d.data()))->m_pixmap;
cacheKey1 = us->cacheKey();
} else
cacheKey1 = (static_cast<QTexturedBrushData *>(d.data()))->image().cacheKey();
if (qHasPixmapTexture(b)) {
them = (static_cast<QTexturedBrushData *>(b.d.data()))->m_pixmap;
cacheKey2 = them->cacheKey();
} else
cacheKey2 = (static_cast<QTexturedBrushData *>(b.d.data()))->image().cacheKey();
if (cacheKey1 != cacheKey2)
return false;
if (!us == !them) // both images or both pixmaps
return true;
// Only raster QPixmaps use the same cachekeys as QImages.
if (us && us->handle()->classId() == QPlatformPixmap::RasterClass)
return true;
if (them && them->handle()->classId() == QPlatformPixmap::RasterClass)
return true;
return false;
}
case Qt::LinearGradientPattern:
case Qt::RadialGradientPattern:

View File

@ -1,5 +1,5 @@
CONFIG += testcase
CONFIG += parallel_test
TARGET = tst_qbrush
QT += testlib
QT += testlib gui-private
SOURCES += tst_qbrush.cpp

View File

@ -37,6 +37,7 @@
#include "qbrush.h"
#include <QPainter>
#include <QBitmap>
#include <private/qpixmap_raster_p.h>
#include <qdebug.h>
@ -71,6 +72,7 @@ private slots:
void debug();
void textureBrushStream();
void textureBrushComparison();
};
@ -446,5 +448,22 @@ void tst_QBrush::textureBrushStream()
QCOMPARE(loadedBrush2.textureImage(), image_source);
}
void tst_QBrush::textureBrushComparison()
{
QImage image1(10, 10, QImage::Format_RGB32);
QRasterPlatformPixmap* ppixmap = new QRasterPlatformPixmap(QPlatformPixmap::PixmapType);
ppixmap->fromImage(image1, Qt::NoFormatConversion);
QPixmap pixmap(ppixmap);
QImage image2(image1);
QBrush pixmapBrush, imageBrush1, imageBrush2;
pixmapBrush.setTexture(pixmap);
imageBrush1.setTextureImage(image1);
imageBrush2.setTextureImage(image2);
QVERIFY(imageBrush1 == imageBrush2);
QVERIFY(pixmapBrush == imageBrush1);
}
QTEST_MAIN(tst_QBrush)
#include "tst_qbrush.moc"