Review Request: Separate caching for unthemed SVGs requesting colorization

[Aaron Seigo]

> On 2010-12-05 14:50:33, Manuel Mommertz wrote:
> > It is now even worse then before. Unthemed graphics still use the colors from plasma theme but don't get informed if the theme changes. this means, the colors that are used when first rendering the graphics stay even if I switch the theme.
> > The plasma elements itself don't get rerendered on theme change. Only graphics that have to be rerendered for another reason (resizing a widget, ...) get the svg's from the new activated theme.
> 
> Marco Martin wrote:
>     well, they shouldn't be informed on theme changes in this case...
> 
> Manuel Mommertz wrote:
>     right, but on colorscheme changes. with doesn't happen.

"Unthemed graphics still use the colors from plasma theme but don't get informed if the theme changes."

unthemed graphics never use colors from the plasma theme. they either use no colors, or they use the system colors. in which case are unthemed graphics using the color scheme from the plasma theme?

"the colors that are used when first rendering the graphics stay even if I switch the theme."

of course. the theme is not relevant. you get to pick between "themed" and "unthemed".

"The plasma elements itself don't get rerendered on theme change."

which plasma elements?

"Only graphics that have to be rerendered for another reason (resizing a widget, ...) get the svg's from the new activated theme."

are you saying that NO svg's (themed or unthemed) are being updated? because that works here.

"right, but on colorscheme changes. with doesn't happen."

do you mean the KDE color scheme change? e.g. when you change the colorscheme in the Colors control panel?

i'm quite confused now by what you mean. can you give me a clear, simple use case with detailed steps of reproduction so that i may identify and reproduce the problem and subsequently fix it? thanks.


- Aaron


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


On 2010-11-28 23:52:16, Aaron Seigo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6008/
> -----------------------------------------------------------
> 
> (Updated 2010-11-28 23:52:16)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> see: http://www.mail-archive.com/plasma-devel@kde.org/msg13494.html
> 
> this introduces a Theme object used by Svg to cache the rendering of svg's asking to be colorized, but which are not part of the theme. this requires an independent cache and set of color files, therefore a second Theme object. SvgPrivate::actualTheme() is thus overridden by SvgPrivate::cacheAndColorsTheme() whenever caching or colors are used.
> 
> there are two remaining defects of varying severity, though neither is very high imho:
> 
> a) if the system color palette changes when the application is not running (or an svg using such a theme is not running) the cache will not be dropped. this is an existing issue, however, not something introduce by this patchset
> 
> b) the new system color theme is shared for performance reasons, but once created it is never deleted. it ought to be reference counted so it can be cleaned up after in use, but this is currently not done.
> 
> several other fixes crept into this patchset (sorry :/) including:
> 
> * not dropping the cache just because we change themes; this made sense when just plasma-desktop used this or when the apps using Plasma::Theme all used the same theme. this is no longer the case, really, and having applications changing themes randomly kicking the cache out from underneath other apps that may still be using it is undesirable. this does mean that cache files will accumulate. not sure that's a big issue as they don't tend to be large and are in an area marked as "cache" (so ripe for clean-without-care)
> 
> * consolidate the signal/slot connections in SvgPrivate::checkColorHints, and now the check is consistent on all paths (previously, it wasn't!)
> 
> * missing (and possible cause of cache key ambiguity) separator in CACHE_ID_WITH_SIZE
> 
> * cleanups such as getting rid of superfluous use of Plasma:: namespacing in code that is now in libplasma, such as the stylesheeting. (it was from a plasmoid sebas wrote originally, iirc, explaining the historical reason for the explicit namespacing)
> 
> in any case, consider this a draft at this stage. i'm interested in geting feedback, improvements and testing.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/plasma/private/svg_p.h 1199366 
>   /trunk/KDE/kdelibs/plasma/svg.cpp 1201684 
>   /trunk/KDE/kdelibs/plasma/theme.cpp 1201685 
> 
> Diff: http://svn.reviewboard.kde.org/r/6008/diff
> 
> 
> Testing
> -------
> 
> just running normal plasma-desktop and what i use. needs testing with svg's that will actually rely on this feature.
> 
> 
> Thanks,
> 
> Aaron
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20101205/7ff2880d/attachment.htm