QQuickAnimatedImagePrivate::setMovie: check for re-assignment; cleanup

There's no need to delete the movie if the same one is being re-assigned.
Also, reorder, rename and clean up the variables.  Followup to
bb02a577f0fbf3bd2a273129e6cb57cfc9a338c8

Task-number: QTBUG-62913
Change-Id: I1fc57b411bfbd819b88be20f5933db8c13e9aba0
Reviewed-by: Kevin Krammer <kevin.krammer@kdab.com>
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>

2 files changed, 64 insertions, 71 deletions

diff --git a/src/quick/items/qquickanimatedimage.cpp b/src/quick/items/qquickanimatedimage.cpp
index 5bc5b0faff..2f76671b1a 100644
--- a/src/quick/items/qquickanimatedimage.cpp
+++ b/src/quick/items/qquickanimatedimage.cpp
@@ -54,26 +54,26 @@ QT_BEGIN_NAMESPACE
 
 QQuickPixmap* QQuickAnimatedImagePrivate::infoForCurrentFrame(QQmlEngine *engine)
 {
-    if (!_movie)
+    if (!movie)
         return 0;
 
-    int current = _movie->currentFrameNumber();
+    int current = movie->currentFrameNumber();
     if (!frameMap.contains(current)) {
         QUrl requestedUrl;
         QQuickPixmap *pixmap = 0;
-        if (engine && !_movie->fileName().isEmpty()) {
+        if (engine && !movie->fileName().isEmpty()) {
             requestedUrl.setUrl(QString::fromUtf8("quickanimatedimage://%1#%2")
-                                .arg(_movie->fileName())
+                                .arg(movie->fileName())
                                 .arg(current));
         }
         if (!requestedUrl.isEmpty()) {
             if (QQuickPixmap::isCached(requestedUrl, QSize(), QQuickImageProviderOptions()))
                 pixmap = new QQuickPixmap(engine, requestedUrl);
             else
-                pixmap = new QQuickPixmap(requestedUrl, _movie->currentImage());
+                pixmap = new QQuickPixmap(requestedUrl, movie->currentImage());
         } else {
             pixmap = new QQuickPixmap;
-            pixmap->setImage(_movie->currentImage());
+            pixmap->setImage(movie->currentImage());
         }
         frameMap.insert(current, pixmap);
     }
@@ -138,7 +138,7 @@ QQuickPixmap* QQuickAnimatedImagePrivate::infoForCurrentFrame(QQmlEngine *engine
 QQuickAnimatedImage::QQuickAnimatedImage(QQuickItem *parent)
     : QQuickImage(*(new QQuickAnimatedImagePrivate), parent)
 {
-    QObject::connect(this, &QQuickImageBase::cacheChanged, this, &QQuickAnimatedImage::onCacheChanged);
+    connect(this, &QQuickImageBase::cacheChanged, this, &QQuickAnimatedImage::onCacheChanged);
 }
 
 QQuickAnimatedImage::~QQuickAnimatedImage()
@@ -148,7 +148,7 @@ QQuickAnimatedImage::~QQuickAnimatedImage()
     if (d->reply)
         d->reply->deleteLater();
 #endif
-    delete d->_movie;
+    delete d->movie;
     qDeleteAll(d->frameMap);
     d->frameMap.clear();
 }
@@ -164,9 +164,9 @@ QQuickAnimatedImage::~QQuickAnimatedImage()
 bool QQuickAnimatedImage::isPaused() const
 {
     Q_D(const QQuickAnimatedImage);
-    if (!d->_movie)
+    if (!d->movie)
         return d->paused;
-    return d->_movie->state()==QMovie::Paused;
+    return d->movie->state()==QMovie::Paused;
 }
 
 void QQuickAnimatedImage::setPaused(bool pause)
@@ -174,11 +174,11 @@ void QQuickAnimatedImage::setPaused(bool pause)
     Q_D(QQuickAnimatedImage);
     if (pause == d->paused)
         return;
-    if (!d->_movie) {
+    if (!d->movie) {
         d->paused = pause;
         emit pausedChanged();
     } else {
-        d->_movie->setPaused(pause);
+        d->movie->setPaused(pause);
     }
 }
 
@@ -203,9 +203,9 @@ void QQuickAnimatedImage::setPaused(bool pause)
 bool QQuickAnimatedImage::isPlaying() const
 {
     Q_D(const QQuickAnimatedImage);
-    if (!d->_movie)
+    if (!d->movie)
         return d->playing;
-    return d->_movie->state()!=QMovie::NotRunning;
+    return d->movie->state()!=QMovie::NotRunning;
 }
 
 void QQuickAnimatedImage::setPlaying(bool play)
@@ -213,15 +213,15 @@ void QQuickAnimatedImage::setPlaying(bool play)
     Q_D(QQuickAnimatedImage);
     if (play == d->playing)
         return;
-    if (!d->_movie) {
+    if (!d->movie) {
         d->playing = play;
         emit playingChanged();
         return;
     }
     if (play)
-        d->_movie->start();
+        d->movie->start();
     else
-        d->_movie->stop();
+        d->movie->stop();
 }
 
 /*!
@@ -237,27 +237,27 @@ void QQuickAnimatedImage::setPlaying(bool play)
 int QQuickAnimatedImage::currentFrame() const
 {
     Q_D(const QQuickAnimatedImage);
-    if (!d->_movie)
-        return d->preset_currentframe;
-    return d->_movie->currentFrameNumber();
+    if (!d->movie)
+        return d->presetCurrentFrame;
+    return d->movie->currentFrameNumber();
 }
 
 void QQuickAnimatedImage::setCurrentFrame(int frame)
 {
     Q_D(QQuickAnimatedImage);
-    if (!d->_movie) {
-        d->preset_currentframe = frame;
+    if (!d->movie) {
+        d->presetCurrentFrame = frame;
         return;
     }
-    d->_movie->jumpToFrame(frame);
+    d->movie->jumpToFrame(frame);
 }
 
 int QQuickAnimatedImage::frameCount() const
 {
     Q_D(const QQuickAnimatedImage);
-    if (!d->_movie)
+    if (!d->movie)
         return 0;
-    return d->_movie->frameCount();
+    return d->movie->frameCount();
 }
 
 void QQuickAnimatedImage::setSource(const QUrl &url)
@@ -278,10 +278,7 @@ void QQuickAnimatedImage::setSource(const QUrl &url)
     d->frameMap.clear();
 
     d->oldPlaying = isPlaying();
-    if (d->_movie) {
-        d->setMovie(nullptr);
-    }
-
+    d->setMovie(nullptr);
     d->url = url;
     emit sourceChanged(d->url);
 
@@ -335,10 +332,8 @@ void QQuickAnimatedImage::load()
             req.setAttribute(QNetworkRequest::HttpPipeliningAllowedAttribute, true);
 
             d->reply = qmlEngine(this)->networkAccessManager()->get(req);
-            QObject::connect(d->reply, SIGNAL(finished()),
-                            this, SLOT(movieRequestFinished()));
-            QObject::connect(d->reply, SIGNAL(downloadProgress(qint64,qint64)),
-                            this, SLOT(requestProgress(qint64,qint64)));
+            connect(d->reply, &QNetworkReply::finished, this, &QQuickAnimatedImage::movieRequestFinished);
+            connect(d->reply, SIGNAL(downloadProgress(qint64,qint64)), this, SLOT(requestProgress(qint64,qint64)));
 #endif
         }
     }
@@ -369,7 +364,7 @@ void QQuickAnimatedImage::movieRequestFinished()
     }
 #endif
 
-    if (!d->_movie || !d->_movie->isValid()) {
+    if (!d->movie || !d->movie->isValid()) {
         qmlWarning(this) << "Error Reading Animated Image File " << d->url.toString();
         d->setMovie(nullptr);
         d->setImage(QImage());
@@ -390,12 +385,10 @@ void QQuickAnimatedImage::movieRequestFinished()
         return;
     }
 
-    connect(d->_movie, SIGNAL(stateChanged(QMovie::MovieState)),
-            this, SLOT(playingStatusChanged()));
-    connect(d->_movie, SIGNAL(frameChanged(int)),
-            this, SLOT(movieUpdate()));
+    connect(d->movie, &QMovie::stateChanged, this, &QQuickAnimatedImage::playingStatusChanged);
+    connect(d->movie, &QMovie::frameChanged, this, &QQuickAnimatedImage::movieUpdate);
     if (d->cache)
-        d->_movie->setCacheMode(QMovie::CacheAll);
+        d->movie->setCacheMode(QMovie::CacheAll);
 
     d->status = Ready;
     emit statusChanged(d->status);
@@ -406,22 +399,21 @@ void QQuickAnimatedImage::movieRequestFinished()
     }
 
     bool pausedAtStart = d->paused;
-    if (d->playing) {
-        d->_movie->start();
-    }
+    if (d->playing)
+        d->movie->start();
     if (pausedAtStart)
-        d->_movie->setPaused(true);
+        d->movie->setPaused(true);
     if (d->paused || !d->playing) {
-        d->_movie->jumpToFrame(d->preset_currentframe);
-        d->preset_currentframe = 0;
+        d->movie->jumpToFrame(d->presetCurrentFrame);
+        d->presetCurrentFrame = 0;
     }
     d->setPixmap(*d->infoForCurrentFrame(qmlEngine(this)));
 
     if (isPlaying() != d->oldPlaying)
         emit playingChanged();
 
-    if (d->_movie)
-        d->currentSourceSize = d->_movie->currentPixmap().size();
+    if (d->movie)
+        d->currentSourceSize = d->movie->currentPixmap().size();
     else
         d->currentSourceSize = QSize(0, 0);
 
@@ -440,7 +432,7 @@ void QQuickAnimatedImage::movieUpdate()
         d->frameMap.clear();
     }
 
-    if (d->_movie) {
+    if (d->movie) {
         d->setPixmap(*d->infoForCurrentFrame(qmlEngine(this)));
         emit frameChanged();
     }
@@ -450,12 +442,12 @@ void QQuickAnimatedImage::playingStatusChanged()
 {
     Q_D(QQuickAnimatedImage);
 
-    if ((d->_movie->state() != QMovie::NotRunning) != d->playing) {
-        d->playing = (d->_movie->state() != QMovie::NotRunning);
+    if ((d->movie->state() != QMovie::NotRunning) != d->playing) {
+        d->playing = (d->movie->state() != QMovie::NotRunning);
         emit playingChanged();
     }
-    if ((d->_movie->state() == QMovie::Paused) != d->paused) {
-        d->paused = (d->_movie->state() == QMovie::Paused);
+    if ((d->movie->state() == QMovie::Paused) != d->paused) {
+        d->paused = (d->movie->state() == QMovie::Paused);
         emit pausedChanged();
     }
 }
@@ -466,13 +458,11 @@ void QQuickAnimatedImage::onCacheChanged()
     if (!cache()) {
         qDeleteAll(d->frameMap);
         d->frameMap.clear();
-        if (d->_movie) {
-            d->_movie->setCacheMode(QMovie::CacheNone);
-        }
+        if (d->movie)
+            d->movie->setCacheMode(QMovie::CacheNone);
     } else {
-        if (d->_movie) {
-            d->_movie->setCacheMode(QMovie::CacheAll);
-        }
+        if (d->movie)
+            d->movie->setCacheMode(QMovie::CacheAll);
     }
 }
 
@@ -488,13 +478,15 @@ void QQuickAnimatedImage::componentComplete()
     load();
 }
 
-void QQuickAnimatedImagePrivate::setMovie(QMovie *movie)
+void QQuickAnimatedImagePrivate::setMovie(QMovie *m)
 {
+    if (movie == m)
+        return;
     Q_Q(QQuickAnimatedImage);
     const int oldFrameCount = q->frameCount();
 
-    delete _movie;
-    _movie = movie;
+    delete movie;
+    movie = m;
 
     if (oldFrameCount != q->frameCount())
         emit q->frameCountChanged();
diff --git a/src/quick/items/qquickanimatedimage_p_p.h b/src/quick/items/qquickanimatedimage_p_p.h
index 68c4f2d359..85508459f9 100644
--- a/src/quick/items/qquickanimatedimage_p_p.h
+++ b/src/quick/items/qquickanimatedimage_p_p.h
@@ -70,28 +70,29 @@ class QQuickAnimatedImagePrivate : public QQuickImagePrivate
 
 public:
     QQuickAnimatedImagePrivate()
-      : playing(true), paused(false), preset_currentframe(0), _movie(0), oldPlaying(false)
+      : playing(true), paused(false), oldPlaying(false), presetCurrentFrame(0)
+      , currentSourceSize(0, 0), movie(nullptr)
 #if QT_CONFIG(qml_network)
-        , reply(0), redirectCount(0)
+      , reply(nullptr), redirectCount(0)
 #endif
-        , currentSourceSize(0, 0)
     {
     }
 
     QQuickPixmap *infoForCurrentFrame(QQmlEngine *engine);
+    void setMovie(QMovie *movie);
 
-    bool playing;
-    bool paused;
-    int preset_currentframe;
-    QMovie *_movie;
-    bool oldPlaying;
+    bool playing : 1;
+    bool paused : 1;
+    bool oldPlaying : 1;
+    unsigned padding: 29;
+    int presetCurrentFrame;
+    QSize currentSourceSize;
+    QMovie *movie;
 #if QT_CONFIG(qml_network)
     QNetworkReply *reply;
     int redirectCount;
 #endif
     QMap<int, QQuickPixmap *> frameMap;
-    QSize currentSourceSize;
-    void setMovie(QMovie *movie);
 };
 
 QT_END_NAMESPACE