Merge pull request #1230 from letsfindaway/fix-thumbnail-sync

Fix thumbnail sync

Author: kaamui

src/core/UBPersistenceManager.cpp

@@ -858,6 +858,13 @@ void UBPersistenceManager::duplicateDocumentScene(std::shared_ptr<UBDocumentProx
 {
     checkIfDocumentRepositoryExists();
 
+    auto scene = mSceneCache.value(proxy, index);
+
+    if (scene && scene->isModified())
+    {
+        persistDocumentScene(proxy, scene, index, false, true);
+    }
+
     for (int i = proxy->pageCount(); i > index + 1; i--)
     {
         renamePage(proxy, i - 1 , i);
@@ -869,7 +876,7 @@ void UBPersistenceManager::duplicateDocumentScene(std::shared_ptr<UBDocumentProx
     copyPage(proxy, index , index + 1);
 
     //TODO: write a proper way to handle object on disk
-    std::shared_ptr<UBGraphicsScene> scene = loadDocumentScene(proxy, index + 1);
+    scene = loadDocumentScene(proxy, index + 1);
 
     foreach(QGraphicsItem* item, scene->items())
     {

src/document/UBDocument.cpp

@@ -69,7 +69,7 @@ void UBDocument::duplicatePage(int index)
 {
     UBPersistenceManager::persistenceManager()->duplicateDocumentScene(mProxy, index);
 
-    mThumbnailScene->insertThumbnail(index);
+    mThumbnailScene->insertThumbnail(index + 1);
 }
 
 void UBDocument::movePage(int fromIndex, int toIndex)

src/frameworks/UBBackgroundLoader.cpp

@@ -44,6 +44,12 @@ UBBackgroundLoader::~UBBackgroundLoader()
     wait();
 }
 
+bool UBBackgroundLoader::isIdle()
+{
+    QMutexLocker lock{&mMutex};
+    return mPaths.empty() && mResults.empty();
+}
+
 bool UBBackgroundLoader::isResultAvailable()
 {
     QMutexLocker lock{&mMutex};

src/frameworks/UBBackgroundLoader.h

@@ -37,6 +37,7 @@ class UBBackgroundLoader : public QThread
     UBBackgroundLoader(QList<std::pair<int, QString>> paths, QObject* parent = nullptr);
     virtual ~UBBackgroundLoader();
 
+    bool isIdle();
     bool isResultAvailable();
     std::pair<int, QByteArray> takeResult();
 

src/gui/UBBoardThumbnailsView.cpp

@@ -165,19 +165,6 @@ void UBBoardThumbnailsView::updateActiveThumbnail(int newActiveIndex)
     update();
 }
 
-void UBBoardThumbnailsView::updateExposure()
-{
-    QRect viewportRect(QPoint(0, 0), viewport()->size());
-    QRectF visibleSceneRect = mapToScene(viewportRect).boundingRect();
-
-    int index = 0;
-
-    while (auto thumbnail = mDocument->thumbnailScene()->thumbnailAt(index++))
-    {
-        thumbnail->setExposed(visibleSceneRect.intersects(thumbnail->sceneBoundingRect()));
-    }
-}
-
 void UBBoardThumbnailsView::resizeEvent(QResizeEvent *event)
 {
     bool scrollbarWasHidden = mScrollbarVisible && !verticalScrollBar()->isVisible();
@@ -275,7 +262,6 @@ void UBBoardThumbnailsView::mouseReleaseEvent(QMouseEvent *event)
 void UBBoardThumbnailsView::scrollContentsBy(int dx, int dy)
 {
     QGraphicsView::scrollContentsBy(dx, dy);
-    updateExposure();
 }
 
 void UBBoardThumbnailsView::dragEnterEvent(QDragEnterEvent *event)

src/gui/UBBoardThumbnailsView.h

@@ -80,8 +80,6 @@ public slots:
     void moveThumbnailRequired(int from, int to);
 
 private:
-    void updateExposure();
-
     std::shared_ptr<UBDocument> mDocument;
 
     int mThumbnailWidth;

src/gui/UBThumbnail.cpp

@@ -45,6 +45,9 @@ UBThumbnail::UBThumbnail()
     // accept hover to show/hide UI buttons
     setAcceptHoverEvents(true);
 
+    // make it selectable
+    setFlag(QGraphicsItem::ItemIsSelectable, true);
+
     // arrange child items behind the transparent parent
     mPixmapItem->setFlag(QGraphicsItem::ItemStacksBehindParent);
     mTextItem->setFlag(QGraphicsItem::ItemStacksBehindParent);
@@ -137,16 +140,6 @@ void UBThumbnail::setPageScene(std::shared_ptr<UBGraphicsScene> pageScene)
     adjustThumbnail();
 }
 
-void UBThumbnail::setExposed(bool exposed)
-{
-    mExposed = exposed;
-}
-
-bool UBThumbnail::isExposed() const
-{
-    return mExposed;
-}
-
 void UBThumbnail::setDeletable(bool deletable)
 {
     mDeletable = deletable;
@@ -156,7 +149,7 @@ void UBThumbnail::updatePixmap(const QRectF& region)
 {
     auto pageScene = mPageScene.lock();
 
-    if (pageScene && !mPixmapItem->pixmap().isNull() && mExposed)
+    if (pageScene && !mPixmapItem->pixmap().isNull())
     {
         QPixmap pixmap = this->pixmap();
         QRectF pixmapRect;

src/gui/UBThumbnail.h

@@ -55,8 +55,6 @@ class UBThumbnail : public QGraphicsRectItem
     int row() const;
 
     void setPageScene(std::shared_ptr<UBGraphicsScene> pageScene);
-    void setExposed(bool exposed);
-    bool isExposed() const;
     void setDeletable(bool deletable);
 
     void updatePixmap(const QRectF& region = QRectF());
@@ -81,7 +79,6 @@ class UBThumbnail : public QGraphicsRectItem
     int mColumn{0};
     int mRow{0};
     std::weak_ptr<UBGraphicsScene> mPageScene{}; // weak, don't keep the scene from being deleted
-    bool mExposed{false};
     QTransform mTransform{};
     bool mEditable{false};
     bool mDeletable{true};

src/gui/UBThumbnailScene.cpp

@@ -33,6 +33,7 @@
 
 UBThumbnailScene::UBThumbnailScene(UBDocument* document)
     : mDocument{document}
+    , mThumbnailItems{document->proxy()->pageCount()}
 {
 }
 
@@ -82,20 +83,22 @@ void UBThumbnailScene::arrangeThumbnails(int fromIndex, int toIndex)
     for (int i = fromIndex; i < toIndex; ++i)
     {
         auto thumbnail = mThumbnailItems.at(i);
-        thumbnail->setThumbnailSize(thumbnailSize);
-        thumbnail->setFlag(QGraphicsItem::ItemIsSelectable, true);
 
-        thumbnail->setColumn(columnIndex);
-        thumbnail->setRow(rowIndex);
-
-        // start with a margin left
-        // step by width + spacing
-        // start with margin top
-        // step by height including label + spacing
-        const auto posX = margins.left() + columnIndex * gridSize.width() + horizontalCenterOffset;
-        const auto posY = margins.top() + rowIndex * gridSize.height();
-
-        thumbnail->setPos(posX, posY);
+        if (thumbnail)
+        {
+            thumbnail->setThumbnailSize(thumbnailSize);
+            thumbnail->setColumn(columnIndex);
+            thumbnail->setRow(rowIndex);
+
+            // start with a margin left
+            // step by width + spacing
+            // start with margin top
+            // step by height including label + spacing
+            const auto posX = margins.left() + columnIndex * gridSize.width() + horizontalCenterOffset;
+            const auto posY = margins.top() + rowIndex * gridSize.height();
+
+            thumbnail->setPos(posX, posY);
+        }
 
         if (++columnIndex >= nbColumns)
         {
@@ -130,7 +133,7 @@ void UBThumbnailScene::hightlightItem(int index, bool only)
 
     if (index >= 0 && index < mThumbnailItems.size())
     {
-        mThumbnailItems.at(index)->setSelected(true);
+        thumbnailAt(index)->setSelected(true);
     }
 }
 
@@ -139,23 +142,36 @@ int UBThumbnailScene::thumbnailCount() const
     return mThumbnailItems.size();
 }
 
-UBThumbnail* UBThumbnailScene::thumbnailAt(int index) const
+UBThumbnail* UBThumbnailScene::thumbnailAt(int index)
 {
     if (index >= 0 && index < mThumbnailItems.size())
     {
+        if (!mThumbnailItems.at(index))
+        {
+            // create the missing thumbnail
+            auto thumbnailItem = new UBThumbnail;
+
+            thumbnailItem->setPixmap(UBThumbnailAdaptor::get(mDocument->proxy(), index));
+            thumbnailItem->setSceneIndex(index);
+
+            mThumbnailItems[index] = thumbnailItem;
+            addItem(thumbnailItem);
+
+            arrangeThumbnails(index, index + 1);
+        }
+
         return mThumbnailItems.at(index);
     }
 
     return nullptr;
 }
 
 /**
- * @brief (Re-)create all thumbnails for this scene.
+ * @brief Create thumbnails for this scene.
  *
- * Already existing thumbnails are first removed from the scene and deleted.
- * Then all thumbnails for the document pages are asynchronously loaded and
- * positioned on the scene. The application remains responsive even while
- * loading thumbnails.
+ * Thumbnails for the document pages above startIndex are asynchronously
+ * loaded and positioned on the scene. The application remains responsive
+ * even while loading thumbnails.
  *
  * It is even possible to interact with the already loaded thumbnails during
  * the loading process. So the already loaded pages can be moved, copied,
@@ -164,13 +180,10 @@ UBThumbnail* UBThumbnailScene::thumbnailAt(int index) const
  */
 void UBThumbnailScene::createThumbnails(int startIndex)
 {
-    // delete current thumbnails above startIndex
-    for (int index = mThumbnailItems.size() - 1; index >= startIndex; --index)
+    // skip already loaded thumbnails
+    while (startIndex < mThumbnailItems.count() && mThumbnailItems.at(startIndex))
     {
-        auto item = mThumbnailItems.at(index);
-        removeItem(item);
-        mThumbnailItems.removeAt(index);
-        delete item;
+        ++startIndex;
     }
 
     // create the list of all thumbnail paths
@@ -187,6 +200,12 @@ void UBThumbnailScene::createThumbnails(int startIndex)
         // abort a running loader
         mLoader->abort();
         delete mLoader;
+        mLoader = nullptr;
+    }
+
+    if (paths.empty())
+    {
+        return;
     }
 
     mLoader = new UBBackgroundLoader{paths, this};
@@ -236,7 +255,7 @@ void UBThumbnailScene::insertThumbnail(int pageIndex, std::shared_ptr<UBGraphics
     if (mLoader)
     {
         // restart loading remaining thumbnails
-        createThumbnails(mThumbnailItems.size());
+        createThumbnails(pageIndex);
     }
 }
 
@@ -259,7 +278,7 @@ void UBThumbnailScene::deleteThumbnail(int pageIndex, bool rearrange)
     if (mLoader)
     {
         // restart loading remaining thumbnails
-        createThumbnails(mThumbnailItems.size());
+        createThumbnails(pageIndex);
     }
 
     if (mThumbnailItems.size() == 1)
@@ -288,7 +307,7 @@ void UBThumbnailScene::moveThumbnail(int fromIndex, int toIndex)
     if (mLoader)
     {
         // restart loading remaining thumbnails
-        createThumbnails(mThumbnailItems.size());
+        createThumbnails(fromIndex);
     }
 }
 
@@ -298,7 +317,7 @@ void UBThumbnailScene::reloadThumbnail(int pageIndex)
     {
         auto thumbnail = mThumbnailItems.at(pageIndex);
 
-        if (thumbnail && !thumbnail->isExposed())
+        if (thumbnail)
         {
             thumbnail->setPixmap(UBThumbnailAdaptor::get(mDocument->proxy(), pageIndex));
         }
@@ -334,7 +353,7 @@ void UBThumbnailScene::loadNextThumbnail()
     // max number of thumbnails to load in one pass
     constexpr int bulkSize{10};
 
-    if (mThumbnailItems.size() < mDocument->proxy()->pageCount())
+    if (!mLoader->isIdle())
     {
         if (UBApplication::isClosing)
         {
@@ -348,7 +367,7 @@ void UBThumbnailScene::loadNextThumbnail()
             return;
         }
 
-        const auto firstIndex = mThumbnailItems.size();
+        int firstIndex = -1;
 
         for (int i = 0; i < bulkSize; ++i)
         {
@@ -357,31 +376,43 @@ void UBThumbnailScene::loadNextThumbnail()
                 break;
             }
 
-            // take next result and determine index from current number of thumbnails and
-            // not from result, because pages may have been added or removed in the meantime
+            // take and process next result
             const auto result = mLoader->takeResult();
-            const auto nextIndex = mThumbnailItems.size();
-            QPixmap pixmap;
+            const auto index = result.first;
+            auto thumbnailItem = mThumbnailItems.at(index);
 
-            if (result.second.isEmpty())
+            if (!thumbnailItem)
             {
-                pixmap = UBThumbnailAdaptor::generateMissingThumbnail(mDocument->proxy(), nextIndex);
-            }
-            else
-            {
-                pixmap.loadFromData(result.second);
-            }
+                QPixmap pixmap;
 
-            auto thumbnailItem = new UBThumbnail;
+                if (result.second.isEmpty())
+                {
+                    pixmap = UBThumbnailAdaptor::generateMissingThumbnail(mDocument->proxy(), index);
+                }
+                else
+                {
+                    pixmap.loadFromData(result.second);
+                }
 
-            thumbnailItem->setPixmap(pixmap);
-            thumbnailItem->setSceneIndex(nextIndex);
+                thumbnailItem = new UBThumbnail;
 
-            mThumbnailItems << thumbnailItem;
-            addItem(thumbnailItem);
+                thumbnailItem->setPixmap(pixmap);
+                thumbnailItem->setSceneIndex(index);
+
+                mThumbnailItems[index] = thumbnailItem;
+                addItem(thumbnailItem);
+
+                if (firstIndex < 0)
+                {
+                    firstIndex = index;
+                }
+            }
         }
 
-        arrangeThumbnails(firstIndex);
+        if (firstIndex >= 0)
+        {
+            arrangeThumbnails(firstIndex);
+        }
 
         // load next thumbnails in a deferred task executed on the main thread when it is idle.
         QTimer::singleShot(1, mLoader, [this]() { loadNextThumbnail(); });

src/gui/UBThumbnailScene.h

@@ -57,7 +57,7 @@ class UBThumbnailScene : public QGraphicsScene
     void arrangeThumbnails(int fromIndex = 0, int toIndex = -1);
     void hightlightItem(int index, bool only = false);
     int thumbnailCount() const;
-    UBThumbnail* thumbnailAt(int index) const;
+    UBThumbnail* thumbnailAt(int index);
 
 protected:
     // only to be called from UBDocument