← Back to team overview

ubuntu-touch-coreapps-reviewers team mailing list archive

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