Re: [PATCH weston v3] xwm: Add icon support to the frame

[Quentin Glidic]

On 4/4/17 4:34 PM, Emmanuel Gil Peyrot wrote:

From: Emmanuel Gil Peyrot 

This fetches the _NET_WM_ICON property of the X11 window, and use the
first image found as the frame icon.

This has been tested with various X11 programs, and improves usability
and user-friendliness a bit.

Changes since v1:
- Changed frame_button_create() to use
   frame_button_create_from_surface() internally.
- Removed a check that should never have been commited.

Changes since v2:
- Request UINT32_MAX items instead of 2048, to avoid cutting valid
   icons.
- Strengthen checks against malformed input.
- Handle XCB_PROPERTY_DELETE to remove the icon.
- Schedule a repaint if the icon changed.

Signed-off-by: Emmanuel Gil Peyrot 
Signed-off-by: Emmanuel Gil Peyrot 


We’re mostly good, still a few comments in the property handling.



---
  clients/window.c   |  4 +--
  libweston/compositor-wayland.c |  2 +-
  shared/cairo-util.h|  2 +-
  shared/frame.c | 54 +++--
  xwayland/window-manager.c  | 61 --
  5 files changed, 103 insertions(+), 20 deletions(-)

diff --git a/clients/window.c b/clients/window.c
index 95796d46..15a86e15 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -2546,7 +2546,7 @@ window_frame_create(struct window *window, void *data)
  
  	frame = xzalloc(sizeof *frame);

frame->frame = frame_create(window->display->theme, 0, 0,
-   buttons, window->title);
+   buttons, window->title, NULL);
  
  	frame->widget = window_add_widget(window, frame);

frame->child = widget_add_widget(frame->widget, data);
@@ -5449,7 +5449,7 @@ create_menu(struct display *display,
menu->user_data = user_data;
menu->widget = window_add_widget(menu->window, menu);
menu->frame = frame_create(window->display->theme, 0, 0,
-  FRAME_BUTTON_NONE, NULL);
+  FRAME_BUTTON_NONE, NULL, NULL);
fail_on_null(menu->frame, 0, __FILE__, __LINE__);
menu->entries = entries;
menu->count = count;
diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c
index 14f2c8db..aa6e5043 100644
--- a/libweston/compositor-wayland.c
+++ b/libweston/compositor-wayland.c
@@ -862,7 +862,7 @@ wayland_output_set_windowed(struct wayland_output *output)
return -1;
}
output->frame = frame_create(b->theme, 100, 100,
-FRAME_BUTTON_CLOSE, output->title);
+FRAME_BUTTON_CLOSE, output->title, NULL);
if (!output->frame)
return -1;
  
diff --git a/shared/cairo-util.h b/shared/cairo-util.h

index 84cf005e..7893ca24 100644
--- a/shared/cairo-util.h
+++ b/shared/cairo-util.h
@@ -126,7 +126,7 @@ enum {
  
  struct frame *

  frame_create(struct theme *t, int32_t width, int32_t height, uint32_t buttons,
-const char *title);
+ const char *title, cairo_surface_t *icon);
  
  void

  frame_destroy(struct frame *frame);
diff --git a/shared/frame.c b/shared/frame.c
index eb0cd77a..32779856 100644
--- a/shared/frame.c
+++ b/shared/frame.c
@@ -106,9 +106,9 @@ struct frame {
  };
  
  static struct frame_button *

-frame_button_create(struct frame *frame, const char *icon,
-   enum frame_status status_effect,
-   enum frame_button_flags flags)
+frame_button_create_from_surface(struct frame *frame, cairo_surface_t *icon,
+ enum frame_status status_effect,
+ enum frame_button_flags flags)
  {
struct frame_button *button;
  
@@ -116,12 +116,7 @@ frame_button_create(struct frame *frame, const char *icon,

if (!button)
return NULL;
  
-	button->icon = cairo_image_surface_create_from_png(icon);

-   if (!button->icon) {
-   free(button);
-   return NULL;
-   }
-
+   button->icon = icon;
button->frame = frame;
button->flags = flags;
button->status_effect = status_effect;
@@ -131,6 +126,30 @@ frame_button_create(struct frame *frame, const char *icon,
return button;
  }
  
+static struct frame_button *

+frame_button_create(struct frame *frame, const char *icon_name,
+enum frame_status status_effect,
+enum frame_button_flags flags)
+{
+   struct frame_button *button;
+   cairo_surface_t *icon;
+
+   icon = cairo_image_surface_create_from_png(icon_name);
+   if (cairo_surface_status(icon) != CAIRO_STATUS_SUCCESS)
+   goto error;
+
+   button = frame_button_create_from_surface(frame, icon, status_effect,
+ flags);
+   if (!button)
+ 

[PATCH weston v3] xwm: Add icon support to the frame

[Emmanuel Gil Peyrot]

From: Emmanuel Gil Peyrot 

This fetches the _NET_WM_ICON property of the X11 window, and use the
first image found as the frame icon.

This has been tested with various X11 programs, and improves usability
and user-friendliness a bit.

Changes since v1:
- Changed frame_button_create() to use
  frame_button_create_from_surface() internally.
- Removed a check that should never have been commited.

Changes since v2:
- Request UINT32_MAX items instead of 2048, to avoid cutting valid
  icons.
- Strengthen checks against malformed input.
- Handle XCB_PROPERTY_DELETE to remove the icon.
- Schedule a repaint if the icon changed.

Signed-off-by: Emmanuel Gil Peyrot 
Signed-off-by: Emmanuel Gil Peyrot 
---
 clients/window.c   |  4 +--
 libweston/compositor-wayland.c |  2 +-
 shared/cairo-util.h|  2 +-
 shared/frame.c | 54 +++--
 xwayland/window-manager.c  | 61 --
 5 files changed, 103 insertions(+), 20 deletions(-)

diff --git a/clients/window.c b/clients/window.c
index 95796d46..15a86e15 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -2546,7 +2546,7 @@ window_frame_create(struct window *window, void *data)
 
frame = xzalloc(sizeof *frame);
frame->frame = frame_create(window->display->theme, 0, 0,
-   buttons, window->title);
+   buttons, window->title, NULL);
 
frame->widget = window_add_widget(window, frame);
frame->child = widget_add_widget(frame->widget, data);
@@ -5449,7 +5449,7 @@ create_menu(struct display *display,
menu->user_data = user_data;
menu->widget = window_add_widget(menu->window, menu);
menu->frame = frame_create(window->display->theme, 0, 0,
-  FRAME_BUTTON_NONE, NULL);
+  FRAME_BUTTON_NONE, NULL, NULL);
fail_on_null(menu->frame, 0, __FILE__, __LINE__);
menu->entries = entries;
menu->count = count;
diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c
index 14f2c8db..aa6e5043 100644
--- a/libweston/compositor-wayland.c
+++ b/libweston/compositor-wayland.c
@@ -862,7 +862,7 @@ wayland_output_set_windowed(struct wayland_output *output)
return -1;
}
output->frame = frame_create(b->theme, 100, 100,
-FRAME_BUTTON_CLOSE, output->title);
+FRAME_BUTTON_CLOSE, output->title, NULL);
if (!output->frame)
return -1;
 
diff --git a/shared/cairo-util.h b/shared/cairo-util.h
index 84cf005e..7893ca24 100644
--- a/shared/cairo-util.h
+++ b/shared/cairo-util.h
@@ -126,7 +126,7 @@ enum {
 
 struct frame *
 frame_create(struct theme *t, int32_t width, int32_t height, uint32_t buttons,
-const char *title);
+ const char *title, cairo_surface_t *icon);
 
 void
 frame_destroy(struct frame *frame);
diff --git a/shared/frame.c b/shared/frame.c
index eb0cd77a..32779856 100644
--- a/shared/frame.c
+++ b/shared/frame.c
@@ -106,9 +106,9 @@ struct frame {
 };
 
 static struct frame_button *
-frame_button_create(struct frame *frame, const char *icon,
-   enum frame_status status_effect,
-   enum frame_button_flags flags)
+frame_button_create_from_surface(struct frame *frame, cairo_surface_t *icon,
+ enum frame_status status_effect,
+ enum frame_button_flags flags)
 {
struct frame_button *button;
 
@@ -116,12 +116,7 @@ frame_button_create(struct frame *frame, const char *icon,
if (!button)
return NULL;
 
-   button->icon = cairo_image_surface_create_from_png(icon);
-   if (!button->icon) {
-   free(button);
-   return NULL;
-   }
-
+   button->icon = icon;
button->frame = frame;
button->flags = flags;
button->status_effect = status_effect;
@@ -131,6 +126,30 @@ frame_button_create(struct frame *frame, const char *icon,
return button;
 }
 
+static struct frame_button *
+frame_button_create(struct frame *frame, const char *icon_name,
+enum frame_status status_effect,
+enum frame_button_flags flags)
+{
+   struct frame_button *button;
+   cairo_surface_t *icon;
+
+   icon = cairo_image_surface_create_from_png(icon_name);
+   if (cairo_surface_status(icon) != CAIRO_STATUS_SUCCESS)
+   goto error;
+
+   button = frame_button_create_from_surface(frame, icon, status_effect,
+ flags);
+   if (!button)
+   goto error;
+
+   return button;
+
+error:
+   cairo_surface_destroy(icon);
+   return NULL;
+}
+
 static void