Review Request: Use correct pixmap cache file for theme

[Aaron Seigo]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6100/#review9220
-----------------------------------------------------------


i agree with the analysis of the problem: the pixmapCache object is not always reset in ThemePrivate::setThemeName. there some issues with the implementation, however, that need to be addressed before it can be committed.


/trunk/KDE/kdelibs/plasma/theme.cpp
<http://svn.reviewboard.kde.org/r/6100/#comment10089>

    whitespace, should be: "if (themeName"



/trunk/KDE/kdelibs/plasma/theme.cpp
<http://svn.reviewboard.kde.org/r/6100/#comment10090>

    whitespace: if (info



/trunk/KDE/kdelibs/plasma/theme.cpp
<http://svn.reviewboard.kde.org/r/6100/#comment10092>

    this method's signature is leading to hard to read code and is going to be error prone. if we indeed move to a two-boolean method here, i'd prefer to see an enumeration used as flags in ThemePrivate. e.g.
    
    enum { NoCache = 0, PixmapCache = 1, SvgElementsCache = 2 } CacheType;
    Q_DECLARE_FLAGS(CacheTypes, CacheType)
    
    void ThemePrivate::discardCache(CacheTyeps caches)
    
    and then calls would become sth like:
    
    discardCache(PixmapCache | SvgElementsCache)
    
    this would reverse the meaning the the current booleans, but then the code becomes far easier to read as a result.



/trunk/KDE/kdelibs/plasma/theme.cpp
<http://svn.reviewboard.kde.org/r/6100/#comment10087>

    it is unintuitive that a boolean called "keepPixmapCache" results in the pixmap cache being deleted.
    
    a comment should be included here noting that deleting the pixmapCache object does not remove the on-disk backing.



/trunk/KDE/kdelibs/plasma/theme.cpp
<http://svn.reviewboard.kde.org/r/6100/#comment10088>

    this may as well just be an else if (pixmapCache)


- Aaron


On 2010-12-12 16:31:51, Manuel Mommertz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6100/
> -----------------------------------------------------------
> 
> (Updated 2010-12-12 16:31:51)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> Currently when switching themes, the ondisk cache for pixmaps don't switch to the correct file. This leads to not updated SVGs on theme change.
> 
> (For some reason folderview doesn't update. But currently no graphic updates.)
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/plasma/theme.cpp 1205716 
> 
> Diff: http://svn.reviewboard.kde.org/r/6100/diff
> 
> 
> Testing
> -------
> 
> Switching themes, system colors and compositing.
> 
> 
> Thanks,
> 
> Manuel
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20101212/43d5cdb0/attachment-0001.htm