Merge lp:~verzegnassi-stefano/ubuntu-docviewer-app/fix-zoom-performance into lp:ubuntu-docviewer-app/trunk

Proposed by Stefano Verzegnassi
Status: Merged
Approved by: Nicholas Skaggs
Approved revision: 77
Merged at revision: 83
Proposed branch: lp:~verzegnassi-stefano/ubuntu-docviewer-app/fix-zoom-performance
Merge into: lp:ubuntu-docviewer-app/trunk
Diff against target: 53 lines (+10/-12)
1 file modified
src/app/qml/PdfView.qml (+10/-12)
To merge this branch: bzr merge lp:~verzegnassi-stefano/ubuntu-docviewer-app/fix-zoom-performance
Reviewer Review Type Date Requested Status
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Alan Pope 🍺🐧🐱 πŸ¦„ (community) Approve
Review via email: mp+249060@code.launchpad.net

Commit message

Fixed zoom performance

Description of the change

QQuickView::releaseResources() is a bit expensive.

In the current trunk code, it's called whenever _zoomHelper.scale value changes, one call for each delegate currently loaded.
That means that (on phones) we have ~5 calls for each pinchUpdated event, and it's too much.

In the proposed patch, QQuickView::releaseResources() has been moved in the onPinchFinished slot, so that it's called just one time per zoom event.

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

LGTM

review: Approve
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/app/qml/PdfView.qml'
--- src/app/qml/PdfView.qml 2015-02-05 19:30:53 +0000
+++ src/app/qml/PdfView.qml 2015-02-09 12:10:55 +0000
@@ -37,25 +37,24 @@
37 PDF.VerticalView {37 PDF.VerticalView {
38 id: pdfView38 id: pdfView
39 objectName: "pdfView"39 objectName: "pdfView"
40
41 anchors.fill: parent40 anchors.fill: parent
42 spacing: units.gu(2)41 spacing: units.gu(2)
4342
44 clip: true43 clip: true
45 boundsBehavior: Flickable.StopAtBounds44 boundsBehavior: Flickable.StopAtBounds
46
47 cacheBuffer: height * poppler.providersNumber * _zoomHelper.scale * 0.5
48
49 flickDeceleration: 1500 * units.gridUnit / 845 flickDeceleration: 1500 * units.gridUnit / 8
50 maximumFlickVelocity: 2500 * units.gridUnit / 846 maximumFlickVelocity: 2500 * units.gridUnit / 8
5147
48 contentWidth: parent.width * _zoomHelper.scale
49 cacheBuffer: height * poppler.providersNumber * _zoomHelper.scale * 0.5
50 interactive: !pinchy.pinch.active
51
52 model: poppler52 model: poppler
53 delegate: PdfViewDelegate {53 delegate: PdfViewDelegate {
54 onWidthChanged: QQuickView.releaseResources()
55 Component.onDestruction: QQuickView.releaseResources()54 Component.onDestruction: QQuickView.releaseResources()
56 }55 }
5756
58 contentWidth: parent.width * _zoomHelper.scale57 // FIXME: On zooming, keep the same content position.
59 PinchArea {58 PinchArea {
60 id: pinchy59 id: pinchy
61 anchors.fill: parent60 anchors.fill: parent
@@ -66,14 +65,13 @@
66 maximumScale: 2.565 maximumScale: 2.5
67 }66 }
6867
69 onPinchStarted: pdfView.interactive = false;
70
71 // FIXME: On zooming, keep the same content position.
72 // onPinchUpdated: {}
73
74 onPinchFinished: {68 onPinchFinished: {
75 pdfView.interactive = true;
76 pdfView.returnToBounds();69 pdfView.returnToBounds();
70
71 // This is a bit expensive, so it's safer to put it here.
72 // It won't be called on desktop (where PinchArea is not used),
73 // but it's not a problem at the moment (our target is phone).
74 QQuickView.releaseResources();
77 }75 }
78 }76 }
7977

Subscribers

People subscribed via source and target branches