Image w/ PreserveAspectFit has its width changed once more than needed.

Avoid an extra setImplicitWidth/setImplicitHeight on image load.

Change-Id: I8bec1c97244068000c7a7f5fb3e937f80f3b36f5
Task-number: QTBUG-18573
Reviewed-by: Michael Brasser

4 files changed, 35 insertions, 11 deletions

diff --git a/src/declarative/graphicsitems/qdeclarativeimage.cpp b/src/declarative/graphicsitems/qdeclarativeimage.cpp
index a62d3d2..b776532 100644
--- a/src/declarative/graphicsitems/qdeclarativeimage.cpp
+++ b/src/declarative/graphicsitems/qdeclarativeimage.cpp
@@ -136,12 +136,10 @@ void QDeclarativeImagePrivate::setPixmap(const QPixmap &pixmap)
     Q_Q(QDeclarativeImage);
     pix.setPixmap(pixmap);
 
-    q->setImplicitWidth(pix.width());
-    q->setImplicitHeight(pix.height());
+    q->pixmapChange();
     status = pix.isNull() ? QDeclarativeImageBase::Null : QDeclarativeImageBase::Ready;
 
     q->update();
-    q->pixmapChange();
 }
 
 /*!
@@ -390,8 +388,11 @@ void QDeclarativeImage::updatePaintedGeometry()
     Q_D(QDeclarativeImage);
 
     if (d->fillMode == PreserveAspectFit) {
-        if (!d->pix.width() || !d->pix.height())
+        if (!d->pix.width() || !d->pix.height()) {
+            setImplicitWidth(0);
+            setImplicitHeight(0);
             return;
+        }
         qreal w = widthValid() ? width() : d->pix.width();
         qreal widthScale = w / qreal(d->pix.width());
         qreal h = heightValid() ? height() : d->pix.height();
@@ -405,9 +406,13 @@ void QDeclarativeImage::updatePaintedGeometry()
         }
         if (widthValid() && !heightValid()) {
             setImplicitHeight(d->paintedHeight);
+        } else {
+            setImplicitHeight(d->pix.height());
         }
         if (heightValid() && !widthValid()) {
             setImplicitWidth(d->paintedWidth);
+        } else {
+            setImplicitWidth(d->pix.width());
         }
     } else if (d->fillMode == PreserveAspectCrop) {
         if (!d->pix.width() || !d->pix.height())
@@ -566,6 +571,13 @@ void QDeclarativeImage::paint(QPainter *p, const QStyleOptionGraphicsItem *, QWi
 
 void QDeclarativeImage::pixmapChange()
 {
+    Q_D(QDeclarativeImage);
+    // PreserveAspectFit calculates the implicit size differently so we
+    // don't call our superclass pixmapChange(), since that would
+    // result in the implicit size being set incorrectly, then updated
+    // in updatePaintedGeometry()
+    if (d->fillMode != PreserveAspectFit)
+        QDeclarativeImageBase::pixmapChange();
     updatePaintedGeometry();
 }
 
diff --git a/src/declarative/graphicsitems/qdeclarativeimagebase.cpp b/src/declarative/graphicsitems/qdeclarativeimagebase.cpp
index daebac4..81eac78 100644
--- a/src/declarative/graphicsitems/qdeclarativeimagebase.cpp
+++ b/src/declarative/graphicsitems/qdeclarativeimagebase.cpp
@@ -191,11 +191,9 @@ void QDeclarativeImageBase::load()
         d->pix.clear(this);
         d->status = Null;
         d->progress = 0.0;
-        setImplicitWidth(0);
-        setImplicitHeight(0);
+        pixmapChange();
         emit progressChanged(d->progress);
         emit statusChanged(d->status);
-        pixmapChange();
         update();
     } else {
         QDeclarativePixmap::Options options;
@@ -246,8 +244,7 @@ void QDeclarativeImageBase::requestFinished()
 
     d->progress = 1.0;
 
-    setImplicitWidth(d->pix.width());
-    setImplicitHeight(d->pix.height());
+    pixmapChange();
 
     if (d->sourcesize.width() != d->pix.width() || d->sourcesize.height() != d->pix.height())
         emit sourceSizeChanged();
@@ -256,7 +253,7 @@ void QDeclarativeImageBase::requestFinished()
         emit statusChanged(d->status);
     if (d->progress != oldProgress)
         emit progressChanged(d->progress);
-    pixmapChange();
+
     update();
 }
 
@@ -279,6 +276,9 @@ void QDeclarativeImageBase::componentComplete()
 
 void QDeclarativeImageBase::pixmapChange()
 {
+    Q_D(QDeclarativeImageBase);
+    setImplicitWidth(d->pix.width());
+    setImplicitHeight(d->pix.height());
 }
 
 QT_END_NAMESPACE
diff --git a/tests/auto/declarative/qdeclarativeimage/data/aspectratio.qml b/tests/auto/declarative/qdeclarativeimage/data/aspectratio.qml
index 739299d..cd092bc 100644
--- a/tests/auto/declarative/qdeclarativeimage/data/aspectratio.qml
+++ b/tests/auto/declarative/qdeclarativeimage/data/aspectratio.qml
@@ -1,6 +1,10 @@
 import QtQuick 1.0
 
 Image { 
+    property int widthChange: 0
+    property int heightChange: 0
     source: "heart.png"
     fillMode: Image.PreserveAspectFit;
+    onWidthChanged: widthChange += 1
+    onHeightChanged: heightChange += 1
 }
diff --git a/tests/auto/declarative/qdeclarativeimage/tst_qdeclarativeimage.cpp b/tests/auto/declarative/qdeclarativeimage/tst_qdeclarativeimage.cpp
index c5a3d14..87e3347 100644
--- a/tests/auto/declarative/qdeclarativeimage/tst_qdeclarativeimage.cpp
+++ b/tests/auto/declarative/qdeclarativeimage/tst_qdeclarativeimage.cpp
@@ -248,14 +248,22 @@ void tst_qdeclarativeimage::preserveAspectRatio()
     canvas->setSource(QUrl::fromLocalFile(SRCDIR "/data/aspectratio.qml"));
     QDeclarativeImage *image = qobject_cast<QDeclarativeImage*>(canvas->rootObject());
     QVERIFY(image != 0);
+    QCOMPARE(image->property("widthChange").toInt(), 1);
+    QCOMPARE(image->property("heightChange").toInt(), 1);
     image->setWidth(80.0);
+    QCOMPARE(image->property("widthChange").toInt(), 2);
+    QCOMPARE(image->property("heightChange").toInt(), 2);
     QCOMPARE(image->width(), 80.);
     QCOMPARE(image->height(), 80.);
 
     canvas->setSource(QUrl::fromLocalFile(SRCDIR "/data/aspectratio.qml"));
     image = qobject_cast<QDeclarativeImage*>(canvas->rootObject());
-    image->setHeight(60.0);
     QVERIFY(image != 0);
+    QCOMPARE(image->property("widthChange").toInt(), 1);
+    QCOMPARE(image->property("heightChange").toInt(), 1);
+    image->setHeight(60.0);
+    QCOMPARE(image->property("widthChange").toInt(), 2);
+    QCOMPARE(image->property("heightChange").toInt(), 2);
     QCOMPARE(image->height(), 60.);
     QCOMPARE(image->width(), 60.);
     delete canvas;