Merge lp:~mspacek/qpdfview/cachezoom into lp:qpdfview

[Martin Spacek]

Here is my naive implementation of caching. It uses a string as the cache key instead of the tile. The key is just a bunch of concatenated values of page number, rotation, inversion, tile left, right, width, and height. Not sure if these are the best parameters to use to ensure unique keys.

No pixmap is ever removed from the cache any more. This only happens (I presume) when the cache hist its size limit and starts removing the entries accessed the furthest ago in time.

This doesn't deal at all with annotations or forms. I haven't thought about that at all, so this almost certainly needs more work.

"(I would suggest defining a struct with a comparison operator in the global header for that.)"

I'm very inexperienced when it comes to C++, so I'm not sure what that means.

[Adam Reichold (adamreichold)]

Hello Martin,

thanks for attacking that problem. However, leaving aside the annotation resp. forms problem, I don't think that keying the index and rendering parameter (in whatever form) is a suitable key because it is not unique: The cache is a static variable and is therefore shared through the whole application, so if you open two documents with equal render parameters and page sizes (e.g. both A4), the pages with the same indices will occupy the same cache entries. And more subtle, if you open two instances of the same document, even their source (e.g. file path or something) will be the same but you could have added an annotation in one instance but not the other. So I think keying at least on the pointer (and invalidating on destruction, so that a newly created object cannot take a stale cache entry) is necessary.

Best regards, Adam.

P.S. The suggestion is really just what I would have tried, e.g. define
struct RenderParamters
{
  qreal m_resolutionX;
  qreal m_resolutionY;
  ...

  bool operator<(const RenderParameters& that) const { /* lexicographical comparison or hashing or something */ }
};
and the use QPair< TileItem*, RenderParameters > as a cache key...

review: Needs Fixing

[lp:~mspacek/qpdfview/cachezoom]

1577. By Martin Spacek <email address hidden> on 2014-06-17

Merge from trunk

1578. By Martin Spacek <email address hidden> on 2014-06-17

Fix merge oops

1579. By Martin Spacek <email address hidden> on 2014-06-18

Try to include document as part of cache key, doesn't compile

[Martin Spacek (mspacek)]

Aren't the tiles created and destroyed every time there's a changing in tiling (like from 1x1 to 2x2)? If so, when don't you then lose access to previously cached pixmaps? Wouldn't it be better to use the document as part of the key, instead?

I tried doing this (haven't tried making a record yet), but it doesn't compile. I get this error:

/usr/include/qt4/QtCore/qobject.h: In instantiation of ‘T qobject_cast(QObject*) [with T = qpdfview::Model::Document*]’:
sources/tileitem.cpp:94:98: required from here
/usr/include/qt4/QtCore/qobject.h:378:5: error: ‘class qpdfview::Model::Document’ has no member named ‘qt_check_for_QOBJECT_macro’
/usr/include/qt4/QtCore/qobject.h:380:85: error: ‘class qpdfview::Model::Document’ has no member named ‘staticMetaObject’
/usr/include/qt4/QtCore/qobject.h: In function ‘T qobject_cast(QObject*) [with T = qpdfview::Model::Document*]’:
/usr/include/qt4/QtCore/qobject.h:381:1: warning: control reaches end of non-void function [-Wreturn-type]

[Adam Reichold (adamreichold)]

Hello,

Am 19.06.2014 00:55, schrieb Martin Spacek:
> Aren't the tiles created and destroyed every time there's a changing in tiling (like from 1x1 to 2x2)? If so, when don't you then lose access to previously cached pixmaps? Wouldn't it be better to use the document as part of the key, instead?

Yes, you are right. Tiles might be destroyed if there is a change in
tiling, i.e. we will reuse as many TileItems and create new ones if the
number of tiles increases and destroy some when the number of tiles
decreases. And so yes, a better candidate for a key is the page object,
i.e. the "m_page" member of type "Model::Page*".

> I tried doing this (haven't tried making a record yet), but it doesn't compile. I get this error:
>
> /usr/include/qt4/QtCore/qobject.h: In instantiation of ‘T qobject_cast(QObject*) [with T = qpdfview::Model::Document*]’:
> sources/tileitem.cpp:94:98: required from here
> /usr/include/qt4/QtCore/qobject.h:378:5: error: ‘class qpdfview::Model::Document’ has no member named ‘qt_check_for_QOBJECT_macro’
> /usr/include/qt4/QtCore/qobject.h:380:85: error: ‘class qpdfview::Model::Document’ has no member named ‘staticMetaObject’
> /usr/include/qt4/QtCore/qobject.h: In function ‘T qobject_cast(QObject*) [with T = qpdfview::Model::Document*]’:
> /usr/include/qt4/QtCore/qobject.h:381:1: warning: control reaches end of non-void function [-Wreturn-type]
>

This looks like you tried to use "qobject_cast" on a class that is no
subtype of QObject and hence has no "Q_OBJECT" macro (meaning no meta
type associated with it). The solution would be to use a normal C++ cast
like "static_cast" if you statically know the correct type or
"dynamic_cast" which is decided at runtime like "qobject_cast" just a
bit slower, c.f. [1] and [2] if you're interested in the background.

Best regards, Adam.

[1] http://qt-project.org/doc/qt-5/qobject.html#qobject_cast

[2] http://qt-project.org/doc/qt-5/metaobjects.html

[lp:~mspacek/qpdfview/cachezoom]

1580. By Martin Spacek <email address hidden> on 2014-06-20

Use page as part of cache key instead of document

1581. By Martin Spacek <email address hidden> on 2014-06-20

Merge trunk

[Martin Spacek (mspacek)]

Hi Adam,

Thanks for that. Yup, using the page seems to work. Now I no longer get cache collisions when multiple files with the same page size are open (although that was pretty amusing :)). The branch has been updated.

I still need to switch to using a struct, and then start thinking about annots and forms.

[lp:~mspacek/qpdfview/cachezoom]

1582. By Martin Spacek <email address hidden> on 2014-06-20

Remove commented out lines

[Adam Reichold (adamreichold)]

Hello again,

yeah, cache collisions can probably give a interesting twist to the more boring documents... Otherwise it seems to start to come together and I made some smaller inline comments.

Concerning the cache key: I would really prefer a custom struct since that could also be used to simplify the code in "RenderTask" and especially in "TileItem::on_renderTask_imageReady". I would also suggest to really include all parameters used in "RenderTask" since I don't want to start evaluating whether different combinations of resolution, device pixel ratio and scale factor could yield the same bounding rectangle (it also seems simpler to use "m_tile" instead of "m_boundingRect"). Just consider the device pixel ratio changing dynamically when you drag the window to another monitor (and this is an explicit meta-datum of the cached pixmap, i.e. QPixmap::devicePixelRatio).

Best regards, Adam.

[Martin Spacek (mspacek)]

Hi Adam,

Yes, that all sounds good. I'm not sure how much of it I can do for now, or even how much of it I'm capable of doing, but when I'm feeling up to it (or feel like procrastinating), I'll give it a shot.

Just a note that I've noticed, only once so far, a cache collision between a page of an open document and that of another previously open document. This is while using the latest version of this branch, which uses the m_page as part of the key (and no longer uses the page number). Is it possible that once in a while the same page object is reused across documents, or is allocated the same memory address as a previously cached one?

[Adam Reichold (adamreichold)]

Hello Martin,

Am 21.06.2014 04:25, schrieb Martin Spacek:
> Hi Adam,
>
> Yes, that all sounds good. I'm not sure how much of it I can do for now, or even how much of it I'm capable of doing, but when I'm feeling up to it (or feel like procrastinating), I'll give it a shot.

Just take your time, there is no pressure involved. (It was deliberate
to set the bug report into "In Progress" but without a target milestone.)

> Just a note that I've noticed, only once so far, a cache collision between a page of an open document and that of another previously open document. This is while using the latest version of this branch, which uses the m_page as part of the key (and no longer uses the page number). Is it possible that once in a while the same page object is reused across documents, or is allocated the same memory address as a previously cached one?
>

Yes this is possible, even though the default memory allocators reuse
storage locations in a FIFO manner, it is possible for a page object to
get allocated at the same address as another.
(Especially since the allocators tend to use several pools for
allocations of different sizes, but a "Model::Page" object for the same
plug-in will have the same size make use of the same pool increasing the
probability of address reuse.)
But since this depends on the actual implementation in question,
probably glibc's malloc, and I am not an expert on any of them, I feel
hard to pressure to numerically estimate the probability.

With trunk, this is impossible since "TileItem" removes itself from the
cache upon destruction. The best solution IMHO would be to key on the
"PageItem*" instead of "Model::Page*" and remove all entries with that
address in the destructor of "PageItem". This would also fit well
together with the fact, the "PageItem" will have to do the same
operation when adding an annotation or after hiding the form or
annotation overlay, i.e. "PageItem" will need a
remove-all-cache-entries-related-to-this-instance method anyway.

Best regards, Adam.

[Adam Reichold (adamreichold)]

Hello,

if you merge from trunk (revision 1593), you'll notice that I packaged the relevant parameter into a strcuture with ordering relations defined. I though that since this all boiler plate code, it might help you to concentrate on the actual functionality.

Best regards, Adam.

[lp:~mspacek/qpdfview/cachezoom]

1583. By Martin Spacek <email address hidden> on 2014-06-23

Merge from trunk

1584. By Martin Spacek <email address hidden> on 2014-06-23

No need to include model.h, clearer variable name

1585. By Martin Spacek <email address hidden> on 2014-06-23

Rename getPixmapKey -> pixmapKey, make it private and const

1586. By Martin Spacek <email address hidden> on 2014-06-23

Use PageItem instead of Page as cache key

[Martin Spacek (mspacek)]

OK, I've merged from the latest trunk. The key taps into the RenderParam structure now for various bits of info. Do the operators you added to the struct allow it to be used more directly as a key, instead of having to check each of its fields individually? I tried doing so, but couldn't get it to work. I kept getting call type errors for qHash. Also, shouldn't m_rect be part of the key? Something like:

QPair< PageItem*, RenderParam, QRect > TileItem::pixmapKey()

I've also tried to address the inline comments regarding private and const. Hope I did it right.

[Martin Spacek (mspacek)]

For the current revision of this branch, I'm noticing cache collisions on page refresh. Do I maybe need to do something in the PageItem destructor?

[Adam Reichold (adamreichold)]

Hello again,

Am 23.06.2014 15:44, schrieb Martin Spacek:
> For the current revision of this branch, I'm noticing cache
> collisions on page refresh. Do I maybe need to do something in the
> PageItem destructor?
>

yes, within the PageItem destructor, you need to find all entries
which belong to this page and remove them so they can't be reused.
This also implies that the PageItem pointer needs to be recoverable
from the cache key. (And as said earlier, you will need to do the same
thing whenever annotations or form fields change.)

Best regards, Adam.

[Adam Reichold (adamreichold)]

Hello again,

Am 23.06.2014 14:04, schrieb Martin Spacek:
> OK, I've merged from the latest trunk. The key taps into the
> RenderParam structure now for various bits of info. Do the
> operators you added to the struct allow it to be used more directly
> as a key, instead of having to check each of its fields
> individually? I tried doing so, but couldn't get it to work. I kept
> getting call type errors for qHash. Also, shouldn't m_rect be part
> of the key? Something like:
>
> QPair< PageItem*, RenderParam, QRect > TileItem::pixmapKey()

The comparison operators do not help. I assumed that QCache was based
on an ordered associative container like QMap, but it uses hashing,
i.e. QHash and hence we have to provide a hash function for our data
structure. We therefore need to provide an implemementation of "qHash"
[1] for our key type, which should be something like

QPair< PageItem*, QPair< RenderParam, QRect > >

or

struct CacheKey
{
 PageItem* pageItem;
 RenderParam renderParam;
 QRect rect;
};

where the first one could make direct use of "qHash(QPair)" if we have
a "qHash" implementation for the partial keys, but we actually only
need to provide "qHash(CacheKey)", maybe using the implementations in
the Qt header "qhash.h" [2] as a guide. (In the end, this will be
effectively the same as using a QString like you currently do to
obtain a hashable key, but to me it doesn't look right to use
"sprintf" for this kind of thing.)

And yes, the tile/rect should be part of the key but I left it out of
RenderParameter so that I can use a single instance of it stored in
PageItem rather than several copies in each TileItem.

> I've also tried to address the inline comments regarding private
> and const. Hope I did it right.
>

Yes, looks fine now. (The include for "model.h" seems unnecessary?)

Best regards, Adam.

[1] http://qt-project.org/doc/qt-5/qhash.html#the-qhash-hashing-function

[2]
https://qt.gitorious.org/qt/qtbase/source/4cb03924c113c74b99e18c7347278600a011e917:src/corelib/tools/qhash.h

[Adam Reichold (adamreichold)]

Hello again,

from what we discussed so far, I think the most efficient cache key type would be

QPair< PageItem*, uint >

with the second member being set to a "qHash-like" value created by hashing the RenderParam and the QRect. This way, we don't store any information in the cache the we don't need to recover for deleting all entries associated with a given PageItem instance.

Best regards, Adam.

[lp:~mspacek/qpdfview/cachezoom]

1587. By Martin Spacek <email address hidden> on 2014-06-23

Merge from trunk

1588. By Martin Spacek <email address hidden> on 2014-06-23

Remove unnecessary model.h include again

1589. By Martin Spacek <email address hidden> on 2014-06-23

Remove pixmap from cache on tile refresh

[Martin Spacek (mspacek)]

"Forgot something as it won't work like that since this would still be
the key and not the hash, i.e. we would have to collision-free w.r.t.
the second member and that seems are rather harsh requirement, so back
to "struct CacheKey" I'd say. Sorry for my thinking going off the rails..."

Hm, this is starting to sound a bit complicated. OK, so a CacheKey struct should be defined in global.h like this:

struct CacheKey
{
 PageItem* pageItem;
 RenderParam renderParam;
 QRect rect;
};

and it needs to be given a qHash method so that QCache knows how to hash it?

How does this solve the problem of removing from the QCache all the keys associated with a specific PageItem that we want to destroy?

[lp:~mspacek/qpdfview/cachezoom]

1590. By Martin Spacek <email address hidden> on 2014-06-24

Fix oops: don't remove cached pixmap on tile refresh

1591. By Martin Spacek <email address hidden> on 2014-06-24

Temporarily add page number back to cache key as hack to mitigate PageItem collisions

[Adam Reichold (adamreichold)]

Hello,

Am 23.06.2014 22:48, schrieb Martin Spacek:
> "Forgot something as it won't work like that since this would still
> be the key and not the hash, i.e. we would have to collision-free
> w.r.t. the second member and that seems are rather harsh
> requirement, so back to "struct CacheKey" I'd say. Sorry for my
> thinking going off the rails..."

I should take less and think more before doing it...

> Hm, this is starting to sound a bit complicated. OK, so a CacheKey
> struct should be defined in global.h like this:
>
> struct CacheKey { PageItem* pageItem; RenderParam renderParam;
> QRect rect; };
>
> and it needs to be given a qHash method so that QCache knows how to
> hash it?

Yes, this sounds about right. (Even though I would probably declare
"struct CacheKey {...};" and "qHash(const CacheKey&);" in "tileitem.h"
and implement the hashing function in "tileitem.cpp".)

But this is mainly a size optimization as to not store a textual
representation of these parameter but the parameter itself.

> How does this solve the problem of removing from the QCache all the
> keys associated with a specific PageItem that we want to destroy?

It doesn't. ;-) As I said, all this is a size (and hence to a degree
speed) optimization. You can definitely keep using "QPair< PageItem*,
QString >" until everything works functionally.

If you intend to continue using "QCache", they I don't think there is an
alternative to enumerating "QCache::keys()" and testing for "pageItem ==
this" in the PageItem destructor.
(If you want this to be faster, you'd need an ordered container like
QMap, where you can lexicographically order w.r.t. "CacheKey::pageItem"
and then the rest, so that "upperBound(this,...)" and
"lowerBound(this+1,...)" will give you any cache items associated with
"this" but you would have to reinvent all the LRU caching stuff.)
I'd suggest putting this into something like "PageItem::invalidateCache"
since you'll then just need to call this when changing annotations or
form fields as well. But especially for large caches with a lot of
entries, it will probably be rather slow...

Best regards, Adam.

[Martin Spacek (mspacek)]

OK. I was thinking another fast way to deal with invalidating a PageItem in the cache would be to keep a second, separate mapping of PageItems to cache keys. That way, all you need to do is a quick look up of the keys associated with the PageItem to be destroyed, and remove those keys from the cache. Of course, this extra mapping would take a bit more memory, but not nearly as much as the cache itself, because it's only storing keys, not pixmaps. If this was Python, I could throw something together in a few of minutes :)

[Adam Reichold (adamreichold)]

Hello Martin,

Am 24.06.2014 20:21, schrieb Martin Spacek:
> OK. I was thinking another fast way to deal with invalidating a PageItem in the cache would be to keep a second, separate mapping of PageItems to cache keys. That way, all you need to do is a quick look up of the keys associated with the PageItem to be destroyed, and remove those keys from the cache. Of course, this extra mapping would take a bit more memory, but not nearly as much as the cache itself, because it's only storing keys, not pixmaps. If this was Python, I could throw something together in a few of minutes :)
>

Yes, you could try to keep a separate list of all partial keys which
belong to a given PageItem instance, but then you would not notice when
a cache key is evicted by the cache and would potentially keep a lot
stale keys around which do not belong to the cache anymore. (Or you
would have to wrap the QPixmap inside a separate object whose destructor
tells your second list that it was destroyed and can be removed from
list. It just keeps getting better...)

Btw., I don't think C/C++ is the main problem, is rather trying to
encode a rather specific set of requirements into a rather general data
structure like QCache have only the cache key type to maneuver with. As
I said, the look-up part would become rather simple by using an ordered
map like QMap, but then you would need to duplicate the LRU logic (and
also penalize read access somewhat).

All in all, I'd also say that you are only a few minutes away from a
working if not optimal solution, just add "PageItem::invalideCache"
enumerating "TileItem::s_cache.keys()" and removing the items whose
pointer part of the key fits. Call this in "PageItem::~PageItem" and
when the annotation and form field overlays are hidden as well when an
annotation is added and you should have a basic working program. Then
you have all the time in the world to work on optimizing it. (And you
have something to do performance comparisons against...)

Best regards, Adam.

[lp:~mspacek/qpdfview/cachezoom]

1592. By Martin Spacek <email address hidden> on 2014-06-26

Merge from trunk

1593. By Martin Spacek <email address hidden> on 2014-06-28

Merge from trunk

1594. By Martin Spacek <email address hidden> on 2014-06-29

Merge from trunk

1595. By Martin Spacek <email address hidden> on 2014-07-21

Merge from lp:~adamreichold/qpdfview/cachezoom

[Martin Spacek (mspacek)]

Hi Adam,

Thanks for that! I've merged it in and pushed it back up to LP. Obviously, I haven't worked on this much lately. I only have a vague idea of what you did, and so far it seems to work. Can you remind me if there's anything else that needs to be done? Does it just require some longer-term testing before merging into trunk?

[Adam Reichold (adamreichold)]

Hello Martin,

I more or less just implemented what we talked about: When a PageItem is destroyed or changes internally (e.g. via a form field), all associated cache entries are searched and removed.

Basically, I would say this is feature complete and just needs more testing (which it will probably not see without being merged to trunk and hence being available via the dailydebs). But I would also like to have some sort of performance metric on how heavy the more involved cache key really is. A more compact representation of the cache key would be further bonus. (QDataStream does a lot of coding to keep its output platform-independent which we obviously don't need, but it's still more terse than a textual representation.)

Best regards, Adam.

[Martin Spacek (mspacek)]

OK, I'll keep running off this branch, see if I find any problems. So far, I haven't found any.

The other potential feature I think you mentioned before is a way to avoid having to iterate over all cache keys in TileItem::dropCachedPixmaps, maybe using a second mapping. But I don't know if the time savings from that would be worth the trouble.

[Adam Reichold (adamreichold)]

Am 23.07.2014 um 07:50 schrieb Martin Spacek:
> OK, I'll keep running off this branch, see if I find any problems. So far, I haven't found any.
>
> The other potential feature I think you mentioned before is a way to avoid having to iterate over all cache keys in TileItem::dropCachedPixmaps, maybe using a second mapping. But I don't know if the time savings from that would be worth the trouble.

From simple time measurements with this code and considering that this
is not done during normal drawing, I'd say the effort isn't worth it.

[Adam Reichold (adamreichold)]

Ok, since you did not seem to have found any more issues with this, I'll merge it into trunk so it receives more widespread testing. Thanks again for your contribution!

review: Approve

[Martin Spacek (mspacek)]

Great, thanks! Yup, still haven't encountered any problems.

=== modified file 'sources/pageitem.cpp'
--- sources/pageitem.cpp	2014-06-23 16:07:02 +0000
+++ sources/pageitem.cpp	2014-07-21 20:51:01 +0000
@@ -94,6 +94,8 @@
     hideAnnotationOverlay(false);
     hideFormFieldOverlay(false);
 
+    TileItem::dropCachedPixmaps(this);
+
     qDeleteAll(m_links);
     qDeleteAll(m_annotations);
     qDeleteAll(m_formFields);
@@ -208,7 +210,7 @@
 }
 
 
-void PageItem::refresh(bool keepObsoletePixmaps)
+void PageItem::refresh(bool keepObsoletePixmaps, bool dropCachedPixmaps)
 {
     if(!s_settings->pageItem().useTiling())
     {
@@ -222,6 +224,11 @@
         }
     }
 
+    if(dropCachedPixmaps)
+    {
+        TileItem::dropCachedPixmaps(this);
+    }
+
     update();
 }
 
@@ -703,7 +710,7 @@
             m_annotations.append(annotation);
             connect(annotation, SIGNAL(wasModified()), SIGNAL(wasModified()));
 
-            refresh();
+            refresh(false, true);
             emit wasModified();
 
             showAnnotationOverlay(annotation);
@@ -728,7 +735,7 @@
 
             annotation->deleteLater();
 
-            refresh();
+            refresh(false, true);
             emit wasModified();
         }
     }
@@ -798,7 +805,7 @@
             }
         }
 
-        refresh();
+        refresh(false, true);
     }
 }
 

=== modified file 'sources/pageitem.h'
--- sources/pageitem.h	2014-06-23 16:07:02 +0000
+++ sources/pageitem.h	2014-07-21 20:51:01 +0000
@@ -105,7 +105,7 @@
     void wasModified();
 
 public slots:
-    void refresh(bool keepObsoletePixmaps = false);
+    void refresh(bool keepObsoletePixmaps = false, bool dropCachedPixmaps = false);
 
     int startRender(bool prefetch = false);
     void cancelRender();

=== modified file 'sources/tileitem.cpp'
--- sources/tileitem.cpp	2014-06-23 16:07:02 +0000
+++ sources/tileitem.cpp	2014-07-21 20:51:01 +0000
@@ -33,7 +33,7 @@
 
 Settings* TileItem::s_settings = 0;
 
-QCache< TileItem*, QPixmap > TileItem::s_cache;
+QCache< TileItem::CacheKey, QPixmap > TileItem::s_cache;
 
 TileItem::TileItem(QObject* parent) : QObject(parent),
     m_rect(),
@@ -59,8 +59,17 @@
 {
     m_renderTask->cancel(true);
     m_renderTask->wait();
+}
 
-    s_cache.remove(this);
+void TileItem::dropCachedPixmaps(PageItem* page)
+{
+    foreach(CacheKey key, s_cache.keys())
+    {
+        if(key.first == page)
+        {
+            s_cache.remove(key);
+        }
+    }
 }
 
 void TileItem::paint(QPainter* painter, const QPointF& topLeft)
@@ -105,9 +114,9 @@
 {
     if(keepObsoletePixmaps && s_settings->pageItem().keepObsoletePixmaps())
     {
-        if(s_cache.contains(this))
+        if(s_cache.contains(cacheKey()))
         {
-            m_obsoletePixmap = *s_cache.object(this);
+            m_obsoletePixmap = *s_cache.object(cacheKey());
         }
     }
     else
@@ -119,12 +128,11 @@
 
     m_pixmapError = false;
     m_pixmap = QPixmap();
-    s_cache.remove(this);
 }
 
 int TileItem::startRender(bool prefetch)
 {
-    if(m_pixmapError || m_renderTask->isRunning() || (prefetch && s_cache.contains(this)))
+    if(m_pixmapError || m_renderTask->isRunning() || (prefetch && s_cache.contains(cacheKey())))
     {
         return 0;
     }
@@ -184,7 +192,7 @@
     if(prefetch && !m_renderTask->wasCanceledForcibly())
     {
         int cost = image.width() * image.height() * image.depth() / 8;
-        s_cache.insert(this, new QPixmap(QPixmap::fromImage(image)), cost);
+        s_cache.insert(cacheKey(), new QPixmap(QPixmap::fromImage(image)), cost);
     }
     else if(!m_renderTask->wasCanceled())
     {
@@ -197,13 +205,31 @@
     return qobject_cast< PageItem* >(parent());
 }
 
+TileItem::CacheKey TileItem::cacheKey() const
+{
+    PageItem* page = parentPage();
+    QByteArray key;
+
+    QDataStream(&key, QIODevice::WriteOnly)
+            << page->m_renderParam.resolution.resolutionX
+            << page->m_renderParam.resolution.resolutionY
+            << page->m_renderParam.scaleFactor
+            << page->m_renderParam.rotation
+            << page->m_renderParam.invertColors
+            << m_rect;
+
+    return qMakePair(page, key);
+}
+
 QPixmap TileItem::takePixmap()
 {
     QPixmap pixmap;
 
-    if(s_cache.contains(this))
+    if(s_cache.contains(cacheKey()))
     {
-        pixmap = *s_cache.object(this);
+        pixmap = *s_cache.object(cacheKey());
+
+        m_obsoletePixmap = QPixmap();
     }
     else
     {
@@ -213,7 +239,7 @@
             m_pixmap = QPixmap();
 
             int cost = pixmap.width() * pixmap.height() * pixmap.depth() / 8;
-            s_cache.insert(this, new QPixmap(pixmap), cost);
+            s_cache.insert(cacheKey(), new QPixmap(pixmap), cost);
         }
         else
         {

=== modified file 'sources/tileitem.h'
--- sources/tileitem.h	2014-06-25 11:59:57 +0000
+++ sources/tileitem.h	2014-07-21 20:51:01 +0000
@@ -49,6 +49,8 @@
     inline void dropPixmap() { m_pixmap = QPixmap(); }
     inline void dropObsoletePixmap() { m_obsoletePixmap = QPixmap(); }
 
+    static void dropCachedPixmaps(PageItem* page);
+
     void paint(QPainter* painter, const QPointF& topLeft);
 
 public slots:
@@ -70,9 +72,11 @@
 
     static Settings* s_settings;
 
-    static QCache< TileItem*, QPixmap > s_cache;
+    typedef QPair< PageItem*, QByteArray > CacheKey;
+    static QCache< CacheKey, QPixmap > s_cache;
 
     PageItem* parentPage() const;
+    CacheKey cacheKey() const;
 
     QRect m_rect;