<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, May 18, 2017 at 6:26 AM, Pekka Paalanen <span dir="ltr"><<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, 2 May 2017 10:24:17 -0600<br>
Scott Moreau <<a href="mailto:oreaus@gmail.com">oreaus@gmail.com</a>> wrote:<br>
<br>
> This fixes wrong input coordinates when using clients like steam,<br>
> that draw their own decorations and have no shadows.<br>
> ---<br>
> xwayland/window-manager.c | 30 +++++++++++++++++-------------<br>
> 1 file changed, 17 insertions(+), 13 deletions(-)<br>
><br>
> diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c<br>
> index 2608075..7cb60d1 100644<br>
> --- a/xwayland/window-manager.c<br>
> +++ b/xwayland/window-manager.c<br>
> @@ -610,15 +610,17 @@ weston_wm_window_get_frame_<wbr>size(struct weston_wm_window *window,<br>
> {<br>
> struct theme *t = window->wm->theme;<br>
><br>
> - if (window->fullscreen) {<br>
> + if (window->decorate) {<br>
> + if (window->frame) {<br>
> + *width = frame_width(window->frame);<br>
> + *height = frame_height(window->frame);<br>
> + } else {<br>
> + *width = window->width + t->margin * 2;<br>
> + *height = window->height + t->margin * 2;<br>
> + }<br>
> + } else {<br>
> *width = window->width;<br>
> *height = window->height;<br>
> - } else if (window->decorate && window->frame) {<br>
> - *width = frame_width(window->frame);<br>
> - *height = frame_height(window->frame);<br>
> - } else {<br>
> - *width = window->width + t->margin * 2;<br>
> - *height = window->height + t->margin * 2;<br>
> }<br>
> }<br>
><br>
> @@ -628,14 +630,16 @@ weston_wm_window_get_child_<wbr>position(struct weston_wm_window *window,<br>
> {<br>
> struct theme *t = window->wm->theme;<br>
><br>
> - if (window->fullscreen) {<br>
> + if (window->decorate) {<br>
> + if (window->frame)<br>
> + frame_interior(window->frame, x, y, NULL, NULL);<br>
> + else {<br>
> + *x = t->margin;<br>
> + *y = t->margin;<br>
> + }<br>
> + } else {<br>
> *x = 0;<br>
> *y = 0;<br>
> - } else if (window->decorate && window->frame) {<br>
> - frame_interior(window->frame, x, y, NULL, NULL);<br>
> - } else {<br>
> - *x = t->margin;<br>
> - *y = t->margin;<br>
> }<br>
> }<br>
><br>
<br>
</div></div>Hi Scott,<br>
<br>
thanks for this, the handling of non-decorated, non-fullscreen, non-OR<br>
windows has probably been broken for a good while if not forever.<br></blockquote><div><br></div><div>Hi Pekka,<br><br></div><div>Thanks for the review.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
I tested with google-chrome-browser as that's what I had at hand, both<br>
with and without system decorations. Without system decorations, this<br>
patch indeed fixes both the input region (the shadow area was not<br>
click-through as it should have been) and removes shadows (that I<br>
assume should not have been there).<br></blockquote><div><br></div><div>I'm glad you mentioned this because I hadn't noticed, but this patch wasn't intended to remove decorations. I have a few other patches I'll submit soon to fix the input offset problem as well as a couple others I noticed. Hopefully they fix the input region for things like google-chrome shadow area problem as you describe.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Is the above exactly what you wanted to say in the commit message, or<br>
did Steam really get wrong input coordinates, i.e. clicks were off also<br>
inside the window?<br></blockquote><div><br></div><div>The commit message is poor at best; I've updated commit messages in the new patches.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Does this patch remove unwanted shadows from Steam or was Steam without<br>
shadows to begin with?<br></blockquote><div><br></div><div>It's not meant to remove any shadows. The new patch doesn't adjust size at all.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
The commit message is a little ambiguous on these terms, so I'd like<br>
(someone) to re-word it.<br>
<br>
When using system decorations in Chrome, the decorations look and work<br>
just fine also after the patch.<br></blockquote><div><br></div><div>The thing that doesn't work is toggling the system decoration switch without having to restart chrome. Also as a side note, moving a window from its original starting position causes override redirect windows to be placed as if the window were never moved.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Maximizing xterm works ok.<br>
<br>
I'm a little worried about removing the checks for window->fullscreen,<br>
because e.g. weston_wm_window_draw_<wbr>decoration() looks at<br>
window->fullscreen and does nothing. It would feel more logical for<br>
draw_decoration to use the same conditions as get_frame_size and<br>
get_child_position. What do you think? Which conditions should those be?<br></blockquote><div><br></div><div>Yes, I see what you mean. I left fullscreen logic alone in the new patch.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
weston_wm_window_configure(), call arranged from send_configure(), is<br>
unconditionally using get_frame_size and get_child_position as well.<br>
<br>
Also weston_wm_window_set_pending_<wbr>state() and send_configure() are<br>
looking at both window->decorate and window->fullscreen. It all<br>
seems to imply that if _NET_WM_STATE_FULLSCREEN is in effect, then<br>
decorations should be avoided regardless of the _MOTIF_WM_HINTS<br>
MWM_DECOR_* flags. Does that make sense?<br>
<br>
Looking at weston_wm_window_draw_<wbr>decoration(), it seems that shadows<br>
for undecorated non-fullscreen surfaces were actually intentional. If<br>
we remove that intention, the drawing of the shadow should also be<br>
removed, and the commit message should explain why that was wrong.<br>
<br>
In summary, I would suggest changing<br>
if (window->decorate)<br>
into<br>
if (window->decorate && !window->fullscreen)<br>
<br>
In fact, testing with e.g. geany, this patch regresses fullscreening: a<br>
fullscreen X11 window is shifted down with a black bar on top and the<br>
bottom going out of screen. The same can be seen with xterm.<br>
<br>
<br>
Thanks,<br>
pq<br>
</blockquote></div><br></div><div class="gmail_extra">Hopefully you can test the new series and see if there are any regressions or concerns.<br><br><br>Thanks again,<br><br></div><div class="gmail_extra">- Scott<br></div></div>