Merge lp:~fboucault/camera-app/photo_roll_reliable_indexes into lp:camera-app

[Florian Boucault]

Photo roll: more fine grained media folder monitoring as to properly forward deletions to the view. Workarounded a couple of Qt bugs related to snapMode SnapOneItem.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:369
http://jenkins.qa.ubuntu.com/job/camera-app-ci/276/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-utopic-amd64-ci/92
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-utopic-armhf-ci/92
        deb: http://jenkins.qa.ubuntu.com/job/camera-app-utopic-armhf-ci/92/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-utopic-i386-ci/92
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-utopic-touch/316
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/3244
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-runner-mako/457
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-click-builder-utopic-armhf/511
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12480
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/2662
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3537
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3537/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/camera-app-ci/276/rebuild

review: Needs Fixing (continuous-integration)

[Ugo Riboni (uriboni)]

Fixes all the mentioned bugs and doesn't seem to introduce any regression. The code is also sensible even though I can't fully understand the QT workaround.

review: Approve

=== modified file 'CameraApp/foldersmodel.cpp'
--- CameraApp/foldersmodel.cpp	2014-08-11 17:42:41 +0000
+++ CameraApp/foldersmodel.cpp	2014-09-02 13:20:32 +0000
@@ -78,19 +78,17 @@
 
 void FoldersModel::updateFileInfoList()
 {
+    beginResetModel();
     m_fileInfoList.clear();
     Q_FOREACH (QString folder, m_folders) {
         QDir currentDir(folder);
         QFileInfoList fileInfoList = currentDir.entryInfoList(QDir::Files | QDir::Readable,
                                                               QDir::Time | QDir::Reversed);
         Q_FOREACH (QFileInfo fileInfo, fileInfoList) {
+            if (fileInfo.isDir()) continue;
             m_watcher->addPath(fileInfo.absoluteFilePath());
-            QString type = m_mimeDatabase.mimeTypeForFile(fileInfo).name();
-            Q_FOREACH (QString filterType, m_typeFilters) {
-                if (type.startsWith(filterType)) {
-                    insertFileInfo(fileInfo);
-                    break;
-                }
+            if (fileMatchesTypeFilters(fileInfo)) {
+                insertFileInfo(fileInfo, false);
             }
         }
     }
@@ -104,19 +102,45 @@
     return fileInfo1.lastModified() < fileInfo2.lastModified();
 }
 
+bool FoldersModel::fileMatchesTypeFilters(const QFileInfo& newFileInfo)
+{
+    QString type = m_mimeDatabase.mimeTypeForFile(newFileInfo).name();
+    Q_FOREACH (QString filterType, m_typeFilters) {
+        if (type.startsWith(filterType)) {
+            return true;
+        }
+    }
+    return false;
+}
+
 // inserts newFileInfo into m_fileInfoList while keeping m_fileInfoList sorted by
 // file modification time with the files most recently modified first
-void FoldersModel::insertFileInfo(const QFileInfo& newFileInfo)
+void FoldersModel::insertFileInfo(const QFileInfo& newFileInfo, bool emitChange)
 {
     QFileInfoList::iterator i;
     for (i = m_fileInfoList.begin(); i != m_fileInfoList.end(); ++i) {
         QFileInfo fileInfo = *i;
         if (!moreRecentThan(newFileInfo, fileInfo)) {
-            m_fileInfoList.insert(i, newFileInfo);
+            if (emitChange) {
+                int index = m_fileInfoList.indexOf(*i);
+                beginInsertRows(QModelIndex(), index, index);
+                m_fileInfoList.insert(i, newFileInfo);
+                endInsertRows();
+            } else {
+                m_fileInfoList.insert(i, newFileInfo);
+            }
             return;
         }
     }
-    m_fileInfoList.append(newFileInfo);
+
+    if (emitChange) {
+        int index = m_fileInfoList.size();
+        beginInsertRows(QModelIndex(), index, index);
+        m_fileInfoList.append(newFileInfo);
+        endInsertRows();
+    } else {
+        m_fileInfoList.append(newFileInfo);
+    }
 }
 
 QHash<int, QByteArray> FoldersModel::roleNames() const
@@ -177,12 +201,58 @@
 
 void FoldersModel::directoryChanged(const QString &directoryPath)
 {
-    updateFileInfoList();
+    /* Only react when a file is added. Ignore when a file was modified or
+     * deleted as this is taken care of by FoldersModel::fileChanged()
+     * To do so we go through all the files in directoryPath and add
+     * all the ones we were not watching before.
+     */
+    QStringList watchedFiles = m_watcher->files();
+    QDir directory(directoryPath);
+    QStringList files = directory.entryList(QDir::Files | QDir::Readable,
+                                            QDir::Time | QDir::Reversed);
+
+    Q_FOREACH (QString fileName, files) {
+        QString filePath = directory.absoluteFilePath(fileName);
+        if (!watchedFiles.contains(filePath)) {
+            QFileInfo fileInfo(filePath);
+            if (fileInfo.isDir()) continue;
+            m_watcher->addPath(filePath);
+            if (fileMatchesTypeFilters(fileInfo)) {
+               insertFileInfo(fileInfo, true);
+            }
+        }
+    }
 }
 
 void FoldersModel::fileChanged(const QString &filePath)
 {
-    updateFileInfoList();
+    /* Act appropriately upon file change or removal */
+    bool exists = QFileInfo::exists(filePath);
+    int fileIndex = m_fileInfoList.indexOf(QFileInfo(filePath));
+
+    if (exists) {
+        // file's content has changed
+        QFileInfo fileInfo = QFileInfo(filePath);
+        if (fileIndex == -1) {
+            // file's type might have changed and file might have to be included
+            if (fileMatchesTypeFilters(fileInfo)) {
+                insertFileInfo(fileInfo, true);
+            }
+        } else {
+            // update file information
+            QModelIndex modelIndex = this->index(fileIndex);
+            m_fileInfoList[fileIndex] = fileInfo;
+            Q_EMIT dataChanged(modelIndex, modelIndex);
+        }
+    } else {
+        // file has either been removed or renamed
+        if (fileIndex != -1) {
+            // FIXME: handle the renamed case
+            beginRemoveRows(QModelIndex(), fileIndex, fileIndex);
+            m_fileInfoList.removeAt(fileIndex);
+            endRemoveRows();
+        }
+    }
 }
 
 void FoldersModel::toggleSelected(int row)

=== modified file 'CameraApp/foldersmodel.h'
--- CameraApp/foldersmodel.h	2014-08-11 17:42:20 +0000
+++ CameraApp/foldersmodel.h	2014-09-02 13:20:32 +0000
@@ -52,7 +52,8 @@
     void setSingleSelectionOnly(bool singleSelectionOnly);
 
     void updateFileInfoList();
-    void insertFileInfo(const QFileInfo& newFileInfo);
+    bool fileMatchesTypeFilters(const QFileInfo& newFileInfo);
+    void insertFileInfo(const QFileInfo& newFileInfo, bool emitChange);
 
     QHash<int, QByteArray> roleNames() const;
     QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const;

=== modified file 'GalleryView.qml'
--- GalleryView.qml	2014-08-11 17:20:01 +0000
+++ GalleryView.qml	2014-09-02 13:20:32 +0000
@@ -35,10 +35,13 @@
     }
 
     property bool gridMode: false
+    property bool showLastPhotoTakenPending: false
 
     function showLastPhotoTaken() {
         galleryView.gridMode = false;
-        slideshowView.showLastPhotoTaken();
+        // do not immediately try to show the photo in the slideshow as it
+        // might not be in the photo roll model yet
+        showLastPhotoTakenPending = true;
     }
 
     onExit: {
@@ -54,6 +57,7 @@
             anchors.fill: parent
             model: galleryView.model
             visible: opacity != 0.0
+            inView: galleryView.inView
             onToggleHeader: header.toggle();
         }
 
@@ -63,6 +67,7 @@
             headerHeight: header.height
             model: galleryView.model
             visible: opacity != 0.0
+            inView: galleryView.inView
             onPhotoClicked: {
                 if (main.contentExportMode) {
                     model.toggleSelected(index);
@@ -102,7 +107,11 @@
 
     onInViewChanged: {
         if (inView) {
-           header.show();
+            header.show();
+            if (showLastPhotoTakenPending) {
+                slideshowView.showLastPhotoTaken();
+                showLastPhotoTakenPending = false;
+            }
         }
     }
 

=== modified file 'PhotogridView.qml'
--- PhotogridView.qml	2014-07-31 18:41:17 +0000
+++ PhotogridView.qml	2014-09-02 13:20:32 +0000
@@ -27,6 +27,7 @@
     property var model
     signal photoClicked(int index)
     property real headerHeight
+    property bool inView
     property list<Action> actions
 
     function showPhotoAtIndex(index) {
@@ -83,7 +84,7 @@
                 
                 asynchronous: true
                 cache: false
-                source: "image://thumbnailer/" + fileURL.toString()
+                source: photogridView.inView ? "image://thumbnailer/" + fileURL.toString() : ""
                 sourceSize {
                     width: width
                     height: height

=== modified file 'SlideshowView.qml'
--- SlideshowView.qml	2014-08-15 17:28:36 +0000
+++ SlideshowView.qml	2014-09-02 13:20:32 +0000
@@ -29,7 +29,7 @@
     property var model
     property int currentIndex: listView.currentIndex
     property bool touchAcquired: listView.currentItem ? listView.currentItem.pinchInProgress : false
-
+    property bool inView
     signal toggleHeader
     property list<Action> actions: [
         Action {
@@ -49,7 +49,7 @@
     }
 
     function showLastPhotoTaken() {
-        listView.positionViewAtBeginning();
+        listView.currentIndex = 0;
     }
 
     function exit() {
@@ -74,11 +74,23 @@
         boundsBehavior: Flickable.StopAtBounds
         cacheBuffer: width
         highlightRangeMode: ListView.StrictlyEnforceRange
+        // FIXME: this disables the animation introduced by highlightRangeMode
+        // happening setting currentIndex; it is necessary at least because we
+        // were hitting https://bugreports.qt-project.org/browse/QTBUG-41035
+        highlightMoveDuration: 0
         snapMode: ListView.SnapOneItem
         spacing: units.gu(1)
         interactive: currentItem ? !currentItem.pinchInProgress : true
         property real maxDimension: Math.max(width, height)
 
+        removeDisplaced: Transition {
+            UbuntuNumberAnimation { property: "x" }
+        }
+        remove: Transition {
+            ParallelAnimation {
+                UbuntuNumberAnimation { property: "opacity"; to: 0 }
+            }
+        }
         delegate: Item {
             id: delegate
             property bool pinchInProgress: zoomPinchArea.active
@@ -167,7 +179,7 @@
                             anchors.fill: parent
                             asynchronous: true
                             cache: false
-                            source: "image://thumbnailer/" + fileURL.toString()
+                            source: slideshowView.inView ? "image://thumbnailer/" + fileURL.toString() : ""
                             sourceSize {
                                 width: listView.maxDimension
                                 height: listView.maxDimension
@@ -175,7 +187,6 @@
                             fillMode: Image.PreserveAspectFit
                             opacity: status == Image.Ready ? 1.0 : 0.0
                             Behavior on opacity { UbuntuNumberAnimation {duration: UbuntuAnimation.FastDuration} }
-
                         }
 
                         Image {
@@ -301,7 +312,15 @@
                 text: i18n.tr("Delete")
                 color: UbuntuColors.orange
                 onClicked: {
-                    var currentFilePath = slideshowView.model.get(slideshowView.currentIndex, "filePath");
+                    // FIXME: workaround bug in ListView with snapMode: ListView.SnapOneItem
+                    // whereby after deleting the last item in the list the first
+                    // item would be shown even though the currentIndex was not set to 0
+                    var toBeDeleted = listView.currentIndex;
+                    if (listView.currentIndex == listView.count - 1) {
+                        listView.currentIndex = listView.currentIndex - 1;
+                    }
+
+                    var currentFilePath = slideshowView.model.get(toBeDeleted, "filePath");
                     fileOperations.remove(currentFilePath);
                     PopupUtils.close(deleteDialog);
                 }