ubuntu-touch-coreapps-reviewers team mailing list archive

[Stefano Verzegnassi <stefano92.100@xxxxxxxxx>]

Stefano Verzegnassi has proposed merging lp:~verzegnassi-stefano/ubuntu-docviewer-app/lok-qml-currentpart-moved into lp:ubuntu-docviewer-app.

Commit message:
[lok-qml] Moved 'currentPart' property from LODocument to LOView.

Requested reviews:
  Ubuntu Document Viewer Developers (ubuntu-docviewer-dev)

For more details, see:
https://code.launchpad.net/~verzegnassi-stefano/ubuntu-docviewer-app/lok-qml-currentpart-moved/+merge/281840

[lok-qml] Moved 'currentPart' property from LODocument to LOView.
-- 
Your team Ubuntu Document Viewer Developers is requested to review the proposed merge of lp:~verzegnassi-stefano/ubuntu-docviewer-app/lok-qml-currentpart-moved into lp:ubuntu-docviewer-app.

=== modified file 'po/com.ubuntu.docviewer.pot'
--- po/com.ubuntu.docviewer.pot	2015-12-27 12:10:06 +0000
+++ po/com.ubuntu.docviewer.pot	2016-01-07 11:26:17 +0000
@@ -8,7 +8,7 @@
 msgstr ""
 "Project-Id-Version: \n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2015-12-27 13:06+0100\n"
+"POT-Creation-Date: 2016-01-07 12:22+0100\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@xxxxxx>\n"
@@ -215,7 +215,7 @@
 msgstr ""
 
 #: ../src/app/qml/documentPage/DocumentPage.qml:23
-#: /tmp/ubuntu-docviewer-app-build/po/com.ubuntu.docviewer.desktop.in.in.h:3
+#: /tmp/lok-qml-currentpart-moved-build/po/com.ubuntu.docviewer.desktop.in.in.h:3
 msgid "Documents"
 msgstr ""
 
@@ -439,10 +439,10 @@
 msgid "copy %1"
 msgstr ""
 
-#: /tmp/ubuntu-docviewer-app-build/po/com.ubuntu.docviewer.desktop.in.in.h:1
+#: /tmp/lok-qml-currentpart-moved-build/po/com.ubuntu.docviewer.desktop.in.in.h:1
 msgid "Document Viewer"
 msgstr ""
 
-#: /tmp/ubuntu-docviewer-app-build/po/com.ubuntu.docviewer.desktop.in.in.h:2
+#: /tmp/lok-qml-currentpart-moved-build/po/com.ubuntu.docviewer.desktop.in.in.h:2
 msgid "documents;viewer;pdf;reader;"
 msgstr ""

=== modified file 'src/app/qml/loView/KeybHelper.js'
--- src/app/qml/loView/KeybHelper.js	2015-11-30 12:12:10 +0000
+++ src/app/qml/loView/KeybHelper.js	2016-01-07 11:26:17 +0000
@@ -22,7 +22,7 @@
 
     if (event.key == Qt.Key_PageUp) {
         if (isPresentation)
-            view.document.currentPart -= 1
+            view.currentPart -= 1
         else
             view.moveView("vertical", -view.height)
  
@@ -31,7 +31,7 @@
  
     if (event.key == Qt.Key_PageDown) {
         if (isPresentation)
-            view.document.currentPart += 1
+            view.currentPart += 1
         else
             view.moveView("vertical", view.height)
  
@@ -42,7 +42,7 @@
         if (event.modifiers & Qt.ControlModifier) {
             view.contentX = 0
             view.contentY = 0
-            view.document.currentPart = 0
+            view.currentPart = 0
         } else {
             view.contentX = 0
             view.contentY = 0
@@ -53,8 +53,8 @@
         if (event.modifiers & Qt.ControlModifier) {
             view.contentX = view.contentWidth - view.width
             view.contentY = view.contentHeight - view.height
-            console.log(view.document.currentPart, view.document.partsCount - 1)
-            view.document.currentPart = view.document.partsCount - 1
+            console.log(view.currentPart, view.document.partsCount - 1)
+            view.currentPart = view.document.partsCount - 1
         } else {
             view.contentX = view.contentWidth - view.width
             view.contentY = view.contentHeight - view.height

=== modified file 'src/app/qml/loView/PartsView.qml'
--- src/app/qml/loView/PartsView.qml	2015-11-13 21:35:22 +0000
+++ src/app/qml/loView/PartsView.qml	2016-01-07 11:26:17 +0000
@@ -31,7 +31,7 @@
     // used in vertical mode
     property bool isWide: width > units.gu(24)
 
-    currentIndex: view.model ? loView.document.currentPart : -1
+    currentIndex: view.model ? loView.currentPart : -1
     highlightMoveDuration: UbuntuAnimation.SnapDuration
 
     delegate: (orientation == ListView.Vertical) ? verticalDelegate : horizontalDelegate
@@ -44,8 +44,8 @@
             width: parent.width
             height: units.gu(16)
 
-            color: (loView.document.currentPart === model.index) ? theme.palette.selected.background
-                                                                 : "transparent"
+            color: (loView.currentPart === model.index) ? theme.palette.selected.background
+                                                        : "transparent"
 
             onClicked: internal.delegate_onClicked(model.index)
 
@@ -77,8 +77,8 @@
                     wrapMode: Text.WordWrap
                     text: model.name
                     visible: view.isWide
-                    color: (loView.document.currentPart === model.index) ? UbuntuColors.orange
-                                                                         : theme.palette.selected.backgroundText
+                    color: (loView.currentPart === model.index) ? UbuntuColors.orange
+                                                                : theme.palette.selected.backgroundText
                 }
 
                 /* UITK 1.3 specs: Slot C */
@@ -86,8 +86,8 @@
                     SlotsLayout.position: SlotsLayout.Trailing
 
                     text: model.index + 1
-                    color: (loView.document.currentPart === model.index) ? UbuntuColors.orange
-                                                                         : theme.palette.selected.backgroundText
+                    color: (loView.currentPart === model.index) ? UbuntuColors.orange
+                                                                : theme.palette.selected.backgroundText
                 }
             }
         }
@@ -100,8 +100,8 @@
             id: delegate
             height: parent.height; width: height
 
-            color: (loView.document.currentPart === model.index) ? theme.palette.selected.background
-                                                                 : "transparent"
+            color: (loView.currentPart === model.index) ? theme.palette.selected.background
+                                                        : "transparent"
 
             onClicked: internal.delegate_onClicked(model.index)
 
@@ -127,8 +127,8 @@
                 Label {
                     Layout.alignment: Qt.AlignHCenter | Qt.AlignTop
                     text: model.index + 1
-                    color: (loView.document.currentPart === model.index) ? UbuntuColors.orange
-                                                                         : theme.palette.selected.backgroundText
+                    color: (loView.currentPart === model.index) ? UbuntuColors.orange
+                                                                : theme.palette.selected.backgroundText
                 }
             }
         }
@@ -138,7 +138,7 @@
         id: internal
 
         function delegate_onClicked(index) {
-            loView.document.currentPart = index
+            loView.currentPart = index
 
             // Check if the view has been included in a nested page (e.g.
             // bottomEdge). If so, close that page and return to the

=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/lodocument.cpp'
--- src/plugin/libreofficetoolkit-qml-plugin/lodocument.cpp	2015-11-28 22:11:54 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/lodocument.cpp	2016-01-07 11:26:17 +0000
@@ -34,7 +34,6 @@
 
 LODocument::LODocument()
   : m_path("")
-  , m_currentPart(-1)
   , m_error(LibreOfficeError::NoError)
   , m_lokDocument(nullptr)
 {
@@ -60,22 +59,6 @@
     loadDocument(m_path);
 }
 
-int LODocument::currentPart() {
-    return m_currentPart;
-}
-
-void LODocument::setCurrentPart(int index)
-{
-    if (!m_lokDocument)
-        return;
-
-    if (m_currentPart == index || index < 0 || index > partsCount() - 1)
-        return;
-
-    m_currentPart = index;
-    Q_EMIT currentPartChanged();
-}
-
 // Load the document
 void LODocument::loadDocument(const QString &pathName)
 {
@@ -122,8 +105,6 @@
     m_docType = DocumentType(m_lokDocument->getDocumentType());
     Q_EMIT documentTypeChanged();
 
-    setCurrentPart(m_lokDocument->getPart());
-
     m_lokDocument->initializeForRendering();
     qDebug() << "Document loaded successfully !";
 
@@ -170,11 +151,13 @@
     return QSize(pWidth, pHeight);
 }
 
-QImage LODocument::paintTile(const QSize& canvasSize, const QRect& tileSize, const qreal &zoom)
+QImage LODocument::paintTile(int part, const QSize& canvasSize, const QRect& tileSize, const qreal &zoom)
 {
     if (!m_lokDocument)
         return QImage();
 
+    m_lokDocument->setPart(part);
+
     QImage result = QImage(canvasSize.width(), canvasSize.height(),  QImage::Format_RGB32);
 
 #ifdef DEBUG_TILE_BENCHMARK
@@ -196,11 +179,13 @@
     return result.rgbSwapped();
 }
 
-QImage LODocument::paintThumbnail(qreal size)
+QImage LODocument::paintThumbnail(int part, qreal size)
 {
     if (!m_lokDocument)
         return QImage();
 
+    m_lokDocument->setPart(part);
+
 #ifdef DEBUG_TILE_BENCHMARK
     QElapsedTimer renderTimer;
     renderTimer.start();

=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/lodocument.h'
--- src/plugin/libreofficetoolkit-qml-plugin/lodocument.h	2015-11-28 22:11:54 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/lodocument.h	2016-01-07 11:26:17 +0000
@@ -33,7 +33,6 @@
     Q_DISABLE_COPY(LODocument)
 
     Q_PROPERTY(QString                 path         READ path         WRITE setPath         NOTIFY pathChanged)
-    Q_PROPERTY(int                     currentPart  READ currentPart  WRITE setCurrentPart  NOTIFY currentPartChanged)
     // Declare partsCount as constant at the moment, since LOK-plugin is just a viewer for now.
     Q_PROPERTY(int                     partsCount   READ partsCount                                                    CONSTANT)
     Q_PROPERTY(int                     documentPart READ documentPart WRITE setDocumentPart NOTIFY documentPartChanged)
@@ -56,9 +55,6 @@
     QString path() const;
     void setPath(const QString& pathName);
 
-    int currentPart();
-    void setCurrentPart(int index);
-
     DocumentType documentType() const;
 
     int documentPart() const;
@@ -66,8 +62,8 @@
 
     QSize documentSize() const;
 
-    QImage paintTile(const QSize& canvasSize, const QRect& tileSize, const qreal& zoom = 1.0);
-    QImage paintThumbnail(qreal size);
+    QImage paintTile(int part, const QSize& canvasSize, const QRect& tileSize, const qreal& zoom = 1.0);
+    QImage paintThumbnail(int part, qreal size);
 
     int partsCount();
     QString getPartName(int index) const;
@@ -77,14 +73,12 @@
 
 Q_SIGNALS:
     void pathChanged();
-    void currentPartChanged();
     void documentTypeChanged();
     void documentPartChanged();
     void errorChanged();
 
 private:
     QString m_path;
-    int m_currentPart;
     DocumentType m_docType;
     LibreOfficeError::Error m_error;
     lok::Document *m_lokDocument;

=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/lorendertask.cpp'
--- src/plugin/libreofficetoolkit-qml-plugin/lorendertask.cpp	2015-12-12 10:06:55 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/lorendertask.cpp	2016-01-07 11:26:17 +0000
@@ -16,10 +16,10 @@
 
 QImage TileRenderTask::doWork()
 {
-    return m_document->paintTile(m_area.size(), m_area, m_zoom);
+    return m_document->paintTile(m_part, m_area.size(), m_area, m_zoom);
 }
 
 QImage ThumbnailRenderTask::doWork()
 {
-    return m_document->paintThumbnail(m_size);
+    return m_document->paintThumbnail(m_part, m_size);
 }

=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/lorendertask.h'
--- src/plugin/libreofficetoolkit-qml-plugin/lorendertask.h	2015-12-12 10:06:55 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/lorendertask.h	2016-01-07 11:26:17 +0000
@@ -16,7 +16,7 @@
 {
 public:
     virtual bool canBeRunInParallel(AbstractRenderTask* prevTask);
-    virtual void prepare() { m_document->setDocumentPart(m_part); }
+    virtual void prepare() { /* do nothing */ }
 
     int part() { return m_part; }
     void setPart(int p) { m_part = p; }

=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/loview.cpp'
--- src/plugin/libreofficetoolkit-qml-plugin/loview.cpp	2015-11-28 22:11:54 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/loview.cpp	2016-01-07 11:26:17 +0000
@@ -36,6 +36,7 @@
     , m_parentFlickable(nullptr)
     , m_document(nullptr)
     , m_partsModel(nullptr)
+    , m_currentPart(0)
     , m_zoomFactor(1.0)
     , m_cacheBuffer(TILE_SIZE * 3)
     , m_visibleArea(0, 0, 0, 0)
@@ -114,7 +115,8 @@
     engine->addImageProvider("lok", m_imageProvider);
     // --------------------------------------------------
 
-    connect(m_document.data(), SIGNAL(currentPartChanged()), this, SLOT(invalidateAllTiles()));
+    setCurrentPart(0);
+    connect(this, SIGNAL(currentPartChanged()), this, SLOT(invalidateAllTiles()));
 
     Q_EMIT documentChanged();
 }
@@ -130,6 +132,22 @@
     return m_partsModel;
 }
 
+int LOView::currentPart() {
+    return m_currentPart;
+}
+
+void LOView::setCurrentPart(int index)
+{
+    if (!m_document)
+        return;
+
+    if (m_currentPart == index || index < 0 || index > (m_document.data()->partsCount() - 1))
+        return;
+
+    m_currentPart = index;
+    Q_EMIT currentPartChanged();
+}
+
 qreal LOView::zoomFactor() const
 {
     return m_zoomFactor;
@@ -399,7 +417,7 @@
 {
     TileRenderTask* task = new TileRenderTask();
     task->setId(id);
-    task->setPart(m_document->currentPart());
+    task->setPart(m_currentPart);
     task->setDocument(m_document);
     task->setArea(rect);
     task->setZoom(m_zoomFactor);

=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/loview.h'
--- src/plugin/libreofficetoolkit-qml-plugin/loview.h	2015-12-12 10:06:55 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/loview.h	2016-01-07 11:26:17 +0000
@@ -39,6 +39,7 @@
     Q_PROPERTY(QQuickItem*              parentFlickable READ parentFlickable WRITE setParentFlickable NOTIFY parentFlickableChanged)
     Q_PROPERTY(LODocument*              document        READ document        /*WRITE setDocument*/    NOTIFY documentChanged)
     Q_PROPERTY(LOPartsModel*            partsModel      READ partsModel                               NOTIFY partsModelChanged)
+    Q_PROPERTY(int                      currentPart     READ currentPart     WRITE setCurrentPart     NOTIFY currentPartChanged)
     Q_PROPERTY(qreal                    zoomFactor      READ zoomFactor      WRITE setZoomFactor      NOTIFY zoomFactorChanged)
     Q_PROPERTY(ZoomMode                 zoomMode        READ zoomMode                                 NOTIFY zoomModeChanged)
     Q_PROPERTY(int                      cacheBuffer     READ cacheBuffer     WRITE setCacheBuffer     NOTIFY cacheBufferChanged)
@@ -61,6 +62,9 @@
     LODocument* document() const;
     LOPartsModel* partsModel() const;
 
+    int currentPart();
+    void setCurrentPart(int index);
+
     qreal       zoomFactor() const;
     void        setZoomFactor(const qreal zoom);
 
@@ -77,6 +81,7 @@
     void parentFlickableChanged();
     void documentChanged();
     void partsModelChanged();
+    void currentPartChanged();
     void zoomFactorChanged();
     void zoomModeChanged();
     void cacheBufferChanged();
@@ -96,6 +101,7 @@
     LOPartsModel*               m_partsModel; // TODO MB move to document.
     LOPartsImageProvider*       m_imageProvider; // The QQmlEngine takes ownership of provider.
 
+    int                         m_currentPart;
     qreal                       m_zoomFactor;
     ZoomMode                    m_zoomMode;
     int                         m_cacheBuffer;

=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/qml/Viewer.qml'
--- src/plugin/libreofficetoolkit-qml-plugin/qml/Viewer.qml	2015-11-27 16:39:21 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/qml/Viewer.qml	2016-01-07 11:26:17 +0000
@@ -26,6 +26,7 @@
     property alias partsModel:  view.partsModel
     property alias zoomMode:    view.zoomMode
     property alias error:       view.error
+    property alias currentPart: view.currentPart
 
     property string documentPath: ""
 
@@ -61,22 +62,22 @@
 
     function goNextPart()
     {
-        document.currentPart = Math.min(document.currentPart + 1, document.partsCount - 1)
+        currentPart = Math.min(currentPart + 1, document.partsCount - 1)
     }
 
     function goPreviousPart()
     {
-        document.currentPart = Math.max(0, document.currentPart - 1)
+        currentPart = Math.max(0, currentPart - 1)
     }
 
     function goFirstPart()
     {
-        document.currentPart = 0
+        currentPart = 0
     }
 
     function goLastPart()
     {
-        document.currentPart = document.partsCount - 1
+        currentPart = document.partsCount - 1
     }
 
     onDocumentPathChanged: {
@@ -97,10 +98,6 @@
         id: view
 
         parentFlickable: rootFlickable
-    }
-
-    Connections {
-        target: view.document
 
         onCurrentPartChanged: {
             // Position view at top-left corner