[PATCH weston 2/8 v2] shell: Implement panel window list.
Scott Moreau
oreaus at gmail.com
Tue Mar 5 21:49:30 PST 2013
Hi Kristian,
Sorry it took so long to get back to you on this but I had to deal with
some other more pressing things. I wanted to reply while I am rebasing this
series to master, in case there are any other concerns. Please see
responses below.
On Wed, Nov 28, 2012 at 7:48 PM, Kristian Høgsberg <hoegsberg at gmail.com>wrote:
> On Thu, Nov 15, 2012 at 12:37:01AM -0700, Scott Moreau wrote:
> > This patch uses the special surface_data interface to send information
> about
> > the surface to the shell. The shell then uses this information to render
> a
> > window list in the panel.
>
> The previous XML patch was good and the overall approach here is fine.
> There's just a few more things below we need to fix and then we can
> commit this.
>
Cool.
>
> > ---
> > clients/desktop-shell.c | 526
> ++++++++++++++++++++++++++++++++++++++++++++++--
> > data/Makefile.am | 1 +
> > data/list_item_icon.png | Bin 0 -> 176 bytes
> > src/compositor.c | 2 +
> > src/compositor.h | 1 +
> > src/shell.c | 170 ++++++++++++++++
> > 6 files changed, 683 insertions(+), 17 deletions(-)
> > create mode 100644 data/list_item_icon.png
> >
> > diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
> > index 1cae789..ea42c66 100644
> > --- a/clients/desktop-shell.c
> > +++ b/clients/desktop-shell.c
> > @@ -44,6 +44,8 @@
> >
> > #include "desktop-shell-client-protocol.h"
> >
> > +#define MIN(x, y) (((x) < (y)) ? (x) : (y))
> > +
> > extern char **environ; /* defined by libc */
> >
> > struct desktop {
> > @@ -51,37 +53,59 @@ struct desktop {
> > struct desktop_shell *shell;
> > struct unlock_dialog *unlock_dialog;
> > struct task unlock_task;
> > + struct wl_list surfaces;
> > struct wl_list outputs;
> > + uint32_t output_count;
> >
> > struct window *grab_window;
> > struct widget *grab_widget;
> >
> > enum cursor_type grab_cursor;
> > +
> > + struct surface_data_manager *surface_data_manager;
> > };
> >
> > struct surface {
> > + struct wl_list item_list;
> > + struct surface_data *surface_data;
> > + uint32_t output_mask;
> > + char *title;
> > +
> > + struct wl_list link;
> > +};
> > +
> > +struct resize {
> > void (*configure)(void *data,
> > struct desktop_shell *desktop_shell,
> > uint32_t edges, struct window *window,
> > int32_t width, int32_t height);
> > };
> >
> > +struct rgba {
> > + float r, g, b, a;
> > +};
> > +
> > struct panel {
> > - struct surface base;
> > + struct resize base;
> > struct window *window;
> > struct widget *widget;
> > struct wl_list launcher_list;
> > + struct wl_list window_list;
> > + struct rectangle window_list_rect;
> > + uint32_t surface_count;
> > + struct rgba focused_item;
> > struct panel_clock *clock;
> > };
> >
> > struct background {
> > - struct surface base;
> > + struct resize base;
> > struct window *window;
> > struct widget *widget;
> > };
> >
> > struct output {
> > struct wl_output *output;
> > + uint32_t id;
> > struct wl_list link;
> >
> > struct panel *panel;
> > @@ -99,6 +123,16 @@ struct panel_launcher {
> > struct wl_array argv;
> > };
> >
> > +struct list_item {
> > + struct surface *surface;
> > + struct widget *widget;
> > + struct panel *panel;
> > + cairo_surface_t *icon;
> > + int focused, highlight;
> > + struct wl_list link;
> > + struct wl_list surface_link;
> > +};
> > +
> > struct panel_clock {
> > struct widget *widget;
> > struct panel *panel;
> > @@ -249,6 +283,15 @@ set_hex_color(cairo_t *cr, uint32_t color)
> > }
> >
> > static void
> > +get_hex_color_rgba(uint32_t color, float *r, float *g, float *b, float
> *a)
> > +{
> > + *r = ((color >> 16) & 0xff) / 255.0;
> > + *g = ((color >> 8) & 0xff) / 255.0;
> > + *b = ((color >> 0) & 0xff) / 255.0;
> > + *a = ((color >> 24) & 0xff) / 255.0;
> > +}
> > +
> > +static void
> > panel_redraw_handler(struct widget *widget, void *data)
> > {
> > cairo_surface_t *surface;
> > @@ -421,15 +464,49 @@ panel_button_handler(struct widget *widget,
> > }
> >
> > static void
> > +panel_window_list_schedule_redraw(struct panel *panel)
> > +{
> > + struct list_item *item;
> > + float x, y, w, h;
> > + float item_width, padding;
> > +
> > + /* If there are no window list items, redraw the panel to clear it
> */
> > + if (wl_list_empty(&panel->window_list)) {
> > + widget_schedule_redraw(panel->widget);
> > + return;
> > + }
> > +
> > + item_width = ((float) panel->window_list_rect.width /
> > +
> panel->surface_count);
> > + padding = MIN(item_width * 0.1f, 2.0f);
>
> We don't want floats here. We're trying to compute an integer number
> of pixels that will be the item width. Also, item padding is always a
> constant number of pixels (2 is fine), not proportional to the width.
>
Yes, you are right.
>
> > + x = panel->window_list_rect.x + padding;
> > + y = 16;
> > + w = MIN(item_width - padding, 200);
> > + h = 24;
> > +
> > + wl_list_for_each(item, &panel->window_list, link) {
> > + widget_set_allocation(item->widget, x, y - h / 2, w + 1, h
> + 1);
>
> It'd be more straightforward to just use x, 4, w, 24 here. I know
> it's done that way elsewhere in desktop-shell.c but it doesn't make
> sense there either.
>
Done.
>
> > + x += w + padding;
> > + widget_schedule_redraw(item->widget);
> > + }
> > +}
> > +
> > +static void
> > panel_resize_handler(struct widget *widget,
> > int32_t width, int32_t height, void *data)
> > {
> > struct panel_launcher *launcher;
> > + struct rectangle launcher_rect;
> > + struct rectangle clock_rect;
> > struct panel *panel = data;
> > int x, y, w, h;
> > -
> > +
> > x = 10;
> > y = 16;
> > +
> > + launcher_rect.x = x;
> > +
> > wl_list_for_each(launcher, &panel->launcher_list, link) {
> > w = cairo_image_surface_get_width(launcher->icon);
> > h = cairo_image_surface_get_height(launcher->icon);
> > @@ -437,12 +514,25 @@ panel_resize_handler(struct widget *widget,
> > x, y - h / 2, w + 1, h + 1);
> > x += w + 10;
>
> We could fix the weird y - h / 2 here too.
>
No problem.
>
> > }
> > - h=20;
> > +
> > + launcher_rect.width = x - launcher_rect.x;
> > +
> > w=170;
> > + h=20;
> >
> > if (panel->clock)
> > widget_set_allocation(panel->clock->widget,
> > width - w - 8, y - h / 2, w + 1, h +
> 1);
> > +
> > + widget_get_allocation(panel->clock->widget, &clock_rect);
> > +
> > + panel->window_list_rect.x = launcher_rect.x + launcher_rect.width;
> > + panel->window_list_rect.y = 2;
> > + panel->window_list_rect.width = width -
> > + panel->window_list_rect.x -
> > + (clock_rect.width + 20);
> > + panel->window_list_rect.height = 28;
> > + panel_window_list_schedule_redraw(panel);
> > }
> >
> > static void
> > @@ -451,7 +541,7 @@ panel_configure(void *data,
> > uint32_t edges, struct window *window,
> > int32_t width, int32_t height)
> > {
> > - struct surface *surface = window_get_user_data(window);
> > + struct resize *surface = window_get_user_data(window);
> > struct panel *panel = container_of(surface, struct panel, base);
> >
> > window_schedule_resize(panel->window, width, 32);
> > @@ -490,6 +580,25 @@ panel_destroy(struct panel *panel)
> > free(panel);
> > }
> >
> > +static void
> > +panel_set_list_item_focus_color(struct panel *panel)
> > +{
> > + float r, g, b, a;
> > +
> > + /* Consider panel color when choosing item highlight color */
> > + get_hex_color_rgba(key_panel_color, &r, &b, &g, &a);
> > + if (r += 0.2, g += 0.2, b += 0.2, r > 1.0 || g > 1.0 || b > 1.0) {
>
> Can we avoid all this stuff inside the if? Also, just saturating to
> white might be better.
>
Yes we can avoid the statements in the conditional but I don't think fading
to white is a good idea. This code attempts to detect lighter color panels
near white and sets the highlight color of the list items to a darker color
to offset. I did restructure it a bit however. This is what I have
currently:
/* Consider panel color when choosing item highlight color */
get_hex_color_rgba(key_panel_color, &r, &b, &g, &a);
r += 0.2;
g += 0.2;
b += 0.2;
panel->focused_item.r = r > 1.0 ? 0.6 : r;
panel->focused_item.g = g > 1.0 ? 0.6 : g;
panel->focused_item.b = b > 1.0 ? 0.6 : b;
panel->focused_item.a = 0.75;
> > + panel->focused_item.r = 0.6;
> > + panel->focused_item.g = 0.6;
> > + panel->focused_item.b = 0.6;
> > + } else {
> > + panel->focused_item.r = r;
> > + panel->focused_item.g = g;
> > + panel->focused_item.b = b;
> > + }
> > + panel->focused_item.a = 0.75;
> > +}
> > +
> > static struct panel *
> > panel_create(struct display *display)
> > {
> > @@ -502,6 +611,7 @@ panel_create(struct display *display)
> > panel->window = window_create_custom(display);
> > panel->widget = window_add_widget(panel->window, panel);
> > wl_list_init(&panel->launcher_list);
> > + wl_list_init(&panel->window_list);
> >
> > window_set_title(panel->window, "panel");
> > window_set_user_data(panel->window, panel);
> > @@ -509,7 +619,9 @@ panel_create(struct display *display)
> > widget_set_redraw_handler(panel->widget, panel_redraw_handler);
> > widget_set_resize_handler(panel->widget, panel_resize_handler);
> > widget_set_button_handler(panel->widget, panel_button_handler);
> > -
> > +
> > + panel->surface_count = 0;
> > + panel_set_list_item_focus_color(panel);
> > panel_add_clock(panel);
> >
> > return panel;
> > @@ -518,18 +630,21 @@ panel_create(struct display *display)
> > static cairo_surface_t *
> > load_icon_or_fallback(const char *icon)
> > {
> > - cairo_surface_t *surface =
> cairo_image_surface_create_from_png(icon);
> > + cairo_surface_t *surface;
> > cairo_t *cr;
> > -
> > - if (cairo_surface_status(surface) == CAIRO_STATUS_SUCCESS)
> > - return surface;
> > -
> > - cairo_surface_destroy(surface);
> > - fprintf(stderr, "ERROR loading icon from file '%s'\n", icon);
> > +#ifdef CAIRO_HAS_PNG_FUNCTIONS
> > + if (icon) {
> > + surface = cairo_image_surface_create_from_png(icon);
>
> We should just use load_cairo_surface() here instead of using the
> cairo png functions. That lets us load other formats and we don't
> need the #ifdef.
>
Yes, this is much better since it adds support for other image types as
well.
>
> > + if (cairo_surface_status(surface) == CAIRO_STATUS_SUCCESS)
> > + return surface;
> > +
> > + cairo_surface_destroy(surface);
> > + fprintf(stderr, "ERROR loading icon from file '%s'\n",
> icon);
> > + }
> > +#endif
> >
> > /* draw fallback icon */
> > - surface = cairo_image_surface_create(CAIRO_FORMAT_ARGB32,
> > - 20, 20);
> > + surface = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, 20, 20);
> > cr = cairo_create(surface);
> >
> > cairo_set_source_rgba(cr, 0.8, 0.8, 0.8, 1);
> > @@ -862,7 +977,7 @@ desktop_shell_configure(void *data,
> > int32_t width, int32_t height)
> > {
> > struct window *window = wl_surface_get_user_data(surface);
> > - struct surface *s = window_get_user_data(window);
> > + struct resize *s = window_get_user_data(window);
> >
> > s->configure(data, desktop_shell, edges, window, width, height);
> > }
> > @@ -946,6 +1061,362 @@ background_destroy(struct background *background)
> > free(background);
> > }
> >
> > +static void
> > +panel_list_item_redraw_handler(struct widget *widget, void *data)
> > +{
> > + cairo_t *cr;
> > + cairo_surface_t *surface;
> > + struct list_item *item = data;
> > + struct rectangle rect;
> > + cairo_text_extents_t extents;
> > + cairo_font_extents_t font_extents;
> > + int icon_width, icon_height;
> > + unsigned int dots;
> > + double padding;
> > + char title[128];
> > +
> > + widget_get_allocation(widget, &rect);
> > + if (rect.width == 0)
> > + return;
> > +
> > + surface = window_get_surface(item->panel->window);
> > + cr = cairo_create(surface);
> > +
> > + if (item->focused) {
> > + cairo_set_source_rgba(cr, item->panel->focused_item.r,
> > + item->panel->focused_item.g,
> > + item->panel->focused_item.b,
> > + item->panel->focused_item.a);
> > + cairo_move_to(cr, rect.x, rect.y);
> > + cairo_line_to(cr, rect.x + rect.width, rect.y);
> > + cairo_line_to(cr, rect.x + rect.width, rect.y +
> rect.height);
> > + cairo_line_to(cr, rect.x, rect.y + rect.height);
> > + cairo_line_to(cr, rect.x, rect.y);
>
> Use cairo_rectangle() instead?
>
Yep.
>
> > + cairo_fill(cr);
> > + }
> > +
> > + icon_width = cairo_image_surface_get_width(item->icon);
> > + icon_height = cairo_image_surface_get_height(item->icon);
> > + padding = (rect.height / 2.f) - (icon_height / 2.f);
> > + if (rect.width > icon_width * 2) {
> > + cairo_set_source_surface(cr, item->icon,
> > + rect.x + padding,
> > + rect.y + padding);
> > + cairo_paint(cr);
> > + } else {
> > + icon_width = 0;
> > + icon_height = 0;
> > + padding = 1;
> > + }
> > +
> > + strcpy(title, item->surface->title);
> > + cairo_select_font_face(cr, "sans",
> > + CAIRO_FONT_SLANT_NORMAL,
> > + CAIRO_FONT_WEIGHT_NORMAL);
> > + cairo_set_font_size(cr, 14);
> > + cairo_text_extents(cr, title, &extents);
> > +
> > + /* If the string is too long, clip text to button width */
> > + while (extents.width > (rect.width - (icon_width + padding * 3))) {
> > + title[strlen(title) - 1] = '\0';
> > + cairo_text_extents(cr, title, &extents);
> > + if (extents.width <= 0) {
> > + title[0] = '\0';
> > + break;
> > + }
> > + }
> > +
> > + /* If the text is clipped, add an ellipsis */
> > + dots = 3;
> > + if (strlen(title) < dots)
> > + dots = strlen(title) + 1;
> > + if (strlen(title) != strlen(item->surface->title))
> > + while (dots-- > 0)
> > + title[strlen(title) - dots] = '.';
>
> I suppose this is fine... if we want to do better, we'll have to use
> something like pango or harfbuzz to ellipsize the text. For our toy
> desktop, this is par for the course.
>
> > + cairo_font_extents (cr, &font_extents);
> > + cairo_move_to(cr, rect.x + icon_width + padding * 3 + 1,
> > + rect.y + 3 * (rect.height >> 2) + 1);
> > + cairo_set_source_rgb(cr, 0, 0, 0);
> > + cairo_show_text(cr, title);
> > + cairo_move_to(cr, rect.x + icon_width + padding * 3,
> > + rect.y + 3 * (rect.height >> 2));
> > + if (item->focused)
> > + cairo_set_source_rgb(cr, 1, 1, 1);
> > + else
> > + cairo_set_source_rgb(cr, 0.85, 0.85, 0.85);
> > + cairo_show_text(cr, title);
> > + cairo_destroy(cr);
> > +}
> > +
> > +static int
> > +panel_list_item_motion_handler(struct widget *widget, struct input
> *input,
> > + uint32_t time, float x, float y, void *data)
> > +{
> > + struct list_item *item = data;
> > +
> > + widget_set_tooltip(widget, basename((char *)item->surface->title),
> x, y);
> > +
> > + return CURSOR_LEFT_PTR;
> > +}
> > +
> > +static int
> > +panel_list_item_enter_handler(struct widget *widget, struct input
> *input,
> > + float x, float y, void *data)
> > +{
> > + struct list_item *item = data;
> > +
> > + item->focused = 1;
> > + widget_schedule_redraw(widget);
> > +
> > + return CURSOR_LEFT_PTR;
> > +}
> > +
> > +static void
> > +panel_list_item_leave_handler(struct widget *widget,
> > + struct input *input, void *data)
> > +{
> > + struct list_item *item = data;
> > +
> > + item->focused = 0;
> > + widget_destroy_tooltip(widget);
> > + widget_schedule_redraw(widget);
> > +}
> > +
> > +static void
> > +panel_list_item_button_handler(struct widget *widget,
> > + struct input *input, uint32_t time,
> > + uint32_t button,
> > + enum wl_pointer_button_state state, void
> *data)
> > +{
> > + widget_schedule_redraw(widget);
> > + /* TODO: Toggle minimize */
> > +}
> > +
> > +static struct list_item *
> > +panel_list_item_add(struct panel *panel, const char *icon, const char
> *text)
> > +{
> > + struct list_item *item;
> > + item = malloc(sizeof *item);
> > + memset(item, 0, sizeof *item);
> > +
> > + item->icon = load_icon_or_fallback(icon);
> > +
> > + item->panel = panel;
> > + wl_list_insert(panel->window_list.prev, &item->link);
> > + panel->surface_count++;
> > +
> > + item->widget = widget_add_widget(panel->widget, item);
> > + widget_set_enter_handler(item->widget,
> panel_list_item_enter_handler);
> > + widget_set_leave_handler(item->widget,
> panel_list_item_leave_handler);
> > + widget_set_button_handler(item->widget,
> panel_list_item_button_handler);
> > + widget_set_redraw_handler(item->widget,
> panel_list_item_redraw_handler);
> > + widget_set_motion_handler(item->widget,
> panel_list_item_motion_handler);
> > +
> > + return item;
> > +}
> > +
> > +static void
> > +panel_list_item_remove(struct list_item *item)
> > +{
> > + item->panel->surface_count--;
> > + wl_list_remove(&item->link);
> > + wl_list_remove(&item->surface_link);
> > + widget_destroy(item->widget);
> > + panel_window_list_schedule_redraw(item->panel);
> > + free(item);
> > +}
> > +
> > +static int
> > +panel_list_item_exists(struct panel *panel, struct surface *surface)
> > +{
> > + struct list_item *p_item, *s_item;
> > +
> > + wl_list_for_each(p_item, &panel->window_list, link) {
> > + wl_list_for_each(s_item, &surface->item_list,
> surface_link) {
> > + if (p_item == s_item)
> > + return 1;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void
> > +output_update_window_list(struct output *output, struct surface
> *surface)
> > +{
> > + struct list_item *item, *next;
> > + struct panel *panel;
> > +
> > + panel = output->panel;
> > +
> > + /* Make a list item for each panel of the surfaces output mask */
> > + if ((1 << output->id) & surface->output_mask) {
> > + if (!panel_list_item_exists(panel, surface)) {
> > + item = panel_list_item_add(panel,
> > + DATADIR
> "/weston/list_item_icon.png",
> > + surface->title);
> > + wl_list_insert(surface->item_list.prev,
> > +
> &item->surface_link);
> > + item->surface = surface;
> > + }
> > + } else {
> > + /* Remove item from panel if surface
> > + * is no longer on the output */
> > + wl_list_for_each_safe(item, next, &surface->item_list,
> > +
> surface_link) {
> > + if (item->panel == panel)
> > + panel_list_item_remove(item);
> > + }
> > + }
> > +
> > + panel_window_list_schedule_redraw(panel);
> > +}
> > +
> > +static struct surface*
> > +desktop_create_surface(struct desktop *desktop,
> > + struct surface_data *surface_data)
> > +{
> > + struct surface *surface;
> > +
> > + surface = calloc(1, sizeof *surface);
> > +
> > + if (!surface) {
> > + fprintf(stderr, "ERROR: Failed to allocate memory!\n");
> > + exit(EXIT_FAILURE);
> > + }
> > +
> > + surface->surface_data = surface_data;
> > + surface->title = strdup("unknown");
> > + surface->output_mask = 1;
> > + wl_list_init(&surface->item_list);
> > + wl_list_insert(&desktop->surfaces, &surface->link);
> > +
> > + return surface;
> > +}
> > +
> > +static void
> > +desktop_destroy_surface(struct surface *surface)
> > +{
> > + struct list_item *item, *next;
> > +
> > + wl_list_for_each_safe(item, next, &surface->item_list,
> surface_link)
> > + panel_list_item_remove(item);
> > +
> > + wl_list_remove(&surface->link);
> > + free(surface->title);
> > + free(surface);
> > +}
> > +
> > +static struct surface *
> > +desktop_get_surface(struct desktop *desktop, struct surface_data
> *surface_data)
> > +{
> > + struct output *output;
> > + struct list_item *item;
> > +
> > + wl_list_for_each(output, &desktop->outputs, link) {
> > + wl_list_for_each(item, &output->panel->window_list, link) {
> > + if (surface_data == item->surface->surface_data)
> > + return item->surface;
> > + }
> > + }
>
> Just store struct surface as user_data for struct surface_data using
> surface_data_set_user_data(). Then this function becomes just
> surface_data_get_user_data().
>
This seems like it will work.
>
> > + return NULL;
> > +}
> > +
> > +static void
> > +desktop_update_list_items(struct desktop *desktop, struct surface
> *surface)
> > +{
> > + struct output *output;
> > +
> > + wl_list_for_each(output, &desktop->outputs, link)
> > + output_update_window_list(output, surface);
> > +}
> > +
> > +static void
> > +surface_data_set_output_mask(void *data,
> > + struct surface_data *surface_data,
> > + uint32_t output_mask)
> > +{
> > + struct desktop *desktop;
> > + struct surface *surface;
> > +
> > + desktop = data;
> > +
> > + surface = desktop_get_surface(desktop, surface_data);
> > +
> > + if (!surface)
> > + surface = desktop_create_surface(desktop, surface_data);
> > +
> > + surface->output_mask = output_mask;
> > +
> > + desktop_update_list_items(desktop, surface);
> > +}
> > +
> > +static void
> > +surface_data_set_title(void *data,
> > + struct surface_data *surface_data,
> > + const char *title)
> > +{
> > + struct desktop *desktop;
> > + struct surface *surface;
> > +
> > + desktop = data;
> > +
> > + surface = desktop_get_surface(desktop, surface_data);
> > +
> > + if (!surface)
> > + surface = desktop_create_surface(desktop, surface_data);
> > +
> > + if (surface->title)
> > + free(surface->title);
> > + surface->title = strdup(title);
> > +
> > + desktop_update_list_items(desktop, surface);
> > +}
> > +
> > +static void
> > +surface_data_destroy_handler(void *data, struct surface_data
> *surface_data)
> > +{
> > + struct list_item *item, *next;
> > + struct desktop *desktop;
> > + struct output *output;
> > + struct panel *panel;
> > +
> > + desktop = data;
> > +
> > + surface_data_destroy(surface_data);
> > +
> > + wl_list_for_each(output, &desktop->outputs, link) {
> > + panel = output->panel;
> > + wl_list_for_each_safe(item, next, &panel->window_list,
> link) {
> > + if (surface_data == item->surface->surface_data) {
> > + desktop_destroy_surface(item->surface);
> > + return;
> > + }
> > + }
> > + }
> > +}
> > +
> > +static const struct surface_data_listener surface_data_listener = {
> > + surface_data_set_output_mask,
> > + surface_data_set_title,
> > + surface_data_destroy_handler
> > +};
> > +
> > +static void
> > +surface_data_receive_surface_object(void *data,
> > + struct surface_data_manager *manager,
> > + struct surface_data *surface_data)
> > +{
> > + surface_data_add_listener(surface_data,
> > + &surface_data_listener, data);
>
> Just always create struct surface here (fold desktop_create_surface()
> into this function and set the surface_data user data. struct surface
> will need a pointer back the struct desktop.
>
Yep, sounds good.
>
> > +}
> > +
> > +static const struct surface_data_manager_listener
> surface_data_manager_listener = {
> > + surface_data_receive_surface_object
> > +};
> > +
> > static struct background *
> > background_create(struct desktop *desktop)
> > {
> > @@ -1022,6 +1493,15 @@ desktop_destroy_outputs(struct desktop *desktop)
> > }
> >
> > static void
> > +desktop_destroy_surfaces(struct desktop *desktop)
> > +{
> > + struct surface *surface, *next;
> > +
> > + wl_list_for_each_safe(surface, next, &desktop->surfaces, link)
> > + desktop_destroy_surface(surface);
> > +}
> > +
> > +static void
> > create_output(struct desktop *desktop, uint32_t id)
> > {
> > struct output *output;
> > @@ -1033,7 +1513,9 @@ create_output(struct desktop *desktop, uint32_t id)
> > output->output =
> > display_bind(desktop->display, id, &wl_output_interface,
> 1);
> >
> > - wl_list_insert(&desktop->outputs, &output->link);
> > + output->id = desktop->output_count++;
> > +
> > + wl_list_insert(desktop->outputs.prev, &output->link);
> > }
> >
> > static void
> > @@ -1046,6 +1528,12 @@ global_handler(struct display *display, uint32_t
> id,
> > desktop->shell = display_bind(desktop->display,
> > id,
> &desktop_shell_interface, 1);
> > desktop_shell_add_listener(desktop->shell, &listener,
> desktop);
> > + } else if (strcmp(interface, "surface_data_manager") == 0) {
> > + desktop->surface_data_manager =
> > + display_bind(display, id,
> > + &surface_data_manager_interface,
> 1);
> > +
> surface_data_manager_add_listener(desktop->surface_data_manager,
> > + &surface_data_manager_listener,
> desktop);
> > } else if (!strcmp(interface, "wl_output")) {
> > create_output(desktop, id);
> > }
> > @@ -1100,6 +1588,9 @@ int main(int argc, char *argv[])
> > return -1;
> > }
> >
> > + wl_list_init(&desktop.surfaces);
> > + desktop.output_count = 0;
> > +
> > display_set_user_data(desktop.display, &desktop);
> > display_set_global_handler(desktop.display, global_handler);
> >
> > @@ -1133,6 +1624,7 @@ int main(int argc, char *argv[])
> >
> > /* Cleanup */
> > grab_surface_destroy(&desktop);
> > + desktop_destroy_surfaces(&desktop);
> > desktop_destroy_outputs(&desktop);
> > if (desktop.unlock_dialog)
> > unlock_dialog_destroy(desktop.unlock_dialog);
> > diff --git a/data/Makefile.am b/data/Makefile.am
> > index a7cc944..293d2bc 100644
> > --- a/data/Makefile.am
> > +++ b/data/Makefile.am
> > @@ -7,6 +7,7 @@ dist_westondata_DATA = \
> > terminal.png \
> > border.png \
> > icon_window.png \
> > + list_item_icon.png \
> > sign_close.png \
> > sign_maximize.png \
> > sign_minimize.png
> > diff --git a/data/list_item_icon.png b/data/list_item_icon.png
> > new file mode 100644
> > index
> 0000000000000000000000000000000000000000..ac987e997313910ec3e02a11be8ccda0dfa581c7
> > GIT binary patch
> > literal 176
> > zcmeAS at N?(olHy`uVBq!ia0vp^0wB!61|;P_|4#%`Y)RhkE)4%caKYZ?lYt_f1s;*b
> > z3=G`DAk4 at xYmNj^kiEpy*OmPaH$RJ!-UgFp7lA^yo-U3d7N_4%cI0AE;5i(0^Z))B
> > z|EBQpgWZ)55eu3H82wmS6TZt#lE`$@S at Cy{aM-usE&4llHh-IQIlXHpO8~cw3TFo|
> > P&>#j+S3j3^P6<r_g#|R0
> >
> > literal 0
> > HcmV?d00001
> >
> > diff --git a/src/compositor.c b/src/compositor.c
> > index 587fded..686a11b 100644
> > --- a/src/compositor.c
> > +++ b/src/compositor.c
> > @@ -374,6 +374,8 @@ weston_surface_update_output_mask(struct
> weston_surface *es, uint32_t mask)
> > if (1 << output->id & left)
> > wl_surface_send_leave(&es->surface.resource,
> resource);
> > }
> > +
> > + es->compositor->shell_interface.send_output_mask(es);
> > }
> >
> > static void
> > diff --git a/src/compositor.h b/src/compositor.h
> > index e5c579b..a60563a 100644
> > --- a/src/compositor.h
> > +++ b/src/compositor.h
> > @@ -88,6 +88,7 @@ struct weston_shell_interface {
> > int (*move)(struct shell_surface *shsurf, struct weston_seat *ws);
> > int (*resize)(struct shell_surface *shsurf,
> > struct weston_seat *ws, uint32_t edges);
> > + void (*send_output_mask)(struct weston_surface *surface);
>
> This doesn't feel right. The weston_shell_interface are for higher
> level modules (right now, only the xwm) to call into the shell module,
> not for the core to call into. This creates a core dependency on the
> shell module, which we don't want. A better approach would be a
> weston_surface wl_signal that we fire from the end of
> weston_surface_update_transform.
>
Yes, a wl_signal fits much more nicely.
>
> > };
> >
> > diff --git a/src/shell.c b/src/shell.c
> > index cccfe66..86f2f6f 100644
> > --- a/src/shell.c
> > +++ b/src/shell.c
> > @@ -97,6 +97,8 @@ struct desktop_shell {
> > uint32_t deathstamp;
> > } child;
> >
> > + struct wl_resource *surface_data_manager;
> > +
> > bool locked;
> > bool showing_input_panels;
> > bool prepare_event_sent;
> > @@ -201,6 +203,7 @@ struct shell_surface {
> > struct wl_list link;
> >
> > const struct weston_shell_client *client;
> > + struct wl_resource *surface_data;
> > };
> >
> > struct shell_grab {
> > @@ -1396,6 +1399,118 @@ shell_surface_pong(struct wl_client *client,
> struct wl_resource *resource,
> > }
> >
> > static void
> > +surface_data_object_destroy(struct wl_resource *resource)
> > +{
> > + struct shell_surface *shsurf = resource->data;
> > +
> > + free(resource);
> > +
> > + if (!shsurf)
> > + return;
> > +
> > + shsurf->surface_data = NULL;
> > +}
> > +
> > +static void
> > +surface_data_destroy_handler(struct wl_client *client,
> > + struct wl_resource *resource)
> > +{
> > + wl_resource_destroy(resource);
> > +}
> > +
> > +static const struct surface_data_interface
> > + surface_data_implementation = {
> > + surface_data_destroy_handler
> > +};
> > +
> > +static int
> > +create_surface_data(struct desktop_shell *shell, struct shell_surface
> *shsurf)
> > +{
> > + struct wl_resource *surface_data;
> > +
> > + surface_data = malloc(sizeof *surface_data);
> > + if (surface_data == NULL)
> > + return -1;
> > +
> > + surface_data->data = shsurf;
> > + surface_data->object.id = 0;
> > + surface_data->object.interface = &surface_data_interface;
> > + surface_data->destroy = surface_data_object_destroy;
> > + surface_data->object.implementation =
> > + (void (**)(void)) &surface_data_implementation;
> > + wl_signal_init(&surface_data->destroy_signal);
> > +
> > + wl_client_add_resource(shell->surface_data_manager->client,
> surface_data);
> > +
> > + shsurf->surface_data = surface_data;
> > +
> > + return 0;
> > +}
> > +
> > +static void
> > +send_surface_data_object(struct desktop_shell *shell, struct
> shell_surface *shsurf)
> > +{
> > +
> surface_data_manager_send_surface_object(shell->surface_data_manager,
> > + shsurf->surface_data);
> > +}
>
> This function doesn't add much, just call
> surface_data_manager_send_surface_object directly below? Or call it
> from create_surface_data?
>
Yes, that's better.
>
> > +static bool
> > +surface_is_window_list_candidate(struct weston_surface *surface,
> > + struct shell_surface *out)
> > +{
> > + struct desktop_shell *shell;
> > + struct shell_surface *shsurf;
> > +
> > + shsurf = get_shell_surface(surface);
> > + if (!shsurf)
> > + return false;
> > +
> > + shell = shsurf->shell;
> > +
> > + if (!shell->surface_data_manager)
> > + return false;
> > +
> > + switch (shsurf->type) {
> > + default:
> > + case SHELL_SURFACE_TRANSIENT:
> > + case SHELL_SURFACE_POPUP:
> > + case SHELL_SURFACE_NONE:
> > + return false;
> > + case SHELL_SURFACE_FULLSCREEN:
> > + case SHELL_SURFACE_MAXIMIZED:
> > + case SHELL_SURFACE_TOPLEVEL:
> > + if (!shsurf->surface_data) {
> > + if (create_surface_data(shell, shsurf))
> > + return false;
> > + send_surface_data_object(shell, shsurf);
> > + }
> > + *out = *shsurf;
>
> This is copying the entire shsurf by value... you could return NULL
> for "not a candidate" or the shsurf pointer for "yes", instead of the
> bool, but don't copy the entire struct. But I'd rather just do all
> this up front when we create the shell_surface.
>
Right, so I've restructured this significantly so that this is fixed in a
cleaner way. We have to wait until shsurf->type is set to create the
surface_data object since we need to know the type to decide if it is a
window list candidate.
>
> > + return true;
> > + }
> > +}
> > +
> > +static void
> > +send_surface_data_output_mask(struct weston_surface *surface)
> > +{
> > + struct shell_surface shsurf;
> > +
> > + if (surface_is_window_list_candidate(surface, &shsurf))
> > + surface_data_send_output_mask(shsurf.surface_data,
> > + surface->output_mask);
> > +}
> > +
> > +static void
> > +send_surface_data_title(struct weston_surface *surface)
> > +{
> > + struct shell_surface shsurf;
> > +
> > + if (surface_is_window_list_candidate(surface, &shsurf))
>
> If we create the surface_data up front, we just need to call
> get_shell_surface(surface) and then check if shsurf->surface_data is
> not NULL.
>
Yes, this works.
>
> > + surface_data_send_title(shsurf.surface_data,
> > + shsurf.title == NULL ?
> > + "Surface" : shsurf.title);
> > +}
> > +
> > +static void
> > shell_surface_set_title(struct wl_client *client,
> > struct wl_resource *resource, const char *title)
> > {
> > @@ -1403,6 +1518,7 @@ shell_surface_set_title(struct wl_client *client,
> >
> > free(shsurf->title);
> > shsurf->title = strdup(title);
> > + send_surface_data_title(shsurf->surface);
> > }
> >
> > static void
> > @@ -1519,6 +1635,8 @@ set_surface_type(struct shell_surface *shsurf)
> > default:
> > break;
> > }
> > +
> > + send_surface_data_title(surface);
> > }
> >
> > static void
> > @@ -1930,6 +2048,8 @@ static const struct wl_shell_surface_interface
> shell_surface_implementation = {
> > static void
> > destroy_shell_surface(struct shell_surface *shsurf)
> > {
> > + if (shsurf->surface_data)
> > + surface_data_send_gone(shsurf->surface_data);
> > if (shsurf->popup.grab.pointer)
> > wl_pointer_end_grab(shsurf->popup.grab.pointer);
> >
> > @@ -2314,6 +2434,20 @@ static const struct desktop_shell_interface
> desktop_shell_implementation = {
> > desktop_shell_set_grab_surface
> > };
> >
> > +static void
> > +surface_data_send_all_info(struct desktop_shell *shell)
> > +{
> > + struct weston_surface *surface;
> > + struct workspace *ws;
> > +
> > + ws = get_current_workspace(shell);
> > +
> > + wl_list_for_each(surface, &ws->layer.surface_list, layer_link) {
>
> And here we'd have to create the surface_data before sending out these
> events.
>
Yep.
>
> > + send_surface_data_output_mask(surface);
> > + send_surface_data_title(surface);
> > + }
> > +}
> > +
> > static enum shell_surface_type
> > get_shell_surface_type(struct weston_surface *surface)
> > {
> > @@ -3143,6 +3277,37 @@ bind_desktop_shell(struct wl_client *client,
> > }
> >
> > static void
> > +unbind_surface_data_manager(struct wl_resource *resource)
> > +{
> > + struct desktop_shell *shell = resource->data;
> > +
> > + shell->surface_data_manager = NULL;
> > + free(resource);
> > +}
> > +
> > +static void
> > +bind_surface_data_manager(struct wl_client *client,
> > + void *data, uint32_t version, uint32_t id)
> > +{
> > + struct desktop_shell *shell = data;
> > + struct wl_resource *resource;
> > +
> > + resource = wl_client_add_object(client,
> &surface_data_manager_interface,
> > + NULL, id, shell);
> > +
> > + if (client == shell->child.client) {
> > + resource->destroy = unbind_surface_data_manager;
> > + shell->surface_data_manager = resource;
> > + surface_data_send_all_info(shell);
> > + return;
> > + }
> > +
> > + wl_resource_post_error(resource, WL_DISPLAY_ERROR_INVALID_OBJECT,
> > + "permission to bind desktop_shell denied");
> > + wl_resource_destroy(resource);
> > +}
> > +
> > +static void
> > screensaver_configure(struct weston_surface *surface, int32_t sx,
> int32_t sy)
> > {
> > struct desktop_shell *shell = surface->private;
> > @@ -3836,6 +4001,7 @@ module_init(struct weston_compositor *ec)
> > ec->shell_interface.set_transient = set_transient;
> > ec->shell_interface.move = surface_move;
> > ec->shell_interface.resize = surface_resize;
> > + ec->shell_interface.send_output_mask =
> send_surface_data_output_mask;
> >
> > wl_list_init(&shell->screensaver.surfaces);
> > wl_list_init(&shell->input_panel.surfaces);
> > @@ -3887,6 +4053,10 @@ module_init(struct weston_compositor *ec)
> > shell, bind_workspace_manager) == NULL)
> > return -1;
> >
> > + if (wl_display_add_global(ec->wl_display,
> &surface_data_manager_interface,
> > + shell, bind_surface_data_manager) ==
> NULL)
> > + return -1;
> > +
> > shell->child.deathstamp = weston_compositor_get_time();
> >
> > loop = wl_display_get_event_loop(ec->wl_display);
> > --
> > 1.7.11.7
>
>
Thanks for the review Kristian, I will probably have this set updated and
ready soon.
- Scott
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130305/b381b6dc/attachment-0001.html>
More information about the wayland-devel
mailing list