> On April 28, 2016, 1:08 p.m., Milian Wolff wrote:
> > src/kiconloader.cpp, line 66
> > <https://git.reviewboard.kde.org/r/127779/diff/5/?file=461490#file461490line66>
> >
> >     this and the line below should be rewritten to
> >     
> >     namespace {
> >     QString NULL_EFFECT_FINGERPRINT() {
> >         return QStringLiteral("noeffect");
> >     }
> >     QString STYLESHEET_TEMPLATE() {
> >         return QStringLiteral("...");
> >     }
> >     }
> 
> Marco Martin wrote:
>     but this would instantiate a new one every time is used?

What would be instantiated? Are you aware of what `QStringLiteral` is doing?


- Milian


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


On April 28, 2016, 12: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, 12: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
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to