[weston] xwm: Support maximizing xwayland windows

Submitted by Giulio Camuffo on Nov. 22, 2014, 5:25 p.m.

Details

Message ID 1416677151-23878-1-git-send-email-giuliocamuffo@gmail.com
State Superseded
Headers show

Not browsing as part of any series.

Commit Message

Giulio Camuffo Nov. 22, 2014, 5:25 p.m.
This patch adds the maximize button to the window frame for the windows
which set the MWM_DECOR_MAXIMIZE hint, and it wires it with the shell
via a new method in weston_shell_interface.
Additionally, it also listens for the wm hints coming from the client,
but it doesn't support maximizing a window only vertically or horizontally.
The window will be maximized only when both directions are maximized.
---

This is the first time i go into this X stuff, so please be understanding if
i made some stupid mistake. :)

 desktop-shell/shell.c     | 81 +++++++++++++++++++++++++++++---------------
 src/compositor.h          |  1 +
 xwayland/window-manager.c | 86 ++++++++++++++++++++++++++++++++++++++++-------
 xwayland/xwayland.h       |  2 ++
 4 files changed, 130 insertions(+), 40 deletions(-)

Patch hide | download patch | download mbox

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 844a322..a6d08c7 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -2931,6 +2931,51 @@  shell_interface_set_fullscreen(struct shell_surface *shsurf,
 	shsurf->state_changed = true;
 }
 
+static struct weston_output *
+get_focused_output(struct weston_compositor *compositor)
+{
+	struct weston_seat *seat;
+	struct weston_output *output = NULL;
+
+	wl_list_for_each(seat, &compositor->seat_list, link) {
+		/* Priority has touch focus, then pointer and
+		 * then keyboard focus. We should probably have
+		 * three for loops and check frist for touch,
+		 * then for pointer, etc. but unless somebody has some
+		 * objections, I think this is sufficient. */
+		if (seat->touch && seat->touch->focus)
+			output = seat->touch->focus->output;
+		else if (seat->pointer && seat->pointer->focus)
+			output = seat->pointer->focus->output;
+		else if (seat->keyboard && seat->keyboard->focus)
+			output = seat->keyboard->focus->output;
+
+		if (output)
+			break;
+	}
+
+	return output;
+}
+
+static void
+shell_interface_set_maximized(struct shell_surface *shsurf)
+{
+	struct weston_output *output;
+
+	surface_clear_next_states(shsurf);
+	shsurf->next_state.maximized = true;
+	shsurf->state_changed = true;
+	shsurf->type = SHELL_SURFACE_TOPLEVEL;
+
+	if (!weston_surface_is_mapped(shsurf->surface))
+		output = get_focused_output(shsurf->surface->compositor);
+	else
+		output = shsurf->surface->output;
+
+	shell_surface_set_output(shsurf, output);
+	send_configure_for_surface(shsurf);
+}
+
 static int
 shell_interface_move(struct shell_surface *shsurf, struct weston_seat *ws)
 {
@@ -3534,32 +3579,6 @@  get_primary_view(void *shell, struct shell_surface *shsurf)
 	return shsurf->view;
 }
 
-static struct weston_output *
-get_focused_output(struct weston_compositor *compositor)
-{
-	struct weston_seat *seat;
-	struct weston_output *output = NULL;
-
-	wl_list_for_each(seat, &compositor->seat_list, link) {
-		/* Priority has touch focus, then pointer and
-		 * then keyboard focus. We should probably have
-		 * three for loops and check frist for touch,
-		 * then for pointer, etc. but unless somebody has some
-		 * objections, I think this is sufficient. */
-		if (seat->touch && seat->touch->focus)
-			output = seat->touch->focus->output;
-		else if (seat->pointer && seat->pointer->focus)
-			output = seat->pointer->focus->output;
-		else if (seat->keyboard && seat->keyboard->focus)
-			output = seat->keyboard->focus->output;
-
-		if (output)
-			break;
-	}
-
-	return output;
-}
-
 static void
 shell_get_shell_surface(struct wl_client *client,
 			struct wl_resource *resource,
@@ -5278,8 +5297,13 @@  set_maximized_position(struct desktop_shell *shell,
 	pixman_box32_t *e;
 
 	get_output_work_area(shell, shsurf->output, &area);
-	surface_subsurfaces_boundingbox(shsurf->surface,
-					&surf_x, &surf_y, NULL, NULL);
+	if (shsurf->has_set_geometry) {
+		surf_x = shsurf->geometry.x;
+		surf_y = shsurf->geometry.y;
+	} else {
+		surface_subsurfaces_boundingbox(shsurf->surface,
+						&surf_x, &surf_y, NULL, NULL);
+	}
 	e = pixman_region32_extents(&shsurf->output->region);
 
 	weston_view_set_position(shsurf->view,
@@ -6520,6 +6544,7 @@  module_init(struct weston_compositor *ec,
 	ec->shell_interface.resize = surface_resize;
 	ec->shell_interface.set_title = set_title;
 	ec->shell_interface.set_window_geometry = set_window_geometry;
+	ec->shell_interface.set_maximized = shell_interface_set_maximized;
 
 	weston_layer_init(&shell->fullscreen_layer, &ec->cursor_layer.link);
 	weston_layer_init(&shell->panel_layer, &shell->fullscreen_layer.link);
diff --git a/src/compositor.h b/src/compositor.h
index 2bec183..22642b5 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -120,6 +120,7 @@  struct weston_shell_interface {
 	void (*set_window_geometry)(struct shell_surface *shsurf,
 				    int32_t x, int32_t y,
 				    int32_t width, int32_t height);
+	void (*set_maximized)(struct shell_surface *shsurf);
 };
 
 struct weston_animation {
diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 4acb534..68f8782 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -144,6 +144,8 @@  struct weston_wm_window {
 	int fullscreen;
 	int has_alpha;
 	int delete_window;
+	int maximized_vert;
+	int maximized_horz;
 	struct wm_size_hints size_hints;
 	struct motif_wm_hints motif_hints;
 	struct wl_list link;
@@ -472,6 +474,10 @@  weston_wm_window_read_properties(struct weston_wm_window *window)
 			for (i = 0; i < reply->value_len; i++)
 				if (atom[i] == wm->atom.net_wm_state_fullscreen)
 					window->fullscreen = 1;
+				if (atom[i] == wm->atom.net_wm_state_maximized_vert)
+					window->maximized_vert = 1;
+				if (atom[i] == wm->atom.net_wm_state_maximized_horz)
+					window->maximized_horz = 1;
 			break;
 		case TYPE_MOTIF_WM_HINTS:
 			memcpy(&window->motif_hints,
@@ -479,7 +485,7 @@  weston_wm_window_read_properties(struct weston_wm_window *window)
 			       sizeof window->motif_hints);
 			if (window->motif_hints.flags & MWM_HINTS_DECORATIONS)
 				window->decorate =
-					window->motif_hints.decorations > 0;
+					window->motif_hints.decorations;
 			break;
 		default:
 			break;
@@ -789,12 +795,16 @@  static void
 weston_wm_window_set_net_wm_state(struct weston_wm_window *window)
 {
 	struct weston_wm *wm = window->wm;
-	uint32_t property[1];
+	uint32_t property[3];
 	int i;
 
 	i = 0;
 	if (window->fullscreen)
 		property[i++] = wm->atom.net_wm_state_fullscreen;
+	if (window->maximized_vert)
+		property[i++] = wm->atom.net_wm_state_maximized_vert;
+	if (window->maximized_horz)
+		property[i++] = wm->atom.net_wm_state_maximized_horz;
 
 	xcb_change_property(wm->conn,
 			    XCB_PROP_MODE_REPLACE,
@@ -811,10 +821,14 @@  weston_wm_window_create_frame(struct weston_wm_window *window)
 	struct weston_wm *wm = window->wm;
 	uint32_t values[3];
 	int x, y, width, height;
+	int buttons = FRAME_BUTTON_CLOSE;
+
+	if (window->decorate & MWM_DECOR_MAXIMIZE)
+		buttons |= FRAME_BUTTON_MAXIMIZE;
 
 	window->frame = frame_create(window->wm->theme,
 				     window->width, window->height,
-				     FRAME_BUTTON_CLOSE, window->name);
+				     buttons, window->name);
 	frame_resize_inside(window->frame, window->width, window->height);
 
 	weston_wm_window_get_frame_size(window, &width, &height);
@@ -1303,6 +1317,22 @@  static void
 weston_wm_window_configure(void *data);
 
 static void
+weston_wm_window_set_toplevel(struct weston_wm_window *window)
+{
+	struct weston_shell_interface *shell_interface =
+		&window->wm->server->compositor->shell_interface;
+
+	shell_interface->set_toplevel(window->shsurf);
+	window->width = window->saved_width;
+	window->height = window->saved_height;
+	if (window->frame)
+		frame_resize_inside(window->frame,
+					window->width,
+					window->height);
+	weston_wm_window_configure(window);
+}
+
+static void
 weston_wm_window_handle_state(struct weston_wm_window *window,
 			      xcb_client_message_event_t *client_message)
 {
@@ -1310,6 +1340,7 @@  weston_wm_window_handle_state(struct weston_wm_window *window,
 	struct weston_shell_interface *shell_interface =
 		&wm->server->compositor->shell_interface;
 	uint32_t action, property;
+	int maximized = window->maximized_horz && window->maximized_vert;
 
 	action = client_message->data.data32[0];
 	property = client_message->data.data32[1];
@@ -1326,14 +1357,26 @@  weston_wm_window_handle_state(struct weston_wm_window *window,
 								WL_SHELL_SURFACE_FULLSCREEN_METHOD_DEFAULT,
 								0, NULL);
 		} else {
-			shell_interface->set_toplevel(window->shsurf);
-			window->width = window->saved_width;
-			window->height = window->saved_height;
-			if (window->frame)
-				frame_resize_inside(window->frame,
-						    window->width,
-						    window->height);
-			weston_wm_window_configure(window);
+			weston_wm_window_set_toplevel(window);
+		}
+	} else {
+		if (property == wm->atom.net_wm_state_maximized_vert &&
+		    update_state(action, &window->maximized_vert))
+			weston_wm_window_set_net_wm_state(window);
+		if (property == wm->atom.net_wm_state_maximized_horz &&
+		    update_state(action, &window->maximized_horz))
+			weston_wm_window_set_net_wm_state(window);
+
+		if (maximized != window->maximized_vert && window->maximized_horz) {
+			if (window->maximized_vert && window->maximized_horz) {
+				window->saved_width = window->width;
+				window->saved_height = window->height;
+
+				if (window->shsurf)
+					shell_interface->set_maximized(window->shsurf);
+			} else {
+				weston_wm_window_set_toplevel(window);
+			}
 		}
 	}
 }
@@ -1665,6 +1708,19 @@  weston_wm_handle_button(struct weston_wm *wm, xcb_generic_event_t *event)
 		weston_wm_window_close(window, button->time);
 		frame_status_clear(window->frame, FRAME_STATUS_CLOSE);
 	}
+
+	if (frame_status(window->frame) & FRAME_STATUS_MAXIMIZE) {
+		window->maximized_horz = !window->maximized_horz;
+		window->maximized_vert = !window->maximized_vert;
+		if (window->maximized_horz && window->maximized_vert) {
+			window->saved_width = window->width;
+			window->saved_height = window->height;
+			shell_interface->set_maximized(window->shsurf);
+		} else {
+			weston_wm_window_set_toplevel(window);
+		}
+		frame_status_clear(window->frame, FRAME_STATUS_MAXIMIZE);
+	}
 }
 
 static void
@@ -1853,6 +1909,8 @@  weston_wm_get_resources(struct weston_wm *wm)
 		{ "_NET_WM_PID",	F(atom.net_wm_pid) },
 		{ "_NET_WM_ICON",	F(atom.net_wm_icon) },
 		{ "_NET_WM_STATE",	F(atom.net_wm_state) },
+		{ "_NET_WM_STATE_MAXIMIZED_VERT", F(atom.net_wm_state_maximized_vert) },
+		{ "_NET_WM_STATE_MAXIMIZED_HORZ", F(atom.net_wm_state_maximized_horz) },
 		{ "_NET_WM_STATE_FULLSCREEN", F(atom.net_wm_state_fullscreen) },
 		{ "_NET_WM_USER_TIME", F(atom.net_wm_user_time) },
 		{ "_NET_WM_ICON_NAME", F(atom.net_wm_icon_name) },
@@ -2029,7 +2087,7 @@  weston_wm_create(struct weston_xserver *wxs, int fd)
 	struct wl_event_loop *loop;
 	xcb_screen_iterator_t s;
 	uint32_t values[1];
-	xcb_atom_t supported[3];
+	xcb_atom_t supported[5];
 
 	wm = zalloc(sizeof *wm);
 	if (wm == NULL)
@@ -2080,6 +2138,8 @@  weston_wm_create(struct weston_xserver *wxs, int fd)
 	supported[0] = wm->atom.net_wm_moveresize;
 	supported[1] = wm->atom.net_wm_state;
 	supported[2] = wm->atom.net_wm_state_fullscreen;
+	supported[3] = wm->atom.net_wm_state_maximized_vert;
+	supported[4] = wm->atom.net_wm_state_maximized_horz;
 	xcb_change_property(wm->conn,
 			    XCB_PROP_MODE_REPLACE,
 			    wm->screen->root,
@@ -2342,6 +2402,8 @@  xserver_map_shell_surface(struct weston_wm_window *window,
 					       parent->surface,
 					       window->x - parent->x,
 					       window->y - parent->y, 0);
+	} else if (window->maximized_vert && window->maximized_horz) {
+		shell_interface->set_maximized(window->shsurf);
 	} else {
 		shell_interface->set_toplevel(window->shsurf);
 	}
diff --git a/xwayland/xwayland.h b/xwayland/xwayland.h
index 312c9b2..69fdcfd 100644
--- a/xwayland/xwayland.h
+++ b/xwayland/xwayland.h
@@ -102,6 +102,8 @@  struct weston_wm {
 		xcb_atom_t		 net_wm_pid;
 		xcb_atom_t		 net_wm_icon;
 		xcb_atom_t		 net_wm_state;
+		xcb_atom_t		 net_wm_state_maximized_vert;
+		xcb_atom_t		 net_wm_state_maximized_horz;
 		xcb_atom_t		 net_wm_state_fullscreen;
 		xcb_atom_t		 net_wm_user_time;
 		xcb_atom_t		 net_wm_icon_name;

Comments

On Sat, 22 Nov 2014 19:25:51 +0200
Giulio Camuffo <giuliocamuffo@gmail.com> wrote:

> This patch adds the maximize button to the window frame for the windows
> which set the MWM_DECOR_MAXIMIZE hint, and it wires it with the shell
> via a new method in weston_shell_interface.
> Additionally, it also listens for the wm hints coming from the client,
> but it doesn't support maximizing a window only vertically or horizontally.
> The window will be maximized only when both directions are maximized.
> ---
> 
> This is the first time i go into this X stuff, so please be understanding if
> i made some stupid mistake. :)

Me too, I mean, I'd appreciate if someone who knows something about X
WM would review the X parts.

Some comments below.

>  desktop-shell/shell.c     | 81 +++++++++++++++++++++++++++++---------------
>  src/compositor.h          |  1 +
>  xwayland/window-manager.c | 86 ++++++++++++++++++++++++++++++++++++++++-------
>  xwayland/xwayland.h       |  2 ++
>  4 files changed, 130 insertions(+), 40 deletions(-)
> 
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index 844a322..a6d08c7 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -2931,6 +2931,51 @@ shell_interface_set_fullscreen(struct shell_surface *shsurf,
>  	shsurf->state_changed = true;
>  }
>  
> +static struct weston_output *
> +get_focused_output(struct weston_compositor *compositor)
> +{
> +	struct weston_seat *seat;
> +	struct weston_output *output = NULL;
> +
> +	wl_list_for_each(seat, &compositor->seat_list, link) {
> +		/* Priority has touch focus, then pointer and
> +		 * then keyboard focus. We should probably have
> +		 * three for loops and check frist for touch,
> +		 * then for pointer, etc. but unless somebody has some
> +		 * objections, I think this is sufficient. */
> +		if (seat->touch && seat->touch->focus)
> +			output = seat->touch->focus->output;
> +		else if (seat->pointer && seat->pointer->focus)
> +			output = seat->pointer->focus->output;
> +		else if (seat->keyboard && seat->keyboard->focus)
> +			output = seat->keyboard->focus->output;
> +
> +		if (output)
> +			break;
> +	}
> +
> +	return output;
> +}
> +
> +static void
> +shell_interface_set_maximized(struct shell_surface *shsurf)
> +{
> +	struct weston_output *output;
> +
> +	surface_clear_next_states(shsurf);
> +	shsurf->next_state.maximized = true;
> +	shsurf->state_changed = true;
> +	shsurf->type = SHELL_SURFACE_TOPLEVEL;
> +
> +	if (!weston_surface_is_mapped(shsurf->surface))
> +		output = get_focused_output(shsurf->surface->compositor);
> +	else
> +		output = shsurf->surface->output;
> +
> +	shell_surface_set_output(shsurf, output);
> +	send_configure_for_surface(shsurf);

I wonder if setting shsurf->type is needed, but otherwise can't see
anything odd here on a first glance.

> +}
> +
>  static int
>  shell_interface_move(struct shell_surface *shsurf, struct weston_seat *ws)
>  {
> @@ -3534,32 +3579,6 @@ get_primary_view(void *shell, struct shell_surface *shsurf)
>  	return shsurf->view;
>  }
>  
> -static struct weston_output *
> -get_focused_output(struct weston_compositor *compositor)
> -{
> -	struct weston_seat *seat;
> -	struct weston_output *output = NULL;
> -
> -	wl_list_for_each(seat, &compositor->seat_list, link) {
> -		/* Priority has touch focus, then pointer and
> -		 * then keyboard focus. We should probably have
> -		 * three for loops and check frist for touch,
> -		 * then for pointer, etc. but unless somebody has some
> -		 * objections, I think this is sufficient. */
> -		if (seat->touch && seat->touch->focus)
> -			output = seat->touch->focus->output;
> -		else if (seat->pointer && seat->pointer->focus)
> -			output = seat->pointer->focus->output;
> -		else if (seat->keyboard && seat->keyboard->focus)
> -			output = seat->keyboard->focus->output;
> -
> -		if (output)
> -			break;
> -	}
> -
> -	return output;
> -}
> -
>  static void
>  shell_get_shell_surface(struct wl_client *client,
>  			struct wl_resource *resource,
> @@ -5278,8 +5297,13 @@ set_maximized_position(struct desktop_shell *shell,
>  	pixman_box32_t *e;
>  
>  	get_output_work_area(shell, shsurf->output, &area);
> -	surface_subsurfaces_boundingbox(shsurf->surface,
> -					&surf_x, &surf_y, NULL, NULL);
> +	if (shsurf->has_set_geometry) {
> +		surf_x = shsurf->geometry.x;
> +		surf_y = shsurf->geometry.y;
> +	} else {
> +		surface_subsurfaces_boundingbox(shsurf->surface,
> +						&surf_x, &surf_y, NULL, NULL);
> +	}

This hunk looks like it should be a patch of its own.

set_maximized_position() is called by map() and configure() which are
both called from shell_surface_configure(), which is set as the
weston_surface::configure for both wl_shell and xdg_shell in
create_common_surface().

I'm guessing that if an xdg_shell app used a maximized view, where the
window geometry did not cover the whole surface, it would be positioned
badly.


>  	e = pixman_region32_extents(&shsurf->output->region);
>  
>  	weston_view_set_position(shsurf->view,
> @@ -6520,6 +6544,7 @@ module_init(struct weston_compositor *ec,
>  	ec->shell_interface.resize = surface_resize;
>  	ec->shell_interface.set_title = set_title;
>  	ec->shell_interface.set_window_geometry = set_window_geometry;
> +	ec->shell_interface.set_maximized = shell_interface_set_maximized;
>  
>  	weston_layer_init(&shell->fullscreen_layer, &ec->cursor_layer.link);
>  	weston_layer_init(&shell->panel_layer, &shell->fullscreen_layer.link);
> diff --git a/src/compositor.h b/src/compositor.h
> index 2bec183..22642b5 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -120,6 +120,7 @@ struct weston_shell_interface {
>  	void (*set_window_geometry)(struct shell_surface *shsurf,
>  				    int32_t x, int32_t y,
>  				    int32_t width, int32_t height);
> +	void (*set_maximized)(struct shell_surface *shsurf);
>  };
>  
>  struct weston_animation {
> diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
> index 4acb534..68f8782 100644
> --- a/xwayland/window-manager.c
> +++ b/xwayland/window-manager.c
> @@ -144,6 +144,8 @@ struct weston_wm_window {
>  	int fullscreen;
>  	int has_alpha;
>  	int delete_window;
> +	int maximized_vert;
> +	int maximized_horz;
>  	struct wm_size_hints size_hints;
>  	struct motif_wm_hints motif_hints;
>  	struct wl_list link;
> @@ -472,6 +474,10 @@ weston_wm_window_read_properties(struct weston_wm_window *window)
>  			for (i = 0; i < reply->value_len; i++)
>  				if (atom[i] == wm->atom.net_wm_state_fullscreen)
>  					window->fullscreen = 1;
> +				if (atom[i] == wm->atom.net_wm_state_maximized_vert)
> +					window->maximized_vert = 1;
> +				if (atom[i] == wm->atom.net_wm_state_maximized_horz)
> +					window->maximized_horz = 1;
>  			break;
>  		case TYPE_MOTIF_WM_HINTS:
>  			memcpy(&window->motif_hints,
> @@ -479,7 +485,7 @@ weston_wm_window_read_properties(struct weston_wm_window *window)
>  			       sizeof window->motif_hints);
>  			if (window->motif_hints.flags & MWM_HINTS_DECORATIONS)
>  				window->decorate =
> -					window->motif_hints.decorations > 0;
> +					window->motif_hints.decorations;
>  			break;
>  		default:
>  			break;
> @@ -789,12 +795,16 @@ static void
>  weston_wm_window_set_net_wm_state(struct weston_wm_window *window)
>  {
>  	struct weston_wm *wm = window->wm;
> -	uint32_t property[1];
> +	uint32_t property[3];
>  	int i;
>  
>  	i = 0;
>  	if (window->fullscreen)
>  		property[i++] = wm->atom.net_wm_state_fullscreen;
> +	if (window->maximized_vert)
> +		property[i++] = wm->atom.net_wm_state_maximized_vert;
> +	if (window->maximized_horz)
> +		property[i++] = wm->atom.net_wm_state_maximized_horz;
>  
>  	xcb_change_property(wm->conn,
>  			    XCB_PROP_MODE_REPLACE,
> @@ -811,10 +821,14 @@ weston_wm_window_create_frame(struct weston_wm_window *window)
>  	struct weston_wm *wm = window->wm;
>  	uint32_t values[3];
>  	int x, y, width, height;
> +	int buttons = FRAME_BUTTON_CLOSE;
> +
> +	if (window->decorate & MWM_DECOR_MAXIMIZE)
> +		buttons |= FRAME_BUTTON_MAXIMIZE;
>  
>  	window->frame = frame_create(window->wm->theme,
>  				     window->width, window->height,
> -				     FRAME_BUTTON_CLOSE, window->name);
> +				     buttons, window->name);
>  	frame_resize_inside(window->frame, window->width, window->height);
>  
>  	weston_wm_window_get_frame_size(window, &width, &height);
> @@ -1303,6 +1317,22 @@ static void
>  weston_wm_window_configure(void *data);
>  
>  static void
> +weston_wm_window_set_toplevel(struct weston_wm_window *window)
> +{
> +	struct weston_shell_interface *shell_interface =
> +		&window->wm->server->compositor->shell_interface;
> +
> +	shell_interface->set_toplevel(window->shsurf);
> +	window->width = window->saved_width;
> +	window->height = window->saved_height;
> +	if (window->frame)
> +		frame_resize_inside(window->frame,
> +					window->width,
> +					window->height);
> +	weston_wm_window_configure(window);
> +}
> +
> +static void
>  weston_wm_window_handle_state(struct weston_wm_window *window,
>  			      xcb_client_message_event_t *client_message)
>  {
> @@ -1310,6 +1340,7 @@ weston_wm_window_handle_state(struct weston_wm_window *window,
>  	struct weston_shell_interface *shell_interface =
>  		&wm->server->compositor->shell_interface;
>  	uint32_t action, property;
> +	int maximized = window->maximized_horz && window->maximized_vert;
>  
>  	action = client_message->data.data32[0];
>  	property = client_message->data.data32[1];
> @@ -1326,14 +1357,26 @@ weston_wm_window_handle_state(struct weston_wm_window *window,
>  								WL_SHELL_SURFACE_FULLSCREEN_METHOD_DEFAULT,
>  								0, NULL);
>  		} else {
> -			shell_interface->set_toplevel(window->shsurf);
> -			window->width = window->saved_width;
> -			window->height = window->saved_height;
> -			if (window->frame)
> -				frame_resize_inside(window->frame,
> -						    window->width,
> -						    window->height);
> -			weston_wm_window_configure(window);
> +			weston_wm_window_set_toplevel(window);
> +		}
> +	} else {
> +		if (property == wm->atom.net_wm_state_maximized_vert &&
> +		    update_state(action, &window->maximized_vert))
> +			weston_wm_window_set_net_wm_state(window);
> +		if (property == wm->atom.net_wm_state_maximized_horz &&
> +		    update_state(action, &window->maximized_horz))
> +			weston_wm_window_set_net_wm_state(window);
> +
> +		if (maximized != window->maximized_vert && window->maximized_horz) {
> +			if (window->maximized_vert && window->maximized_horz) {
> +				window->saved_width = window->width;
> +				window->saved_height = window->height;
> +
> +				if (window->shsurf)
> +					shell_interface->set_maximized(window->shsurf);
> +			} else {
> +				weston_wm_window_set_toplevel(window);
> +			}
>  		}
>  	}
>  }
> @@ -1665,6 +1708,19 @@ weston_wm_handle_button(struct weston_wm *wm, xcb_generic_event_t *event)
>  		weston_wm_window_close(window, button->time);
>  		frame_status_clear(window->frame, FRAME_STATUS_CLOSE);
>  	}
> +
> +	if (frame_status(window->frame) & FRAME_STATUS_MAXIMIZE) {
> +		window->maximized_horz = !window->maximized_horz;
> +		window->maximized_vert = !window->maximized_vert;

I wonder if an X app can get maximized_horz != maximized_vert somehow.
Should we be more defensive here against that?

> +		if (window->maximized_horz && window->maximized_vert) {
> +			window->saved_width = window->width;
> +			window->saved_height = window->height;
> +			shell_interface->set_maximized(window->shsurf);
> +		} else {
> +			weston_wm_window_set_toplevel(window);
> +		}
> +		frame_status_clear(window->frame, FRAME_STATUS_MAXIMIZE);
> +	}
>  }
>  
>  static void
> @@ -1853,6 +1909,8 @@ weston_wm_get_resources(struct weston_wm *wm)
>  		{ "_NET_WM_PID",	F(atom.net_wm_pid) },
>  		{ "_NET_WM_ICON",	F(atom.net_wm_icon) },
>  		{ "_NET_WM_STATE",	F(atom.net_wm_state) },
> +		{ "_NET_WM_STATE_MAXIMIZED_VERT", F(atom.net_wm_state_maximized_vert) },
> +		{ "_NET_WM_STATE_MAXIMIZED_HORZ", F(atom.net_wm_state_maximized_horz) },
>  		{ "_NET_WM_STATE_FULLSCREEN", F(atom.net_wm_state_fullscreen) },
>  		{ "_NET_WM_USER_TIME", F(atom.net_wm_user_time) },
>  		{ "_NET_WM_ICON_NAME", F(atom.net_wm_icon_name) },
> @@ -2029,7 +2087,7 @@ weston_wm_create(struct weston_xserver *wxs, int fd)
>  	struct wl_event_loop *loop;
>  	xcb_screen_iterator_t s;
>  	uint32_t values[1];
> -	xcb_atom_t supported[3];
> +	xcb_atom_t supported[5];
>  
>  	wm = zalloc(sizeof *wm);
>  	if (wm == NULL)
> @@ -2080,6 +2138,8 @@ weston_wm_create(struct weston_xserver *wxs, int fd)
>  	supported[0] = wm->atom.net_wm_moveresize;
>  	supported[1] = wm->atom.net_wm_state;
>  	supported[2] = wm->atom.net_wm_state_fullscreen;
> +	supported[3] = wm->atom.net_wm_state_maximized_vert;
> +	supported[4] = wm->atom.net_wm_state_maximized_horz;
>  	xcb_change_property(wm->conn,
>  			    XCB_PROP_MODE_REPLACE,
>  			    wm->screen->root,
> @@ -2342,6 +2402,8 @@ xserver_map_shell_surface(struct weston_wm_window *window,
>  					       parent->surface,
>  					       window->x - parent->x,
>  					       window->y - parent->y, 0);
> +	} else if (window->maximized_vert && window->maximized_horz) {
> +		shell_interface->set_maximized(window->shsurf);
>  	} else {
>  		shell_interface->set_toplevel(window->shsurf);
>  	}
> diff --git a/xwayland/xwayland.h b/xwayland/xwayland.h
> index 312c9b2..69fdcfd 100644
> --- a/xwayland/xwayland.h
> +++ b/xwayland/xwayland.h
> @@ -102,6 +102,8 @@ struct weston_wm {
>  		xcb_atom_t		 net_wm_pid;
>  		xcb_atom_t		 net_wm_icon;
>  		xcb_atom_t		 net_wm_state;
> +		xcb_atom_t		 net_wm_state_maximized_vert;
> +		xcb_atom_t		 net_wm_state_maximized_horz;
>  		xcb_atom_t		 net_wm_state_fullscreen;
>  		xcb_atom_t		 net_wm_user_time;
>  		xcb_atom_t		 net_wm_icon_name;

Thanks,
pq
2014-12-01 15:11 GMT+02:00 Pekka Paalanen <ppaalanen@gmail.com>:
> On Sat, 22 Nov 2014 19:25:51 +0200
> Giulio Camuffo <giuliocamuffo@gmail.com> wrote:
>
>> This patch adds the maximize button to the window frame for the windows
>> which set the MWM_DECOR_MAXIMIZE hint, and it wires it with the shell
>> via a new method in weston_shell_interface.
>> Additionally, it also listens for the wm hints coming from the client,
>> but it doesn't support maximizing a window only vertically or horizontally.
>> The window will be maximized only when both directions are maximized.
>> ---
>>
>> This is the first time i go into this X stuff, so please be understanding if
>> i made some stupid mistake. :)
>
> Me too, I mean, I'd appreciate if someone who knows something about X
> WM would review the X parts.
>
> Some comments below.
>
>>  desktop-shell/shell.c     | 81 +++++++++++++++++++++++++++++---------------
>>  src/compositor.h          |  1 +
>>  xwayland/window-manager.c | 86 ++++++++++++++++++++++++++++++++++++++++-------
>>  xwayland/xwayland.h       |  2 ++
>>  4 files changed, 130 insertions(+), 40 deletions(-)
>>
>> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
>> index 844a322..a6d08c7 100644
>> --- a/desktop-shell/shell.c
>> +++ b/desktop-shell/shell.c
>> @@ -2931,6 +2931,51 @@ shell_interface_set_fullscreen(struct shell_surface *shsurf,
>>       shsurf->state_changed = true;
>>  }
>>
>> +static struct weston_output *
>> +get_focused_output(struct weston_compositor *compositor)
>> +{
>> +     struct weston_seat *seat;
>> +     struct weston_output *output = NULL;
>> +
>> +     wl_list_for_each(seat, &compositor->seat_list, link) {
>> +             /* Priority has touch focus, then pointer and
>> +              * then keyboard focus. We should probably have
>> +              * three for loops and check frist for touch,
>> +              * then for pointer, etc. but unless somebody has some
>> +              * objections, I think this is sufficient. */
>> +             if (seat->touch && seat->touch->focus)
>> +                     output = seat->touch->focus->output;
>> +             else if (seat->pointer && seat->pointer->focus)
>> +                     output = seat->pointer->focus->output;
>> +             else if (seat->keyboard && seat->keyboard->focus)
>> +                     output = seat->keyboard->focus->output;
>> +
>> +             if (output)
>> +                     break;
>> +     }
>> +
>> +     return output;
>> +}
>> +
>> +static void
>> +shell_interface_set_maximized(struct shell_surface *shsurf)
>> +{
>> +     struct weston_output *output;
>> +
>> +     surface_clear_next_states(shsurf);
>> +     shsurf->next_state.maximized = true;
>> +     shsurf->state_changed = true;
>> +     shsurf->type = SHELL_SURFACE_TOPLEVEL;
>> +
>> +     if (!weston_surface_is_mapped(shsurf->surface))
>> +             output = get_focused_output(shsurf->surface->compositor);
>> +     else
>> +             output = shsurf->surface->output;
>> +
>> +     shell_surface_set_output(shsurf, output);
>> +     send_configure_for_surface(shsurf);
>
> I wonder if setting shsurf->type is needed, but otherwise can't see
> anything odd here on a first glance.
>
>> +}
>> +
>>  static int
>>  shell_interface_move(struct shell_surface *shsurf, struct weston_seat *ws)
>>  {
>> @@ -3534,32 +3579,6 @@ get_primary_view(void *shell, struct shell_surface *shsurf)
>>       return shsurf->view;
>>  }
>>
>> -static struct weston_output *
>> -get_focused_output(struct weston_compositor *compositor)
>> -{
>> -     struct weston_seat *seat;
>> -     struct weston_output *output = NULL;
>> -
>> -     wl_list_for_each(seat, &compositor->seat_list, link) {
>> -             /* Priority has touch focus, then pointer and
>> -              * then keyboard focus. We should probably have
>> -              * three for loops and check frist for touch,
>> -              * then for pointer, etc. but unless somebody has some
>> -              * objections, I think this is sufficient. */
>> -             if (seat->touch && seat->touch->focus)
>> -                     output = seat->touch->focus->output;
>> -             else if (seat->pointer && seat->pointer->focus)
>> -                     output = seat->pointer->focus->output;
>> -             else if (seat->keyboard && seat->keyboard->focus)
>> -                     output = seat->keyboard->focus->output;
>> -
>> -             if (output)
>> -                     break;
>> -     }
>> -
>> -     return output;
>> -}
>> -
>>  static void
>>  shell_get_shell_surface(struct wl_client *client,
>>                       struct wl_resource *resource,
>> @@ -5278,8 +5297,13 @@ set_maximized_position(struct desktop_shell *shell,
>>       pixman_box32_t *e;
>>
>>       get_output_work_area(shell, shsurf->output, &area);
>> -     surface_subsurfaces_boundingbox(shsurf->surface,
>> -                                     &surf_x, &surf_y, NULL, NULL);
>> +     if (shsurf->has_set_geometry) {
>> +             surf_x = shsurf->geometry.x;
>> +             surf_y = shsurf->geometry.y;
>> +     } else {
>> +             surface_subsurfaces_boundingbox(shsurf->surface,
>> +                                             &surf_x, &surf_y, NULL, NULL);
>> +     }
>
> This hunk looks like it should be a patch of its own.
>
> set_maximized_position() is called by map() and configure() which are
> both called from shell_surface_configure(), which is set as the
> weston_surface::configure for both wl_shell and xdg_shell in
> create_common_surface().
>
> I'm guessing that if an xdg_shell app used a maximized view, where the
> window geometry did not cover the whole surface, it would be positioned
> badly.

You're right, the lazyness was too strong ;). I'll take it out to a
separate patch.

>
>
>>       e = pixman_region32_extents(&shsurf->output->region);
>>
>>       weston_view_set_position(shsurf->view,
>> @@ -6520,6 +6544,7 @@ module_init(struct weston_compositor *ec,
>>       ec->shell_interface.resize = surface_resize;
>>       ec->shell_interface.set_title = set_title;
>>       ec->shell_interface.set_window_geometry = set_window_geometry;
>> +     ec->shell_interface.set_maximized = shell_interface_set_maximized;
>>
>>       weston_layer_init(&shell->fullscreen_layer, &ec->cursor_layer.link);
>>       weston_layer_init(&shell->panel_layer, &shell->fullscreen_layer.link);
>> diff --git a/src/compositor.h b/src/compositor.h
>> index 2bec183..22642b5 100644
>> --- a/src/compositor.h
>> +++ b/src/compositor.h
>> @@ -120,6 +120,7 @@ struct weston_shell_interface {
>>       void (*set_window_geometry)(struct shell_surface *shsurf,
>>                                   int32_t x, int32_t y,
>>                                   int32_t width, int32_t height);
>> +     void (*set_maximized)(struct shell_surface *shsurf);
>>  };
>>
>>  struct weston_animation {
>> diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
>> index 4acb534..68f8782 100644
>> --- a/xwayland/window-manager.c
>> +++ b/xwayland/window-manager.c
>> @@ -144,6 +144,8 @@ struct weston_wm_window {
>>       int fullscreen;
>>       int has_alpha;
>>       int delete_window;
>> +     int maximized_vert;
>> +     int maximized_horz;
>>       struct wm_size_hints size_hints;
>>       struct motif_wm_hints motif_hints;
>>       struct wl_list link;
>> @@ -472,6 +474,10 @@ weston_wm_window_read_properties(struct weston_wm_window *window)
>>                       for (i = 0; i < reply->value_len; i++)
>>                               if (atom[i] == wm->atom.net_wm_state_fullscreen)
>>                                       window->fullscreen = 1;
>> +                             if (atom[i] == wm->atom.net_wm_state_maximized_vert)
>> +                                     window->maximized_vert = 1;
>> +                             if (atom[i] == wm->atom.net_wm_state_maximized_horz)
>> +                                     window->maximized_horz = 1;
>>                       break;
>>               case TYPE_MOTIF_WM_HINTS:
>>                       memcpy(&window->motif_hints,
>> @@ -479,7 +485,7 @@ weston_wm_window_read_properties(struct weston_wm_window *window)
>>                              sizeof window->motif_hints);
>>                       if (window->motif_hints.flags & MWM_HINTS_DECORATIONS)
>>                               window->decorate =
>> -                                     window->motif_hints.decorations > 0;
>> +                                     window->motif_hints.decorations;
>>                       break;
>>               default:
>>                       break;
>> @@ -789,12 +795,16 @@ static void
>>  weston_wm_window_set_net_wm_state(struct weston_wm_window *window)
>>  {
>>       struct weston_wm *wm = window->wm;
>> -     uint32_t property[1];
>> +     uint32_t property[3];
>>       int i;
>>
>>       i = 0;
>>       if (window->fullscreen)
>>               property[i++] = wm->atom.net_wm_state_fullscreen;
>> +     if (window->maximized_vert)
>> +             property[i++] = wm->atom.net_wm_state_maximized_vert;
>> +     if (window->maximized_horz)
>> +             property[i++] = wm->atom.net_wm_state_maximized_horz;
>>
>>       xcb_change_property(wm->conn,
>>                           XCB_PROP_MODE_REPLACE,
>> @@ -811,10 +821,14 @@ weston_wm_window_create_frame(struct weston_wm_window *window)
>>       struct weston_wm *wm = window->wm;
>>       uint32_t values[3];
>>       int x, y, width, height;
>> +     int buttons = FRAME_BUTTON_CLOSE;
>> +
>> +     if (window->decorate & MWM_DECOR_MAXIMIZE)
>> +             buttons |= FRAME_BUTTON_MAXIMIZE;
>>
>>       window->frame = frame_create(window->wm->theme,
>>                                    window->width, window->height,
>> -                                  FRAME_BUTTON_CLOSE, window->name);
>> +                                  buttons, window->name);
>>       frame_resize_inside(window->frame, window->width, window->height);
>>
>>       weston_wm_window_get_frame_size(window, &width, &height);
>> @@ -1303,6 +1317,22 @@ static void
>>  weston_wm_window_configure(void *data);
>>
>>  static void
>> +weston_wm_window_set_toplevel(struct weston_wm_window *window)
>> +{
>> +     struct weston_shell_interface *shell_interface =
>> +             &window->wm->server->compositor->shell_interface;
>> +
>> +     shell_interface->set_toplevel(window->shsurf);
>> +     window->width = window->saved_width;
>> +     window->height = window->saved_height;
>> +     if (window->frame)
>> +             frame_resize_inside(window->frame,
>> +                                     window->width,
>> +                                     window->height);
>> +     weston_wm_window_configure(window);
>> +}
>> +
>> +static void
>>  weston_wm_window_handle_state(struct weston_wm_window *window,
>>                             xcb_client_message_event_t *client_message)
>>  {
>> @@ -1310,6 +1340,7 @@ weston_wm_window_handle_state(struct weston_wm_window *window,
>>       struct weston_shell_interface *shell_interface =
>>               &wm->server->compositor->shell_interface;
>>       uint32_t action, property;
>> +     int maximized = window->maximized_horz && window->maximized_vert;
>>
>>       action = client_message->data.data32[0];
>>       property = client_message->data.data32[1];
>> @@ -1326,14 +1357,26 @@ weston_wm_window_handle_state(struct weston_wm_window *window,
>>                                                               WL_SHELL_SURFACE_FULLSCREEN_METHOD_DEFAULT,
>>                                                               0, NULL);
>>               } else {
>> -                     shell_interface->set_toplevel(window->shsurf);
>> -                     window->width = window->saved_width;
>> -                     window->height = window->saved_height;
>> -                     if (window->frame)
>> -                             frame_resize_inside(window->frame,
>> -                                                 window->width,
>> -                                                 window->height);
>> -                     weston_wm_window_configure(window);
>> +                     weston_wm_window_set_toplevel(window);
>> +             }
>> +     } else {
>> +             if (property == wm->atom.net_wm_state_maximized_vert &&
>> +                 update_state(action, &window->maximized_vert))
>> +                     weston_wm_window_set_net_wm_state(window);
>> +             if (property == wm->atom.net_wm_state_maximized_horz &&
>> +                 update_state(action, &window->maximized_horz))
>> +                     weston_wm_window_set_net_wm_state(window);
>> +
>> +             if (maximized != window->maximized_vert && window->maximized_horz) {
>> +                     if (window->maximized_vert && window->maximized_horz) {
>> +                             window->saved_width = window->width;
>> +                             window->saved_height = window->height;
>> +
>> +                             if (window->shsurf)
>> +                                     shell_interface->set_maximized(window->shsurf);
>> +                     } else {
>> +                             weston_wm_window_set_toplevel(window);
>> +                     }
>>               }
>>       }
>>  }
>> @@ -1665,6 +1708,19 @@ weston_wm_handle_button(struct weston_wm *wm, xcb_generic_event_t *event)
>>               weston_wm_window_close(window, button->time);
>>               frame_status_clear(window->frame, FRAME_STATUS_CLOSE);
>>       }
>> +
>> +     if (frame_status(window->frame) & FRAME_STATUS_MAXIMIZE) {
>> +             window->maximized_horz = !window->maximized_horz;
>> +             window->maximized_vert = !window->maximized_vert;
>
> I wonder if an X app can get maximized_horz != maximized_vert somehow.
> Should we be more defensive here against that?

Well, in that case the behavior would be what we have now, i.e. do nothing.


Thanks,
Giulio

>
>> +             if (window->maximized_horz && window->maximized_vert) {
>> +                     window->saved_width = window->width;
>> +                     window->saved_height = window->height;
>> +                     shell_interface->set_maximized(window->shsurf);
>> +             } else {
>> +                     weston_wm_window_set_toplevel(window);
>> +             }
>> +             frame_status_clear(window->frame, FRAME_STATUS_MAXIMIZE);
>> +     }
>>  }
>>
>>  static void
>> @@ -1853,6 +1909,8 @@ weston_wm_get_resources(struct weston_wm *wm)
>>               { "_NET_WM_PID",        F(atom.net_wm_pid) },
>>               { "_NET_WM_ICON",       F(atom.net_wm_icon) },
>>               { "_NET_WM_STATE",      F(atom.net_wm_state) },
>> +             { "_NET_WM_STATE_MAXIMIZED_VERT", F(atom.net_wm_state_maximized_vert) },
>> +             { "_NET_WM_STATE_MAXIMIZED_HORZ", F(atom.net_wm_state_maximized_horz) },
>>               { "_NET_WM_STATE_FULLSCREEN", F(atom.net_wm_state_fullscreen) },
>>               { "_NET_WM_USER_TIME", F(atom.net_wm_user_time) },
>>               { "_NET_WM_ICON_NAME", F(atom.net_wm_icon_name) },
>> @@ -2029,7 +2087,7 @@ weston_wm_create(struct weston_xserver *wxs, int fd)
>>       struct wl_event_loop *loop;
>>       xcb_screen_iterator_t s;
>>       uint32_t values[1];
>> -     xcb_atom_t supported[3];
>> +     xcb_atom_t supported[5];
>>
>>       wm = zalloc(sizeof *wm);
>>       if (wm == NULL)
>> @@ -2080,6 +2138,8 @@ weston_wm_create(struct weston_xserver *wxs, int fd)
>>       supported[0] = wm->atom.net_wm_moveresize;
>>       supported[1] = wm->atom.net_wm_state;
>>       supported[2] = wm->atom.net_wm_state_fullscreen;
>> +     supported[3] = wm->atom.net_wm_state_maximized_vert;
>> +     supported[4] = wm->atom.net_wm_state_maximized_horz;
>>       xcb_change_property(wm->conn,
>>                           XCB_PROP_MODE_REPLACE,
>>                           wm->screen->root,
>> @@ -2342,6 +2402,8 @@ xserver_map_shell_surface(struct weston_wm_window *window,
>>                                              parent->surface,
>>                                              window->x - parent->x,
>>                                              window->y - parent->y, 0);
>> +     } else if (window->maximized_vert && window->maximized_horz) {
>> +             shell_interface->set_maximized(window->shsurf);
>>       } else {
>>               shell_interface->set_toplevel(window->shsurf);
>>       }
>> diff --git a/xwayland/xwayland.h b/xwayland/xwayland.h
>> index 312c9b2..69fdcfd 100644
>> --- a/xwayland/xwayland.h
>> +++ b/xwayland/xwayland.h
>> @@ -102,6 +102,8 @@ struct weston_wm {
>>               xcb_atom_t               net_wm_pid;
>>               xcb_atom_t               net_wm_icon;
>>               xcb_atom_t               net_wm_state;
>> +             xcb_atom_t               net_wm_state_maximized_vert;
>> +             xcb_atom_t               net_wm_state_maximized_horz;
>>               xcb_atom_t               net_wm_state_fullscreen;
>>               xcb_atom_t               net_wm_user_time;
>>               xcb_atom_t               net_wm_icon_name;
>
> Thanks,
> pq