Review Request: allow SVGs to use systemcolors

[Manuel Mommertz]

> On 2010-09-30 13:10:03, Marco Martin wrote:
> > still not completely happy having to keep a separate xml dom, however, there are probably not going to be other ways for a while and the implementation seems pretty good, so modulo those little things it could go in

If QXmlStreamReader::parse() would be virtual, we could create a stream reader, that replaces this on the fly. But for now I see no other way of doing this. The only idea that could work is: Create a QIODevice that reads from another QIODevice with help of QXmlStreamReader and writes the data again with QXmlStreamWriter... But this would be much more code and I don't think that this would be faster at all.


> On 2010-09-30 13:10:03, Marco Martin wrote:
> > /trunk/KDE/kdelibs/plasma/svg.cpp, line 107
> > <http://svn.reviewboard.kde.org/r/5495/diff/1/?file=38739#file38739line107>
> >
> >     a check elements with the same id aren't existing should be done

You are right. But this would add more clutter. Maybe it is better to just leave the id as is. So no hint-uses-color-scheme but only current-color-scheme.


> On 2010-09-30 13:10:03, Marco Martin wrote:
> > /trunk/KDE/kdelibs/plasma/svg.cpp, line 441
> > <http://svn.reviewboard.kde.org/r/5495/diff/1/?file=38739#file38739line441>
> >
> >     so the coloring of the pixmap is still kept as retrocompatibility?

I don't want to break existing themes ;)


> On 2010-09-30 13:10:03, Marco Martin wrote:
> > /trunk/KDE/kdelibs/plasma/theme.cpp, line 320
> > <http://svn.reviewboard.kde.org/r/5495/diff/1/?file=38741#file38741line320>
> >
> >     i wonder how much is the benefit vs cost of caching this?

Both is quite low. ;) But as this stylesheet has to be generated for every Plasma::Svg object, which is 259 times for a start of plasma with the default setup + analog clock + calculator, it feels wrong to regenerate it that often.


> On 2010-09-30 13:10:03, Marco Martin wrote:
> > /trunk/KDE/kdelibs/plasma/theme.cpp, line 790
> > <http://svn.reviewboard.kde.org/r/5495/diff/1/?file=38741#file38741line790>
> >
> >     the button widget should be modified if this will start to be used

For what reason?


- Manuel


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


On 2010-09-30 11:41:09, Manuel Mommertz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5495/
> -----------------------------------------------------------
> 
> (Updated 2010-09-30 11:41:09)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> With this patch applied SVGs can put a style-element with id 'current-system-colors' in it, which gets replaced by a style with the current systemcolors. This allows SVGs to use colors like background color and text color from the system palette. Giving themes much more possibilitys then just coloring the resulting pixmap.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/plasma/theme.h 1180314 
>   /trunk/KDE/kdelibs/plasma/theme.cpp 1180314 
>   /trunk/KDE/kdelibs/plasma/svg.cpp 1180314 
> 
> Diff: http://svn.reviewboard.kde.org/r/5495/diff
> 
> 
> Testing
> -------
> 
> Changing theme, changing colorscheme
> 
> 
> Thanks,
> 
> Manuel
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20100930/57b6ddc7/attachment.htm