> On April 7, 2016, 9:45 a.m., Martin Gräßlin wrote: > > src/kwindowsystem.cpp, lines 495-496 > > <https://git.reviewboard.kde.org/r/124648/diff/1/?file=390797#file390797line495> > > > > This part looks wrong to me and I think is the reason why reading icons > > fails in kwin_wayland now. > > > > We are dealing here with real pixels. E.g. kwin asking for the icon in > > size 16x16, because the icon is in 16x16 on the window property. By > > multiplying here it picks a different icon, e.g. 32x32, which might not > > even exist on the window. > > > > I think the scaling needs to be done by the user of the API. > > David Edmundson wrote: > kwin doesn't have a DPR set, and shouldn't. I doubt that's the reason. > Also how are you checking what icons exist in the window? Arguably they > should be scaled down. > > Martin Gräßlin wrote: > > Also how are you checking what icons exist in the window? > > KWin code just tries to get all available icon sizes which are normally > installed on a window. That is currently: > > > readIcon(16); > readIcon(32); > readIcon(48, false); > readIcon(64, false); > readIcon(128, false); > > All those pixmaps are combined into one QIcon. So it just shouldn't need > any scaled values. Also note that the method call takes an argument "bool > scale" - I find it strange that I have an API where I say "don't scale the > icon" and it scales it. > > The problem with adjusting the value here is that this is a really low > level method with different behavior. Depending on the flags it tries to get > the icon from different sources. E.g. it first tries to get the icon > installed as a property, then tries to resolve from the legacy installed > bitmap, then from class hint. Due to the added scaling it can happen that a > smaller icon gets returned. E.g. try Firefox, it installs a max icon of 48, > but also has the legacy bitmap set. When you now request the icon in size 48, > you cannot get it (there is never an icon with size 96) and falls back to the > 16x16 legacy bitmap. Not what one wants.
If you want to give it a try: https://git.reviewboard.kde.org/r/122086/ contains a test app for all different icons on a window. It should help understanding what will happen if you start to scale the passed in values. I'll try to extract that test app without the API change. - Martin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124648/#review94357 ----------------------------------------------------------- On Aug. 7, 2015, 11:47 p.m., David Edmundson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/124648/ > ----------------------------------------------------------- > > (Updated Aug. 7, 2015, 11:47 p.m.) > > > Review request for KDE Frameworks and Plasma. > > > Repository: kwindowsystem > > > Description > ------- > > Qt scales down the size of QScreens by the device pixel ratio; we should > make our windowing APIs match. > > X (and KWin) deal with device dependent pixels, so everything needs to > be converted when communitcating geometry. > > Abstraction happens in the main kwindowsystem so X and Wayland are both > supported. > > BUG: 350865 > BUG: 350614 > BUG: 347951 > > > Diffs > ----- > > src/kwindowsystem.cpp 0f8ec0ef470b3a3dcd353a1052dc80ed2bb3f992 > > Diff: https://git.reviewboard.kde.org/r/124648/diff/ > > > Testing > ------- > > Ran yakuake, it looks all right again. > Re-enabled system DPR support in plasmashell (which is currently disabled) > and tested notifications and panel struts are all sensible. > > > Thanks, > > David Edmundson > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
