<table><tr><td style="">kmaterka removed a reviewer: Plasma: Workspaces.<br />kmaterka added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D22767">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D22767#inline-128759">View Inline</a><span style="color: #4b4d51; font-weight: bold;">davidedmundson</span> wrote in <span style="color: #4b4d51; font-weight: bold;">sniproxy.cpp:576</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">This point was marked as closed, but not changed.  Probably my fault for not expanding.</p>

<p style="padding: 0; margin: 8px;">We move the window so that it's at x-clickPoint<br />
The relative point is clickPoint.x</p>

<p style="padding: 0; margin: 8px;">The absolute position of the click should be offset by the same amount. I would have expected</p>

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p style="padding: 0; margin: 8px;">event->root_x = x + clickPoint.x<br />
event->root_y = y + clickPoint.y</p></blockquote>

<p style="padding: 0; margin: 8px;">in both places</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;"><em>root_x</em><em>/root_y</em> coordinates are relative to the <em>root</em> window (in most cases = screen). Local <em>x</em> and <em>y</em> variables hold coordinates of the pointer, which is also relative to the <em>root</em> window. We are not changing pointer position nor root window position.<br />
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 <em>event_x</em><em>/event_y</em>. I don't have any experience with XCB, I just followed this documentation:<br />
<a href="https://www.x.org/releases/X11R7.7/doc/man/man3/xcb_button_press_event_t.3.xhtml" class="remarkup-link" target="_blank" rel="noreferrer">https://www.x.org/releases/X11R7.7/doc/man/man3/xcb_button_press_event_t.3.xhtml</a></p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D22767#inline-128745">View Inline</a><span style="color: #4b4d51; font-weight: bold;">davidedmundson</span> wrote in <span style="color: #4b4d51; font-weight: bold;">sniproxy.cpp:239</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">This is quite expensive to do every frame (some animate), I think we can do something else, see below.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">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.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D22767#inline-128746">View Inline</a><span style="color: #4b4d51; font-weight: bold;">davidedmundson</span> wrote in <span style="color: #4b4d51; font-weight: bold;">sniproxy.cpp:530</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">I don't think clickpoint has to be a member variable, we only use it within the context of this method.</p>

<p style="padding: 0; margin: 8px;">We can make it a local variable evaluated within sendClick. Relative to frame updates, clicks are quite rare.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">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.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D22767#inline-128744">View Inline</a><span style="color: #4b4d51; font-weight: bold;">davidedmundson</span> wrote in <span style="color: #4b4d51; font-weight: bold;">sniproxy.cpp:551</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">+ m_clickPoint ?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">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.<br />
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.</p></div></div></div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D22767">https://phabricator.kde.org/D22767</a></div></div><br /><div><strong>To: </strong>kmaterka, Plasma: Workspaces<br /><strong>Cc: </strong>davidedmundson, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart<br /></div>