D22767: Calculate first clickable point, from the top-left

[Konrad Materka]

kmaterka removed a reviewer: Plasma: Workspaces.
kmaterka added inline comments.

INLINE COMMENTS

> davidedmundson wrote in sniproxy.cpp:576
> This point was marked as closed, but not changed.  Probably my fault for not expanding.
> 
> We move the window so that it's at x-clickPoint
> The relative point is clickPoint.x
> 
> The absolute position of the click should be offset by the same amount. I would have expected
> 
> > event->root_x = x + clickPoint.x
> > event->root_y = y + clickPoint.y
> 
> in both places

//root_x/////root_y// coordinates are relative to the //root// window (in most cases = screen). Local //x// and //y// variables hold coordinates of the pointer, which is also relative to the //root// window. We are not changing pointer position nor root window position.
In original implementation embedable tray window was moved to the pointer location, so that pointer was at (0,0) relative to the tray window. Now we are moving it a little bit further (in the top-left direction), so the coordinates relative to the tray windows must change. If I understand this correctly, it is stored in //event_x/////event_y//. I don't have any experience with XCB, I just followed this documentation:
https://www.x.org/releases/X11R7.7/doc/man/man3/xcb_button_press_event_t.3.xhtml

> davidedmundson wrote in sniproxy.cpp:239
> This is quite expensive to do every frame (some animate), I think we can do something else, see below.

You are correct, calculate click point might be heavy (when set, it goers thou all window regions and calculates vector length). I will change that.

> davidedmundson wrote in sniproxy.cpp:530
> I don't think clickpoint has to be a member variable, we only use it within the context of this method.
> 
> We can make it a local variable evaluated within sendClick. Relative to frame updates, clicks are quite rare.

You are correct, calculate click point might be heavy (when set, it goers thou all window regions and calculates vector length). I will change that.

> davidedmundson wrote in sniproxy.cpp:551
> + m_clickPoint ?

Root X/Y should remain the same, we are not moving mouse pointer, only the underlying window. GTK is checking mouse state and mosue location, better not to mess with it.
Setting event_x/y is optional and my first implementation ignored this. Without event_x/y it looks bad for Wine applications. Wine is always showing context menu at [0,0] (or [event_x, event_y] to be precise) of the tray icon regardless of the pointer location. It is fine when underlying X window is aligned with the icon in the system tray - it is OK when menu always appears at the corner of the icon. Unfortunately our icon is not aligned, we are moving underlying window (so that click at x,y will hit the underlying window). Changing event_x tricks Wine and menu is shown at the mouse location.

REVISION DETAIL
  https://phabricator.kde.org/D22767

To: kmaterka, #plasma_workspaces
Cc: davidedmundson, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190805/31f1ea7d/attachment-0001.html>