xwm: Send configure event when moving

Submitted by Scott Moreau on March 18, 2018, 6:22 p.m.

Details

Message ID 1521397337-19298-2-git-send-email-oreaus@gmail.com
State New
Headers show
Series "xwm: Handle changing override redirect flag" ( rev: 3 ) in Wayland

Not browsing as part of any series.

Commit Message

Scott Moreau March 18, 2018, 6:22 p.m.
Send a synthetic configure notify event to the reparented window to
update the position in Xwayland. This fixes menu positioning in clients
like VLC after moving the window.
---
 xwayland/window-manager.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index c307e19..542e9fb 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -659,13 +659,12 @@  weston_wm_window_get_input_rect(struct weston_wm_window *window,
 }
 
 static void
-weston_wm_window_send_configure_notify(struct weston_wm_window *window)
+weston_wm_window_send_configure_notify(struct weston_wm_window *window,
+										int x, int y)
 {
 	xcb_configure_notify_event_t configure_notify;
 	struct weston_wm *wm = window->wm;
-	int x, y;
 
-	weston_wm_window_get_child_position(window, &x, &y);
 	configure_notify.response_type = XCB_CONFIGURE_NOTIFY;
 	configure_notify.pad0 = 0;
 	configure_notify.event = window->id;
@@ -702,7 +701,7 @@  weston_wm_handle_configure_request(struct weston_wm *wm, xcb_generic_event_t *ev
 		return;
 
 	if (window->fullscreen) {
-		weston_wm_window_send_configure_notify(window);
+		weston_wm_window_send_configure_notify(window, 0, 0);
 		return;
 	}
 
@@ -2734,6 +2733,7 @@  send_position(struct weston_surface *surface, int32_t x, int32_t y)
 	struct weston_wm_window *window = get_wm_window(surface);
 	struct weston_wm *wm;
 	uint32_t mask, values[2];
+	int sx, sy;
 
 	if (!window || !window->wm)
 		return;
@@ -2750,6 +2750,10 @@  send_position(struct weston_surface *surface, int32_t x, int32_t y)
 		mask = XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y;
 
 		xcb_configure_window(wm->conn, window->frame_id, mask, values);
+
+		weston_wm_window_get_child_position(window, &sx, &sy);
+		weston_wm_window_send_configure_notify(window, x + sx, y + sy);
+
 		xcb_flush(wm->conn);
 	}
 }

Comments

On Sun, 18 Mar 2018 12:22:16 -0600
Scott Moreau <oreaus@gmail.com> wrote:

> Send a synthetic configure notify event to the reparented window to
> update the position in Xwayland. This fixes menu positioning in clients
> like VLC after moving the window.

Hi,

Signed-off-by missing, I would strongly recommend to add it to say you
comply with https://developercertificate.org/ . You can do it by simply
replying to this email with the tag.

It's not absolutely required by our contribution guidelines yet, but it
might in the future. It is what we have always assumed, S-o-b just
makes it explicit.

> ---
>  xwayland/window-manager.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
> index c307e19..542e9fb 100644
> --- a/xwayland/window-manager.c
> +++ b/xwayland/window-manager.c
> @@ -659,13 +659,12 @@ weston_wm_window_get_input_rect(struct weston_wm_window *window,
>  }
>  
>  static void
> -weston_wm_window_send_configure_notify(struct weston_wm_window *window)
> +weston_wm_window_send_configure_notify(struct weston_wm_window *window,
> +										int x, int y)

The first "int" should be aligned with "struct" on the previous line.

>  {
>  	xcb_configure_notify_event_t configure_notify;
>  	struct weston_wm *wm = window->wm;
> -	int x, y;
>  
> -	weston_wm_window_get_child_position(window, &x, &y);
>  	configure_notify.response_type = XCB_CONFIGURE_NOTIFY;
>  	configure_notify.pad0 = 0;
>  	configure_notify.event = window->id;
> @@ -702,7 +701,7 @@ weston_wm_handle_configure_request(struct weston_wm *wm, xcb_generic_event_t *ev
>  		return;
>  
>  	if (window->fullscreen) {
> -		weston_wm_window_send_configure_notify(window);
> +		weston_wm_window_send_configure_notify(window, 0, 0);
>  		return;
>  	}
>  
> @@ -2734,6 +2733,7 @@ send_position(struct weston_surface *surface, int32_t x, int32_t y)
>  	struct weston_wm_window *window = get_wm_window(surface);
>  	struct weston_wm *wm;
>  	uint32_t mask, values[2];
> +	int sx, sy;
>  
>  	if (!window || !window->wm)
>  		return;
> @@ -2750,6 +2750,10 @@ send_position(struct weston_surface *surface, int32_t x, int32_t y)
>  		mask = XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y;
>  
>  		xcb_configure_window(wm->conn, window->frame_id, mask, values);
> +
> +		weston_wm_window_get_child_position(window, &sx, &sy);
> +		weston_wm_window_send_configure_notify(window, x + sx, y + sy);

https://tronche.com/gui/x/xlib/events/window-state-change/configure.html
says:
	"The x and y members are set to the coordinates relative to the
	parent window's origin and indicate the position of the
	upper-left outside corner of the window."

I tested with kcachegrind, which without this patch gets combobox popup
windows positioned wrong, and this patch as is fixes it indeed. I also
tried sending just sx instead of x + sx, and it breaks it again. So
while this code seems to work right, I would like to know why Tronche
is seemingly wrong?

https://stackoverflow.com/questions/25391791/x11-configurenotify-always-returning-x-y-0-0
seems to agree with Tronche.

So perhaps this is not the right fix?

A quick look at
https://lists.freedesktop.org/archives/xorg/2008-November/040184.html
makes this ever more... interesting.

Is this one of those things that someone once got wrong in the ancient
days, and then literally everyone fixed their programs to comply with
the spec-violating behaviour?

> +
>  		xcb_flush(wm->conn);
>  	}
>  }

Thanks,
pq