Re: [PATCH 02/12] shell.c: Restore maximized and fullscreen window on destroyed output

[Ander Conselvan de Oliveira]

On 03/18/2014 07:39 AM, Zhang, Xiong Y wrote:

On Mon, 2014-03-17 at 19:17 +0200, Ander Conselvan de Oliveira wrote:

On 03/07/2014 10:27 AM, Xiong Zhang wrote:
> When maximized or fullscreen window is on destroyed output, compositor
> can't change these windows to normal window without notify client,
> otherwise maximize icon or F11 buttion lose its effect after output unplug.
>
> Instead we keep these window as maximized or fullscreen, just change
> it's size to target output.

I'm not sure about this. xdg-shell lets us handle this properly and
wl_shell should be deprecated at some point, so I'm more inclined to
keep the current behavior.




Cheers,
Ander


yes, using xdg-shell, maximize icon and F11 button can take effect after
output unplug.
But server secretly changes window's state from maximized to unmaximized
without notifying client, it's unreasonable.


What I meant is that xdg-shell provides mechanism to notify the client 
if it looses the maximized or fullscreen state. If I understand 
correctly, xdg-shell is an attempt to do wl_shell right and once it is 
finished wl_shell will be deprecated. So in my understanding, as long as 
we do the right thing with xdg-shell we are OK.


But as I said, I'm not sure. I don't like the behavior of taking a 
fullscreen surface from an unplugged output and making it fullscreen in 
another output. I'd prefer to unfullscreen it. But if we must do 
something sensible with wl_shell, then I guess this is the way to go.


Cheers,
Ander


If the unplugged output is
much larger than remaining output, the maximized window can't been fully
displayed on remaining output when the output of maximized window is
unplugged. In order to restore this window to normal, user must move
this window to see the maximize icon. It isn't convenient for user.

thanks.

>
> Signed-off-by: Xiong Zhang mailto:xiong.y.zh...@intel.com>>
> ---
>   desktop-shell/shell.c | 24 +---
>   1 file changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index bee1b0b..02dd1b8 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -5534,6 +5534,7 @@ shell_reposition_view_on_output_destroy(struct 
weston_view *view)
>struct shell_surface *shsurf;
>float x, y;
>int visible;
> +  struct weston_view *black_view;
>
>x = view->geometry.x;
>y = view->geometry.y;
> @@ -5557,6 +5558,8 @@ shell_reposition_view_on_output_destroy(struct 
weston_view *view)
>x = first_output->x + first_output->width / 4;
>y = first_output->y + first_output->height / 4;
>
> +  output = first_output;
> +
>weston_view_set_position(view, x, y);
>} else
>weston_view_geometry_dirty(view);
> @@ -5566,9 +5569,24 @@ shell_reposition_view_on_output_destroy(struct 
weston_view *view)
>
>if (shsurf) {
>shsurf->saved_position_valid = false;
> -  shsurf->next_state.maximized = false;
> -  shsurf->next_state.fullscreen = false;
> -  shsurf->state_changed = true;
> +
> +  /* Resize maxmized window to target output. */
> +  if (shsurf->state.maximized)
> +  set_maximized(shsurf, output);
> +
> +  /* Resize fullscreen window to target output. */
> +  if (shsurf->state.fullscreen) {
> +  black_view = shsurf->fullscreen.black_view->surface;
> +  weston_surface_destroy(black_view->surface);
> +  shsurf->fullscreen.black_view = NULL;
> +
> +  set_fullscreen(shsurf,
> + shsurf->fullscreen.type,
> + shsurf->fullscreen.framerate,
> + output);
> +  }
> +
> +  shsurf->state_changed = false;
>}
>   }
>
>

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org  

http://lists.freedesktop.org/mailman/listinfo/wayland-devel




-
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 


This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH 02/12] shell.c: Restore maximized and fullscreen window on destroyed output

[Zhang, Xiong Y]

On Tue, 2014-03-18 at 10:21 +0200, Conselvan De Oliveira, Ander wrote:
> On 03/18/2014 07:39 AM, Zhang, Xiong Y wrote:
> > On Mon, 2014-03-17 at 19:17 +0200, Ander Conselvan de Oliveira wrote:
> >> On 03/07/2014 10:27 AM, Xiong Zhang wrote:
> >> > When maximized or fullscreen window is on destroyed output, compositor
> >> > can't change these windows to normal window without notify client,
> >> > otherwise maximize icon or F11 buttion lose its effect after output 
> >> > unplug.
> >> >
> >> > Instead we keep these window as maximized or fullscreen, just change
> >> > it's size to target output.
> >>
> >> I'm not sure about this. xdg-shell lets us handle this properly and
> >> wl_shell should be deprecated at some point, so I'm more inclined to
> >> keep the current behavior.
> >
> >>
> >> Cheers,
> >> Ander
> >>
> > yes, using xdg-shell, maximize icon and F11 button can take effect after
> > output unplug.
> > But server secretly changes window's state from maximized to unmaximized
> > without notifying client, it's unreasonable.
> 
> What I meant is that xdg-shell provides mechanism to notify the client 
> if it looses the maximized or fullscreen state. If I understand 
> correctly, xdg-shell is an attempt to do wl_shell right and once it is 
> finished wl_shell will be deprecated. So in my understanding, as long as 
> we do the right thing with xdg-shell we are OK.
> 
> But as I said, I'm not sure. I don't like the behavior of taking a 
> fullscreen surface from an unplugged output and making it fullscreen in 
> another output. I'd prefer to unfullscreen it. But if we must do 
> something sensible with wl_shell, then I guess this is the way to go.
> 
> Cheers,
> Ander
I like your ideal that restore maximized and full screen window to
normal , the patch should be like this:

shell.c: Restore maximized and fullscreen window on destroyed output

Signed-off-by: Xiong Zhang 

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 2cc9407..4c2b409 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -5529,7 +5529,7 @@ workspace_move_surface_down_binding(struct
weston_seat *seat, uint32_t time,
 static void
 shell_reposition_view_on_output_destroy(struct weston_view *view)
 {
-   struct weston_output *output, *first_output;
+   struct weston_output *output;
struct weston_compositor *ec = view->surface->compositor;
struct shell_surface *shsurf;
float x, y;
@@ -5566,9 +5566,18 @@ shell_reposition_view_on_output_destroy(struct
weston_view *view)
 
if (shsurf) {
shsurf->saved_position_valid = false;
-   shsurf->next_state.maximized = false;
-   shsurf->next_state.fullscreen = false;
-   shsurf->state_changed = true;
+
+   /* Restore maxmized window */
+   if (shsurf->state.maximized)
+   xdg_surface_change_state(shsurf,
+
XDG_SURFACE_STATE_MAXIMIZED,
+0, 0);
+
+   /* Restore fullscreen window */
+   if (shsurf->state.fullscreen)
+   xdg_surface_change_state(shsurf,
+
XDG_SURFACE_STATE_FULLSCREEN,
+0, 0);
}
 }

thanks




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

[PATCH] Add error handling for wl_cursors

[Hardening]

This patch adds some error management in wayland cursors
---
 cursor/wayland-cursor.c | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/cursor/wayland-cursor.c b/cursor/wayland-cursor.c
index b16f530..dba3b51 100644
--- a/cursor/wayland-cursor.c
+++ b/cursor/wayland-cursor.c
@@ -94,6 +94,8 @@ shm_pool_resize(struct shm_pool *pool, int size)
 
pool->data = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED,
  pool->fd, 0);
+   if (pool->data == (void *)-1)
+   return 0;
pool->size = size;
 
return 1;
@@ -391,17 +393,15 @@ wl_cursor_theme_load(const char *name, int size, struct 
wl_shm *shm)
name = "default";
 
theme->name = strdup(name);
+   if (!theme->name)
+   goto out_error_name;
theme->size = size;
theme->cursor_count = 0;
theme->cursors = NULL;
 
-   theme->pool =
-   shm_pool_create(shm, size * size * 4);
-   if (!theme->pool) {
-   free(theme->name);
-   free(theme);
-   return NULL;
-   }
+   theme->pool = shm_pool_create(shm, size * size * 4);
+   if (!theme->pool)
+   goto out_error_pool;
 
xcursor_load_theme(name, size, load_callback, theme);
 
@@ -409,6 +409,12 @@ wl_cursor_theme_load(const char *name, int size, struct 
wl_shm *shm)
load_default_theme(theme);
 
return theme;
+
+out_error_pool:
+   free(theme->name);
+out_error_name:
+   free(theme);
+   return NULL;
 }
 
 /** Destroys a cursor theme object
-- 
1.8.1.2

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

[PATCH] Check return value of wl_cursor functions

[Hardening]

This patch adds checks for themes and cursors returned by wl_cursor functions.
---
 clients/simple-egl.c | 10 ++
 clients/window.c |  4 
 src/compositor-wayland.c |  7 ++-
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/clients/simple-egl.c b/clients/simple-egl.c
index a6deff6..410e3ab 100644
--- a/clients/simple-egl.c
+++ b/clients/simple-egl.c
@@ -501,6 +501,8 @@ pointer_handle_enter(void *data, struct wl_pointer *pointer,
else if (cursor) {
image = display->default_cursor->images[0];
buffer = wl_cursor_image_get_buffer(image);
+   if (!buffer)
+   return;
wl_pointer_set_cursor(pointer, serial,
  display->cursor_surface,
  image->hotspot_x,
@@ -715,8 +717,16 @@ registry_handle_global(void *data, struct wl_registry 
*registry,
d->shm = wl_registry_bind(registry, name,
  &wl_shm_interface, 1);
d->cursor_theme = wl_cursor_theme_load(NULL, 32, d->shm);
+   if (!d->cursor_theme) {
+   fprintf(stderr, "unable to load default theme\n");
+   return;
+   }
d->default_cursor =
wl_cursor_theme_get_cursor(d->cursor_theme, "left_ptr");
+   if (!d->default_cursor) {
+   fprintf(stderr, "unable to load default left 
pointer\n");
+   // TODO: abort ?
+   }
}
 }
 
diff --git a/clients/window.c b/clients/window.c
index 3136a7d..96b1731 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -1280,6 +1280,10 @@ create_cursors(struct display *display)
weston_config_destroy(config);
 
display->cursor_theme = wl_cursor_theme_load(theme, size, display->shm);
+   if (!display->cursor_theme) {
+   fprintf(stderr, "could not load theme '%s'\n", theme);
+   return;
+   }
free(theme);
display->cursors =
xmalloc(ARRAY_LENGTH(cursors) * sizeof display->cursors[0]);
diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
index 238946b..7d9e01b 100644
--- a/src/compositor-wayland.c
+++ b/src/compositor-wayland.c
@@ -968,7 +968,8 @@ input_set_cursor(struct wayland_input *input)
 
image = input->compositor->cursor->images[0];
buffer = wl_cursor_image_get_buffer(image);
-
+   if (!buffer)
+   return;
 
wl_pointer_set_cursor(input->parent.pointer, input->enter_serial,
  input->parent.cursor.surface,
@@ -1428,6 +1429,10 @@ create_cursor(struct wayland_compositor *c, struct 
weston_config *config)
weston_config_section_get_int(s, "cursor-size", &size, 32);
 
c->cursor_theme = wl_cursor_theme_load(theme, size, c->parent.shm);
+   if (!c->cursor_theme) {
+   fprintf(stderr, "could not load cursor theme\n");
+   return;
+   }
 
free(theme);
 
-- 
1.8.1.2

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

[PATCH weston-ivi-shell v5 9/9] Modify example clients to support ivi-application.xml

[Nobuhiko Tanibata]

Signed-off-by: Nobuhiko Tanibata 
---

Changes for v2, v3 and v4
  - nothing. Version number aligned to the first patch

Changes for v5
  - support weston-dnd-ivi to verify wl_pointer::set_cursor and 
wl_data_device::start_drag

 clients/.gitignore   |  6 +
 clients/Makefile.am  | 72 
 clients/simple-egl.c | 70 ++
 clients/simple-shm.c | 53 +-
 clients/window.c | 45 ++--
 5 files changed, 228 insertions(+), 18 deletions(-)

diff --git a/clients/.gitignore b/clients/.gitignore
index d23027c..b4ff991 100644
--- a/clients/.gitignore
+++ b/clients/.gitignore
@@ -20,6 +20,12 @@ weston-stacking
 weston-subsurfaces
 weston-transformed
 weston-view
+weston-clickdot-ivi
+weston-flower-ivi
+weston-simple-egl-ivi
+weston-simple-shm-ivi
+weston-smoke-ivi
+weston-dnd-ivi
 
 desktop-shell-client-protocol.h
 desktop-shell-protocol.c
diff --git a/clients/Makefile.am b/clients/Makefile.am
index 4f8d4a6..c8238a1 100644
--- a/clients/Makefile.am
+++ b/clients/Makefile.am
@@ -7,6 +7,11 @@ demo_clients = \
$(simple_clients_programs)  \
$(simple_egl_clients_programs)
 
+if ENABLE_IVI_SHELL
+demo_clients += \
+   $(ivi_shell_clients_programs)
+endif
+
 if INSTALL_DEMO_CLIENTS
 bin_PROGRAMS += $(demo_clients)
 else
@@ -246,6 +251,73 @@ endif
 
 endif
 
+if ENABLE_IVI_SHELL
+noinst_LTLIBRARIES = libivitoytoolkit.la
+
+libivitoytoolkit_la_SOURCES =  \
+   window.c\
+   window.h\
+   text-cursor-position-protocol.c \
+   text-cursor-position-client-protocol.h  \
+   scaler-protocol.c   \
+   scaler-client-protocol.h\
+   workspaces-protocol.c   \
+   workspaces-client-protocol.h
+
+libivitoytoolkit_la_CPPFLAGS = $(AM_CPPFLAGS) -DENABLE_IVI_CLIENT
+
+libivitoytoolkit_la_LIBADD =   \
+   $(CLIENT_LIBS)  \
+   $(CAIRO_EGL_LIBS)   \
+   ../shared/libshared-cairo.la -lrt -lm
+
+ivi_shell_clients_programs =   \
+   weston-simple-egl-ivi   \
+   weston-simple-shm-ivi   \
+   weston-flower-ivi   \
+   weston-smoke-ivi\
+   weston-clickdot-ivi \
+   weston-dnd-ivi
+
+weston_simple_egl_ivi_SOURCES = simple-egl.c \
+   ../ivi-shell/ivi-application-protocol.c \
+   ../ivi-shell/ivi-application-client-protocol.h
+weston_simple_egl_ivi_CPPFLAGS = $(SIMPLE_EGL_CLIENT_CFLAGS) 
-DENABLE_IVI_CLIENT
+weston_simple_egl_ivi_LDADD = $(SIMPLE_EGL_CLIENT_LIBS) -lm
+
+weston_simple_shm_ivi_SOURCES = simple-shm.c   \
+   ../shared/os-compatibility.c\
+   ../shared/os-compatibility.h\
+   ../ivi-shell/ivi-application-protocol.c \
+   ../ivi-shell/ivi-application-client-protocol.h
+weston_simple_shm_ivi_CPPFLAGS = $(SIMPLE_CLIENT_CFLAGS) -DENABLE_IVI_CLIENT
+weston_simple_shm_ivi_LDADD = $(SIMPLE_CLIENT_LIBS)
+
+weston_flower_ivi_SOURCES = flower.c \
+   ../ivi-shell/ivi-application-protocol.c \
+   ../ivi-shell/ivi-application-client-protocol.h
+weston_flower_ivi_CPPFLAGS = $(AM_CPPFLAGS) -DENABLE_IVI_CLIENT
+weston_flower_ivi_LDADD = libivitoytoolkit.la
+
+weston_smoke_ivi_SOURCES = smoke.c \
+   ../ivi-shell/ivi-application-protocol.c \
+   ../ivi-shell/ivi-application-client-protocol.h
+weston_smoke_ivi_CPPFLAGS = $(AM_CPPFLAGS) -DENABLE_IVI_CLIENT
+weston_smoke_ivi_LDADD = libivitoytoolkit.la
+
+weston_clickdot_ivi_SOURCES = clickdot.c \
+   ../ivi-shell/ivi-application-protocol.c \
+   ../ivi-shell/ivi-application-client-protocol.h
+weston_clickdot_ivi_CPPFLAGS = $(AM_CPPFLAGS) -DENABLE_IVI_CLIENT
+weston_clickdot_ivi_LDADD = libivitoytoolkit.la
+
+weston_dnd_ivi_SOURCES = dnd.c \
+   ../ivi-shell/ivi-application-protocol.c \
+   ../ivi-shell/ivi-application-client-protocol.h
+weston_dnd_ivi_CPPFLAGS = $(AM_CPPFLAGS) -DENABLE_IVI_CLIENT
+weston_dnd_ivi_LDADD = libivitoytoolkit.la
+endif
+
 wayland_protocoldir = $(top_srcdir)/protocol
 include $(top_srcdir)/wayland-scanner.mk
 
diff --git a/clients/simple-egl.c b/clients/simple-egl.c
index 2c009ee..bc90b91 100644
--- a/clients/simple-egl.c
+++ b/clients/simple-egl.c
@@ -38,6 +38,13 @@
 #include 
 #include 
 
+#ifdef ENABLE_IVI_CLIENT
+#include 
+#include 
+#include "../ivi-shell/ivi-application-client-protocol.h"
+#define IVI_SURFACE_ID 9000
+#endif
+
 #ifndef EGL_EXT_swap_buffers_with_damage
 #define EGL_EXT_swap_buffers_with_damage 1
 typedef EGLBoolean (EGLAPIENTRYP 
PFNEGLSWAPBUFFERSWITHDAMAGEEXTPROC)(EGLDisplay dpy, EGLSurface surface, EGLint 
*rects, EGL

Re: [PATCH weston-ivi-shell v3 02/10] ivi application protocol:

[Nobuhiko Tanibata]

2014-03-17 18:11 に Nobuhiko Tanibata さんは書きました:

2014-03-17 16:15 に Pekka Paalanen さんは書きました:



Hi pq,

I see. In case of ivi, ivi_application shall be exclusive from others.
In wayland client manner, client manages which role wl_surface shall
do, I understand.

Sub-surface: it can have a wl_surface of ivi_surface as a parent. So
client can use it exclusively.
Pointer cursor: it is for cursor. It can use it exclusively.
wl_shell: this is desktop style. It can use it exclusively.
drag icon: wl_surface is registered for icon surface around cursor
during drag. It can be used exclusively.

So I think above request can be used for ivi-application request.
However, I have to take care wl_surface which is set to set_cursor and
icon of start_drag shall be visible in ivi-shell. However for the time
being, it is not critical for ivi use case.

Thank you very much. This comment was good to start investigation.
BR,
Nobuhiko



Hi again,

Just for your information
I am sending a patch of a sample, weston_dnd_ivi, to verify 
wl_pointer.set_cursor and wl_data_device::start_drag.


BR,
Nobuhiko
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH] Make RDP backend supports arbitrary modes

[Hardening]

This patch removes the extra modes parameter for the RDP compositor. And
make it support any mode that is requested (be aware that RDP client may not
support all possible modes, especially odd resolution).
This is definitely useful for the fullscreen shell.
---
 src/compositor-rdp.c | 105 +++
 src/compositor.c |   1 -
 2 files changed, 38 insertions(+), 68 deletions(-)

diff --git a/src/compositor-rdp.c b/src/compositor-rdp.c
index e8e4a8d..22380cb 100644
--- a/src/compositor-rdp.c
+++ b/src/compositor-rdp.c
@@ -49,6 +49,7 @@
 
 #define MAX_FREERDP_FDS 32
 #define DEFAULT_AXIS_STEP_DISTANCE wl_fixed_from_int(10)
+#define RDP_MODE_FREQ 60 * 1000
 
 struct rdp_compositor_config {
int width;
@@ -58,7 +59,6 @@ struct rdp_compositor_config {
char *rdp_key;
char *server_cert;
char *server_key;
-   char *extra_modes;
int env_socket;
 };
 
@@ -121,7 +121,6 @@ rdp_compositor_config_init(struct rdp_compositor_config 
*config) {
config->rdp_key = NULL;
config->server_cert = NULL;
config->server_key = NULL;
-   config->extra_modes = NULL;
config->env_socket = 0;
 }
 
@@ -320,11 +319,13 @@ rdp_output_repaint(struct weston_output *output_base, 
pixman_region32_t *damage)
pixman_renderer_output_set_buffer(output_base, output->shadow_surface);
ec->renderer->repaint_output(&output->base, damage);
 
-   wl_list_for_each(outputPeer, &output->peers, link) {
-   if ((outputPeer->flags & RDP_PEER_ACTIVATED) &&
-   (outputPeer->flags & RDP_PEER_OUTPUT_ENABLED))
-   {
-   rdp_peer_refresh_region(damage, outputPeer->peer);
+   if (pixman_region32_n_rects(damage)) {
+   wl_list_for_each(outputPeer, &output->peers, link) {
+   if ((outputPeer->flags & RDP_PEER_ACTIVATED) &&
+   (outputPeer->flags & 
RDP_PEER_OUTPUT_ENABLED))
+   {
+   rdp_peer_refresh_region(damage, 
outputPeer->peer);
+   }
}
}
 
@@ -352,16 +353,29 @@ finish_frame_handler(void *data)
return 1;
 }
 
+static struct weston_mode *
+rdp_insert_new_mode(struct weston_output *output, int width, int height, int 
rate) {
+   struct weston_mode *ret;
+   ret = zalloc(sizeof *ret);
+   if(!ret)
+   return ret;
+   ret->width = width;
+   ret->height = height;
+   ret->refresh = rate;
+   wl_list_insert(&output->mode_list, &ret->link);
+   return ret;
+}
 
 static struct weston_mode *
-find_matching_mode(struct weston_output *output, struct weston_mode *target) {
+ensure_matching_mode(struct weston_output *output, struct weston_mode *target) 
{
struct weston_mode *local;
 
wl_list_for_each(local, &output->mode_list, link) {
if((local->width == target->width) && (local->height == 
target->height))
return local;
}
-   return 0;
+
+   return rdp_insert_new_mode(output, target->width, target->height, 
RDP_MODE_FREQ);
 }
 
 static int
@@ -372,7 +386,7 @@ rdp_switch_mode(struct weston_output *output, struct 
weston_mode *target_mode) {
pixman_image_t *new_shadow_buffer;
struct weston_mode *local_mode;
 
-   local_mode = find_matching_mode(output, target_mode);
+   local_mode = ensure_matching_mode(output, target_mode);
if(!local_mode) {
weston_log("mode %dx%d not available\n", target_mode->width, 
target_mode->height);
return -ENOENT;
@@ -398,6 +412,9 @@ rdp_switch_mode(struct weston_output *output, struct 
weston_mode *target_mode) {
 
wl_list_for_each(rdpPeer, &rdpOutput->peers, link) {
settings = rdpPeer->peer->settings;
+   if (settings->DesktopWidth == target_mode->width && 
settings->DesktopHeight == target_mode->height)
+   continue;
+
if(!settings->DesktopResize) {
/* too bad this peer does not support desktop resize */
rdpPeer->peer->Close(rdpPeer->peer);
@@ -411,49 +428,12 @@ rdp_switch_mode(struct weston_output *output, struct 
weston_mode *target_mode) {
 }
 
 static int
-parse_extra_modes(const char *modes_str, struct rdp_output *output) {
-   const char *startAt = modes_str;
-   const char *nextPos;
-   int w, h;
-   struct weston_mode *mode;
-
-   while(startAt && *startAt) {
-   nextPos = strchr(startAt, 'x');
-   if(!nextPos)
-   return -1;
-
-   w = strtoul(startAt, NULL, 0);
-   startAt = nextPos + 1;
-   if(!*startAt)
-   return -1;
-
-   h = strtoul(startAt, NULL, 0);
-
-   if(!w || (w > 3000) || !h || (h > 3000))
-   

Re: [PATCH] Add error handling for wl_cursors

[Bryce W. Harrington]

LGTM

Reviewed-by: Bryce Harrington 

On Tue, Mar 18, 2014 at 11:29:00AM +0100, Hardening wrote:
> This patch adds some error management in wayland cursors
> ---
>  cursor/wayland-cursor.c | 20 +---
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/cursor/wayland-cursor.c b/cursor/wayland-cursor.c
> index b16f530..dba3b51 100644
> --- a/cursor/wayland-cursor.c
> +++ b/cursor/wayland-cursor.c
> @@ -94,6 +94,8 @@ shm_pool_resize(struct shm_pool *pool, int size)
>  
>   pool->data = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED,
> pool->fd, 0);
> + if (pool->data == (void *)-1)
> + return 0;
>   pool->size = size;
>  
>   return 1;
> @@ -391,17 +393,15 @@ wl_cursor_theme_load(const char *name, int size, struct 
> wl_shm *shm)
>   name = "default";
>  
>   theme->name = strdup(name);
> + if (!theme->name)
> + goto out_error_name;
>   theme->size = size;
>   theme->cursor_count = 0;
>   theme->cursors = NULL;
>  
> - theme->pool =
> - shm_pool_create(shm, size * size * 4);
> - if (!theme->pool) {
> - free(theme->name);
> - free(theme);
> - return NULL;
> - }
> + theme->pool = shm_pool_create(shm, size * size * 4);
> + if (!theme->pool)
> + goto out_error_pool;
>  
>   xcursor_load_theme(name, size, load_callback, theme);
>  
> @@ -409,6 +409,12 @@ wl_cursor_theme_load(const char *name, int size, struct 
> wl_shm *shm)
>   load_default_theme(theme);
>  
>   return theme;
> +
> +out_error_pool:
> + free(theme->name);
> +out_error_name:
> + free(theme);
> + return NULL;
>  }
>  
>  /** Destroys a cursor theme object
> -- 
> 1.8.1.2
> 
> ___
> 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

Re: [PATCH 1/3] wayland: Add wl_output name event

[Bryce W. Harrington]

On Mon, Mar 17, 2014 at 11:34:50AM +0800, Quanxian Wang wrote:
> This event contains name of output. It may be sent after
> binding the output object. It is intended that client could
> input a character output name as a parameter or for log output.
> 
> Signed-off-by: Quanxian Wang 
> ---
>  protocol/wayland.xml | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> index d47ee62..0332e1a 100644
> --- a/protocol/wayland.xml
> +++ b/protocol/wayland.xml
> @@ -1756,6 +1756,17 @@
>
>
>  
> +
> +
> +  
> + This event contains name of output. It may be sent after
> + binding the output object. It will provide useful name
> + for client when they generate the log for user.
> + Also it is intended that client could input a character
> + output name as a parameter or log.

Fixing up the grammar a bit:

"""
This event contains the name of the output.  It may be sent after
binding the output object.  It will provide a useful name for the
client when it generates the log for the user.  Also, it is intended
that the client could input a character output name as a parameter or
log.
"""

If I understand the purpose of this patchset properly, this might be
clearer phrasing:

"""
This event allows assigning a human-readable alias to an output; it may
be sent after binding the output object.  It is intended that the client
can use this output name as a parameter or display it in logs.
"""

I glanced at the other patches and they look technically correct, but I
think what might be missing here is a stronger justification about why
this is needed?

> +  
> +  
> +
>
>  
>
> -- 
> 1.8.1.2
> 
> ___
> 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

Re: [PATCH 0/3] Add wl_output name event

[Bryce W. Harrington]

Apart from my phrasing suggestions on the first patch, the series looks
good now, so feel free to add:

Reviewed-by: Bryce Harrington 

On Mon, Mar 17, 2014 at 11:34:49AM +0800, Quanxian Wang wrote:
> This event contains name of output. It may be sent after
> binding the output object. It is intended that client could
> input a character output name as a parameter or for log output.
> 
> Mainly for client, provide a sensible character name
> instead of object.
> 
> Quanxian Wang (3):
>   wayland: Add wl_output name event
>   shell: Add wl_output name event
>   weston:Add wl_output name event
> 
> Wayland:
>  protocol/wayland.xml | 11 +++
>  1 file changed, 11 insertions(+)
> 
> Shell:
>  clients/desktop-shell.c | 10 +-
>  clients/window.c| 10 +-
>  2 files changed, 18 insertions(+), 2 deletions(-)
> 
> Weston:
>  src/compositor.c|  3 +++
>  1 files changed, 3 insertions(+)
> 
> -- 
> 1.8.1.2
> 
> ___
> 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

[PATCH] compositor: Use weston_log rather than perror for error messages

[Bryce W. Harrington]

weston_log() seems to be the standard elsewhere in the codebase for
errors.  These are the only two instances where perror() is used
instead, and their error messages aren't that informative anyway.

Signed-off-by: Bryce Harrington 
---
 src/compositor-wayland.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
index 238946b..83eb59c 100644
--- a/src/compositor-wayland.c
+++ b/src/compositor-wayland.c
@@ -211,13 +211,13 @@ wayland_output_get_shm_buffer(struct wayland_output 
*output)
 
fd = os_create_anonymous_file(height * stride);
if (fd < 0) {
-   perror("os_create_anonymous_file");
+   weston_log("could not create an anonymous file buffer\n");
return NULL;
}
 
data = mmap(NULL, height * stride, PROT_READ | PROT_WRITE, MAP_SHARED, 
fd, 0);
if (data == MAP_FAILED) {
-   perror("mmap");
+   weston_log("could not mmap %d memory for data\n", height * 
stride);
close(fd);
return NULL;
}
-- 
1.7.9.5
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH] compositor: Use weston_log rather than perror for error messages

[Hardening]

Le 18/03/2014 20:34, Bryce W. Harrington a écrit :

weston_log() seems to be the standard elsewhere in the codebase for
errors.  These are the only two instances where perror() is used
instead, and their error messages aren't that informative anyway.

Signed-off-by: Bryce Harrington 
---
  src/compositor-wayland.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
index 238946b..83eb59c 100644
--- a/src/compositor-wayland.c
+++ b/src/compositor-wayland.c
@@ -211,13 +211,13 @@ wayland_output_get_shm_buffer(struct wayland_output 
*output)

fd = os_create_anonymous_file(height * stride);
if (fd < 0) {
-   perror("os_create_anonymous_file");
+   weston_log("could not create an anonymous file buffer\n");


perror writes a human readable message, perhaps
weston_log("could not create an anonymous file buffer: %s\n", 
strerror(error));


would be better ?

[...]

Regards

--
David FORT
website: http://www.hardening-consulting.com/
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH 1/3] wayland: Add wl_output name event

[Jason Ekstrand]

On Mar 18, 2014 2:09 PM, "Bryce W. Harrington" 
wrote:
>
> On Mon, Mar 17, 2014 at 11:34:50AM +0800, Quanxian Wang wrote:
> > This event contains name of output. It may be sent after
> > binding the output object. It is intended that client could
> > input a character output name as a parameter or for log output.
> >
> > Signed-off-by: Quanxian Wang 
> > ---
> >  protocol/wayland.xml | 11 +++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> > index d47ee62..0332e1a 100644
> > --- a/protocol/wayland.xml
> > +++ b/protocol/wayland.xml
> > @@ -1756,6 +1756,17 @@
> >
> >
> >  
> > +
> > +

You also need to bump the version field on the wl_output interface

> > +  
> > + This event contains name of output. It may be sent after
> > + binding the output object. It will provide useful name
> > + for client when they generate the log for user.
> > + Also it is intended that client could input a character
> > + output name as a parameter or log.
>
> Fixing up the grammar a bit:
>
> """
> This event contains the name of the output.  It may be sent after
> binding the output object.  It will provide a useful name for the
> client when it generates the log for the user.  Also, it is intended
> that the client could input a character output name as a parameter or
> log.
> """
>
> If I understand the purpose of this patchset properly, this might be
> clearer phrasing:
>
> """
> This event allows assigning a human-readable alias to an output; it may
> be sent after binding the output object.  It is intended that the client
> can use this output name as a parameter or display it in logs.
> """
>
> I glanced at the other patches and they look technically correct, but I
> think what might be missing here is a stronger justification about why
> this is needed?

While I kind of like the idea of a human-readable name, I agree with Bryce
that some justification is needed.

Thanks,
Jason Ekstrand

>
> > +  
> > +  
> > +
> >
> >
> >
> > --
> > 1.8.1.2
> >
> > ___
> > 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

Re: [PATCH 2/3] weston: Add wl_output name event

[Jason Ekstrand]

On Mar 16, 2014 10:20 PM, "Quanxian Wang"  wrote:
>
> Signed-off-by: Quanxian Wang 
> ---
>  src/compositor.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/src/compositor.c b/src/compositor.c
> index 98a4f6f..f82f771 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -3044,6 +3044,9 @@ bind_output(struct wl_client *client,
> mode->refresh);
> }
>
> +   if (version >= 3)
> +   wl_output_send_name(resource, output->name);
> +

You need to actually advertise that the compositor sports version 3.

--Jason Ekstrand

> if (version >= 2)
> wl_output_send_done(resource);
>  }
> --
> 1.8.1.2
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH 3/3] shell: Add wl_output name event

[Jason Ekstrand]

On Mar 16, 2014 10:20 PM, "Quanxian Wang"  wrote:
>
> Signed-off-by: Quanxian Wang 
> ---
>  clients/desktop-shell.c | 10 +-
>  clients/window.c| 10 +-
>  2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
> index a0c6b6d..cefe936 100644
> --- a/clients/desktop-shell.c
> +++ b/clients/desktop-shell.c
> @@ -1172,6 +1172,13 @@ output_handle_mode(void *data,
>  }
>
>  static void
> +output_handle_name(void *data,
> +  struct wl_output *wl_output,
> +  const char *name)
> +{
> +}
> +
> +static void
>  output_handle_done(void *data,
> struct wl_output *wl_output)
>  {
> @@ -1192,7 +1199,8 @@ static const struct wl_output_listener
output_listener = {
> output_handle_geometry,
> output_handle_mode,
> output_handle_done,
> -   output_handle_scale
> +   output_handle_scale,
> +   output_handle_name
>  };

If you we're going to add the listeners, we should actually bind to version
3.

>
>  static void
> diff --git a/clients/window.c b/clients/window.c
> index 3136a7d..4946b7a 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -4714,11 +4714,19 @@ display_handle_mode(void *data,
> }
>  }
>
> +static void
> +display_handle_name(void *data,
> +   struct wl_output *wl_output,
> +   const char *name)
> +{
> +}
> +
>  static const struct wl_output_listener output_listener = {
> display_handle_geometry,
> display_handle_mode,
> display_handle_done,
> -   display_handle_scale
> +   display_handle_scale,
> +   display_handle_name
>  };

Same here.

--Jason Ekstrand

>
>  static void
> --
> 1.8.1.2
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH 2/3] weston:Add wl_output name event

[Jason Ekstrand]

On Mar 16, 2014 5:30 AM, "Wang, Quanxian"  wrote:
>
>
>
>
>
> From: Jason Ekstrand [mailto:ja...@jlekstrand.net]
> Sent: Saturday, March 15, 2014 9:10 PM
>
> To: Wang, Quanxian
> Cc: ppaala...@gmail.com; wayland-devel@lists.freedesktop.org
> Subject: RE: [PATCH 2/3] weston:Add wl_output name event
>
>
>
>
> On Mar 15, 2014 4:57 AM, "Wang, Quanxian"  wrote:
> >
> >
> >
> >
> >
> > From: Jason Ekstrand [mailto:ja...@jlekstrand.net]
> > Sent: Saturday, March 15, 2014 3:54 AM
> > To: Wang, Quanxian
> > Cc: ppaala...@gmail.com; wayland-devel@lists.freedesktop.org
> >
> > Subject: Re: [PATCH 2/3] weston:Add wl_output name event
> >
> >
> >
> >
> > On Mar 13, 2014 9:12 PM, "Quanxian Wang" 
wrote:
> > >
> > > Signed-off-by: Quanxian Wang 
> > > ---
> > >  src/compositor.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/src/compositor.c b/src/compositor.c
> > > index 98a4f6f..8e8964b 100644
> > > --- a/src/compositor.c
> > > +++ b/src/compositor.c
> > > @@ -3045,6 +3045,9 @@ bind_output(struct wl_client *client,
> > > }
> > >
> > > if (version >= 2)
> > > +   wl_output_send_name(resource, output->name);
> >
> > As with my comment on the protocol, wl_output_send_name should be
version 3, not version 2.  That should also clear up Bryce's comment.
> >
> > [Wang, Quanxian] if it is 2, patch 3/3 is not needed because  currently
shell only support version 2. Right?
>
> We should still have patch 3 since weston should always support the
latest version of everything.
>
> [Wang, Quanxian]That is fine according to my testing. If no patch 3/3,
desktop-shell will crash. I am not sure if more updates are needed. For
example, in our shell code, we use such code  min(2, version) for wl_output
interface, should we change it to minal(3,version) if merge this patch?

Of anything is crashing, chances are that something is wrong. Perhaps this
is tied to adding events without bumping the version number?

--Jason Ekstrand

>
> >
> > Thanks,
> > --Jason Ekstrand
> >
> > > +
> > > +   if (version >= 2)
> > > wl_output_send_done(resource);
> > >  }
> > >
> > > --
> > > 1.8.1.2
> > >
> > > ___
> > > 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

RE: [PATCH 1/3] wayland: Add wl_output name event

[Wang, Quanxian]

>-Original Message-
>From: Bryce W. Harrington [mailto:b.harring...@samsung.com]
>Sent: Wednesday, March 19, 2014 3:09 AM
>To: Wang, Quanxian
>Cc: wayland-devel@lists.freedesktop.org; ja...@jlekstrand.net
>Subject: Re: [PATCH 1/3] wayland: Add wl_output name event
>
>On Mon, Mar 17, 2014 at 11:34:50AM +0800, Quanxian Wang wrote:
>> This event contains name of output. It may be sent after binding the
>> output object. It is intended that client could input a character
>> output name as a parameter or for log output.
>>
>> Signed-off-by: Quanxian Wang 
>> ---
>>  protocol/wayland.xml | 11 +++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/protocol/wayland.xml b/protocol/wayland.xml index
>> d47ee62..0332e1a 100644
>> --- a/protocol/wayland.xml
>> +++ b/protocol/wayland.xml
>> @@ -1756,6 +1756,17 @@
>>
>>
>>  
>> +
>> +
>> +  
>> +This event contains name of output. It may be sent after
>> +binding the output object. It will provide useful name
>> +for client when they generate the log for user.
>> +Also it is intended that client could input a character
>> +output name as a parameter or log.
>
>Fixing up the grammar a bit:
>
>"""
>This event contains the name of the output.  It may be sent after binding the
>output object.  It will provide a useful name for the client when it generates 
>the
>log for the user.  Also, it is intended that the client could input a 
>character output
>name as a parameter or log.
>"""
>
>If I understand the purpose of this patchset properly, this might be clearer
>phrasing:
>
>"""
>This event allows assigning a human-readable alias to an output; it may be sent
>after binding the output object.  It is intended that the client can use this 
>output
>name as a parameter or display it in logs.
>"""
>
[Wang, Quanxian] Thanks Bryce.
>I glanced at the other patches and they look technically correct, but I think 
>what
>might be missing here is a stronger justification about why this is needed?
[Wang, Quanxian] name is one property of output which is assigned detected by 
backend. In compositor, it is visible. Why is it assigned as a name in 
compositor? I think display it in compositor logs will be one of reason. 
Therefore the same thing in client.  At the same time, it provides a good 
communication with compositor as a choice. Client could take it as a parameter 
to communicate with compositor, instead of only by wl_output object. I like 
your word 'human-readable' especially for client. Give client a choice to 
communicate with compositor without wl_output object, and also give client a 
choice to display human-readable name in log which is visible to customer. 
Currently I am designing weston randr protocol, customer(admin or developer) 
could input a char name to stand for an output from command line. It is just 
one usage. Anyway, name of output should be visible for both of compositor and 
client. 
>
>> +  
>> +  
>> +
>>
>>
>>
>> --
>> 1.8.1.2
>>
>> ___
>> 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

Re: [PATCH 1/3] wayland: Add wl_output name event

[Bryce W. Harrington]

On Wed, Mar 19, 2014 at 01:54:13AM +, Wang, Quanxian wrote:
> [Wang, Quanxian] Thanks Bryce.
> >I glanced at the other patches and they look technically correct, but I 
> >think what
> >might be missing here is a stronger justification about why this is needed?

> [Wang, Quanxian] name is one property of output which is assigned
> detected by backend. In compositor, it is visible. Why is it assigned
> as a name in compositor? I think display it in compositor logs will be
> one of reason. Therefore the same thing in client.  At the same time,
> it provides a good communication with compositor as a choice. Client
> could take it as a parameter to communicate with compositor, instead
> of only by wl_output object. I like your word 'human-readable'
> especially for client. Give client a choice to communicate with
> compositor without wl_output object, and also give client a choice to
> display human-readable name in log which is visible to
> customer. Currently I am designing weston randr protocol,
> customer(admin or developer) could input a char name to stand for an
> output from command line. It is just one usage. Anyway, name of output

That's a good example to include for justification; maybe include
mention of the randr use case in your commit message.

> should be visible for both of compositor and client.
> 

> >> +  
> >> +  
> >> +
> >>
> >>
> >>
> >> --
> >> 1.8.1.2
> >>
> >> ___
> >> 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

RE: [PATCH 2/3] weston:Add wl_output name event

[Wang, Quanxian]

From: Jason Ekstrand [mailto:ja...@jlekstrand.net]
Sent: Wednesday, March 19, 2014 8:55 AM
To: Wang, Quanxian
Cc: ppaala...@gmail.com; wayland-devel@lists.freedesktop.org
Subject: RE: [PATCH 2/3] weston:Add wl_output name event


On Mar 16, 2014 5:30 AM, "Wang, Quanxian" 
mailto:quanxian.w...@intel.com>> wrote:
>
>
>
>
>
> From: Jason Ekstrand 
> [mailto:ja...@jlekstrand.net]
> Sent: Saturday, March 15, 2014 9:10 PM
>
> To: Wang, Quanxian
> Cc: ppaala...@gmail.com; 
> wayland-devel@lists.freedesktop.org
> Subject: RE: [PATCH 2/3] weston:Add wl_output name event
>
>
>
>
> On Mar 15, 2014 4:57 AM, "Wang, Quanxian" 
> mailto:quanxian.w...@intel.com>> wrote:
> >
> >
> >
> >
> >
> > From: Jason Ekstrand 
> > [mailto:ja...@jlekstrand.net]
> > Sent: Saturday, March 15, 2014 3:54 AM
> > To: Wang, Quanxian
> > Cc: ppaala...@gmail.com; 
> > wayland-devel@lists.freedesktop.org
> >
> > Subject: Re: [PATCH 2/3] weston:Add wl_output name event
> >
> >
> >
> >
> > On Mar 13, 2014 9:12 PM, "Quanxian Wang" 
> > mailto:quanxian.w...@intel.com>> wrote:
> > >
> > > Signed-off-by: Quanxian Wang 
> > > mailto:quanxian.w...@intel.com>>
> > > ---
> > >  src/compositor.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/src/compositor.c b/src/compositor.c
> > > index 98a4f6f..8e8964b 100644
> > > --- a/src/compositor.c
> > > +++ b/src/compositor.c
> > > @@ -3045,6 +3045,9 @@ bind_output(struct wl_client *client,
> > > }
> > >
> > > if (version >= 2)
> > > +   wl_output_send_name(resource, output->name);
> >
> > As with my comment on the protocol, wl_output_send_name should be version 
> > 3, not version 2.  That should also clear up Bryce's comment.
> >
> > [Wang, Quanxian] if it is 2, patch 3/3 is not needed because  currently 
> > shell only support version 2. Right?
>
> We should still have patch 3 since weston should always support the latest 
> version of everything.
>
> [Wang, Quanxian]That is fine according to my testing. If no patch 3/3, 
> desktop-shell will crash. I am not sure if more updates are needed. For 
> example, in our shell code, we use such code  min(2, version) for wl_output 
> interface, should we change it to minal(3,version) if merge this patch?

Of anything is crashing, chances are that something is wrong. Perhaps this is 
tied to adding events without bumping the version number?

[Wang, Quanxian] maybe, I will have a try and check if it is the bump issue 
which cause client crash. Thanks for your reminder. If it is, patch 3 is not 
needed. If we want to support latest version, we should bump wl_output to 
version 3 and add name listener.

--Jason Ekstrand

>
> >
> > Thanks,
> > --Jason Ekstrand
> >
> > > +
> > > +   if (version >= 2)
> > > wl_output_send_done(resource);
> > >  }
> > >
> > > --
> > > 1.8.1.2
> > >
> > > ___
> > > 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

RE: [PATCH 1/3] wayland: Add wl_output name event

[Wang, Quanxian]

>-Original Message-
>From: Bryce W. Harrington [mailto:b.harring...@samsung.com]
>Sent: Wednesday, March 19, 2014 10:01 AM
>To: Wang, Quanxian
>Cc: wayland-devel@lists.freedesktop.org; ja...@jlekstrand.net
>Subject: Re: [PATCH 1/3] wayland: Add wl_output name event
>
>On Wed, Mar 19, 2014 at 01:54:13AM +, Wang, Quanxian wrote:
>> [Wang, Quanxian] Thanks Bryce.
>> >I glanced at the other patches and they look technically correct, but
>> >I think what might be missing here is a stronger justification about why 
>> >this is
>needed?
>
>> [Wang, Quanxian] name is one property of output which is assigned
>> detected by backend. In compositor, it is visible. Why is it assigned
>> as a name in compositor? I think display it in compositor logs will be
>> one of reason. Therefore the same thing in client.  At the same time,
>> it provides a good communication with compositor as a choice. Client
>> could take it as a parameter to communicate with compositor, instead
>> of only by wl_output object. I like your word 'human-readable'
>> especially for client. Give client a choice to communicate with
>> compositor without wl_output object, and also give client a choice to
>> display human-readable name in log which is visible to customer.
>> Currently I am designing weston randr protocol, customer(admin or
>> developer) could input a char name to stand for an output from command
>> line. It is just one usage. Anyway, name of output
>
>That's a good example to include for justification; maybe include mention of 
>the
>randr use case in your commit message.
[Wang, Quanxian] Got it. Thanks
>
>> should be visible for both of compositor and client.
>>
>
>> >> +  
>> >> +  
>> >> +
>> >>
>> >>
>> >>
>> >> --
>> >> 1.8.1.2
>> >>
>> >> ___
>> >> 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

RE: [PATCH 2/3] weston:Add wl_output name event

[Wang, Quanxian]

From: Wang, Quanxian
Sent: Wednesday, March 19, 2014 10:03 AM
To: 'Jason Ekstrand'
Cc: ppaala...@gmail.com; wayland-devel@lists.freedesktop.org
Subject: RE: [PATCH 2/3] weston:Add wl_output name event



From: Jason Ekstrand [mailto:ja...@jlekstrand.net]
Sent: Wednesday, March 19, 2014 8:55 AM
To: Wang, Quanxian
Cc: ppaala...@gmail.com; 
wayland-devel@lists.freedesktop.org
Subject: RE: [PATCH 2/3] weston:Add wl_output name event


On Mar 16, 2014 5:30 AM, "Wang, Quanxian" 
mailto:quanxian.w...@intel.com>> wrote:
>
>
>
>
>
> From: Jason Ekstrand 
> [mailto:ja...@jlekstrand.net]
> Sent: Saturday, March 15, 2014 9:10 PM
>
> To: Wang, Quanxian
> Cc: ppaala...@gmail.com; 
> wayland-devel@lists.freedesktop.org
> Subject: RE: [PATCH 2/3] weston:Add wl_output name event
>
>
>
>
> On Mar 15, 2014 4:57 AM, "Wang, Quanxian" 
> mailto:quanxian.w...@intel.com>> wrote:
> >
> >
> >
> >
> >
> > From: Jason Ekstrand 
> > [mailto:ja...@jlekstrand.net]
> > Sent: Saturday, March 15, 2014 3:54 AM
> > To: Wang, Quanxian
> > Cc: ppaala...@gmail.com; 
> > wayland-devel@lists.freedesktop.org
> >
> > Subject: Re: [PATCH 2/3] weston:Add wl_output name event
> >
> >
> >
> >
> > On Mar 13, 2014 9:12 PM, "Quanxian Wang" 
> > mailto:quanxian.w...@intel.com>> wrote:
> > >
> > > Signed-off-by: Quanxian Wang 
> > > mailto:quanxian.w...@intel.com>>
> > > ---
> > >  src/compositor.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/src/compositor.c b/src/compositor.c
> > > index 98a4f6f..8e8964b 100644
> > > --- a/src/compositor.c
> > > +++ b/src/compositor.c
> > > @@ -3045,6 +3045,9 @@ bind_output(struct wl_client *client,
> > > }
> > >
> > > if (version >= 2)
> > > +   wl_output_send_name(resource, output->name);
> >
> > As with my comment on the protocol, wl_output_send_name should be version 
> > 3, not version 2.  That should also clear up Bryce's comment.
> >
> > [Wang, Quanxian] if it is 2, patch 3/3 is not needed because  currently 
> > shell only support version 2. Right?
>
> We should still have patch 3 since weston should always support the latest 
> version of everything.
>
> [Wang, Quanxian]That is fine according to my testing. If no patch 3/3, 
> desktop-shell will crash. I am not sure if more updates are needed. For 
> example, in our shell code, we use such code  min(2, version) for wl_output 
> interface, should we change it to minal(3,version) if merge this patch?

Of anything is crashing, chances are that something is wrong. Perhaps this is 
tied to adding events without bumping the version number?

[Wang, Quanxian] maybe, I will have a try and check if it is the bump issue 
which cause client crash. Thanks for your reminder. If it is, patch 3 is not 
needed. If we want to support latest version, we should bump wl_output to 
version 3 and add name listener.

[Wang, Quanxian] I have tested it. It works as your said. The crash happens 
without bumping the version number. I will send the patch series later with 
updates in shell. Thanks

--Jason Ekstrand

>
> >
> > Thanks,
> > --Jason Ekstrand
> >
> > > +
> > > +   if (version >= 2)
> > > wl_output_send_done(resource);
> > >  }
> > >
> > > --
> > > 1.8.1.2
> > >
> > > ___
> > > 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

[PATCH 2/3] weston:Add wl_output name event

[Quanxian Wang]

Signed-off-by: Quanxian Wang 
---
 src/compositor.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/compositor.c b/src/compositor.c
index 40e4b11..78e2b48 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -3115,7 +3115,7 @@ bind_output(struct wl_client *client,
struct wl_resource *resource;
 
resource = wl_resource_create(client, &wl_output_interface,
- MIN(version, 2), id);
+ MIN(version, 3), id);
if (resource == NULL) {
wl_client_post_no_memory(client);
return;
@@ -3144,6 +3144,9 @@ bind_output(struct wl_client *client,
mode->refresh);
}
 
+   if (version >= 3)
+   wl_output_send_name(resource, output->name);
+
if (version >= 2)
wl_output_send_done(resource);
 }
@@ -3400,7 +3403,7 @@ weston_output_init(struct weston_output *output, struct 
weston_compositor *c,
output->compositor->output_id_pool |= 1 << output->id;
 
output->global =
-   wl_global_create(c->wl_display, &wl_output_interface, 2,
+   wl_global_create(c->wl_display, &wl_output_interface, 3,
 output, bind_output);
wl_signal_emit(&c->output_created_signal, output);
 }
-- 
1.8.1.2

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

[PATCH 1/3] wayland: Add wl_output name event

[Quanxian Wang]

This event contains a human-readable name of output.
It may be sent after binding the output object.
It is intended that the client can use this output name
as a parameter or display it in logs.

For example, in weston randr application, output name can
be a parameter in command line to stand for an output.

Signed-off-by: Quanxian Wang 
---
 protocol/wayland.xml | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/protocol/wayland.xml b/protocol/wayland.xml
index d47ee62..881ebad 100644
--- a/protocol/wayland.xml
+++ b/protocol/wayland.xml
@@ -1618,7 +1618,7 @@
 
   
 
-  
+  
 
   An output describes part of the compositor geometry.  The
   compositor works in the 'compositor coordinate system' and an
@@ -1756,6 +1756,16 @@
   
   
 
+
+
+  
+   This event contains a human-readable name of output.
+   It may be sent after binding the output object.
+   It is intended that the client can use this output name
+   as a parameter or display it in logs.
+  
+  
+
   
 
   
-- 
1.8.1.2

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

[PATCH 0/3] Add wl_output name event

[Quanxian Wang]

This event contains a human-readable name of output.
It may be sent after binding the output object.
It is intended that the client can use this output name
as a parameter or display it in logs.

For example, in weston randr application, output name can
be a parameter in command line to stand for an output.

Quanxian Wang (3):
  wayland: Add wl_output name event
  shell: Add wl_output name event
  weston:Add wl_output name event

Wayland:
 protocol/wayland.xml | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Shell:
 clients/desktop-shell.c | 12 ++--
 clients/window.c| 12 ++--
 2 files changed, 20 insertions(+), 4 deletions(-)

Weston:
 src/compositor.c|  7 +--
 1 files changed, 5 insertions(+), 2 deletions(-)

-- 
1.8.1.2

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

[PATCH 3/3] shell: Add wl_output name event

[Quanxian Wang]

Signed-off-by: Quanxian Wang 
---
 clients/desktop-shell.c | 12 ++--
 clients/window.c| 12 ++--
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
index a0c6b6d..9b15e3c 100644
--- a/clients/desktop-shell.c
+++ b/clients/desktop-shell.c
@@ -1172,6 +1172,13 @@ output_handle_mode(void *data,
 }
 
 static void
+output_handle_name(void *data,
+  struct wl_output *wl_output,
+  const char *name)
+{
+}
+
+static void
 output_handle_done(void *data,
struct wl_output *wl_output)
 {
@@ -1192,7 +1199,8 @@ static const struct wl_output_listener output_listener = {
output_handle_geometry,
output_handle_mode,
output_handle_done,
-   output_handle_scale
+   output_handle_scale,
+   output_handle_name
 };
 
 static void
@@ -1221,7 +1229,7 @@ create_output(struct desktop *desktop, uint32_t id)
return;
 
output->output =
-   display_bind(desktop->display, id, &wl_output_interface, 2);
+   display_bind(desktop->display, id, &wl_output_interface, 3);
output->server_output_id = id;
 
wl_output_add_listener(output->output, &output_listener, output);
diff --git a/clients/window.c b/clients/window.c
index c8287e2..c3e9633 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -4758,11 +4758,19 @@ display_handle_mode(void *data,
}
 }
 
+static void
+display_handle_name(void *data,
+   struct wl_output *wl_output,
+   const char *name)
+{
+}
+
 static const struct wl_output_listener output_listener = {
display_handle_geometry,
display_handle_mode,
display_handle_done,
-   display_handle_scale
+   display_handle_scale,
+   display_handle_name
 };
 
 static void
@@ -4774,7 +4782,7 @@ display_add_output(struct display *d, uint32_t id)
output->display = d;
output->scale = 1;
output->output =
-   wl_registry_bind(d->registry, id, &wl_output_interface, 2);
+   wl_registry_bind(d->registry, id, &wl_output_interface, 3);
output->server_output_id = id;
wl_list_insert(d->output_list.prev, &output->link);
 
-- 
1.8.1.2

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