> On Май 2, 2016, 5:02 след обяд, Martin Gräßlin wrote: > > What is the "best" size if it's not specified? Why is your change better > > than how it was? > > > > This change adjusts a very important part of the icon lookup functionality > > used by KWin and Plasma. I'm a little bit scared of touching it. > > > > The change does not come with a test, so I have problems understanding what > > you want to change. Could you please extend the tests to ensure that this > > does not break existing functionality? > > Anthony Fieroni wrote: > I, personally, don't know when NETWinInfo->icon or icccmIconPixmap can > fail but when it happend function returns icon with size 16x16 when is called > like this -> KWindowSystem::icon (wid); > > Martin Gräßlin wrote: > well that's what we have unit test for. They can show us whether the code > fails. > > Anthony Fieroni wrote: > xprop of dragonplayer > > _NET_WM_ICON_GEOMETRY(CARDINAL) = 674, 736, 164, 30 > _NET_WM_ALLOWED_ACTIONS(ATOM) = _NET_WM_ACTION_MOVE, > _NET_WM_ACTION_RESIZE, _NET_WM_ACTION_MINIMIZE, _NET_WM_ACTION_SHADE, > _NET_WM_ACTION_MAXIMIZE_VERT, _NET_WM_ACTION_MAXIMIZE_HORZ, > _NET_WM_ACTION_FULLSCREEN, _NET_WM_ACTION_CHANGE_DESKTOP, _NET_WM_ACTION_CLOSE > _KDE_NET_WM_FRAME_STRUT(CARDINAL) = 2, 2, 24, 4 > _NET_FRAME_EXTENTS(CARDINAL) = 2, 2, 24, 4 > _NET_WM_DESKTOP(CARDINAL) = 0 > _KDE_NET_WM_ACTIVITIES(STRING) = "153b4652-e23a-44c6-baf7-e7a0d812498a" > WM_STATE(WM_STATE): > window state: Normal > icon window: 0x0 > _NET_WM_STATE(ATOM) = > _NET_WM_ICON_NAME(UTF8_STRING) = > XdndAware(ATOM) = BITMAP > WM_NAME(STRING) = > _NET_WM_NAME(UTF8_STRING) = "???????? ?? ????? Dragon" > _KDE_NET_WM_USER_CREATION_TIME(CARDINAL) = 49919374 > _MOTIF_WM_HINTS(_MOTIF_WM_HINTS) = 0x3, 0x3e, 0x7e, 0x0, 0x0 > _NET_WM_WINDOW_TYPE(ATOM) = _NET_WM_WINDOW_TYPE_NORMAL > _XEMBED_INFO(_XEMBED_INFO) = 0x0, 0x1 > WM_CLIENT_LEADER(WINDOW): window id # 0x6c00006 > WM_HINTS(WM_HINTS): > Client accepts input or input focus: True > Initial state is Normal State. > _NET_WM_PID(CARDINAL) = 917 > _NET_WM_SYNC_REQUEST_COUNTER(CARDINAL) = 113246213 > WM_CLASS(STRING) = "dragon", "dragonplayer" > WM_PROTOCOLS(ATOM): protocols WM_DELETE_WINDOW, WM_TAKE_FOCUS, > _NET_WM_PING, _NET_WM_SYNC_REQUEST > WM_NORMAL_HINTS(WM_SIZE_HINTS): > user specified size: 651 by 295 > program specified minimum size: 651 by 295 > window gravity: Static > > NETWinInfo icon and icccmIconPixmap fails, i can add such a test, if it > not present. > So > http://api.kde.org/frameworks-api/frameworks5-apidocs/kwindowsystem/html/netwm_8cpp_source.html#l03544 > NETWinInfo trys to return the larger one, so this function must be > identical, at functionality > > Martin Gräßlin wrote: > Why do you show me the xprop of dragonplayer? What do you want to show me? > > I really have a hard problem understanding why you want to change the > behavior. Especially as the API documentation doesn't say anything about what > it does when width/height are not specified. We might break existing code > here! That's why I need to understand the motivation behind the change. I > need to understand why the change is better than the current one. > > Especially we need to exclude the possiblity of a misuse of the API. > Given the xprop there is no proper icon set on the window. This is a bug in > dragon player. > > Martin Gräßlin wrote: > If the main problem is the incorrect icon of dragon player I suggest to > try to fix it there: set a proper default icon with > QGuiApplication::setWindowIcon. Let's fix the bugs, where they are :-) > > Anthony Fieroni wrote: > So focus at > http://api.kde.org/frameworks-api/frameworks5-apidocs/kwindowsystem/html/netwm_8cpp_source.html#l03544 > when icon is found NETWinInfo::icon returns largest one (width/height == > -1), when is not find KWindowSystem::icon will return the smaller one. This > is api inconsistemcy. > > Anthony Fieroni wrote: > The problem is not only in Dragon, many applications not provide their > icons in that way. I can decide it's wrong or right, this is the facts.
I will correct plasma-framework too, for thumbnail icon size, but this must be correct too, about me. - Anthony ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127809/#review95084 ----------------------------------------------------------- On Май 2, 2016, 5:04 след обяд, Anthony Fieroni wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127809/ > ----------------------------------------------------------- > > (Updated Май 2, 2016, 5:04 след обяд) > > > 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