Merge lp:~verzegnassi-stefano/ubuntu-docviewer-app/reboot-lok-dont-be-dumb into lp:ubuntu-docviewer-app

Proposed by Stefano Verzegnassi
Status: Merged
Approved by: Stefano Verzegnassi
Approved revision: 199
Merged at revision: 202
Proposed branch: lp:~verzegnassi-stefano/ubuntu-docviewer-app/reboot-lok-dont-be-dumb
Merge into: lp:ubuntu-docviewer-app
Prerequisite: lp:~verzegnassi-stefano/ubuntu-docviewer-app/fix-1515037
Diff against target: 127 lines (+45/-33)
2 files modified
src/plugin/libreofficetoolkit-qml-plugin/loview.cpp (+44/-32)
src/plugin/libreofficetoolkit-qml-plugin/loview.h (+1/-1)
To merge this branch: bzr merge lp:~verzegnassi-stefano/ubuntu-docviewer-app/reboot-lok-dont-be-dumb
Reviewer Review Type Date Requested Status
Alan Pope 🍺🐧🐱 πŸ¦„ (community) Approve
Jenkins Bot continuous-integration Approve
Review via email: mp+277816@code.launchpad.net

Commit message

LibreofficeKit-qml-plugin:
  * Fix wrong size for last tile in a row/column
  * Don't create tiles outside the visible/buffer grid (i.e. wrong size for m_visibleArea and m_gridArea)

Description of the change

LibreofficeKit-qml-plugin:
  * Fix wrong size for last tile in a row/column
  * Don't create tiles outside the visible/buffer grid (i.e. wrong size for m_visibleArea and m_gridArea)

To post a comment you must log in.
Revision history for this message
Jenkins Bot (ubuntu-core-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Jenkins Bot (ubuntu-core-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alan Pope 🍺🐧🐱 πŸ¦„ (popey) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/loview.cpp'
2--- src/plugin/libreofficetoolkit-qml-plugin/loview.cpp 2015-10-18 21:27:16 +0000
3+++ src/plugin/libreofficetoolkit-qml-plugin/loview.cpp 2015-11-18 12:50:41 +0000
4@@ -238,17 +238,24 @@
5 }
6 }
7
8- // Update information about the visible area
9- m_visibleArea.setRect(m_parentFlickable->property("contentX").toInt(),
10- m_parentFlickable->property("contentY").toInt(),
11- m_parentFlickable->width(),
12- m_parentFlickable->height());
13-
14- // Update information about the buffer area
15- m_bufferArea.setRect(qMax(0, m_visibleArea.x() - m_cacheBuffer),
16- qMax(0, m_visibleArea.y() - m_cacheBuffer),
17- qMin(int(this->width() - m_bufferArea.x()), m_visibleArea.width() + (m_cacheBuffer * 2)),
18- qMin(int(this->height() - m_bufferArea.y()), m_visibleArea.height() + (m_cacheBuffer * 2)));
19+ // Just for convenience.
20+ QRect documentRect(this->boundingRect().toRect());
21+
22+ // Update visible area
23+ QRect visibleRect(m_parentFlickable->property("contentX").toInt(),
24+ m_parentFlickable->property("contentY").toInt(),
25+ m_parentFlickable->width(),
26+ m_parentFlickable->height());
27+
28+ m_visibleArea = visibleRect.intersected(documentRect);
29+
30+ // Update buffer area
31+ QRect bufferRect(m_visibleArea.left() - m_cacheBuffer,
32+ m_visibleArea.top() - m_cacheBuffer,
33+ m_visibleArea.width() + (m_cacheBuffer * 2),
34+ m_visibleArea.height() + (m_cacheBuffer * 2));
35+
36+ m_bufferArea = bufferRect.intersected(documentRect);
37
38 // Delete tiles that are outside the loading area
39 if (!m_tiles.isEmpty()) {
40@@ -273,21 +280,9 @@
41 }
42 }
43
44- /*
45- FIXME: It seems that LOView loads more tiles than necessary.
46- This can be easily tested with DEBUG_SHOW_TILE_BORDER enabled.
47-
48- Step to reproduce:
49- 1) Open Document Viewer
50- 2) Resize the window, BEFORE opening any LibreOffice document
51- (Trying to resize the window or scrolling the Flickable when the
52- document is already loaded causes bad flickering)
53- 3) Outside the document area, at the bottom-right corner, there are
54- a few tiles that should not be visible/rendered/generated.
55- */
56-
57 // Number of tiles per row
58 int tilesPerWidth = qCeil(this->width() / TILE_SIZE);
59+ int tilesPerHeight = qCeil(this->height() / TILE_SIZE);
60
61 // Get indexes for visible tiles
62 int visiblesFromWidth = int(m_visibleArea.left() / TILE_SIZE);
63@@ -301,18 +296,35 @@
64 int bufferToWidth = qCeil(qreal(m_bufferArea.right()) / TILE_SIZE);
65 int bufferToHeight = qCeil(qreal(m_bufferArea.bottom()) / TILE_SIZE);
66
67- this->generateTiles(visiblesFromWidth, visiblesFromHeight, visiblesToWidth, visiblesToHeight, tilesPerWidth);
68- this->generateTiles(bufferFromWidth, bufferFromHeight, bufferToWidth, bufferToHeight, tilesPerWidth);
69+#ifdef DEBUG_VERBOSE
70+ qDebug() << "Visible area - Left:" << visiblesFromWidth << "Right:" << visiblesToWidth << "Top:" << visiblesFromHeight << "Bottom:" << visiblesToHeight;
71+ qDebug() << "Buffer area - Left:" << bufferFromWidth << "Right:" << bufferToWidth << "Top:" << bufferFromHeight << "Bottom:" << bufferToHeight;
72+#endif
73+
74+ this->generateTiles(visiblesFromWidth, visiblesFromHeight, visiblesToWidth, visiblesToHeight, tilesPerWidth, tilesPerHeight);
75+ this->generateTiles(bufferFromWidth, bufferFromHeight, bufferToWidth, bufferToHeight, tilesPerWidth, tilesPerHeight);
76 }
77
78-void LOView::generateTiles(int x1, int y1, int x2, int y2, int tilesPerWidth)
79+void LOView::generateTiles(int x1, int y1, int x2, int y2, int tilesPerWidth, int tilesPerHeight)
80 {
81 for (int x = x1; x < x2; x++) {
82 for (int y = y1; y < y2; y++) {
83- QRect tileRect(x * TILE_SIZE, y * TILE_SIZE, TILE_SIZE, TILE_SIZE);
84- int index = y * tilesPerWidth + x;
85-
86- this->createTile(index, tileRect);
87+ bool lastRow = (y == (tilesPerHeight - 1));
88+ bool lastColumn = (x == (tilesPerWidth - 1));
89+
90+ int left = TILE_SIZE * x;
91+ int top = TILE_SIZE * y;
92+ int width = lastColumn ? this->width() - left : TILE_SIZE;
93+ int height = lastRow ? this->height() - top : TILE_SIZE;
94+
95+ QRect tileRect(left, top, width, height);
96+ int index = x + tilesPerWidth * y;
97+
98+ createTile(index, tileRect);
99+
100+#ifdef DEBUG_VERBOSE
101+ qDebug() << "Generating tile - Index:" << index << "X:" << x << "Y:" << y;
102+#endif
103 }
104 }
105 }
106@@ -327,7 +339,7 @@
107 {
108 if (!m_tiles.contains(index)) {
109 #ifdef DEBUG_VERBOSE
110- qDebug() << "Creating tile indexed as" << index;
111+ qDebug() << "Creating tile indexed as" << index << "- Rect:" << rect;
112 #endif
113
114 auto tile = new SGTileItem(rect, m_zoomFactor, RenderEngine::getNextId(), this);
115
116=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/loview.h'
117--- src/plugin/libreofficetoolkit-qml-plugin/loview.h 2015-10-11 11:27:29 +0000
118+++ src/plugin/libreofficetoolkit-qml-plugin/loview.h 2015-11-18 12:50:41 +0000
119@@ -103,7 +103,7 @@
120
121 QMap<int, SGTileItem*> m_tiles;
122
123- void generateTiles(int x1, int y1, int x2, int y2, int tilesPerWidth);
124+ void generateTiles(int x1, int y1, int x2, int y2, int tilesPerWidth, int tilesPerHeight);
125 void createTile(int index, QRect rect);
126 void setZoomMode(const ZoomMode zoomMode);
127 bool updateZoomIfAutomatic();

Subscribers

People subscribed via source and target branches