> On Jan. 29, 2014, 1:50 p.m., Aaron J. Seigo wrote: > > plasma/theme.cpp, lines 273-276 > > <https://git.reviewboard.kde.org/r/115326/diff/3/?file=240803#file240803line273> > > > > it's nice to clean up the svg elements files; however without an active > > cache file it doesn't matter and they are a couple dozen K in size. didn't > > seem worth it for cache directory maintenance. > > > > leaving them on disk would result them in them being used again if the > > user installs a newer version of the theme, then later rolls back to an > > older version. however, given that the svg file should be tied to the > > version of the theme, all this accomplishes is having to re-calculate all > > the svg geometry data. > > > > i don't see the point in doing this at all. > > Harald Sitter wrote: > well, from a downgrade POV perhaps the kimagecache cleanup also ought not > delte all versions but those older than one or two versions down? that is, if > downgrading is actually a viable use case of course. otherwise the logic > would seem a bit incosistent. yeah, the kcache is sustantially larger, but I > doubt any piece of software iterating the cache dir will care much for that > reasoning when the dir suddenly contains > > plasma-svgelements-yolo_v1 > plasma-svgelements-yolo_v2 > plasma-svgelements-yolo_v3 > plasma-svgelements-yolo_v4 > plasma-svgelements-yolo_v5 > plasma-svgelements-yolo_v6 > ... > > > > Aaron J. Seigo wrote: > "from a downgrade POV perhaps the kimagecache cleanup also ought not > delte all versions but those older than one or two versions down?" > > besides requiring a version parser / standard that gets followed, this is > overly complex. downgrading themes is not common, which is why the caches are > removed. > > "otherwise the logic would seem a bit incosistent." > > i don't see how: it keeps the cache for the current version, drops the > rest. > > "yeah, the kcache is sustantially larger" > > which is a significant issue on devices with smaller disks or many users. > > "but I doubt any piece of software iterating the cache dir will care much > for that reasoning when the dir suddenly contains" > > it's not about iterating the dir, but using substantial amounts of disk > space for no reason. > > Harald Sitter wrote: > So, what do you want me to do, or not to do? I am not following sorry :/ > > Aaron J. Seigo wrote: > The code ought to: > > * version both cache files to reflect the version # of the theme > * drop all other image caches (new / old, doesn't matter -> whatever > isn't being used should be removed; elements cache files could also be > removed but aren't a significant issue as they are a few dozen kb) > > Can you try the patch posted here: http://pastebin.kde.org/p6czlim0q >
Okeydokey, new diff should cover everything then. - Harald ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115326/#review48535 ----------------------------------------------------------- On Jan. 30, 2014, 10:56 a.m., Harald Sitter wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115326/ > ----------------------------------------------------------- > > (Updated Jan. 30, 2014, 10:56 a.m.) > > > Review request for Plasma, Aaron J. Seigo and Martin Klapetek. > > > Repository: kdelibs > > > Description > ------- > > - to decide whether or not to discard a cache type now the mtimes of > metadata.desktop and the pixmap cache file (previously this was an invalid > file compared to cache change time) > - the discard check now compares the mtime of the actual pixmap cache file > and the actual metadata file to ensure that we are comparing values with > equal meaning (previously the kimagecache modification time was used which > appears to be the creation time of the object by default) > - whether the cache needs to be discard is decided before kimagecache is > created to avoid it altering the mtime, actual discarding still happens after > initialization of the pixmap cache > - introduced a new themeVersion member on the private class > - svgelements cache is now using a versioned cache file whenever themeVersion > is not empty > - introduce svgelements cache maintenance in useCache() > - pixmap cache is now using a versioned cache file whenever themeVersion is > not empty (previously the wrong name var was used) > - various variables inside useCache had their names adjusted to clearify > their purpose. > > > Diffs > ----- > > plasma/theme.cpp cb44878 > > Diff: https://git.reviewboard.kde.org/r/115326/diff/ > > > Testing > ------- > > cache file lineup: > cache/plasma-svgelements-default > cache/plasma-svgelements-default_v1.9 > cache/plasma-svgelements-default_v2.0 (no pix cache) > cache/plasma_theme_default.kcache > cache/plasma_theme_default_v1.9.kcache > cache/plasma_theme_default_v2.1.kcache (no svg cache) > > version of default theme for testing: 2.2 > > [T1] initial run without v2.2 caches: > deleted all previous caches, correctly created > plasma-svgelements-default_v2.2 and plasma_theme_default_v2.2.kcache. > > [T2] subsequent run with v2.2 cache present: > deleted all prevoius caches, existing v2.2 cache not deleted or discard by > useCache(). > > [T3] subsequent run with v2.2 cache present and `touch metadata.desktop`: > deleted all prevoius caches, existing v2.2 cache not deleted, but discarded > by useCache(). > > [T4] subsequent run with v2.2. cache, but default theme has no version > anymore: > deleted *all* caches, discarded (newly created) empty caches > plasma-svgelements-default and plasma_theme_default.kcache. > > > Thanks, > > Harald Sitter > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
