[PATCH] Weston: window.c: Frame buttons

Kristian Høgsberg hoegsberg at gmail.com
Mon May 7 13:44:41 PDT 2012


On Fri, May 04, 2012 at 02:34:24PM +0200, Martin Minarik wrote:
> Thanks for all the feedback.
> We are adding frame buttons to wl_list, as suggested by vignatti.

I'm still getting this through squirrelmail which breaks the newlines.
Can you resend with git send-email?

The patch looks good, but call it frame_button or just button please.
Good progress, just a few more comments below.

thanks,
Kristian

> ---
>  clients/window.c |  254
> +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  data/Makefile.am |    4 +
>  2 files changed, 255 insertions(+), 3 deletions(-)
> 
> diff --git a/clients/window.c b/clients/window.c
> index 2dcf421..613050c 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -200,12 +200,38 @@ struct output {
>  	void *user_data;
>  };
> 
> +enum framebutton_action {
> +	FRAMEBUTTON_NULL = 0,
> +	FRAMEBUTTON_ICON = 1,
> +	FRAMEBUTTON_CLOSE = 2,
> +	FRAMEBUTTON_MINIMIZE = 3,
> +	FRAMEBUTTON_MAXIMIZE = 4,
> +};
> +
> +enum framebutton_pointer {
> +	FRAMEBUTTON_DEFAULT = 0,
> +	FRAMEBUTTON_OVER = 1,
> +	FRAMEBUTTON_ACTIVE = 2,
> +};
> +
> +struct framebutton {
> +	struct widget *widget;
> +	struct frame *frame;
> +	cairo_surface_t *icon;
> +	enum framebutton_action type;
> +	enum framebutton_pointer state;
> +	struct wl_list link;	/* buttons_list */
> +	int position_left;
> +	int border;
> +};
> +
>  struct frame {
>  	struct widget *widget;
>  	struct widget *child;
>  	int margin;
>  	int width;
>  	int titlebar_height;
> +	struct wl_list buttons_list;
>  };
> 
>  struct menu {
> @@ -980,6 +1006,8 @@ frame_resize_handler(struct widget *widget,
>  	struct widget *child = frame->child;
>  	struct rectangle allocation;
>  	struct display *display = widget->window->display;
> +	struct framebutton * button;
> +	int x_l, x_r, y, w, h;
>  	int decoration_width, decoration_height;
>  	int opaque_margin;
> 
> @@ -1001,6 +1029,10 @@ frame_resize_handler(struct widget *widget,
>  			      height - 2 * frame->margin);
> 
>  		opaque_margin = frame->margin + display->frame_radius;
> +
> +		wl_list_for_each(button, &frame->buttons_list, link) {
> +			button->widget->opaque=0;

Please follow the coding convention in the code.  Specifically, use
spaces around '=' in the assignment.

> +		}
>  	} else {
>  		decoration_width = 0;
>  		decoration_height = 0;
> @@ -1010,6 +1042,10 @@ frame_resize_handler(struct widget *widget,
>  		allocation.width = width;
>  		allocation.height = height;
>  		opaque_margin = 0;
> +
> +		wl_list_for_each(button, &frame->buttons_list, link) {
> +			button->widget->opaque=1;

Missing spaces here too.

> +		}
>  	}
> 
>  	widget_set_allocation(child, allocation.x, allocation.y,
> @@ -1033,6 +1069,180 @@ frame_resize_handler(struct widget *widget,
>  			      widget->allocation.width - 2 * opaque_margin,
>  			      widget->allocation.height - 2 * opaque_margin);
>  	}
> +	/* frame internal buttons */
> +	x_r = frame->widget->allocation.width - frame->width - frame->margin;
> +	x_l = frame->width + frame->margin;
> +	y = frame->width + frame->margin;
> +	wl_list_for_each(button, &frame->buttons_list, link) {
> +		w = cairo_image_surface_get_width(button->icon);
> +		h = cairo_image_surface_get_height(button->icon);
> +
> +		if (button->border){w += 10;}

Again, please follow the coding convention.  The if statement body
goes on its own line, don't use {} for just a single statement.

> +
> +		if (button->position_left){
> +			widget_set_allocation(button->widget,
> +				x_l, y , w + 1, h + 1);
> +			x_l += w;
> +			x_l += 4;

Let's use a const int button_padding = 4; here instead of the hard coded 4.

> +		} else {
> +			x_r -= w;
> +			widget_set_allocation(button->widget,
> +				x_r, y , w + 1, h + 1);
> +			x_r -= 4;
> +		}
> +	}
> +}
> +
> +static int
> +framebutton_enter_handler(struct widget *widget,
> +		struct input *input, uint32_t time,
> +		int32_t x, int32_t y, void *data)
> +{
> +	struct framebutton *framebutton = data;
> +	widget_schedule_redraw(framebutton->widget);
> +	framebutton->state=FRAMEBUTTON_OVER;
> +	return POINTER_LEFT_PTR;
> +}
> +
> +static int
> +framebutton_leave_handler(struct widget *widget, struct input *input,
> void *data)
> +{
> +	struct framebutton *framebutton = data;
> +	widget_schedule_redraw(framebutton->widget);
> +	framebutton->state=FRAMEBUTTON_DEFAULT;
> +	return POINTER_LEFT_PTR;
> +}
> +
> +static void
> +framebutton_button_handler(struct widget *widget,
> +		struct input *input, uint32_t time,
> +		int button, int state, void *data)
> +
> +{
> +	struct framebutton *framebutton = data;
> +	struct window *window = widget->window;
> +
> +	if (button != BTN_LEFT)
> +		return;
> +
> +	switch (state){
> +		case 1:

'case' should align with 'switch'.

> +			framebutton->state=FRAMEBUTTON_ACTIVE;
> +			widget_schedule_redraw(framebutton->widget);
> +			return;
> +		break;
> +		case 0:
> +			framebutton->state=FRAMEBUTTON_DEFAULT;
> +			widget_schedule_redraw(framebutton->widget);
> +		break;
> +	}
> +
> +	switch (framebutton->type){
> +	case FRAMEBUTTON_ICON:
> +		/* FIXME: window_show_frame_menu(window, input, time); */
> +	break;
> +	case FRAMEBUTTON_CLOSE:
> +		if (window->close_handler)
> +			window->close_handler(window->parent,
> +					window->user_data);
> +		else
> +			display_exit(window->display);
> +	break;

'break' should be indented with the case body.

> +	case FRAMEBUTTON_MINIMIZE:
> +		fprintf(stderr,"Minimize stub\n");
> +		return;
> +	break;
> +	case FRAMEBUTTON_MAXIMIZE:
> +		window_set_maximized(window, window->type != TYPE_MAXIMIZED);
> +		return;
> +	break;
> +	default: /* Unknown operation */ return; break;
> +	}
> +}
> +
> +static void
> +framebutton_redraw_handler(struct widget *widget, void *data)
> +{
> +	struct framebutton *framebutton = data;
> +	cairo_t *cr;
> +	int width, height, x, y;
> +	struct window *window = widget->window;
> +
> +	x = widget->allocation.x;
> +	y = widget->allocation.y;
> +	width = widget->allocation.width;
> +	height = widget->allocation.height;
> +
> +	if (!width)
> +		return;
> +	if (!height)
> +		return;
> +	if (widget->opaque)
> +		return;
> +
> +	assert(window!=NULL);
> +	assert(window->cairo_surface!=NULL);
> +
> +	cr = cairo_create(window->cairo_surface);
> +
> +	if (framebutton->border){

Space between ) and {.

> +		cairo_set_line_width(cr, 1);
> +
> +		cairo_set_source_rgb(cr, 0.0, 0.0, 0.0);
> +		cairo_rectangle (cr, x, y, 25, 16);
> +
> +		cairo_stroke_preserve(cr);
> +		switch (framebutton->state){
> +		case FRAMEBUTTON_DEFAULT:
> +		cairo_set_source_rgb(cr, 0.88, 0.88, 0.88);
> +		break;

Indent.

> +		case FRAMEBUTTON_OVER:
> +		cairo_set_source_rgb(cr, 1.0, 1.0, 1.0);
> +		break;
> +		case FRAMEBUTTON_ACTIVE:
> +		cairo_set_source_rgb(cr, 0.7, 0.7, 0.7);
> +		break;
> +		}
> +		cairo_fill (cr);
> +
> +		x += 4;
> +	}
> +
> +	cairo_set_source_surface(cr, framebutton->icon,
> +			x, y);
> +	cairo_paint(cr);
> +
> +	cairo_destroy(cr);
> +}
> +
> +struct widget *
> +framebutton_create(struct frame *frame, void *data)
> +{
> +	struct framebutton *framebutton;
> +	const char *icon = data;
> +
> +	framebutton = malloc (sizeof *framebutton);
> +	memset(framebutton, 0, sizeof *framebutton);
> +
> +	framebutton->icon = cairo_image_surface_create_from_png(icon);
> +	framebutton->widget = widget_add_widget(frame->widget, framebutton);
> +	framebutton->frame = frame;
> +
> +	wl_list_insert(frame->buttons_list.prev, &framebutton->link);
> +
> +	widget_set_redraw_handler(framebutton->widget, framebutton_redraw_handler);
> +	widget_set_enter_handler(framebutton->widget, framebutton_enter_handler);
> +	widget_set_leave_handler(framebutton->widget, framebutton_leave_handler);
> +	widget_set_button_handler(framebutton->widget, framebutton_button_handler);
> +	return framebutton->widget;
> +}
> +
> +void
> +framebutton_destroy(struct framebutton *framebutton)
> +{
> +	wl_list_remove(&framebutton->link);
> +	free(framebutton);

Destroy the button->icon here too.

> +	return;
>  }
> 
>  static void
> @@ -1278,6 +1488,8 @@ struct widget *
>  frame_create(struct window *window, void *data)
>  {
>  	struct frame *frame;
> +	struct widget *fr_w;
> +	struct framebutton *fr_b;
> 
>  	frame = malloc(sizeof *frame);
>  	memset(frame, 0, sizeof *frame);
> @@ -1285,15 +1497,46 @@ frame_create(struct window *window, void *data)
>  	frame->widget = window_add_widget(window, frame);
>  	frame->child = widget_add_widget(frame->widget, data);
>  	frame->margin = 32;
> -	frame->width = 4;
> -	frame->titlebar_height = 30
> -;
> +	frame->width = 6;
> +	frame->titlebar_height = 27;
> +
>  	widget_set_redraw_handler(frame->widget, frame_redraw_handler);
>  	widget_set_resize_handler(frame->widget, frame_resize_handler);
>  	widget_set_enter_handler(frame->widget, frame_enter_handler);
>  	widget_set_motion_handler(frame->widget, frame_motion_handler);
>  	widget_set_button_handler(frame->widget, frame_button_handler);
> 
> +	/* Create empty list for frame buttons */
> +	wl_list_init(&frame->buttons_list);
> +
> +	/* Create icon */
> +	fr_w = framebutton_create(frame, DATADIR "/weston/icon_window.png");
> +	fr_b = fr_w->user_data;
> +	fr_b->type = FRAMEBUTTON_ICON;
> +	fr_b->position_left = 1;
> +	fr_b->border = 0;

Just pass these details to frame_button_create and use an enum with
FRAME_BUTTON_LEFT and FRAME_BUTTON_RIGHT instead of the position_left
bool:

  frame_button_create(frame, DATADIR "/weston/icon_window.png".
                      FRAME_BUTTON_ICON, FRAME_BUTTON_LEFT, 0);

> +	/* Create close button */
> +	fr_w = framebutton_create(frame, DATADIR "/weston/sign_close.png");
> +	fr_b = fr_w->user_data;
> +	fr_b->type = FRAMEBUTTON_CLOSE;
> +	fr_b->position_left = 0;
> +	fr_b->border = 1;
> +
> +	/* Create maximize button */
> +	fr_w = framebutton_create(frame, DATADIR "/weston/sign_maximize.png");
> +	fr_b = fr_w->user_data;
> +	fr_b->type = FRAMEBUTTON_MAXIMIZE;
> +	fr_b->position_left = 0;
> +	fr_b->border = 1;
> +
> +	/* Create minimize button */
> +	fr_w = framebutton_create(frame, DATADIR "/weston/sign_minimize.png");
> +	fr_b = fr_w->user_data;
> +	fr_b->type = FRAMEBUTTON_MINIMIZE;
> +	fr_b->position_left = 0;
> +	fr_b->border = 1;
> +
>  	window->frame = frame;
> 
>  	return frame->child;
> @@ -1302,6 +1545,11 @@ frame_create(struct window *window, void *data)
>  static void
>  frame_destroy(struct frame *frame)
>  {
> +	struct framebutton* button;
> +	wl_list_for_each(button, &frame->buttons_list, link) {
> +		framebutton_destroy(button);
> +	}
> +
>  	/* frame->child must be destroyed by the application */
>  	widget_destroy(frame->widget);
>  	free(frame);
> diff --git a/data/Makefile.am b/data/Makefile.am
> index ec2723a..a8441ef 100644
> --- a/data/Makefile.am
> +++ b/data/Makefile.am
> @@ -3,6 +3,10 @@ westondatadir = $(datadir)/weston
>  dist_westondata_DATA =				\
>  	wayland.svg				\
>  	$(wayland_icon_png)			\
> +	icon_window.png				\
> +	sign_close.png				\
> +	sign_maximize.png			\
> +	sign_minimize.png			\

Again, I don't see these images in the patch, please use git
send-email to send the patch and it will just work.

>  	pattern.png				\
>  	terminal.png				\
>  	border.png
> -- 
> 1.7.5.4


> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel



More information about the wayland-devel mailing list