ubuntu-touch-coreapps-reviewers team mailing list archive
-
ubuntu-touch-coreapps-reviewers team
-
Mailing list archive
-
Message #03736
Re: [Merge] lp:~verzegnassi-stefano/ubuntu-docviewer-app/lo-use-timer-update-view into lp:~ubuntu-docviewer-dev/ubuntu-docviewer-app/lo-tiled-rendering
Review: Needs Fixing
See in-diff comment.
Diff comments:
>
> === modified file 'src/plugin/libreofficetoolkit-qml-plugin/loview.h'
> --- src/plugin/libreofficetoolkit-qml-plugin/loview.h 2015-07-04 16:00:33 +0000
> +++ src/plugin/libreofficetoolkit-qml-plugin/loview.h 2015-07-14 12:22:24 +0000
> @@ -68,6 +70,8 @@
> qreal m_zoomFactor;
> QRect m_visibleArea;
>
> + QTimer* m_updateTimer;
I recommend you to use simply QTimer instead of pointer to QTimer. It is better in terms of design and will fix bug - there are no "delete" statement for "m_updateTimer" in destructor.
> +
> // TODO: Should we move tiles management in another class (e.g. TileBuffer)?
> QMap<int, TileItem*> m_tiles;
> };
--
https://code.launchpad.net/~verzegnassi-stefano/ubuntu-docviewer-app/lo-use-timer-update-view/+merge/264687
Your team Ubuntu Document Viewer Developers is subscribed to branch lp:~ubuntu-docviewer-dev/ubuntu-docviewer-app/lo-tiled-rendering.
References