Code review comment for lp:~mspacek/qpdfview/cachezoom

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.