> On May 4, 2016, 10:18 a.m., Martin Gräßlin wrote:
> > src/platforms/xcb/kwindowsystem.cpp, lines 711-717
> > <https://git.reviewboard.kde.org/r/127809/diff/2/?file=462316#file462316line711>
> >
> >     This changes the behavior of the method. Now always NETWM is preferred, 
> > even if it doesn't provide the best icon.
> >     
> >     By returning early at this point you cannot evaluate what's the best 
> > fitting size.
> 
> Anthony Fieroni wrote:
>     How changes it? Return is 2 line below, at original code and all next 
> checks are !result.isNull(), i remove them and returning earlier, like 
> optimization :) Even with original it can returning null image because check 
> is above.

> Even with original it can returning null image because check is above.

Yes which is rather important. I showed you the code from KWin which is 
strongly written against the actual implementation and not written against the 
documentation (KWindowsystem used to be the "KWin lib" back in the days). If 
there are NETWM icons it uses them, even if there is a size missing. Thus the 
code needs to get the null icon. Otherwise it might add a wrong icon.

This is why I'm so scaptical about changing the code. We have two valid users 
of this API (KWin and libtaskmanager) and both have an implementation based on 
the insides of this method.


- Martin


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


On May 5, 2016, 6:50 p.m., Anthony Fieroni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127809/
> -----------------------------------------------------------
> 
> (Updated May 5, 2016, 6:50 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and Martin Gräßlin.
> 
> 
> Bugs: 362324
>     https://bugs.kde.org/show_bug.cgi?id=362324
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> -------
> 
> The api function is KWindowSystem::icon (WId win, int width=-1, int 
> height=-1, bool scale=false) so caller must get best size not worst when 
> width/height is not specified.
> 
> 
> Diffs
> -----
> 
>   src/platforms/xcb/kwindowsystem.cpp 5b7c65a 
> 
> Diff: https://git.reviewboard.kde.org/r/127809/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> before
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/05/01/6d718ef6-26cf-4866-94d2-4ffbdfc906fe__Screenshot_20160426_232109.png
> after
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/05/01/7dcab4ae-e451-4d43-8799-a0fcab471a3d__Screenshot_20160501_224642.png
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Reply via email to