ubuntu-touch-coreapps-reviewers team mailing list archive

[Stefano Verzegnassi <stefano92.100@xxxxxxxxx>]

Stefano Verzegnassi has proposed merging lp:~verzegnassi-stefano/ubuntu-docviewer-app/reboot-lok-dont-be-dumb into lp:ubuntu-docviewer-app/reboot with lp:~verzegnassi-stefano/ubuntu-docviewer-app/fix-1515037 as a prerequisite.

Requested reviews:
  Ubuntu Document Viewer Developers (ubuntu-docviewer-dev)
Related bugs:
  Bug #1516239 in Ubuntu Document Viewer App: "Last tile in a row/column has a wrong size"
  https://bugs.launchpad.net/ubuntu-docviewer-app/+bug/1516239
  Bug #1516241 in Ubuntu Document Viewer App: "LO-viewer creates tiles outside the visible/buffer grid"
  https://bugs.launchpad.net/ubuntu-docviewer-app/+bug/1516241

For more details, see:
https://code.launchpad.net/~verzegnassi-stefano/ubuntu-docviewer-app/reboot-lok-dont-be-dumb/+merge/277816
-- 
Your team Ubuntu Document Viewer Developers is requested to review the proposed merge of lp:~verzegnassi-stefano/ubuntu-docviewer-app/reboot-lok-dont-be-dumb into lp:ubuntu-docviewer-app/reboot.

=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/config.h'
--- src/plugin/libreofficetoolkit-qml-plugin/config.h	2015-11-18 12:03:39 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/config.h	2015-11-18 12:03:39 +0000
@@ -21,13 +21,13 @@
 #define TILE_SIZE 256.0
 
 // Uncomment it if you want to see tiles boundaries
-//#define DEBUG_SHOW_TILE_BORDER
+#define DEBUG_SHOW_TILE_BORDER
 
 // Uncomment for benchmarking tile rendering performance
 //#define DEBUG_TILE_BENCHMARK
 
 // Uncomment if you want more verbose application output
-//#define DEBUG_VERBOSE
+#define DEBUG_VERBOSE
 
 #include <QDir>
 #include <QStandardPaths>

=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/loview.cpp'
--- src/plugin/libreofficetoolkit-qml-plugin/loview.cpp	2015-10-18 21:27:16 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/loview.cpp	2015-11-18 12:03:39 +0000
@@ -238,17 +238,24 @@
         }
     }
 
-    // Update information about the visible area
-    m_visibleArea.setRect(m_parentFlickable->property("contentX").toInt(),
-                          m_parentFlickable->property("contentY").toInt(),
-                          m_parentFlickable->width(),
-                          m_parentFlickable->height());
-
-    // Update information about the buffer area
-    m_bufferArea.setRect(qMax(0, m_visibleArea.x() - m_cacheBuffer),
-                         qMax(0, m_visibleArea.y() - m_cacheBuffer),
-                         qMin(int(this->width() - m_bufferArea.x()), m_visibleArea.width() + (m_cacheBuffer * 2)),
-                         qMin(int(this->height() - m_bufferArea.y()), m_visibleArea.height() + (m_cacheBuffer * 2)));
+    // Just for convenience.
+    QRect documentRect(this->boundingRect().toRect());
+
+    // Update visible area
+    QRect visibleRect(m_parentFlickable->property("contentX").toInt(),
+                      m_parentFlickable->property("contentY").toInt(),
+                      m_parentFlickable->width(),
+                      m_parentFlickable->height());
+
+    m_visibleArea = visibleRect.intersected(documentRect);
+
+    // Update buffer area
+    QRect bufferRect(m_visibleArea.left()   -  m_cacheBuffer,
+                     m_visibleArea.top()    -  m_cacheBuffer,
+                     m_visibleArea.width()  + (m_cacheBuffer * 2),
+                     m_visibleArea.height() + (m_cacheBuffer * 2));
+
+    m_bufferArea = bufferRect.intersected(documentRect);
 
     // Delete tiles that are outside the loading area
     if (!m_tiles.isEmpty()) {
@@ -273,21 +280,9 @@
         }
     }
 
-    /*
-      FIXME: It seems that LOView loads more tiles than necessary.
-      This can be easily tested with DEBUG_SHOW_TILE_BORDER enabled.
-
-      Step to reproduce:
-        1) Open Document Viewer
-        2) Resize the window, BEFORE opening any LibreOffice document
-           (Trying to resize the window or scrolling the Flickable when the
-           document is already loaded causes bad flickering)
-        3) Outside the document area, at the bottom-right corner, there are
-           a few tiles that should not be visible/rendered/generated.
-    */
-
     // Number of tiles per row
     int tilesPerWidth           = qCeil(this->width() / TILE_SIZE);
+    int tilesPerHeight           = qCeil(this->height() / TILE_SIZE);
 
     // Get indexes for visible tiles
     int visiblesFromWidth       = int(m_visibleArea.left() / TILE_SIZE);
@@ -301,18 +296,35 @@
     int bufferToWidth           = qCeil(qreal(m_bufferArea.right()) / TILE_SIZE);
     int bufferToHeight          = qCeil(qreal(m_bufferArea.bottom()) / TILE_SIZE);
 
-    this->generateTiles(visiblesFromWidth, visiblesFromHeight, visiblesToWidth, visiblesToHeight, tilesPerWidth);
-    this->generateTiles(bufferFromWidth, bufferFromHeight, bufferToWidth, bufferToHeight, tilesPerWidth);
+#ifdef DEBUG_VERBOSE
+    qDebug() << "Visible area - Left:" << visiblesFromWidth << "Right:" << visiblesToWidth << "Top:" << visiblesFromHeight << "Bottom:" << visiblesToHeight;
+    qDebug() << "Buffer area  - Left:" << bufferFromWidth   << "Right:" << bufferToWidth   << "Top:" << bufferFromHeight   << "Bottom:" << bufferToHeight;
+#endif
+
+    this->generateTiles(visiblesFromWidth, visiblesFromHeight, visiblesToWidth, visiblesToHeight, tilesPerWidth, tilesPerHeight);
+    this->generateTiles(bufferFromWidth,   bufferFromHeight,   bufferToWidth,   bufferToHeight,   tilesPerWidth, tilesPerHeight);
 }
 
-void LOView::generateTiles(int x1, int y1, int x2, int y2, int tilesPerWidth)
+void LOView::generateTiles(int x1, int y1, int x2, int y2, int tilesPerWidth, int tilesPerHeight)
 {
     for (int x = x1; x < x2; x++) {
         for (int y = y1; y < y2; y++) {
-            QRect tileRect(x * TILE_SIZE, y * TILE_SIZE, TILE_SIZE, TILE_SIZE);
-            int index = y * tilesPerWidth + x;
-
-            this->createTile(index, tileRect);
+            bool lastRow    = (y == (tilesPerHeight - 1));
+            bool lastColumn = (x == (tilesPerWidth  - 1));
+
+            int left   = TILE_SIZE * x;
+            int top    = TILE_SIZE * y;
+            int width  = lastColumn ? this->width() - left : TILE_SIZE;
+            int height = lastRow    ? this->height() - top : TILE_SIZE;
+
+            QRect tileRect(left, top, width, height);
+            int index = x + tilesPerWidth * y;
+
+            createTile(index, tileRect);
+
+#ifdef DEBUG_VERBOSE
+            qDebug() << "Generating tile - Index:" << index << "X:" << x << "Y:" << y;
+#endif
         }
     }
 }
@@ -327,7 +339,7 @@
 {
     if (!m_tiles.contains(index)) {
 #ifdef DEBUG_VERBOSE
-        qDebug() << "Creating tile indexed as" << index;
+        qDebug() << "Creating tile indexed as" << index << "- Rect:" << rect;
 #endif
 
         auto tile = new SGTileItem(rect, m_zoomFactor, RenderEngine::getNextId(), this);

=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/loview.h'
--- src/plugin/libreofficetoolkit-qml-plugin/loview.h	2015-10-11 11:27:29 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/loview.h	2015-11-18 12:03:39 +0000
@@ -103,7 +103,7 @@
 
     QMap<int, SGTileItem*>      m_tiles;
 
-    void generateTiles(int x1, int y1, int x2, int y2, int tilesPerWidth);
+    void generateTiles(int x1, int y1, int x2, int y2, int tilesPerWidth, int tilesPerHeight);
     void createTile(int index, QRect rect);
     void setZoomMode(const ZoomMode zoomMode);
     bool updateZoomIfAutomatic();