> On Jan. 29, 2014, 1:50 p.m., Aaron J. Seigo wrote:
> > this patch doesn't actually improve anything. in fact, the actual bug is 
> > left:
> > 
> >  pixmapCache = new KImageCache(pixmapCacheFileName, config.themeCacheKb() * 
> > 1024);
> > 
> > pixmapCacheFileName should have the version # in it. this was an oversight 
> > on my part when fixing br320855 in 59e88c777b92935cc95f28e52f84fe97563dec9d
> > 
> > this patch should not go in as is, but instead have the cache file name 
> > ammended to include the version information.
> 
> Harald Sitter wrote:
>     the point of this patch is not to fix cache versioning but cache cleaning 
> such that http://i.imgur.com/v0Zyees.png does not happen. further more as 
> mentioned in the description, fixing the cache versioning goes beyond the 
> KImageCache construction but also requires the entire logic to be created for 
> the svgelements cache. to that extent this patch is nothing but a hotfix and 
> a step towards correct cache handling, not a complete fix.
> 
> Aaron J. Seigo wrote:
>     "fixing the cache versioning goes beyond the KImageCache construction but 
> also requires the entire logic to be created for the svgelements cache."
>     
>     right; so what should be happening is that the image cache and the 
> elements cache should both be created with the version number in their name. 
> that keeps them in sync with each other and with the current contents of the 
> theme.
>     
>     the cache file is indeed being created w/out the version # in it, which 
> is a regression introduced in 59e88c777b92935cc95f28e52f84fe97563dec9d. 
> putting the version # back in the cache file name should be sufficient.
>     
>     then in discardCache the version string used in the image cache filename 
> should also be used to initialize svgElementsCache.
>     
>     probably the easiest way to manage this is to add a member to the Private 
> class for the version # string..

something like this: http://pastebin.kde.org/p6czlim0q


- Aaron J.


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


On Jan. 28, 2014, 9:18 a.m., Harald Sitter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115326/
> -----------------------------------------------------------
> 
> (Updated Jan. 28, 2014, 9:18 a.m.)
> 
> 
> Review request for Plasma, Aaron J. Seigo and Martin Klapetek.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> -------
> 
> - introduce svgelements cache handling in useCache()
> - to decide whether or not to discard a cache type now the mtimes of 
> metadata.desktop and the respective cache file are compared (previously this 
> was an invalid file compared to cache change time)
> - cache cleanup now happens entirely before creation of the KImageCache 
> instance to prevent any sort of problem there
> 
> - add fixme comment about broken version-based cache handling. for one the 
> version specific file name is not actually used for the KImageCache and for 
> another the entire shebang is missing from svgelements all together. this has 
> the very unfortunate side effect that the *active* cache files are deleted 
> ever so often, it may be worth removing the code portions until someone finds 
> time/interest to actually implement this in a working fashion.
> 
> 
> Diffs
> -----
> 
>   plasma/theme.cpp cb44878 
> 
> Diff: https://git.reviewboard.kde.org/r/115326/diff/
> 
> 
> Testing
> -------
> 
> upgraded from 4.8 straight to 4.12 -> theme caches correctly discarded
> 
> 
> Thanks,
> 
> Harald Sitter
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to