On Thu, May 18, 2017 at 6:26 AM, Pekka Paalanen <[email protected]> wrote:
> On Tue, 2 May 2017 10:24:17 -0600 > Scott Moreau <[email protected]> wrote: > > > This fixes wrong input coordinates when using clients like steam, > > that draw their own decorations and have no shadows. > > --- > > xwayland/window-manager.c | 30 +++++++++++++++++------------- > > 1 file changed, 17 insertions(+), 13 deletions(-) > > > > diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c > > index 2608075..7cb60d1 100644 > > --- a/xwayland/window-manager.c > > +++ b/xwayland/window-manager.c > > @@ -610,15 +610,17 @@ weston_wm_window_get_frame_size(struct > weston_wm_window *window, > > { > > struct theme *t = window->wm->theme; > > > > - if (window->fullscreen) { > > + if (window->decorate) { > > + if (window->frame) { > > + *width = frame_width(window->frame); > > + *height = frame_height(window->frame); > > + } else { > > + *width = window->width + t->margin * 2; > > + *height = window->height + t->margin * 2; > > + } > > + } else { > > *width = window->width; > > *height = window->height; > > - } else if (window->decorate && window->frame) { > > - *width = frame_width(window->frame); > > - *height = frame_height(window->frame); > > - } else { > > - *width = window->width + t->margin * 2; > > - *height = window->height + t->margin * 2; > > } > > } > > > > @@ -628,14 +630,16 @@ weston_wm_window_get_child_position(struct > weston_wm_window *window, > > { > > struct theme *t = window->wm->theme; > > > > - if (window->fullscreen) { > > + if (window->decorate) { > > + if (window->frame) > > + frame_interior(window->frame, x, y, NULL, NULL); > > + else { > > + *x = t->margin; > > + *y = t->margin; > > + } > > + } else { > > *x = 0; > > *y = 0; > > - } else if (window->decorate && window->frame) { > > - frame_interior(window->frame, x, y, NULL, NULL); > > - } else { > > - *x = t->margin; > > - *y = t->margin; > > } > > } > > > > Hi Scott, > > thanks for this, the handling of non-decorated, non-fullscreen, non-OR > windows has probably been broken for a good while if not forever. > Hi Pekka, Thanks for the review. > > I tested with google-chrome-browser as that's what I had at hand, both > with and without system decorations. Without system decorations, this > patch indeed fixes both the input region (the shadow area was not > click-through as it should have been) and removes shadows (that I > assume should not have been there). > 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. > > Is the above exactly what you wanted to say in the commit message, or > did Steam really get wrong input coordinates, i.e. clicks were off also > inside the window? > The commit message is poor at best; I've updated commit messages in the new patches. > > Does this patch remove unwanted shadows from Steam or was Steam without > shadows to begin with? > It's not meant to remove any shadows. The new patch doesn't adjust size at all. > > The commit message is a little ambiguous on these terms, so I'd like > (someone) to re-word it. > > When using system decorations in Chrome, the decorations look and work > just fine also after the patch. > 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. > > Maximizing xterm works ok. > > I'm a little worried about removing the checks for window->fullscreen, > because e.g. weston_wm_window_draw_decoration() looks at > window->fullscreen and does nothing. It would feel more logical for > draw_decoration to use the same conditions as get_frame_size and > get_child_position. What do you think? Which conditions should those be? > Yes, I see what you mean. I left fullscreen logic alone in the new patch. > > weston_wm_window_configure(), call arranged from send_configure(), is > unconditionally using get_frame_size and get_child_position as well. > > Also weston_wm_window_set_pending_state() and send_configure() are > looking at both window->decorate and window->fullscreen. It all > seems to imply that if _NET_WM_STATE_FULLSCREEN is in effect, then > decorations should be avoided regardless of the _MOTIF_WM_HINTS > MWM_DECOR_* flags. Does that make sense? > > Looking at weston_wm_window_draw_decoration(), it seems that shadows > for undecorated non-fullscreen surfaces were actually intentional. If > we remove that intention, the drawing of the shadow should also be > removed, and the commit message should explain why that was wrong. > > In summary, I would suggest changing > if (window->decorate) > into > if (window->decorate && !window->fullscreen) > > In fact, testing with e.g. geany, this patch regresses fullscreening: a > fullscreen X11 window is shifted down with a black bar on top and the > bottom going out of screen. The same can be seen with xterm. > > > Thanks, > pq > Hopefully you can test the new series and see if there are any regressions or concerns. Thanks again, - Scott
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
