Review Request 122086: Add new method KWindowSystem::icons

[Thomas Lübking]

> On Jan. 21, 2015, 9:07 vorm., Martin Gräßlin wrote:
> > ping - do we want this approach to go in?

I've two concerns about this.
1. the name "icons" implies "several" (it's unfortunate that ::icon isn't ::iconPixmap) - maybe "windowIcon()" or similar
2. the behavior how an icon is build up from multiple sources (isn't)
   right now, you could end up w/ one 16x16 NETWM icon pixmap.
   Considerations: NETWM and WM_HINTS could be more specific than an icon from the appname, the appname could though be more size complete and finally WM_HINTS would likely be "ugly" compared to the other two (bitmasked). Overmore it might be undesirable to end up w/ an icon that has completely different looks for different sizes.
   As we don't have a specific hint on the required target size, it gets harder to ever take an informed decision on what to return here, but one might want to at least check the sizes provided by NEWTM before returning early (if other sources are allowed by flags) - yes, I know that this is not a straight suggestion for improvement :-(


- Thomas


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


On Jan. 21, 2015, 9:06 vorm., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122086/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2015, 9:06 vorm.)
> 
> 
> Review request for KDE Frameworks and kwin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> -------
> 
> ::icons tries to retrieve all available icon sizes and combine them
> into a single QIcon. On the X11 platform this can significantly
> reduce the time needed to fetch all available icons compared to the
> already existing ::icon methods which return a QPixmap of a specific
> size.
> 
> The change includes a new test application "icontest" which shows all
> available icons in all available sizes for a window id passed as
> command line argument.
> 
> CHANGELOG: New method QIcon KWindowSystem::icons(WId win, int flags)
> 
> 
> Diffs
> -----
> 
>   src/kwindowsystem.h 322322f12dda7279567be8420533ed22cffffd52 
>   src/kwindowsystem.cpp 65d215b6dfbf4df22e871fd7187fface75abb61b 
>   src/kwindowsystem_p.h 1f01145b5c7efe925fcb8242f92af17b704e01c9 
>   src/kwindowsystem_p_x11.h 0d4b6ba551776d2fa08030f78226ecdb3c80c889 
>   src/kwindowsystem_x11.cpp bf958ae63b48424fc412405259f082b740928f9a 
>   tests/CMakeLists.txt ce68cc505a69ea9a3cf645e9ae587bd89abe1648 
>   tests/icontest.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/122086/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

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