Re: [PATCH weston-ivi-shell 04/15] ivi-shell supports a type of shell for In-Vehicle Infotainment system.

[Pekka Paalanen]

On Tue, 11 Mar 2014 14:51:21 +0900
Nobuhiko Tanibata  wrote:

> 2014-03-10 19:47 に Pekka Paalanen さんは書きました:
> > On Thu,  6 Mar 2014 18:56:58 +0900
> > Nobuhiko Tanibata  wrote:
> > 
> >> In-Vehicle Infotainment system traditionally manages surfaces with 
> >> global
> >> identification. A protocol, ivi_application, supports such a feature 
> >> by
> >> implementing a request, ivi_application::surface_creation defined in
> >> ivi_application.xml. Additionally, it initialize a library, 
> >> weston-layout,
> >> to manage properties of surfaces and group surfaces in layer. In 
> >> detail,
> >> refer library, weston_layout.
> >> 
> >> Signed-off-by: Nobuhiko Tanibata 
> >> ---
> >>  ivi-shell/ivi-shell.c | 333 
> >> ++
> >>  1 file changed, 333 insertions(+)
> >>  create mode 100644 ivi-shell/ivi-shell.c

...

> >> +static void
> >> +init_ivi_shell(struct weston_compositor *ec, struct ivi_shell *shell)
> >> +{
> >> +shell->compositor = ec;
> >> +
> >> +wl_list_init(&ec->layer_list);
> >> +wl_list_init(&shell->list_surface);
> >> +}
> >> +
> >> +/**
> >> + * Initialization of ivi-shell. A library, weston_layout, is also 
> >> initialized
> >> + * here by calling weston_layout_initWithCompositor.
> >> + *
> >> + */
> >> +
> >> +WL_EXPORT int
> >> +module_init(struct weston_compositor *ec,
> >> +int *argc, char *argv[])
> >> +{
> >> +struct ivi_shell  *shell = NULL;
> >> +struct weston_seat *seat = NULL;
> >> +
> >> +shell = calloc(1, sizeof *shell);
> >> +if (shell == NULL) {
> >> +return -1;
> >> +}
> >> +
> >> +init_ivi_shell(ec, shell);
> >> +weston_layout_initWithCompositor(ec);
> > 
> > It's kind of surprising... this module's init requires another module 
> > to
> > have been loaded before, otherwise this module will not load, right?
> > I wonder if you get a sensible error message when that happens.
> 
> westen_layout is not a module to be loaded by ivi-shell. It is shared 
> library.
> If it is not weston manner, I can link it statically. Please give me 
> your suggestion?

Oh, ok, that should be alright then. I just jumped into a conclusion
that all .so's were plugins. I stand corrected. I read these patches
one by one, when I can.

Is there a reason weston_layout should be a dynamic library? Do you
expect it's ABI to be stable, and have a use case for swapping one of
the two while keeping the other compiled binary intact?

If you are not committing to a stable library ABI, then I'd suggest not
having it as a dynamic library to avoid any confusion and abuse; to
avoid the risk of suddenly realizing you cannot change the API, because
someone else is using it or having a different implementation of it.

> >> +
> >> +shell->destroy_listener.notify = shell_destroy;
> >> +wl_signal_add(&ec->destroy_signal, &shell->destroy_listener);
> >> +
> >> +if (wl_global_create(ec->wl_display, &ivi_application_interface, 
> >> 1,
> >> + shell, bind_ivi_application) == NULL) {
> >> +return -1;
> >> +}
> >> +
> >> +wl_list_for_each(seat, &ec->seat_list, link) {
> >> +if (seat) {
> >> +shell->seat = seat;
> > 
> > 'seat' cannot be NULL here. It seems you are assuming that there can be
> > only one seat? Why?
> > 
> 
> Ideally, I agree. However I am in automotive software, and I have habit 
> to avoid NULL access as much as possible in case of the worst scenario.

Then put an assert(), though in this case even that would be too much.
You can never get a NULL from wl_list_for_each(). You might get a bad
pointer if the list is corrupted, but the odds of getting exactly NULL
are diminishing even with corruption.

Silently ignoring NULLs where they should never appear may hide real
problems in testing, and even in production you'd probably want to log
it somehow to help post mortem.


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

Re: [PATCH] Bug fix client apps because of output change

[Pekka Paalanen]

On Tue, 11 Mar 2014 01:48:49 +
"Wang, Quanxian"  wrote:

> Thanks.
> 
> Quanxian
> 
> >> >binds with version 2 regardless of what the compositor advertised.
> >> >
> >> >You can compare to the "desktop_shell" global handling which seems
> >> >correct.
> >> [Wang, Quanxian] I will check and update that. The change will be
> >> like that 1293 output->output =
> >> 1294 display_bind(desktop->display, id,
> >> &wl_output_interface, version); 1295 output->server_output_id
> >> = id; 1296 output->interface_version = (version < 2) ?
> >> version : 2;
> >
> >Do not use the server advertized version in bind without checking it first. 
> >If the
> >server version grows, the client still needs to bind with the old version, 
> >until the
> >client code is ported to support the new version.
> [Wang, Quanxian] Sorry for misunderstanding the interface version issue. 
> Currently my understanding is that client apps(desktop-shell) is based on 
> wl_output which version is under 2(including 2), so whatever wl_output 
> upgrade or not, we can use only the function provided by wl_output interface 
> under the version 2. Right?
> Another question is, since version is from server, why not keep it as 
> original value? when you do some operation, you just check and choose what 
> needed based on the value(just like update_output). My suggestion is when you 
> upgrade client app to use wl_output, we don't need to continue maintain such 
> code.(For example from 2 to 3). The only update is the new code which want to 
> use new function provided by newly wl_output interface. What is your idea 
> about that?
> >

Let me put it more clearly. The client has been coded to support
version 2. The server advertises version N. The agreed interface
version that both can deal with is then min(2, N).

You have to tell the server, what the agreed version is: you have to
call display_bind() with version min(2, N). This way the server knows,
that it must not send events added in versions 3 or greater, because
those would crash the client.

Also the client knows, that it cannot use requests added in versions
N+1 or greater.

It does not matter whether you set output->interface_version = N or
min(2, N), because the code in the client is anyway comparing that to
what it actually is coded for. It's just more logical to me to store
the agreed version, not the server version.


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

Re: [PATCH] Bug fix client apps because of output change

[Pekka Paalanen]

On Tue, 11 Mar 2014 01:40:18 +
"Wang, Quanxian"  wrote:

> 
> 
> >-Original Message-
> >From: Pekka Paalanen [mailto:ppaala...@gmail.com]
> >Sent: Monday, March 10, 2014 7:05 PM
> >To: Wang, Quanxian
> >Cc: wayland-devel@lists.freedesktop.org
> >Subject: Re: [PATCH] Bug fix client apps because of output change
> >
> >On Mon, 10 Mar 2014 10:58:00 +
> >"Wang, Quanxian"  wrote:
> >
> >>
> >>
> >> >-Original Message-
> >> >From: Pekka Paalanen [mailto:ppaala...@gmail.com]
> >> >Sent: Monday, March 10, 2014 5:58 PM
> >> >To: Wang, Quanxian
> >> >Cc: wayland-devel@lists.freedesktop.org
> >> >Subject: Re: [PATCH] Bug fix client apps because of output change
> >> >
> >> >On Mon, 10 Mar 2014 08:23:45 +
> >> >"Wang, Quanxian"  wrote:
> >> >
> >> >> Thanks Pq. Comments below.
> >> >...
> >> >> >> @@ -1145,6 +1154,45 @@ desktop_destroy_outputs(struct desktop
> >> >> >> *desktop) }
> >> >> >>
> >> >> >>  static void
> >> >> >> +update_output(struct output *output) {
> >> >> >> +struct panel *panel = output->panel;
> >> >> >> +struct background *background = output->background;
> >> >> >> +int width, height;
> >> >> >> +
> >> >> >> +if (!output)
> >> >> >
> >> >> >You already dereferenced 'output' above, checking for NULL here is
> >> >> >useless. 'output' can never be NULL anyway, right?
> >> >> [Wang, Quanxian] right.
> >> >> >
> >> >> >> +return;
> >> >> >> +
> >> >> >> +width = output->mode.width;
> >> >> >> +height = output->mode.height;
> >> >> >> +
> >> >> >> +switch (output->transform) {
> >> >> >> +case WL_OUTPUT_TRANSFORM_90:
> >> >> >> +case WL_OUTPUT_TRANSFORM_270:
> >> >> >> +case WL_OUTPUT_TRANSFORM_FLIPPED_90:
> >> >> >> +case WL_OUTPUT_TRANSFORM_FLIPPED_270:
> >> >> >> +/* Swap width and height */
> >> >> >> +width = output->mode.height;
> >> >> >> +height = output->mode.width;
> >> >> >> +break;
> >> >> >> +default:
> >> >> >> +break;
> >> >> >> +}
> >> >> >> +
> >> >> >> +if (output->scale != 0) {
> >> >> >
> >> >> >If scale was initialized to 1, there would be no need for 'if'.
> >> >> >Right?
> >> >> [Wang, Quanxian] in testing, this cause the error because scale is
> >> >> set to 0, it happens when weston is started at the very beginning.
> >> >> Here we need to do that.
> >> >
> >> >Why does that happen? Is Weston sending an event with scale=0? If so,
> >> >then that is a Weston bug and should be fixed. Until you get any
> >> >event the scale is 1, because that is the implicit scale in interface 
> >> >version 1.
> >> [Wang, Quanxian] yes. Got geometry event with scale=0.
> >
> >What do you mean? wl_output.geometry does not carry scale at all, hence until
> >you receive the first wl_output.scale you should assume scale=1.
> >The easiest way to do that is to initialize output::scale to 1 in 
> >create_output(). You
> >should never have zero.
> [Wang, Quanxian] sorry, is scale event instead of geometry. Yes, I can 
> initialize scale to be 1, but at the same time, I need to get the scale event 
> from server. Basically I still need to do some checking for that.
> 

You cannot require receiving the scale event, because the
protocol specification says:

This event contains scaling geometry information
that is not in the geometry event. It may be sent after
binding the output object or if the output scale changes
later. If it is not sent, the client should assume a
scale of 1.

However, if wl_output agreed interface version is at least 2, then you
are guaranteed to receive the done event when you have received all
output property events.

> After that, I will check why scale(0) is sent by wl_output and push the patch 
> to fix that. 

I suspect it doesn't really happen, but if it does, it is a bug in the
compositor, yes.


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

RE: [PATCH] Bug fix client apps because of output change

[Wang, Quanxian]

>-Original Message-
>From: wayland-devel-boun...@lists.freedesktop.org
>[mailto:wayland-devel-boun...@lists.freedesktop.org] On Behalf Of Pekka
>Paalanen
>Sent: Tuesday, March 11, 2014 4:26 PM
>To: Wang, Quanxian
>Cc: wayland-devel@lists.freedesktop.org
>Subject: Re: [PATCH] Bug fix client apps because of output change
>
>On Tue, 11 Mar 2014 01:48:49 +
>"Wang, Quanxian"  wrote:
>
>> Thanks.
>>
>> Quanxian
>>
>> >> >binds with version 2 regardless of what the compositor advertised.
>> >> >
>> >> >You can compare to the "desktop_shell" global handling which seems
>> >> >correct.
>> >> [Wang, Quanxian] I will check and update that. The change will be
>> >> like that 1293 output->output =
>> >> 1294 display_bind(desktop->display, id,
>> >> &wl_output_interface, version); 1295 output->server_output_id
>> >> = id; 1296 output->interface_version = (version < 2) ?
>> >> version : 2;
>> >
>> >Do not use the server advertized version in bind without checking it
>> >first. If the server version grows, the client still needs to bind
>> >with the old version, until the client code is ported to support the new 
>> >version.
>> [Wang, Quanxian] Sorry for misunderstanding the interface version issue.
>Currently my understanding is that client apps(desktop-shell) is based on
>wl_output which version is under 2(including 2), so whatever wl_output upgrade
>or not, we can use only the function provided by wl_output interface under the
>version 2. Right?
>> Another question is, since version is from server, why not keep it as 
>> original
>value? when you do some operation, you just check and choose what needed
>based on the value(just like update_output). My suggestion is when you upgrade
>client app to use wl_output, we don't need to continue maintain such code.(For
>example from 2 to 3). The only update is the new code which want to use new
>function provided by newly wl_output interface. What is your idea about that?
>> >
>
>Let me put it more clearly. The client has been coded to support version 2. The
>server advertises version N. The agreed interface version that both can deal 
>with
>is then min(2, N).
>
>You have to tell the server, what the agreed version is: you have to call
>display_bind() with version min(2, N). This way the server knows, that it must 
>not
>send events added in versions 3 or greater, because those would crash the 
>client.
>
>Also the client knows, that it cannot use requests added in versions
>N+1 or greater.
>
>It does not matter whether you set output->interface_version = N or min(2, N),
>because the code in the client is anyway comparing that to what it actually is 
>coded
>for. It's just more logical to me to store the agreed version, not the server 
>version.
[Wang, Quanxian] Thanks. Got that.
>
>
>Thanks,
>pq
>___
>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] tests: Fix build of noinst fixed-benchmark test

[Pekka Paalanen]

On Tue, 11 Mar 2014 00:01:47 +
"Bryce W. Harrington"  wrote:

> Solves this build error:
> 
>   tests/fixed-benchmark.o: In function `benchmark':
>   ./wayland/tests/fixed-benchmark.c:82: undefined reference to `clock_gettime'
>   ./wayland/tests/fixed-benchmark.c:84: undefined reference to `clock_gettime'
> 
> Signed-off-by: Bryce Harrington 
> ---
>  Makefile.am |1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Makefile.am b/Makefile.am
> index cb7a186..f1584d5 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -175,6 +175,7 @@ resources_test_SOURCES = tests/resources-test.c
>  resources_test_LDADD = libtest-runner.la
>  
>  fixed_benchmark_SOURCES = tests/fixed-benchmark.c
> +fixed_benchmark_LDADD = libtest-runner.la

Hi,

I think more appropriate would be to just link it with -lrt, it's not a
test suite runnable test.


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

[PATCH] desktop-shell: Bug fix client apps because of output change

[Quanxian Wang]

1)
Width and height of Panel and Background depend on output's, therefore
they should be bound with output changes including mode, transform and
scale.

2)
Update the min_allocation before resize the panel and background
window. Add window_set_min_allocation function because it is invisible
outside window.c.

Signed-off-by: Quanxian Wang 
---
 clients/desktop-shell.c | 75 +++--
 clients/window.c|  7 +
 clients/window.h|  2 ++
 3 files changed, 81 insertions(+), 3 deletions(-)

diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
index a0c6b6d..dc98ea1 100644
--- a/clients/desktop-shell.c
+++ b/clients/desktop-shell.c
@@ -103,6 +103,15 @@ struct output {
 
struct panel *panel;
struct background *background;
+   struct {
+   int height;
+   int width;
+   uint32_t refresh;
+   } mode;
+
+   uint32_t interface_version;
+   uint32_t transform;
+   uint32_t scale;
 };
 
 struct panel_launcher {
@@ -1145,6 +1154,39 @@ desktop_destroy_outputs(struct desktop *desktop)
 }
 
 static void
+update_output(struct output *output)
+{
+   struct panel *panel = output->panel;
+   struct background *background = output->background;
+   int width, height;
+
+   width = output->mode.width;
+   height = output->mode.height;
+
+   switch (output->transform) {
+   case WL_OUTPUT_TRANSFORM_90:
+   case WL_OUTPUT_TRANSFORM_270:
+   case WL_OUTPUT_TRANSFORM_FLIPPED_90:
+   case WL_OUTPUT_TRANSFORM_FLIPPED_270:
+   /* Swap width and height */
+   width = output->mode.height;
+   height = output->mode.width;
+   break;
+   default:
+   break;
+   }
+
+   width /= output->scale;
+   height /= output->scale;
+
+   window_set_min_allocation(panel->window, width, 32);
+   window_set_min_allocation(background->window, width, height);
+
+   window_schedule_resize(background->window, width, height);
+   window_schedule_resize(panel->window, width, 32);
+}
+
+static void
 output_handle_geometry(void *data,
struct wl_output *wl_output,
int x, int y,
@@ -1157,6 +1199,11 @@ output_handle_geometry(void *data,
 {
struct output *output = data;
 
+   output->transform = transform;
+
+   if (output->interface_version < 2)
+   update_output(output);
+
window_set_buffer_transform(output->panel->window, transform);
window_set_buffer_transform(output->background->window, transform);
 }
@@ -1169,12 +1216,28 @@ output_handle_mode(void *data,
   int height,
   int refresh)
 {
+   struct output *output = data;
+
+   if (flags & WL_OUTPUT_MODE_CURRENT) {
+   if (!output)
+   return;
+
+   output->mode.width = width;
+   output->mode.height = height;
+   output->mode.refresh = refresh;
+
+   if (output->interface_version < 2)
+   update_output(output);
+   }
 }
 
 static void
 output_handle_done(void *data,
struct wl_output *wl_output)
 {
+   struct output *output = data;
+
+   update_output(output);
 }
 
 static void
@@ -1184,6 +1247,8 @@ output_handle_scale(void *data,
 {
struct output *output = data;
 
+   output->scale  = scale;
+
window_set_buffer_scale(output->panel->window, scale);
window_set_buffer_scale(output->background->window, scale);
 }
@@ -1212,7 +1277,7 @@ output_init(struct output *output, struct desktop 
*desktop)
 }
 
 static void
-create_output(struct desktop *desktop, uint32_t id)
+create_output(struct desktop *desktop, uint32_t id, uint32_t version)
 {
struct output *output;
 
@@ -1220,9 +1285,13 @@ create_output(struct desktop *desktop, uint32_t id)
if (!output)
return;
 
+   output->interface_version = (version < 2) ? version : 2;
output->output =
-   display_bind(desktop->display, id, &wl_output_interface, 2);
+   display_bind(desktop->display, id,
+&wl_output_interface,
+output->interface_version);
output->server_output_id = id;
+   output->scale = 1;
 
wl_output_add_listener(output->output, &output_listener, output);
 
@@ -1247,7 +1316,7 @@ global_handler(struct display *display, uint32_t id,
  desktop->interface_version);
desktop_shell_add_listener(desktop->shell, &listener, desktop);
} else if (!strcmp(interface, "wl_output")) {
-   create_output(desktop, id);
+   create_output(desktop, id, version);
}
 }
 
diff --git a/clients/window.c b/clients/window.c
index c8287e2..6c01222 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ 

Wayland window transparency

[Neal.Kunkle]

Hi,

I am currently struggling with transparency on a system with Wayland 1.2 and 
using the Weston compositor.  I have looked through the documentation and read 
through previous mails, but I did not find an answer to what I seek.  Can you 
please let me know if this is possible/ how to interact with Wayland for the 
following functionality?

What I would like to do:
I would like my window to have an opaque background, such that when objects in 
my application are transparent, I see through them to the window's background.  
This is what is occurring on other systems and I would like to remain 
consistent across platforms.

What is currently occurring:
When I have transparent objects on my window (meaning low alpha values), I see 
through the window to whatever is underneath.  If another application is 
running underneath mine, it will show through transparent places in my window.

Thanks,

Neal Kunkle
Software Engineer, ASW

EB - Driving the Future of Software
Am Wolfsmantel 46 91058 Erlangen, Deutschland
Phone: +49 9131 7701 7375
neal.kun...@elektrobit.com
http://automotive.elektrobit.com





Please note: This e-mail may contain confidential information
intended solely for the addressee. If you have received this
e-mail in error, please do not disclose it to anyone, notify
the sender promptly, and delete the message from your system.
Thank you.

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

Re: Wayland window transparency

[Giulio Camuffo]

Hi Neal,

You don't need to use any Wayland or Weston specific thing. Wayland
compositor just use buffers you provide to them as you provide them,
so if you want the buffer to be opaque give Wayland an opaque buffer.
Either use one without an alpha channel or clear the whole buffer with
an opaque color before drawing your application content.

Giulio

2014-03-11 14:27 GMT+02:00  :
> Hi,
>
>
>
> I am currently struggling with transparency on a system with Wayland 1.2 and
> using the Weston compositor.  I have looked through the documentation and
> read through previous mails, but I did not find an answer to what I seek.
> Can you please let me know if this is possible/ how to interact with Wayland
> for the following functionality?
>
>
>
> What I would like to do:
>
> I would like my window to have an opaque background, such that when objects
> in my application are transparent, I see through them to the window's
> background.  This is what is occurring on other systems and I would like to
> remain consistent across platforms.
>
>
>
> What is currently occurring:
>
> When I have transparent objects on my window (meaning low alpha values), I
> see through the window to whatever is underneath.  If another application is
> running underneath mine, it will show through transparent places in my
> window.
>
>
>
> Thanks,
>
>
>
> Neal Kunkle
> Software Engineer, ASW
>
>
>
> EB - Driving the Future of Software
>
> Am Wolfsmantel 46 91058 Erlangen, Deutschland
> Phone: +49 9131 7701 7375
> neal.kun...@elektrobit.com
> http://automotive.elektrobit.com
>
>
>
>
>
> 
> Please note: This e-mail may contain confidential information
> intended solely for the addressee. If you have received this
> e-mail in error, please do not disclose it to anyone, notify
> the sender promptly, and delete the message from your system.
> Thank you.
>
>
> ___
> 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: Wayland window transparency

[Pekka Paalanen]

On Tue, 11 Mar 2014 14:50:51 +0200
Giulio Camuffo  wrote:

> Hi Neal,
> 
> You don't need to use any Wayland or Weston specific thing. Wayland
> compositor just use buffers you provide to them as you provide them,
> so if you want the buffer to be opaque give Wayland an opaque buffer.
> Either use one without an alpha channel or clear the whole buffer with
> an opaque color before drawing your application content.
> 
> Giulio
> 
> 2014-03-11 14:27 GMT+02:00  :
> > Hi,
> >
> >
> >
> > I am currently struggling with transparency on a system with Wayland 1.2 and
> > using the Weston compositor.  I have looked through the documentation and
> > read through previous mails, but I did not find an answer to what I seek.
> > Can you please let me know if this is possible/ how to interact with Wayland
> > for the following functionality?
> >
> >
> >
> > What I would like to do:
> >
> > I would like my window to have an opaque background, such that when objects
> > in my application are transparent, I see through them to the window's
> > background.  This is what is occurring on other systems and I would like to
> > remain consistent across platforms.
> >
> >
> >
> > What is currently occurring:
> >
> > When I have transparent objects on my window (meaning low alpha values), I
> > see through the window to whatever is underneath.  If another application is
> > running underneath mine, it will show through transparent places in my
> > window.
> >

Or in other words, it seems like you are just using a wrong blending
mode or algorithm.

On X11 you are probably used to getting an opaque window, no matter
what values you write to the alpha channel. On Wayland, the alpha
channel is actually used by the server to blend your window to the rest
of the desktop, always.

Therefore your problem isn't really about getting transparency, it's
about ensuring that the alpha channel in the final rendered result is
*not* transparent.


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

RE: Wayland window transparency

[Neal.Kunkle]

Ok, this makes sense now.  I am indeed used to always getting an opaque window. 
 

Thank you for your responses!
Neal Kunkle

-Original Message-
From: Pekka Paalanen [mailto:ppaala...@gmail.com] 
Sent: Tuesday, March 11, 2014 2:31 PM
To: Kunkle Neal
Cc: Giulio Camuffo; wayland mailing list
Subject: Re: Wayland window transparency

On Tue, 11 Mar 2014 14:50:51 +0200
Giulio Camuffo  wrote:

> Hi Neal,
> 
> You don't need to use any Wayland or Weston specific thing. Wayland 
> compositor just use buffers you provide to them as you provide them, 
> so if you want the buffer to be opaque give Wayland an opaque buffer.
> Either use one without an alpha channel or clear the whole buffer with 
> an opaque color before drawing your application content.
> 
> Giulio
> 
> 2014-03-11 14:27 GMT+02:00  :
> > Hi,
> >
> >
> >
> > I am currently struggling with transparency on a system with Wayland 
> > 1.2 and using the Weston compositor.  I have looked through the 
> > documentation and read through previous mails, but I did not find an answer 
> > to what I seek.
> > Can you please let me know if this is possible/ how to interact with 
> > Wayland for the following functionality?
> >
> >
> >
> > What I would like to do:
> >
> > I would like my window to have an opaque background, such that when 
> > objects in my application are transparent, I see through them to the 
> > window's background.  This is what is occurring on other systems and 
> > I would like to remain consistent across platforms.
> >
> >
> >
> > What is currently occurring:
> >
> > When I have transparent objects on my window (meaning low alpha 
> > values), I see through the window to whatever is underneath.  If 
> > another application is running underneath mine, it will show through 
> > transparent places in my window.
> >

Or in other words, it seems like you are just using a wrong blending mode or 
algorithm.

On X11 you are probably used to getting an opaque window, no matter what values 
you write to the alpha channel. On Wayland, the alpha channel is actually used 
by the server to blend your window to the rest of the desktop, always.

Therefore your problem isn't really about getting transparency, it's about 
ensuring that the alpha channel in the final rendered result is
*not* transparent.


Thanks,
pq



Please note: This e-mail may contain confidential information
intended solely for the addressee. If you have received this
e-mail in error, please do not disclose it to anyone, notify
the sender promptly, and delete the message from your system.
Thank you.

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

Re: [PATCH weston v4 15/15] Add a screen sharing plugin

[Jason Ekstrand]

On Mar 10, 2014 9:29 PM, "Bryce W. Harrington" 
wrote:
>
> On Mon, Mar 10, 2014 at 08:42:41PM -0500, Jason Ekstrand wrote:
> > > > +static void
> > > > +ss_seat_handle_pointer_enter(void *data, struct wl_pointer
*pointer,
> > > > +  uint32_t serial, struct wl_surface
*surface,
> > > > +  wl_fixed_t x, wl_fixed_t y)
> > > > +{
> > > > + struct ss_seat *seat = data;
> > > > +
> > > > + /* We make the tacit assumption here that we are always
recieving
> > >
> > > receiving
> > >
> > > > +  * input in output coordinates.
> > > > + weston_output_transform_coordinate(&seat->output->base, x, y,
&x,
> > &y);
> > > > +  */
> > >
> > > Is that function call supposed to be commented out?
> > > If so, prefix with "*".
> >
> > Yes, it's supposed to be commented out.  However, the comment needs more
> > explanation.  In the wayland backend we have to transform the input into
> > output coordinates.  Because the screen-share plugin is untransforming
the
> > output before sending it to the RDP backend, this isn't needed.  Leaving
> > the function there but commented out was mostly a note to me that it
needs
> > to go back in if this ever changes.  That said, I should leave a far
better
> > comment than I did.  I'll fix it.
>
> Cool, makes sense.
>
> > >
> > > I know this function is cribbed from an analogous routine from
> > > compositor-wayland.c, but I'm curious why perror() is used here, when
> > > weston_log() is used elsewhere?
> >
> > No good reason.  We should probably change it in compositor-wayland.c
too
>
> Shall I do the honors or do you want to change it to keep it consistent
> with this patch?

Go ahead.  This is going to sit on the list for a little while (I've got
changes to make).  No sense making compositor-wayland wait on it.

>
> > > Also since there is some similarity, could any of this be refactored
> > > into shared code to avoid the redundancy?
> >
> > Yes, it could.  However, this really only shares the shm buffer
> > implementation with compositor-wayland.c and even there it's not
> > identical.  I thought about trying to make them share this, input code,
and
> > another thing or two.  However, there turned out to be enough
differences
> > that it wasn't worth it.
>
> *Nod*
>
> > > > +
> > > > + sb = zalloc(sizeof *sb);
> > > > +
> > > > + sb->output = so;
> > > > + wl_list_init(&sb->free_link);
> > > > + wl_list_insert(&so->shm.buffers, &sb->link);
> > > > +
> > > > + pixman_region32_init_rect(&sb->damage, 0, 0, width, height);
> > > > +
> > > > + sb->data = data;
> > > > + sb->size = height * stride;
> > > > +
> > > > + pool = wl_shm_create_pool(so->parent.shm, fd, sb->size);
> > > > +
> > > > + sb->buffer = wl_shm_pool_create_buffer(pool, 0,
> > > > +width, height, stride,
> > > > +
 WL_SHM_FORMAT_ARGB);
> > > > + wl_buffer_add_listener(sb->buffer, &buffer_listener, sb);
> > > > + wl_shm_pool_destroy(pool);
> > > > + close(fd);
> > > > +
> > > > + memset(data, 0, sb->size);
> > > > +
> > > > + sb->pm_image =
> > > > + pixman_image_create_bits(PIXMAN_a8r8g8b8, width,
height,
> > > > +  (uint32_t *)data, stride);
> > > > +
> > > > + return sb;
> > > > +}
> > > > +
> > > > +static void
> > > > +output_compute_transform(struct weston_output *output,
> > > > +  pixman_transform_t *transform)
> > > > +{
> > > > + pixman_fixed_t fw, fh;
> > > > +
> > > > + pixman_transform_init_identity(transform);
> > > > +
> > > > + fw = pixman_int_to_fixed(output->width);
> > > > + fh = pixman_int_to_fixed(output->height);
> > > > +
> > > > + switch (output->transform) {
> > > > + case WL_OUTPUT_TRANSFORM_FLIPPED:
> > > > + case WL_OUTPUT_TRANSFORM_FLIPPED_90:
> > > > + case WL_OUTPUT_TRANSFORM_FLIPPED_180:
> > > > + case WL_OUTPUT_TRANSFORM_FLIPPED_270:
> > > > + pixman_transform_scale(transform, NULL,
> > > > +pixman_int_to_fixed (-1),
> > > > +pixman_int_to_fixed (1));
> > > > + pixman_transform_translate(transform, NULL, fw, 0);
> > > > + }
> > > > +
> > > > + switch (output->transform) {
> > > > + default:
> > > > + case WL_OUTPUT_TRANSFORM_NORMAL:
> > > > + case WL_OUTPUT_TRANSFORM_FLIPPED:
> > > > + break;
> > > > + case WL_OUTPUT_TRANSFORM_90:
> > > > + case WL_OUTPUT_TRANSFORM_FLIPPED_90:
> > > > + pixman_transform_rotate(transform, NULL, 0,
> > pixman_fixed_1);
> > > > + pixman_transform_translate(transform, NULL, fh, 0);
> > > > + break;
> > > > + case WL_OUTPUT_TRANSFORM_180:
> > > > + case WL_OUTPUT_TRANSFORM_FLIPPED_180:
> > > > + pixman_transform_rotate(transform, NULL,
-pixman_fixed_1,
> > 0);
> > > > + pixman_transform_tran

Re: [PATCH wayland v3] protocol: try to clarify frame callback semantics

[Kristian Høgsberg]

On Tue, Mar 11, 2014 at 03:53:06PM +0200, Pekka Paalanen wrote:
> Ping?

Pong, thanks.  As we discussed, I agree with the changes here.
Patch committed.

Kristian

> On Mon, 24 Feb 2014 10:00:33 +0200
> Pekka Paalanen  wrote:
> 
> > From: Pekka Paalanen 
> > 
> > "the callback event will arrive after the next output refresh" is wrong,
> > if you interpret "output refresh" as framebuffer flip or the moment when
> > the new pixels turn into light the first time. Weston has probably never
> > worked this way.
> > 
> > Weston triggers the frame callbacks when it submits repainting commands
> > to the GPU, which is before the framebuffer flip.
> > 
> > Strike the incorrect claim, and the rest of the paragraph which no
> > longer offers useful information.
> > 
> > As a replacement, expand on the "throttling and driving animations"
> > characteristic. The main purpose is to let clients animate at the
> > display refresh rate, while avoiding drawing frames that will never be
> > presented.
> > 
> > The new claim is that the server should give some time between
> > triggering frame callbacks and repainting itself, for clients to draw
> > and commit. This is somewhat intimate with the repaint scheduling
> > algorithm a compositor uses, but hopefully the right intention.
> > 
> > Another point of this update is to imply, that frame callbacks should
> > not be used to count compositor repaint cycles nor monitor refresh
> > cycles. It has never been guaranteed to work. Removing the mention of
> > frame callback without an attach hopefully discourages such use.
> > 
> > v2: Don't just remove a paragraph, but add useful information about the
> > request's intent.
> > 
> > v3: Specify the order of posting frame callbacks.
> > 
> > Signed-off-by: Pekka Paalanen 
> > Cc: Axel Davy 
> > Cc: Jason Ekstrand 
> > ---
> >  protocol/wayland.xml | 29 -
> >  1 file changed, 20 insertions(+), 9 deletions(-)
> > 
> > diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> > index e1edbe5..3aa89af 100644
> > --- a/protocol/wayland.xml
> > +++ b/protocol/wayland.xml
> > @@ -1059,22 +1059,33 @@
> >  
> >  
> >  
> > -  
> > -   Request notification when the next frame is displayed. Useful
> > -   for throttling redrawing operations, and driving animations.
> > +  
> > +   Request a notification when it is a good time start drawing a new
> > +   frame, by creating a frame callback. This is useful for throttling
> > +   redrawing operations, and driving animations.
> > +
> > +   When a client is animating on a wl_surface, it can use the 'frame'
> > +   request to get notified when it is a good time to draw and commit the
> > +   next frame of animation. If the client commits an update earlier than
> > +   that, it is likely that some updates will not make it to the display,
> > +   and the client is wasting resources by drawing too often.
> > +
> > The frame request will take effect on the next wl_surface.commit.
> > The notification will only be posted for one frame unless
> > -   requested again.
> > +   requested again. For a wl_surface, the notifications are posted in
> > +   the order the frame requests were committed.
> > +
> > +   The server must send the notifications so that a client
> > +   will not send excessive updates, while still allowing
> > +   the highest possible update rate for clients that wait for the reply
> > +   before drawing again. The server should give some time for the client
> > +   to draw and commit after sending the frame callback events to let them
> > +   hit the next output refresh.
> >  
> > A server should avoid signalling the frame callbacks if the
> > surface is not visible in any way, e.g. the surface is off-screen,
> > or completely obscured by other opaque surfaces.
> >  
> > -   A client can request a frame callback even without an attach,
> > -   damage, or any other state changes. wl_surface.commit triggers a
> > -   display update, so the callback event will arrive after the next
> > -   output refresh where the surface is visible.
> > -
> > The object returned by this request will be destroyed by the
> > compositor after the callback is fired and as such the client must not
> > attempt to use it after that point.
> 
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston-ivi-shell 04/15] ivi-shell supports a type of shell for In-Vehicle Infotainment system.

[Nobuhiko Tanibata]

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

On Tue, 11 Mar 2014 14:51:21 +0900
Nobuhiko Tanibata  wrote:


2014-03-10 19:47 に Pekka Paalanen さんは書きました:
> On Thu,  6 Mar 2014 18:56:58 +0900
> Nobuhiko Tanibata  wrote:
>
>> In-Vehicle Infotainment system traditionally manages surfaces with
>> global
>> identification. A protocol, ivi_application, supports such a feature
>> by
>> implementing a request, ivi_application::surface_creation defined in
>> ivi_application.xml. Additionally, it initialize a library,
>> weston-layout,
>> to manage properties of surfaces and group surfaces in layer. In
>> detail,
>> refer library, weston_layout.
>>
>> Signed-off-by: Nobuhiko Tanibata 
>> ---
>>  ivi-shell/ivi-shell.c | 333
>> ++
>>  1 file changed, 333 insertions(+)
>>  create mode 100644 ivi-shell/ivi-shell.c


...


>> +static void
>> +init_ivi_shell(struct weston_compositor *ec, struct ivi_shell *shell)
>> +{
>> +shell->compositor = ec;
>> +
>> +wl_list_init(&ec->layer_list);
>> +wl_list_init(&shell->list_surface);
>> +}
>> +
>> +/**
>> + * Initialization of ivi-shell. A library, weston_layout, is also
>> initialized
>> + * here by calling weston_layout_initWithCompositor.
>> + *
>> + */
>> +
>> +WL_EXPORT int
>> +module_init(struct weston_compositor *ec,
>> +int *argc, char *argv[])
>> +{
>> +struct ivi_shell  *shell = NULL;
>> +struct weston_seat *seat = NULL;
>> +
>> +shell = calloc(1, sizeof *shell);
>> +if (shell == NULL) {
>> +return -1;
>> +}
>> +
>> +init_ivi_shell(ec, shell);
>> +weston_layout_initWithCompositor(ec);
>
> It's kind of surprising... this module's init requires another module
> to
> have been loaded before, otherwise this module will not load, right?
> I wonder if you get a sensible error message when that happens.

westen_layout is not a module to be loaded by ivi-shell. It is shared
library.
If it is not weston manner, I can link it statically. Please give me
your suggestion?


Oh, ok, that should be alright then. I just jumped into a conclusion
that all .so's were plugins. I stand corrected. I read these patches
one by one, when I can.

Is there a reason weston_layout should be a dynamic library? Do you
expect it's ABI to be stable, and have a use case for swapping one of
the two while keeping the other compiled binary intact?

If you are not committing to a stable library ABI, then I'd suggest not
having it as a dynamic library to avoid any confusion and abuse; to
avoid the risk of suddenly realizing you cannot change the API, because
someone else is using it or having a different implementation of it.



Hi pq,

I see. I agree with your comments.


>> +
>> +shell->destroy_listener.notify = shell_destroy;
>> +wl_signal_add(&ec->destroy_signal, &shell->destroy_listener);
>> +
>> +if (wl_global_create(ec->wl_display, &ivi_application_interface,
>> 1,
>> + shell, bind_ivi_application) == NULL) {
>> +return -1;
>> +}
>> +
>> +wl_list_for_each(seat, &ec->seat_list, link) {
>> +if (seat) {
>> +shell->seat = seat;
>
> 'seat' cannot be NULL here. It seems you are assuming that there can be
> only one seat? Why?
>

Ideally, I agree. However I am in automotive software, and I have 
habit
to avoid NULL access as much as possible in case of the worst 
scenario.


Then put an assert(), though in this case even that would be too much.
You can never get a NULL from wl_list_for_each(). You might get a bad
pointer if the list is corrupted, but the odds of getting exactly NULL
are diminishing even with corruption.

Silently ignoring NULLs where they should never appear may hide real
problems in testing, and even in production you'd probably want to log
it somehow to help post mortem.


OK. I will modify my patches.

Thank you very much for many comments!

BR,
Nobuhiko



Thanks,
pq

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

Re: [PATCH weston v4 00/15] Add a fullscreen shell protocol

[Andrew Wedgbury]

Hi Jason,

On Wed, 26 Feb 2014, Jason Ekstrand wrote:


4) Screen sharing and recording.  The security discussion on how to allow
   clients to request screen sharing and things of the like is still going
   on and I have no intention of solving that question here.  Most
   proposals for screen sharing and capture have been based on weston's
   screenshooter protocol which allows the screenshooter client to hand a
   surface to weston that weston then fills with the contents of the
   screen.  While this works for screenshots, there are a lot of race
   conditions involved in doing it this way.  People have also suggested
   screen sharing protocols.  However, these usually involve re-writing
   most of the wayland protocol in the other (client -> server) direction.

   For this reason, I propose that, instead of having complicated screen
   sharing protocols, the screen sharing or recording program simply act
   as a wayland compositor supporting the fullscreen shell protocol.  The
   primary compositor (the DE) can then spawn the sharing program and
   connect to it as a client.  This way we can use the normal Wayland
   protocol for communication rather than re-writing it in reverse.


Thanks for this. I've been putting together a prototype using our VNC 
server - I've implemented the required interfaces along with your new 
fullscreen shell, and I'm launching it in place of weston from your 
screen-share module. It appears to be working well.


One query - I notice you've added a "sprawl" option in compositor-wayland,
which creates an output for every output in the fullscreen shell
compositor. This makes sense in the "system compositor" use cases. But for
screen capture you really want the opposite - i.e. the fullscreen shell
compositor needs to create an output for each output of the real
compositor. I guess we could just connect back to the real compositor and
get the output configuration, but there still needs to be some way of
connecting this up in screen-share. (At the moment I'm simply creating a
single output in my fullscreen shell compositor)

Andrew

---
Andrew Wedgbury 

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

Re: [PATCH] tests: Fix build of noinst fixed-benchmark test

[Kristian Høgsberg]

On Tue, Mar 11, 2014 at 12:01:47AM +, Bryce W. Harrington wrote:
> Solves this build error:
> 
>   tests/fixed-benchmark.o: In function `benchmark':
>   ./wayland/tests/fixed-benchmark.c:82: undefined reference to `clock_gettime'
>   ./wayland/tests/fixed-benchmark.c:84: undefined reference to `clock_gettime'
> 
> Signed-off-by: Bryce Harrington 

Thanks Bryce, applied.

Kristian

> ---
>  Makefile.am |1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Makefile.am b/Makefile.am
> index cb7a186..f1584d5 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -175,6 +175,7 @@ resources_test_SOURCES = tests/resources-test.c
>  resources_test_LDADD = libtest-runner.la
>  
>  fixed_benchmark_SOURCES = tests/fixed-benchmark.c
> +fixed_benchmark_LDADD = libtest-runner.la
>  
>  os_wrappers_test_SOURCES = tests/os-wrappers-test.c
>  os_wrappers_test_LDADD = libtest-runner.la
> -- 
> 1.7.9.5
> ___
> 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] Link only against rt, as fixed-benchmark isn't runnable by test suite

[Bryce W. Harrington]

Thanks to feedback from Pekka Paalanen.  Using -lrt instead of
libtest-runner.la still results in successful compilation and no change
in test behavior, and fixed-benchmark still appears to run properly.

Signed-off-by: Bryce Harrington 
---
 Makefile.am |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index f1584d5..bdfabb7 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -175,7 +175,7 @@ resources_test_SOURCES = tests/resources-test.c
 resources_test_LDADD = libtest-runner.la
 
 fixed_benchmark_SOURCES = tests/fixed-benchmark.c
-fixed_benchmark_LDADD = libtest-runner.la
+fixed_benchmark_LDADD = -lrt
 
 os_wrappers_test_SOURCES = tests/os-wrappers-test.c
 os_wrappers_test_LDADD = libtest-runner.la
-- 
1.7.9.5
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH] gitignore log files, now in root directory

[Bryce W. Harrington]

Signed-off-by: Bryce Harrington 
---
 .gitignore |2 ++
 1 file changed, 2 insertions(+)

diff --git a/.gitignore b/.gitignore
index e0a73c0..d3044a2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -2,6 +2,7 @@
 *.jpg
 *.la
 *.lo
+*.log
 *.o
 *.pc
 *.so
@@ -21,6 +22,7 @@ cscope.out
 /config.status
 /configure
 /libtool
+/logs
 /stamp-h1
 /test-driver
 /weston.ini
-- 
1.7.9.5
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston-ivi-shell 04/15] ivi-shell supports a type of shell for In-Vehicle Infotainment system.

[Bill Spitzak]

Pekka Paalanen wrote:


+wl_list_for_each(seat, &ec->seat_list, link) {
+if (seat) {
+shell->seat = seat;

'seat' cannot be NULL here. It seems you are assuming that there can be
only one seat? Why?

Ideally, I agree. However I am in automotive software, and I have habit 
to avoid NULL access as much as possible in case of the worst scenario.


Then put an assert(), though in this case even that would be too much.
You can never get a NULL from wl_list_for_each(). You might get a bad
pointer if the list is corrupted, but the odds of getting exactly NULL
are diminishing even with corruption.

Silently ignoring NULLs where they should never appear may hide real
problems in testing, and even in production you'd probably want to log
it somehow to help post mortem.


In addition, seeing an "if (x)" is a strong indication to the programmer 
that x *can* be NULL, and they will then make modifications to perceived 
bugs on the assumption that NULL is a possible value. This often leads 
to this same mistake being propagated to every function called by this 
and every use of a variable this value is stored into.


An assert(x) however is a strong indication that x is *not* NULL.

If you really are tempted to make a test so that if you missed a bug 
there is less chance of the optimized program crashing, what I do is 
something like this:


assert(x);
if (!x) return; // THIS SHOULD NOT HAPPEN

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

Re: [PATCH] Link only against rt, as fixed-benchmark isn't runnable by test suite

[Pekka Paalanen]

On Tue, 11 Mar 2014 18:16:52 +
"Bryce W. Harrington"  wrote:

> Thanks to feedback from Pekka Paalanen.  Using -lrt instead of
> libtest-runner.la still results in successful compilation and no change
> in test behavior, and fixed-benchmark still appears to run properly.
> 
> Signed-off-by: Bryce Harrington 
> ---
>  Makefile.am |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index f1584d5..bdfabb7 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -175,7 +175,7 @@ resources_test_SOURCES = tests/resources-test.c
>  resources_test_LDADD = libtest-runner.la
>  
>  fixed_benchmark_SOURCES = tests/fixed-benchmark.c
> -fixed_benchmark_LDADD = libtest-runner.la
> +fixed_benchmark_LDADD = -lrt
>  
>  os_wrappers_test_SOURCES = tests/os-wrappers-test.c
>  os_wrappers_test_LDADD = libtest-runner.la

Yup, this was the intention. :-)


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

Re: [PATCH weston-ivi-shell 04/15] ivi-shell supports a type of shell for In-Vehicle Infotainment system.

[Nobuhiko Tanibata]

2014-03-12 04:10 に Bill Spitzak さんは書きました:

Pekka Paalanen wrote:


+wl_list_for_each(seat, &ec->seat_list, link) {
+if (seat) {
+shell->seat = seat;
'seat' cannot be NULL here. It seems you are assuming that there can 
be

only one seat? Why?

Ideally, I agree. However I am in automotive software, and I have 
habit to avoid NULL access as much as possible in case of the worst 
scenario.


Then put an assert(), though in this case even that would be too much.
You can never get a NULL from wl_list_for_each(). You might get a bad
pointer if the list is corrupted, but the odds of getting exactly NULL
are diminishing even with corruption.

Silently ignoring NULLs where they should never appear may hide real
problems in testing, and even in production you'd probably want to log
it somehow to help post mortem.


In addition, seeing an "if (x)" is a strong indication to the
programmer that x *can* be NULL, and they will then make modifications
to perceived bugs on the assumption that NULL is a possible value.
This often leads to this same mistake being propagated to every
function called by this and every use of a variable this value is
stored into.

An assert(x) however is a strong indication that x is *not* NULL.

If you really are tempted to make a test so that if you missed a bug
there is less chance of the optimized program crashing, what I do is
something like this:

assert(x);
if (!x) return; // THIS SHOULD NOT HAPPEN


Hi pq and Bill,

Thank you a lot. I will follow the above way.

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

Re: [PATCH weston v4 00/15] Add a fullscreen shell protocol

[Jason Ekstrand]

On Mar 11, 2014 11:36 AM, "Andrew Wedgbury" 
wrote:
>
> Hi Jason,
>
>
> On Wed, 26 Feb 2014, Jason Ekstrand wrote:
>
>> 4) Screen sharing and recording.  The security discussion on how to allow
>>clients to request screen sharing and things of the like is still
going
>>on and I have no intention of solving that question here.  Most
>>proposals for screen sharing and capture have been based on weston's
>>screenshooter protocol which allows the screenshooter client to hand a
>>surface to weston that weston then fills with the contents of the
>>screen.  While this works for screenshots, there are a lot of race
>>conditions involved in doing it this way.  People have also suggested
>>screen sharing protocols.  However, these usually involve re-writing
>>most of the wayland protocol in the other (client -> server)
direction.
>>
>>For this reason, I propose that, instead of having complicated screen
>>sharing protocols, the screen sharing or recording program simply act
>>as a wayland compositor supporting the fullscreen shell protocol.  The
>>primary compositor (the DE) can then spawn the sharing program and
>>connect to it as a client.  This way we can use the normal Wayland
>>protocol for communication rather than re-writing it in reverse.
>
>
> Thanks for this. I've been putting together a prototype using our VNC
server - I've implemented the required interfaces along with your new
fullscreen shell, and I'm launching it in place of weston from your
screen-share module. It appears to be working well.

Glad to hear it!  Not sure if it's helpful, but you might want to checkout
libwlb:

https://github.com/jekstrand/libwlb

I'm currently in the process of getting the latest version of
wl_fullscreen_shell implemented in it.  Unfortunately, it's not API-stable
yet, but I think it's getting close.

> One query - I notice you've added a "sprawl" option in compositor-wayland,
> which creates an output for every output in the fullscreen shell
> compositor. This makes sense in the "system compositor" use cases. But for
> screen capture you really want the opposite - i.e. the fullscreen shell
> compositor needs to create an output for each output of the real
> compositor. I guess we could just connect back to the real compositor and
> get the output configuration, but there still needs to be some way of
> connecting this up in screen-share. (At the moment I'm simply creating a
> single output in my fullscreen shell compositor)

Yes, this is an issue.  The wl_fullscreen_shell protocol only solves half
of the problem.  The other half is, as you've mentioned, is setting up
everything.  There are multiple reasons why this is a challenge.  First is
the question of how slave compositor (RDP or VNC server) gets launched
connected to.  One way to do this would be to develop a priviledged client
interface whereby a client can ask the server to connect to it for
screen-sharing and hand it the connection fd.  However, that's kind of out
of the scope for the fullscreen shell.  For the sake of my little demo,
I've made weston launch it.  (Note that the --width and --height parameters
that get passed should not be needed.  I have those in there because the
RDP backend for weston can't properly resize without crashing.  Ideally,
they would not be there)

The second issue is configuring outputs.  I don't know much about VNC or
RDP but I know that they have some mechanism for multiple outputs and we
would like to expose this.  One option would be to have the VNC server
connect as a client to the compositor.  If we go this route, you have to be
very careful that it doesn't dead-lock anyware because of the client-server
loop.  There are also a lot of race conditions there between the server
creating an output and the slave compositor creating one.  Even if we solve
those problems, this doesn't immediatly provide a mechanism for associating
the outputs that the VNC server sees as a client with the ones it provides
as a server.

Another possible way to solve this is to have an "add_output" request.
Obviously, many compositors won't allow this.  However, I am already
planning to add a set of capabilities flags so the compositor could easily
advertise whether or not this is supported.  If we did this, we would
probably also want to add some sort of move_output request to allow the
server to tell the slave compositor where the outputs are.  I talked to
Jasper a bit about this and I'm not 100% sure what I think of it.

I would be very glad to hear your input on this.
Thanks,
--Jason Ekstrand

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