[Okular-devel] Review Request 113986: Multiple Tiles Managers per Page

[Albert Astals Cid]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/113986/#review49312
-----------------------------------------------------------



core/document.cpp
<https://git.reviewboard.kde.org/r/113986/#comment34811>

    Why this change?



core/generator.h
<https://git.reviewboard.kde.org/r/113986/#comment34810>

    Since you're only using this from document.cpp i'd appreciate if you hide this in generator_p.h



core/page.h
<https://git.reviewboard.kde.org/r/113986/#comment34812>

    You don't need the get and set in the public api, just move them to the _p.h file



core/page.cpp
<https://git.reviewboard.kde.org/r/113986/#comment34806>

    You don't really need this m_tilesManagers()



core/page.cpp
<https://git.reviewboard.kde.org/r/113986/#comment34807>

    This loop is NlogN + unneed allocation of all the keys in memory, please use a regular iterator over m_tilesManagers



core/page.cpp
<https://git.reviewboard.kde.org/r/113986/#comment34809>

    Just qDeleteAll, will do what you want



core/page.cpp
<https://git.reviewboard.kde.org/r/113986/#comment34808>

    deleting null is fine, you don't need the if


- Albert Astals Cid


On Nov. 21, 2013, 9 p.m., Michal Humpula wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/113986/
> -----------------------------------------------------------
> 
> (Updated Nov. 21, 2013, 9 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Repository: okular
> 
> 
> Description
> -------
> 
> First shot for multiple tiles managers way. Let's face it -- it's nasty. But the compiled okular is displaying my testing pdf without crashing, so at least that.
> 
> It's straighforward implementation. Every single place, where there were call for (or with) TilesManager, now has a DocumentObserver as companion. The m_tiledManager reference in PagePrivate was changed to QMap<DocumentObserver, TilesManager>.
> 
> It's adding more code then I expected, but I think that now, when every request can be back-supported by TilesManager, all the request can now be served trought them and not by that direct Pixmap cache in Document. So with that, it would actually be more deletion (imho).
> 
> 
> Diffs
> -----
> 
>   active/components/documentitem.cpp aaf98b3 
>   core/document.h fe296e0 
>   core/document.cpp 265ee09 
>   core/document_p.h 3a257de 
>   core/generator.h a5a971b 
>   core/generator.cpp 41beb92 
>   core/page.h bc8c09e 
>   core/page.cpp 0bafa99 
>   core/page_p.h 63d4da1 
>   part.cpp 6951947 
>   ui/pagepainter.cpp d5d9c3e 
>   ui/pageview.cpp 65967bf 
> 
> Diff: https://git.reviewboard.kde.org/r/113986/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michal Humpula
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20140209/4dea3b33/attachment.html>