Review Request 127779: use system colors for monochrome icons

[Aleix Pol Gonzalez]

> On April 28, 2016, 3:52 p.m., Aleix Pol Gonzalez wrote:
> > src/kiconloader.cpp, line 825
> > <https://git.reviewboard.kde.org/r/127779/diff/5/?file=461490#file461490line825>
> >
> >     maybe return contents?
> 
> Marco Martin wrote:
>     contents of what?

`return contents;`


> On April 28, 2016, 3:52 p.m., Aleix Pol Gonzalez wrote:
> > src/kiconloader.cpp, line 870
> > <https://git.reviewboard.kde.org/r/127779/diff/5/?file=461490#file461490line870>
> >
> >     I'd only use QBuffer in the case of svg.
> 
> Marco Martin wrote:
>     I have to get back to the first version of the patch with processSvg inline then tough

Why?
```
QScopedPointer<QIODevice> device;
if (svg) device.reset(processStuff());
else device.reset(new QFile(path));
```


> On April 28, 2016, 3:52 p.m., Aleix Pol Gonzalez wrote:
> > src/kiconloader.cpp, line 823
> > <https://git.reviewboard.kde.org/r/127779/diff/5/?file=461490#file461490line823>
> >
> >     Isn't it possible to use text processing or QXmlReader?
> >     
> >     DOM penalty is not negligible.
> 
> Marco Martin wrote:
>     well, the fact is that i have to actually write in the xml, not just read

How about QXmlStreamReader + QXmlStreamWriter?


- Aleix


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


On April 28, 2016, 2:59 p.m., Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127779/
> -----------------------------------------------------------
> 
> (Updated April 28, 2016, 2:59 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> -------
> 
> Breeze uses stylesheet information to color its icons with some "named" colors that change depending from the current system color scheme, this is already used since some time in the icons used by the Plasma shell.
> But QWidget applications have the same problem, when the user changes the color scheme from breeze to breeze dark (or any color scheme), most of the icons will be black on black.
> This patch clones (a bit simplified and taking only the most "important" colors) the logic used by Plasma::Svg to color icons with the stylesheet.
> 
> even tough it's doing more things at icon generation, an application that uses a lot of icons like Dolphin doesn't seem to have noticeable startup time difference, even when the image cache is not present yet, so i hope is not an unacceptable performance tradeoff (successive loads are unchanged as are from the image cache).
> 
> still some questions and things that can be optimized, like
> 
> * an optional key in the theme metadata file to explicitly enable this, to not have it running in themes that don't care?
> 
> * can i use karchive in this framework?, so it would work with svgz icons as well
> 
> * right now to refresh icons at runtime it depends from a patch in the colors kcm to emit iconchanges as well, alternatively kiconloader could watch for kcolorscheme changes as well, but i don't think is strictly necessary
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 2e838e8 
>   src/CMakeLists.txt 0e30a35 
>   src/kiconloader.cpp 75ab482 
> 
> Diff: https://git.reviewboard.kde.org/r/127779/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> dadel1.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/04/28/0fc42425-947c-479e-9759-09c7a703a456__dadel1.png
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160428/d8fed476/attachment.html>