> On Jan. 21, 2015, 10:07 a.m., Martin Gräßlin wrote: > > ping - do we want this approach to go in? > > Thomas Lübking wrote: > 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 :-(
I completely agree with your two concerns and also thought about the same. Concerning point 1 I wanted to make them another ::icon overload but that's not possible - it would be ambiguous :-(. windowIcon might indeed be a better name. Concerning point 2: I thought about combining all items. E.g. if NETWM and WM_HINTS are set, it will include both. But if NETWM provides a size which WM_HINTS doesn't it wouldn't be set. But that wouldn't work with the app or the X icon. I have to think a little bit more about it. - Martin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122086/#review74460 ----------------------------------------------------------- On Jan. 21, 2015, 10:06 a.m., 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, 10:06 a.m.) > > > 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 > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
