Review Request 112079: Add utility function to paint icon overlays

[Aurélien Gâteau]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112079/#review37765
-----------------------------------------------------------



staging/kguiaddons/src/util/qiconoverlay.h
<http://git.reviewboard.kde.org/r/112079/#comment27976>

    Since the methods are in a KIconUtils namespace, maybe their name can be simplified to "addOverlay()"?



staging/kguiaddons/src/util/qiconoverlay.h
<http://git.reviewboard.kde.org/r/112079/#comment27975>

    The QHash should be passed as a const-ref



staging/kguiaddons/src/util/qiconoverlay.cpp
<http://git.reviewboard.kde.org/r/112079/#comment27964>

    Qt doc says you should inherit from QIconEngineV2 instead



staging/kguiaddons/src/util/qiconoverlay.cpp
<http://git.reviewboard.kde.org/r/112079/#comment27970>

    Maybe mark this method as Q_DECL_OVERRIDE now that we can use it?



staging/kguiaddons/src/util/qiconoverlay.cpp
<http://git.reviewboard.kde.org/r/112079/#comment27971>

    Q_DECL_OVERRIDE as well if you change the class to inherit from QIconEngineV2



staging/kguiaddons/src/util/qiconoverlay.cpp
<http://git.reviewboard.kde.org/r/112079/#comment27965>

    Should be const



staging/kguiaddons/src/util/qiconoverlay.cpp
<http://git.reviewboard.kde.org/r/112079/#comment27972>

    Q_DECL_OVERRIDE



staging/kguiaddons/src/util/qiconoverlay.cpp
<http://git.reviewboard.kde.org/r/112079/#comment27973>

    Q_DECL_OVERRIDE



staging/kguiaddons/src/util/qiconoverlay.cpp
<http://git.reviewboard.kde.org/r/112079/#comment27974>

    Q_DECL_OVERRIDE



staging/kguiaddons/src/util/qiconoverlay.cpp
<http://git.reviewboard.kde.org/r/112079/#comment27967>

    Maybe prefix member vars with "m_"? (I think it is part of kdelibs coding style)



staging/kguiaddons/src/util/qiconoverlay.cpp
<http://git.reviewboard.kde.org/r/112079/#comment27968>

    ditto


- Aurélien Gâteau


On Aug. 14, 2013, 12:49 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112079/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2013, 12:49 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Description
> -------
> 
> As this was rejected from QIcon, I decided to implement it in frameworks. The idea is new private icon engine (KOverlayIconEngine), which paints the overlays itself, and then an utility function in KIconUtils:: namespace, which takes the base icon and the overlay icons as parameters and constructs new QIcon using KOverlayIconEngine. As QPixmapIconEngine (QIcon default) is private in Qt and couldn't be simply extended, the KOverlayIconEngine simply forwards the calls to QIcon, which then forwards those calls internally to QPixmapIconEngine (QIcon is really mostly just a wrapper around an engine).
> 
> There are two functions
> 
> QIcon KIconUtils::kIconAddOverlay(const QIcon &icon, const QIcon &overlay, Qt::Corner position);
> 
> and
> 
> QIcon KIconUtils::kIconAddOverlay(const QIcon &icon, QHash<Qt::Corner, QIcon> overlays);
> 
> The first one serves for single overlay icon, the second one is then for multiple overlays, requires the QHash to be built up before passing though.
> 
> 
> Diffs
> -----
> 
>   staging/kguiaddons/src/CMakeLists.txt fc8941a 
>   staging/kguiaddons/src/util/qiconoverlay.h PRE-CREATION 
>   staging/kguiaddons/src/util/qiconoverlay.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/112079/diff/
> 
> 
> Testing
> -------
> 
> I have a small testing app with 3 QLabels, testing both ::paint() and ::pixmap() methods, all works \o/ I'm thinking about bundling the app with kguiaddons and/or additionally creating a unittest for it (I have one for Qt already), but I'll do that later as I'm off to vacation and I'd like to get comments for the code.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130814/93a844ce/attachment.html>