> On Май 6, 2016, 8:50 преди обяд, Martin Gräßlin wrote: > > src/declarativeimports/core/windowthumbnail.cpp, line 245 > > <https://git.reviewboard.kde.org/r/127826/diff/1/?file=464078#file464078line245> > > > > showing you the icon look up code from KWin, which is close to > > perfection (doesn't get the 256x256 icon yet): > > > > QIcon icon; > > auto readIcon = [this, &icon](int size, bool scale = true) { > > const QPixmap pix = KWindowSystem::icon(window(), size, size, > > scale, KWindowSystem::NETWM | KWindowSystem::WMHints, info); > > if (!pix.isNull()) { > > icon.addPixmap(pix); > > } > > }; > > readIcon(16); > > readIcon(32); > > readIcon(48, false); > > readIcon(64, false); > > readIcon(128, false); > > > > The problem here is that we don't specify from where to take the icon > > (we should always try to prefer NETWM) and don't fetch all possible sizes. > > This is rather important because we want neither upscaling, nor downscaling. > > Anthony Fieroni wrote: > We specified > http://api.kde.org/frameworks-api/frameworks5-apidocs/kwindowsystem/html/kwindowsystem_8cpp_source.html#l00485 > *return icon(win, width, height, scale, NETWM | WMHints | ClassHint | > XApp);* > > Martin Gräßlin wrote: > having written the code in question: I think that was just bad code on my > side. It's the fallback and I didn't spend much time on it. If I would have > thought properly about it, I would have taken KWin's code.
So, this code is correct. - Anthony ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127826/#review95221 ----------------------------------------------------------- On Май 3, 2016, 9:28 след обяд, Anthony Fieroni wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127826/ > ----------------------------------------------------------- > > (Updated Май 3, 2016, 9:28 след обяд) > > > Review request for Plasma, David Edmundson and Martin Gräßlin. > > > Repository: plasma-framework > > > Description > ------- > > I'm toking when app is minimized and when hover taskitem in task manager it's > drawed a thumnail icon window. So what is the real problem, some apps, many > as KDE', not provides their icons in window props and NETWinInfo fails to get > them. So in this case we must specified size explicitely to > KWindowSystem::icon. But this is not enough because KWindowSystem::icon makes > incorrect decision to crop icon to 48x48. Explanation the shots: > 1. Current code not specified size explicitely, returned icon is 16x16 > 2. New code wants icon 256x256 but KWindowSystem::icon ignore it and > returning icon 48x48 > 3. New code in 2 frameworks both, it works. > So i makes shots with Dragon but many applications are affected, even KDE' > akregator, ktimer, kalarm and so on, i don't start to comment other toolkit > apps, when we violates KDE'. > > > Diffs > ----- > > src/declarativeimports/core/windowthumbnail.cpp 251aaa4 > > Diff: https://git.reviewboard.kde.org/r/127826/diff/ > > > Testing > ------- > > Depends on https://git.reviewboard.kde.org/r/127809/ > > > File Attachments > ---------------- > > current plasma-framework and current kwindowsystem > > https://git.reviewboard.kde.org/media/uploaded/files/2016/05/03/f81bc88b-fe3b-4f00-b2f2-502effe4c245__Screenshot_20160503_202946.png > new plasma-framework, current kwindowsystem > > https://git.reviewboard.kde.org/media/uploaded/files/2016/05/03/36d7baa3-e875-420f-b2c2-39c2e5d409b9__Screenshot_20160503_203337.png > new plasma-framework, new kwindowsystem > > https://git.reviewboard.kde.org/media/uploaded/files/2016/05/03/857380fe-cdfd-478d-9df2-743f87d4c1ec__Screenshot_20160503_203946.png > > > Thanks, > > Anthony Fieroni > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
