Message info
 
To:Martin Minarik From:Kristian Høgsberg Subject:Re: [PATCH] Weston: window.c: Frame buttons Date:Mon, 7 May 2012 16:44:41 -0400
 

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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel

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