date:20140509

Re: Bug 78372 - create multiple windows with offset

2014-05-09 Thread Pekka Paalanen

On Thu, 08 May 2014 10:52:51 -0700
Bill Spitzak  wrote:

> On 05/07/2014 10:54 PM, Pekka Paalanen wrote:
> 
> > This is similar to session save/restore, lacking a better term for it.
> > We do not even pretend to support or enable this yet. It is just yet one
> > more feature that the shell protocol suite for desktop should cover,
> > but so far no-one has done any work on it AFAIK.
> >
> > If there is not one already, you are welcome to open a feature request
> > bug about application layout save/restore.
> 
> But the only practical method I can see is to allow the client to query 
> and set the output and x/y position of any surface directly.

You have just been starting at X11 for too long.

> Possibly you are reading the words "save/restore" literally, in that you 
> are imagining some blob of data stored in the compositor that is 
> recognized to restore the layout. However this is NOT what is wanted. 

Sure, that's the first thing comes to my mind. Let the client save a
cookie or an encrypted blob, that it can pass back to the server when
it creates the windows again. /handwaving/

When I say that I have not heard anyone working on it, it really means
I do not have nor seen any plan *yet*. It is a complicated issue, which
cannot be designed in an email, and especially not by me.

> Clients must be able to generate a usable layout the first time they are 
> run, they must be able to do something intelligent if the set of windows 
> changes from that in the saved layout, and use a layout from one system 
> on another (including cross-platform), and be able to choose layouts 
> from a list and add the current layout as a new item on the list.

No. With separate windows you simply cannot do that. The client has no
clue whatsoever on e.g. what positions on an output might be ok, what
else is on the screen, or which outputs would be ok to conquer. You
also might be on a tiling compositor, where your expectations simply
totally fail.

Your program would have to be a mind-reader to get the initial layout
right for any user.

Instead, what an application could do, is describe the expectations it
has for its initial window layout, if possible. That is again left up
for the desktop shell protocol suite to develop.

If you want to be in absolute control of the layout (you the app, not
even the user), use a single big window with compartments or something,
maybe per output. If you think that sucks, I would probably agree. But
I still cannot see how you could guess which output to pick for what,
but that's probably just me never using special purpose monitors.

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

Re: [PATCH] event: Cheking for NULL before dereferencing the pointer.

2014-05-09 Thread Hardening


Le 09/05/2014 08:43, Srivardhan Hebbar a écrit :

Checking for NULL before dereferencing the wl_event_source
pointer so as to avoid crash.

Signed-off-by: Srivardhan Hebbar 
---
  src/event-loop.c |7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/event-loop.c b/src/event-loop.c
index 9790cde..b62d16e 100644
--- a/src/event-loop.c
+++ b/src/event-loop.c
@@ -312,7 +312,12 @@ wl_event_source_check(struct wl_event_source *source)
  WL_EXPORT int
  wl_event_source_remove(struct wl_event_source *source)
  {
-   struct wl_event_loop *loop = source->loop;
+   struct wl_event_loop *loop;
+
+   if (source == NULL)
+   return 0;
+
+   loop = source->loop;

/* We need to explicitly remove the fd, since closing the fd
 * isn't enough in case we've dup'ed the fd. */



Hello Srivardhan,

do you have a case where this check is hit ? I may be wrong but having 
no loop associated with a source event is not supposed to happen. So my 
guess is that a caller of wl_event_source_remove has forgotten to 
nullify the event source after the call.


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: [Weston] More discussion about weston output relative motion algorithm

2014-05-09 Thread Wang, Quanxian

Hi, Pq

Thanks for your comment and idea.

 I list several cases for discussion.

Case 1:
Original:
├──
│ B   │ 
└──
Action: Move A leftof B
├───┼───┤
│ A │ B │
└───┴───┘ 
As you said, B must not change, A will have negative coordinates. That is fine.

Case 2:
Original:
├───┼───┤
│ C │ D │E
└───┴───┘ 

Move D leftof C:
C must not change.
How about E? E does not need change.
├───┼───┤
│ D │ C  │ | E
└───┴───┘
D will have negative coordinates. A big hole is left. It is not accepted by 
user at least I think so.
An optimization is E will move left and is side with C 
├───┼───┤
│ D │ C  │E
└───┴───┘

Case 3:
├───┼───┤
│ C │ D │E  |  F
└───┴───┘
Action: Move E leftof D
C, D's coordinate will not change. E and C will have the same relation with D, 
however they are not clone.
F will be move forward. Right?
├───┼───┤
│C/E│D |  F
└───┴───┘

Case 4:
├───┼───┤
│ C │ D │E  |  F
└───┴───┘
││  │ G   |  
└───┴───┘
Action: Move E leftof D
C, D's coordinate will not change. E and C will have the same relation with D.
F will be move forward. The relation between F and G will be built up. Maybe 
you want G to be moved with E. That will be crazy. 
├───┼───┤
│C/E│D |  F |
└───┴───┘
│  ││ G|  
└───┴───┘

After the cases above, list a complex one. 

├───┼───┤
│  │H  |   I |
└───┴───┘
│  ││ G|  
├───┼───┤
│C/E│D |  F | K/L│M
└───┴─┘
│  ││J || | N|  
└───┴───┘

Move F leftof H, and find some output to take F's position. Then chain reaction 
will happen.
Here you will define an order who take the place. For example, K/L will take 
the place of F. What is the relation between G and K or L? what is the relation 
between J and K or L?
Things turns to be much more complex. Just have a think what will be. It is an 
interesting thing.

After that, you find a conclusion.
1) position to new place will be easy.
2) find replacer and rebuild the relations will be complex. 
3) Chain reaction will make you crazy. Because no one expect how many outputs 
will be available and how many links after that. Maybe 1, or 2 or 3 or more...
 It is very easy to be in a loop. And more hole in layout happens.

Anyway, it is a little hard to make everything happy. :)

Thanks

Regards

Quanxian Wang


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

Re: Bug 78372 - create multiple windows with offset

2014-05-09 Thread Daniel Stone

Hi,

On 9 May 2014 08:34, Pekka Paalanen  wrote:

> On Thu, 08 May 2014 10:52:51 -0700
> Bill Spitzak  wrote:
> > Possibly you are reading the words "save/restore" literally, in that you
> > are imagining some blob of data stored in the compositor that is
> > recognized to restore the layout. However this is NOT what is wanted.
>
> Sure, that's the first thing comes to my mind. Let the client save a
> cookie or an encrypted blob, that it can pass back to the server when
> it creates the windows again. /handwaving/
>
> When I say that I have not heard anyone working on it, it really means
> I do not have nor seen any plan *yet*. It is a complicated issue, which
> cannot be designed in an email, and especially not by me.


I wouldn't say it's that much of a handwave: it's actually exactly what we
came up with a couple of years ago when we were talking about session
management and persistent state.  Seemed to be the best option indeed.

But, like you say, no-one's really approached it yet.

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

Re: [PATCH] desktop-shell: Fix black edges on scaled desktop pattern

2014-05-09 Thread Pekka Paalanen

On Thu, 08 May 2014 20:00:35 -0700
Bill Spitzak  wrote:

> Filter sampling outside the source image can leak black into the edges 
> of the
> desktop image. This is most easily seen by scaling the default tiled image
> with this weston.ini:
> 
>   # no background-image and no background-color
>   background-type=scale-crop
> ---
>   clients/desktop-shell.c |2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
> index 4880888..e121cc7 100644
> --- a/clients/desktop-shell.c
> +++ b/clients/desktop-shell.c
> @@ -724,6 +724,7 @@ background_draw(struct widget *widget, void *data)
>   case BACKGROUND_SCALE:
>   cairo_matrix_init_scale(&matrix, sx, sy);
>   cairo_pattern_set_matrix(pattern, &matrix);
> + cairo_pattern_set_extend(pattern, CAIRO_EXTEND_PAD);
>   break;
>   case BACKGROUND_SCALE_CROP:
>   s = (sx < sy) ? sx : sy;
> @@ -733,6 +734,7 @@ background_draw(struct widget *widget, void *data)
>   cairo_matrix_init_translate(&matrix, tx, ty);
>   cairo_matrix_scale(&matrix, s, s);
>   cairo_pattern_set_matrix(pattern, &matrix);
> + cairo_pattern_set_extend(pattern, CAIRO_EXTEND_PAD);
>   break;
>   case BACKGROUND_TILE:
>   cairo_pattern_set_extend(pattern, CAIRO_EXTEND_REPEAT);

Ok, I see the problem, and I see this fix should be good, but for some
reason this patch does not apply at all. I had to do the change
manually. Anyway, it works right and fixes the issue.

Hm, it does apply with --ignore-whitespace, though. When I save the raw
email, there seems to be extra spaces in the beginning of all context
lines. Maybe Format="flowed" messes it up?


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

RE: [PATCH] event: Cheking for NULL before dereferencing the pointer.

2014-05-09 Thread Srivardhan



> -Original Message-
> From: wayland-devel [mailto:wayland-devel-
> boun...@lists.freedesktop.org] On Behalf Of Hardening
> Sent: Friday, May 09, 2014 1:08 PM
> To: wayland-devel@lists.freedesktop.org
> Subject: Re: [PATCH] event: Cheking for NULL before dereferencing the
> pointer.
> 
> Le 09/05/2014 08:43, Srivardhan Hebbar a écrit :
> > Checking for NULL before dereferencing the wl_event_source pointer so
> > as to avoid crash.
> >
> > Signed-off-by: Srivardhan Hebbar 
> > ---
> >   src/event-loop.c |7 ++-
> >   1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/event-loop.c b/src/event-loop.c index
> > 9790cde..b62d16e 100644
> > --- a/src/event-loop.c
> > +++ b/src/event-loop.c
> > @@ -312,7 +312,12 @@ wl_event_source_check(struct wl_event_source
> *source)
> >   WL_EXPORT int
> >   wl_event_source_remove(struct wl_event_source *source)
> >   {
> > -   struct wl_event_loop *loop = source->loop;
> > +   struct wl_event_loop *loop;
> > +
> > +   if (source == NULL)
> > +   return 0;
> > +
> > +   loop = source->loop;
> >
> > /* We need to explicitly remove the fd, since closing the fd
> >  * isn't enough in case we've dup'ed the fd. */
> >
> 
> Hello Srivardhan,
> 
> do you have a case where this check is hit ? I may be wrong but having no
> loop associated with a source event is not supposed to happen. So my guess
> is that a caller of wl_event_source_remove has forgotten to nullify the
event
> source after the call.

Hi,

I was going through the code of Weston. In the main function in compositor.c
wl_event_loop_add_signal() is called to allocate the memory for
signals[](Line no: 4196. struct wl_event_source *signals[4]). The function
returns NULL if memory allocation failed. After that there is no NULL check
for 'signals'. In the end while clearing up, this function is called. So if
memory allocation failed then a NULL is passed to this function. Hence
adding code to check for the NULL.

Thank-you,
Hebbar
> 
> 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

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

Re: [PATCH] event: Cheking for NULL before dereferencing the pointer.

2014-05-09 Thread Pekka Paalanen

On Fri, 09 May 2014 14:56:14 +0530
Srivardhan  wrote:

> 
> 
> > -Original Message-
> > From: wayland-devel [mailto:wayland-devel-
> > boun...@lists.freedesktop.org] On Behalf Of Hardening
> > Sent: Friday, May 09, 2014 1:08 PM
> > To: wayland-devel@lists.freedesktop.org
> > Subject: Re: [PATCH] event: Cheking for NULL before dereferencing the
> > pointer.
> > 
> > Le 09/05/2014 08:43, Srivardhan Hebbar a écrit :
> > > Checking for NULL before dereferencing the wl_event_source pointer so
> > > as to avoid crash.
> > >
> > > Signed-off-by: Srivardhan Hebbar 
> > > ---
> > >   src/event-loop.c |7 ++-
> > >   1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/event-loop.c b/src/event-loop.c index
> > > 9790cde..b62d16e 100644
> > > --- a/src/event-loop.c
> > > +++ b/src/event-loop.c
> > > @@ -312,7 +312,12 @@ wl_event_source_check(struct wl_event_source
> > *source)
> > >   WL_EXPORT int
> > >   wl_event_source_remove(struct wl_event_source *source)
> > >   {
> > > - struct wl_event_loop *loop = source->loop;
> > > + struct wl_event_loop *loop;
> > > +
> > > + if (source == NULL)
> > > + return 0;
> > > +
> > > + loop = source->loop;
> > >
> > >   /* We need to explicitly remove the fd, since closing the fd
> > >* isn't enough in case we've dup'ed the fd. */
> > >
> > 
> > Hello Srivardhan,
> > 
> > do you have a case where this check is hit ? I may be wrong but having no
> > loop associated with a source event is not supposed to happen. So my guess
> > is that a caller of wl_event_source_remove has forgotten to nullify the
> event
> > source after the call.
> 
> Hi,
> 
> I was going through the code of Weston. In the main function in compositor.c
> wl_event_loop_add_signal() is called to allocate the memory for
> signals[](Line no: 4196. struct wl_event_source *signals[4]). The function
> returns NULL if memory allocation failed. After that there is no NULL check
> for 'signals'. In the end while clearing up, this function is called. So if
> memory allocation failed then a NULL is passed to this function. Hence
> adding code to check for the NULL.

Hi,

I think we should be fixing the caller instead of
wl_event_source_remove(). I do not believe NULL is a valid argument for
it, so the bug is in the caller.

If any wl_event_loop_add_signal() in Weston fails, it would be a reason
to exit with an error.


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

RE: [PATCH] event: Cheking for NULL before dereferencing the pointer.

2014-05-09 Thread Srivardhan



> -Original Message-
> From: Pekka Paalanen [mailto:ppaala...@gmail.com]
> Sent: Friday, May 09, 2014 3:09 PM
> To: Srivardhan
> Cc: 'Hardening'; wayland-devel@lists.freedesktop.org
> Subject: Re: [PATCH] event: Cheking for NULL before dereferencing the
> pointer.
> 
> On Fri, 09 May 2014 14:56:14 +0530
> Srivardhan  wrote:
> 
> >
> >
> > > -Original Message-
> > > From: wayland-devel [mailto:wayland-devel-
> > > boun...@lists.freedesktop.org] On Behalf Of Hardening
> > > Sent: Friday, May 09, 2014 1:08 PM
> > > To: wayland-devel@lists.freedesktop.org
> > > Subject: Re: [PATCH] event: Cheking for NULL before dereferencing
> > > the pointer.
> > >
> > > Le 09/05/2014 08:43, Srivardhan Hebbar a écrit :
> > > > Checking for NULL before dereferencing the wl_event_source pointer
> > > > so as to avoid crash.
> > > >
> > > > Signed-off-by: Srivardhan Hebbar 
> > > > ---
> > > >   src/event-loop.c |7 ++-
> > > >   1 file changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/src/event-loop.c b/src/event-loop.c index
> > > > 9790cde..b62d16e 100644
> > > > --- a/src/event-loop.c
> > > > +++ b/src/event-loop.c
> > > > @@ -312,7 +312,12 @@ wl_event_source_check(struct
> wl_event_source
> > > *source)
> > > >   WL_EXPORT int
> > > >   wl_event_source_remove(struct wl_event_source *source)
> > > >   {
> > > > -   struct wl_event_loop *loop = source->loop;
> > > > +   struct wl_event_loop *loop;
> > > > +
> > > > +   if (source == NULL)
> > > > +   return 0;
> > > > +
> > > > +   loop = source->loop;
> > > >
> > > > /* We need to explicitly remove the fd, since closing the fd
> > > >  * isn't enough in case we've dup'ed the fd. */
> > > >
> > >
> > > Hello Srivardhan,
> > >
> > > do you have a case where this check is hit ? I may be wrong but
> > > having no loop associated with a source event is not supposed to
> > > happen. So my guess is that a caller of wl_event_source_remove has
> > > forgotten to nullify the
> > event
> > > source after the call.
> >
> > Hi,
> >
> > I was going through the code of Weston. In the main function in
> > compositor.c
> > wl_event_loop_add_signal() is called to allocate the memory for
> > signals[](Line no: 4196. struct wl_event_source *signals[4]). The
> > function returns NULL if memory allocation failed. After that there is
> > no NULL check for 'signals'. In the end while clearing up, this
> > function is called. So if memory allocation failed then a NULL is
> > passed to this function. Hence adding code to check for the NULL.
> 
> Hi,
> 
> I think we should be fixing the caller instead of
wl_event_source_remove(). I
> do not believe NULL is a valid argument for it, so the bug is in the
caller.
> 
> If any wl_event_loop_add_signal() in Weston fails, it would be a reason to
> exit with an error.
> 

Oh... k... Will fix that and send the patch...

In general isn't it fine to check for NULL before dereferencing? 

Thank-you,
Hebbar
> 
> Thanks,
> pq

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

Re: [PATCH] event: Cheking for NULL before dereferencing the pointer.

2014-05-09 Thread Pekka Paalanen

On Fri, 09 May 2014 15:21:51 +0530
Srivardhan  wrote:

> 
> 
> > -Original Message-
> > From: Pekka Paalanen [mailto:ppaala...@gmail.com]
> > Sent: Friday, May 09, 2014 3:09 PM
> > To: Srivardhan
> > Cc: 'Hardening'; wayland-devel@lists.freedesktop.org
> > Subject: Re: [PATCH] event: Cheking for NULL before dereferencing the
> > pointer.
> > 
> > On Fri, 09 May 2014 14:56:14 +0530
> > Srivardhan  wrote:
> > 
> > >
> > >
> > > > -Original Message-
> > > > From: wayland-devel [mailto:wayland-devel-
> > > > boun...@lists.freedesktop.org] On Behalf Of Hardening
> > > > Sent: Friday, May 09, 2014 1:08 PM
> > > > To: wayland-devel@lists.freedesktop.org
> > > > Subject: Re: [PATCH] event: Cheking for NULL before dereferencing
> > > > the pointer.
> > > >
> > > > Le 09/05/2014 08:43, Srivardhan Hebbar a écrit :
> > > > > Checking for NULL before dereferencing the wl_event_source pointer
> > > > > so as to avoid crash.
> > > > >
> > > > > Signed-off-by: Srivardhan Hebbar 
> > > > > ---
> > > > >   src/event-loop.c |7 ++-
> > > > >   1 file changed, 6 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/src/event-loop.c b/src/event-loop.c index
> > > > > 9790cde..b62d16e 100644
> > > > > --- a/src/event-loop.c
> > > > > +++ b/src/event-loop.c
> > > > > @@ -312,7 +312,12 @@ wl_event_source_check(struct
> > wl_event_source
> > > > *source)
> > > > >   WL_EXPORT int
> > > > >   wl_event_source_remove(struct wl_event_source *source)
> > > > >   {
> > > > > - struct wl_event_loop *loop = source->loop;
> > > > > + struct wl_event_loop *loop;
> > > > > +
> > > > > + if (source == NULL)
> > > > > + return 0;
> > > > > +
> > > > > + loop = source->loop;
> > > > >
> > > > >   /* We need to explicitly remove the fd, since closing the fd
> > > > >* isn't enough in case we've dup'ed the fd. */
> > > > >
> > > >
> > > > Hello Srivardhan,
> > > >
> > > > do you have a case where this check is hit ? I may be wrong but
> > > > having no loop associated with a source event is not supposed to
> > > > happen. So my guess is that a caller of wl_event_source_remove has
> > > > forgotten to nullify the
> > > event
> > > > source after the call.
> > >
> > > Hi,
> > >
> > > I was going through the code of Weston. In the main function in
> > > compositor.c
> > > wl_event_loop_add_signal() is called to allocate the memory for
> > > signals[](Line no: 4196. struct wl_event_source *signals[4]). The
> > > function returns NULL if memory allocation failed. After that there is
> > > no NULL check for 'signals'. In the end while clearing up, this
> > > function is called. So if memory allocation failed then a NULL is
> > > passed to this function. Hence adding code to check for the NULL.
> > 
> > Hi,
> > 
> > I think we should be fixing the caller instead of
> wl_event_source_remove(). I
> > do not believe NULL is a valid argument for it, so the bug is in the
> caller.
> > 
> > If any wl_event_loop_add_signal() in Weston fails, it would be a reason to
> > exit with an error.
> > 
> 
> Oh... k... Will fix that and send the patch...
> 
> In general isn't it fine to check for NULL before dereferencing? 

Checking is one thing, silently hiding bugs is another thing.

If NULL is a legal input, then of course it needs to be checked.

If NULL can happen, but is a runtime error, the program needs to be
vocal about it, e.g. relay the error back to the caller.

If API specification says NULL is not a valid input, putting an
assert() would be fine, since violating that is a programmer error in
the caller.

I think wl_event_source_remove() falls into the last category. All
functions in wayland-util.h belong to this category, too.


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

[PATCH weston] rpi: build fix for compute_rects debug

2014-05-09 Thread Pekka Paalanen

From: Pekka Paalanen 

See 918f2dd4cfb8b177f67b45653efbbe4325cbe9dc

Signed-off-by: Pekka Paalanen 
---
 src/rpi-renderer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/rpi-renderer.c b/src/rpi-renderer.c
index 3a7f65c..c222eb6 100644
--- a/src/rpi-renderer.c
+++ b/src/rpi-renderer.c
@@ -858,8 +858,8 @@ rpir_view_compute_rects(struct rpir_view *view,
src_height = int_max(src_height, 0);
 
DBG("rpir_view %p %dx%d: p1 %f, %f; p2 %f, %f\n", view,
-   view->view->geometry.width,
-   view->view->geometry.height,
+   view->view->surface->width,
+   view->view->surface->height,
p1.f[0], p1.f[1], p2.f[0], p2.f[1]);
DBG("src rect %d;%d, %d;%d, %d;%dx%d;%d\n",
src_x >> 16, src_x & 0x,
-- 
1.8.5.5

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

Re: [PATCH] event: Cheking for NULL before dereferencing the pointer.

2014-05-09 Thread Hardening


Le 09/05/2014 12:20, Pekka Paalanen a écrit :

On Fri, 09 May 2014 15:21:51 +0530
Srivardhan  wrote:





-Original Message-
From: Pekka Paalanen [mailto:ppaala...@gmail.com]
Sent: Friday, May 09, 2014 3:09 PM
To: Srivardhan
Cc: 'Hardening'; wayland-devel@lists.freedesktop.org
Subject: Re: [PATCH] event: Cheking for NULL before dereferencing the
pointer.

On Fri, 09 May 2014 14:56:14 +0530
Srivardhan  wrote:



[...]



Checking is one thing, silently hiding bugs is another thing.

If NULL is a legal input, then of course it needs to be checked.

If NULL can happen, but is a runtime error, the program needs to be
vocal about it, e.g. relay the error back to the caller.

If API specification says NULL is not a valid input, putting an
assert() would be fine, since violating that is a programmer error in
the caller.

I think wl_event_source_remove() falls into the last category. All
functions in wayland-util.h belong to this category, too.



IMHO wl_event_source_remove() should take a wl_event_source ** as 
parameter and set to NULL the event_source pointer (preventing anyone 
to use it). Using eclipse call hierarchy, I've seen many places where 
this extra precaution is not taken.
I don't know if wl_event_source_remove() can be considered as part of 
the libwayland API and so fixed in stone ?


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

[PATCH weston] vaapi-recorder: Don't loop trying to write on out of space condition

2014-05-09 Thread Ander Conselvan de Oliveira

From: Ander Conselvan de Oliveira 

The error handling for the function that writes the encoded frame on
the disk was bogus, always assuming the buffer supplied to the encoder
was too small. That would cause a bigger buffer to be allocated and
another attempt to encode the frame was done. In the case of a failure
to write to disk (due to ENOSPC, for instance) that would cause an
endless loop.

---
Possibly related to
https://bugs.freedesktop.org/show_bug.cgi?id=69330
---
 src/compositor-drm.c | 27 +++
 src/vaapi-recorder.c | 42 +-
 src/vaapi-recorder.h |  2 +-
 3 files changed, 53 insertions(+), 18 deletions(-)

diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index 5f59789..7d514e4 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -2558,6 +2558,18 @@ planes_binding(struct weston_seat *seat, uint32_t time, 
uint32_t key, void *data
 
 #ifdef BUILD_VAAPI_RECORDER
 static void
+recorder_destroy(struct drm_output *output)
+{
+   vaapi_recorder_destroy(output->recorder);
+   output->recorder = NULL;
+
+   output->base.disable_planes--;
+
+   wl_list_remove(&output->recorder_frame_listener.link);
+   weston_log("[libva recorder] done\n");
+}
+
+static void
 recorder_frame_notify(struct wl_listener *listener, void *data)
 {
struct drm_output *output;
@@ -2579,7 +2591,12 @@ recorder_frame_notify(struct wl_listener *listener, void 
*data)
return;
}
 
-   vaapi_recorder_frame(output->recorder, fd, output->current->stride);
+   ret = vaapi_recorder_frame(output->recorder, fd,
+  output->current->stride);
+   if (ret < 0) {
+   weston_log("[libva recorder] aborted: %m\n");
+   recorder_destroy(output);
+   }
 }
 
 static void *
@@ -2637,13 +2654,7 @@ recorder_binding(struct weston_seat *seat, uint32_t 
time, uint32_t key,
 
weston_log("[libva recorder] initialized\n");
} else {
-   vaapi_recorder_destroy(output->recorder);
-   output->recorder = NULL;
-
-   output->base.disable_planes--;
-
-   wl_list_remove(&output->recorder_frame_listener.link);
-   weston_log("[libva recorder] done\n");
+   recorder_destroy(output);
}
 }
 #else
diff --git a/src/vaapi-recorder.c b/src/vaapi-recorder.c
index 0095a42..921494d 100644
--- a/src/vaapi-recorder.c
+++ b/src/vaapi-recorder.c
@@ -50,6 +50,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -93,6 +94,7 @@ struct vaapi_recorder {
int width, height;
int frame_count;
 
+   int error;
int destroying;
pthread_t worker_thread;
pthread_mutex_t mutex;
@@ -761,7 +763,13 @@ encoder_create_output_buffer(struct vaapi_recorder *r)
return VA_INVALID_ID;
 }
 
-static int
+enum output_write_status {
+   OUTPUT_WRITE_SUCCESS,
+   OUTPUT_WRITE_OVERFLOW,
+   OUTPUT_WRITE_FATAL
+};
+
+static enum output_write_status
 encoder_write_output(struct vaapi_recorder *r, VABufferID output_buf)
 {
VACodedBufferSegment *segment;
@@ -770,19 +778,22 @@ encoder_write_output(struct vaapi_recorder *r, VABufferID 
output_buf)
 
status = vaMapBuffer(r->va_dpy, output_buf, (void **) &segment);
if (status != VA_STATUS_SUCCESS)
-   return -1;
+   return OUTPUT_WRITE_FATAL;
 
if (segment->status & VA_CODED_BUF_STATUS_SLICE_OVERFLOW_MASK) {
r->encoder.output_size *= 2;
vaUnmapBuffer(r->va_dpy, output_buf);
-   return -1;
+   return OUTPUT_WRITE_OVERFLOW;
}
 
count = write(r->output_fd, segment->buf, segment->size);
 
vaUnmapBuffer(r->va_dpy, output_buf);
 
-   return count;
+   if (count < 0)
+   return OUTPUT_WRITE_FATAL;
+
+   return OUTPUT_WRITE_SUCCESS;
 }
 
 static void
@@ -792,9 +803,8 @@ encoder_encode(struct vaapi_recorder *r, VASurfaceID input)
 
VABufferID buffers[8];
int count = 0;
-
-   int slice_type;
-   int ret, i;
+   int i, slice_type;
+   enum output_write_status ret;
 
if ((r->frame_count % r->encoder.intra_period) == 0)
slice_type = SLICE_TYPE_I;
@@ -829,7 +839,10 @@ encoder_encode(struct vaapi_recorder *r, VASurfaceID input)
output_buf = VA_INVALID_ID;
 
vaDestroyBuffer(r->va_dpy, buffers[--count]);
-   } while (ret < 0);
+   } while (ret == OUTPUT_WRITE_OVERFLOW);
+
+   if (ret == OUTPUT_WRITE_FATAL)
+   r->error = errno;
 
for (i = 0; i < count; i++)
vaDestroyBuffer(r->va_dpy, buffers[i]);
@@ -1138,11 +1151,19 @@ worker_thread_function(void *data)
return NULL;
 }
 
-void
+int
 vaapi_recorder_frame(struct vaapi_recorder *r, int prime_fd, int stride)
 {
+   int ret = 0;
+
pthread_mut

Re: [PATCH] event: Cheking for NULL before dereferencing the pointer.

2014-05-09 Thread Pekka Paalanen

On Fri, 09 May 2014 14:50:19 +0200
Hardening  wrote:

> Le 09/05/2014 12:20, Pekka Paalanen a écrit :
> > On Fri, 09 May 2014 15:21:51 +0530
> > Srivardhan  wrote:
> >
> >>
> >>
> >>> -Original Message-
> >>> From: Pekka Paalanen [mailto:ppaala...@gmail.com]
> >>> Sent: Friday, May 09, 2014 3:09 PM
> >>> To: Srivardhan
> >>> Cc: 'Hardening'; wayland-devel@lists.freedesktop.org
> >>> Subject: Re: [PATCH] event: Cheking for NULL before dereferencing the
> >>> pointer.
> >>>
> >>> On Fri, 09 May 2014 14:56:14 +0530
> >>> Srivardhan  wrote:
> >>>
> 
> [...]
> 
> >
> > Checking is one thing, silently hiding bugs is another thing.
> >
> > If NULL is a legal input, then of course it needs to be checked.
> >
> > If NULL can happen, but is a runtime error, the program needs to be
> > vocal about it, e.g. relay the error back to the caller.
> >
> > If API specification says NULL is not a valid input, putting an
> > assert() would be fine, since violating that is a programmer error in
> > the caller.
> >
> > I think wl_event_source_remove() falls into the last category. All
> > functions in wayland-util.h belong to this category, too.
> >
> 
> IMHO wl_event_source_remove() should take a wl_event_source ** as 
> parameter and set to NULL the event_source pointer (preventing anyone 
> to use it). Using eclipse call hierarchy, I've seen many places where 
> this extra precaution is not taken.
> I don't know if wl_event_source_remove() can be considered as part of 
> the libwayland API and so fixed in stone ?

If it is exported in a release, it is set in stone. And so it is.

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

Re: [PATCH] Destroy resources when destroying input and output

2014-05-09 Thread Neil Roberts

Perhaps we should consider applying the patch anyway even though it's
not ideal. Currently if a client uses a dead output in a request such as
xdg_surface.set_output Weston will end up with a weston_output pointer
that points to freed memory. This could cause the compositor to crash.
That is worse than the alternative provided by this patch which is to
make the client abort. The clients know about the output being destroyed
via the wl_registry.global_remove event so in practice they would only
hit the problem in the unlikely event that they used the output in a
request in the short time between the output being unplugged and
noticing the removal event.

In the longer term I was thinking maybe it would be good to handle the
inert resource idea within libwayland-server. We could add a function
like wl_resource_zombify() which would mark the resource as a zombie and
call the destroy handlers. From the compositor's perspective it can then
act as if the resource has been destroyed. We could detect zombie
resources being used within the request marshalling code and ignore the
request. If the request creates new resource we can internally create
new zombie resources too and Weston would never need to know about it. I
am planning to experiment with this approach now.

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

Re: [PATCH] Destroy resources when destroying input and output

2014-05-09 Thread Pekka Paalanen

On Fri, 09 May 2014 14:33:58 +0100
Neil Roberts  wrote:

> Perhaps we should consider applying the patch anyway even though it's
> not ideal. Currently if a client uses a dead output in a request such as
> xdg_surface.set_output Weston will end up with a weston_output pointer
> that points to freed memory. This could cause the compositor to crash.

True, it's very bad now.

> That is worse than the alternative provided by this patch which is to
> make the client abort. The clients know about the output being destroyed
> via the wl_registry.global_remove event so in practice they would only
> hit the problem in the unlikely event that they used the output in a
> request in the short time between the output being unplugged and
> noticing the removal event.

That is also true, but if it is fixed improperly now, there is a very
good chance we just forget about the problem, and never fix it for
real. Especially when it becomes very hard to trigger.

At least make sure we have an open bug report about it, please.

> In the longer term I was thinking maybe it would be good to handle the
> inert resource idea within libwayland-server. We could add a function
> like wl_resource_zombify() which would mark the resource as a zombie and
> call the destroy handlers. From the compositor's perspective it can then
> act as if the resource has been destroyed. We could detect zombie
> resources being used within the request marshalling code and ignore the
> request. If the request creates new resource we can internally create
> new zombie resources too and Weston would never need to know about it. I
> am planning to experiment with this approach now.

Hmm... will be interesting to see, if that works out. It does feel like
quite a lot of magic in libwayland-server, while also making life a lot
easier.


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

[PATCH 1/2] Set to NULL event source after a call to wl_event_source_remove

2014-05-09 Thread Hardening

This patch sets to NULL event sources after a call to wl_event_source_remove()
has been made.
---
 src/clipboard.c  | 3 ++-
 src/compositor-drm.c | 3 +++
 src/compositor-rpi.c | 1 +
 src/compositor-x11.c | 2 ++
 src/compositor.c | 6 +-
 src/evdev-touchpad.c | 1 +
 src/evdev.c  | 4 +++-
 src/libinput-seat.c  | 1 +
 src/logind-util.c| 2 ++
 xwayland/launcher.c  | 6 +-
 xwayland/selection.c | 9 +++--
 11 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/src/clipboard.c b/src/clipboard.c
index 5a3a02d..0e25dc1 100644
--- a/src/clipboard.c
+++ b/src/clipboard.c
@@ -61,6 +61,7 @@ clipboard_source_unref(struct clipboard_source *source)
 
if (source->event_source) {
wl_event_source_remove(source->event_source);
+   source->event_source = NULL;
close(source->fd);
}
wl_signal_emit(&source->base.destroy_signal,
@@ -90,8 +91,8 @@ clipboard_source_data(int fd, uint32_t mask, void *data)
len = read(fd, p, size);
if (len == 0) {
wl_event_source_remove(source->event_source);
-   close(fd);
source->event_source = NULL;
+   close(fd);
} else if (len < 0) {
clipboard_source_unref(source);
clipboard->source = NULL;
diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index 5f59789..0110f41 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -2379,7 +2379,9 @@ drm_destroy(struct weston_compositor *ec)
udev_input_destroy(&d->input);
 
wl_event_source_remove(d->udev_drm_source);
+   d->udev_drm_source = NULL;
wl_event_source_remove(d->drm_source);
+   d->drm_source = NULL;
 
destroy_sprites(d);
 
@@ -2849,6 +2851,7 @@ drm_compositor_create(struct wl_display *display,
 
 err_udev_monitor:
wl_event_source_remove(ec->udev_drm_source);
+   ec->udev_drm_source = NULL;
udev_monitor_unref(ec->udev_monitor);
 err_drm_source:
wl_event_source_remove(ec->drm_source);
diff --git a/src/compositor-rpi.c b/src/compositor-rpi.c
index 287451d..cbfb770 100644
--- a/src/compositor-rpi.c
+++ b/src/compositor-rpi.c
@@ -213,6 +213,7 @@ static void
 rpi_flippipe_release(struct rpi_flippipe *flippipe)
 {
wl_event_source_remove(flippipe->source);
+   flippipe->source = NULL;
close(flippipe->readfd);
close(flippipe->writefd);
 }
diff --git a/src/compositor-x11.c b/src/compositor-x11.c
index 56b3228..9f171e7 100644
--- a/src/compositor-x11.c
+++ b/src/compositor-x11.c
@@ -485,6 +485,7 @@ x11_output_destroy(struct weston_output *output_base)
(struct x11_compositor *)output->base.compositor;
 
wl_event_source_remove(output->finish_frame_timer);
+   output->finish_frame_timer = NULL;
 
if (compositor->use_pixman) {
pixman_renderer_output_destroy(output_base);
@@ -1424,6 +1425,7 @@ x11_destroy(struct weston_compositor *ec)
struct x11_compositor *compositor = (struct x11_compositor *)ec;
 
wl_event_source_remove(compositor->xcb_source);
+   compositor->xcb_source = NULL;
x11_input_destroy(compositor);
 
weston_compositor_shutdown(ec); /* destroys outputs, too */
diff --git a/src/compositor.c b/src/compositor.c
index cd1ca9a..6ad3387 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -3736,8 +3736,12 @@ weston_compositor_shutdown(struct weston_compositor *ec)
struct weston_output *output, *next;
 
wl_event_source_remove(ec->idle_source);
-   if (ec->input_loop_source)
+   ec->idle_source = NULL;
+
+   if (ec->input_loop_source) {
wl_event_source_remove(ec->input_loop_source);
+   ec->input_loop_source = NULL;
+   }
 
/* Destroy all outputs associated with this compositor */
wl_list_for_each_safe(output, next, &ec->output_list, link)
diff --git a/src/evdev-touchpad.c b/src/evdev-touchpad.c
index 69f913a..360f87f 100644
--- a/src/evdev-touchpad.c
+++ b/src/evdev-touchpad.c
@@ -663,6 +663,7 @@ touchpad_destroy(struct evdev_dispatch *dispatch)
 
touchpad->filter->interface->destroy(touchpad->filter);
wl_event_source_remove(touchpad->fsm.timer_source);
+   touchpad->fsm.timer_source = NULL;
free(dispatch);
 }
 
diff --git a/src/evdev.c b/src/evdev.c
index 888dfbd..a1bce2c 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -697,8 +697,10 @@ evdev_device_destroy(struct evdev_device *device)
if (dispatch)
dispatch->interface->destroy(dispatch);
 
-   if (device->source)
+   if (device->source) {
wl_event_source_remove(device->source);
+   device->source = NULL;
+   }
if (device->output)
wl_list_remove(&device->output_destroy_listener.link);
wl_list_remove(&device->link);
diff --git a/src/libinput-seat.c b/src/libinput-seat.c
index a38d470..34f1aab 100644
--

[PATCH 2/2] Handle OOM with signal events

2014-05-09 Thread Hardening

This patch handles the case where a signal event source can not be created.
---
 src/compositor.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/compositor.c b/src/compositor.c
index 6ad3387..047df8a 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -4197,6 +4197,7 @@ int main(int argc, char *argv[])
display = wl_display_create();
 
loop = wl_display_get_event_loop(display);
+   memset(signals, 0, sizeof(signals));
signals[0] = wl_event_loop_add_signal(loop, SIGTERM, on_term_signal,
  display);
signals[1] = wl_event_loop_add_signal(loop, SIGINT, on_term_signal,
@@ -4208,6 +4209,9 @@ int main(int argc, char *argv[])
signals[3] = wl_event_loop_add_signal(loop, SIGCHLD, sigchld_handler,
  NULL);
 
+   if (!signals[0] || !signals[1] || !signals[2] || !signals[3])
+   goto out_signals;
+
config = weston_config_parse("weston.ini");
if (config != NULL) {
weston_log("Using config file '%s'\n",
@@ -4321,8 +4325,11 @@ int main(int argc, char *argv[])
 
wl_signal_emit(&ec->destroy_signal, ec);
 
-   for (i = ARRAY_LENGTH(signals); i;)
-   wl_event_source_remove(signals[--i]);
+ out_signals:
+   for (i = ARRAY_LENGTH(signals); i; i--) {
+   if (signals[i-1])
+   wl_event_source_remove(signals[i-1]);
+   }
 
weston_compositor_xkb_destroy(ec);
 
-- 
1.8.1.2

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

Re: [PATCH] Destroy resources when destroying input and output

2014-05-09 Thread Jason Ekstrand

On Fri, May 9, 2014 at 1:37 AM, Pekka Paalanen  wrote:
> Looking at it in general, there is one more fun complication.
>
> If the inert object has requests in its interface, that create new
> objects, the server cannot just ignore those requests. I think the
> server will need to actually create new objects if requested, but
> make those inert too. Otherwise there would again be a mismatch
> between the server and the client on which objects exist.

I was hoping that this wasn't actually going to be a problem, but on
wl_seat it is.  The server can destroy a seat while a client is calling
wl_seat.get_pointer for instance.  Then, yes, we do have to create an
intert object.

On Fri, May 9, 2014 at 9:02 AM, Pekka Paalanen  wrote:

> On Fri, 09 May 2014 14:33:58 +0100
> Neil Roberts  wrote:
>
> > Perhaps we should consider applying the patch anyway even though it's
> > not ideal. Currently if a client uses a dead output in a request such as
> > xdg_surface.set_output Weston will end up with a weston_output pointer
> > that points to freed memory. This could cause the compositor to crash.
>
> True, it's very bad now.
>
> > That is worse than the alternative provided by this patch which is to
> > make the client abort. The clients know about the output being destroyed
> > via the wl_registry.global_remove event so in practice they would only
> > hit the problem in the unlikely event that they used the output in a
> > request in the short time between the output being unplugged and
> > noticing the removal event.
>
> That is also true, but if it is fixed improperly now, there is a very
> good chance we just forget about the problem, and never fix it for
> real. Especially when it becomes very hard to trigger.
>
> At least make sure we have an open bug report about it, please.
>

I agree.  This is bad and we need to make sure it gets fixed properly.

>
> > In the longer term I was thinking maybe it would be good to handle the
> > inert resource idea within libwayland-server. We could add a function
> > like wl_resource_zombify() which would mark the resource as a zombie and
> > call the destroy handlers. From the compositor's perspective it can then
> > act as if the resource has been destroyed. We could detect zombie
> > resources being used within the request marshalling code and ignore the
> > request. If the request creates new resource we can internally create
> > new zombie resources too and Weston would never need to know about it. I
> > am planning to experiment with this approach now.
>
> Hmm... will be interesting to see, if that works out. It does feel like
> quite a lot of magic in libwayland-server, while also making life a lot
> easier.
>

Most of the magic there is in allowing resources with no handler in
libwayland-server.  The patch would be about 4 lines.  Right now,
client-side wl_proxy objects are allowed to have a NULL implementation and
there's no problem;  server-side, this is not currently allowed.  If we
allowed this ten Neil's wl_resource_zombify would only have to set the
destructor, and implementation to NULL.  For that matter, we could just do
that explicitly in weston and not add API for it.

The big chunk of magic is in handling new_id requests.  We would need to
create zombie wl_resource's for each new_id argument on a call to a
zombified wl_resource.  Now that Pekka mentioned it, I'm not sure that
we're handling those correctly client-side if there's no implementation set.

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

Re: [PATCH 2/2] compositor-wayland: Add touch support

2014-05-09 Thread Jason Ekstrand

On Fri, May 9, 2014 at 1:09 AM, Boyan Ding  wrote:

> At 2014-05-08 09:55:23, "Jason Ekstrand"  wrote:
>
> Boyan,
> By and large, this looks really good!  I have just a few comments below.
> As I said in another e-mail, I don't have any touch hardware I can test
> this on, so I wasn't able to actually test it.
>
>
> Unfortunately, I don't have touch hardware either. However I found a
> interesting project [1] on github, though I'm still figuring out how to use
> it.
>

I'll be curious to see how well that works.  I don't have a touch screen
either and it would be nice to be able to test things anyway.


>
> On Tue, May 6, 2014 at 9:46 PM, Boyan Ding  wrote:
>
>> Adding touch support to weston's nested wayland backend to make testing
>> easier.
>>
>> https://bugs.freedesktop.org/show_bug.cgi?id=77769
>> ---
>>  src/compositor-wayland.c | 95
>> 
>>  1 file changed, 95 insertions(+)
>>
>> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
>> index a08b71a..45040d4 100644
>> --- a/src/compositor-wayland.c
>> +++ b/src/compositor-wayland.c
>> @@ -1582,6 +1582,91 @@ static const struct wl_keyboard_listener
>> keyboard_listener = {
>>  };
>>
>>  static void
>> +input_handle_touch_down(void *data, struct wl_touch *touch, uint32_t
>> serial,
>> +   uint32_t time, struct wl_surface *surface,
>> int32_t id,
>> +   wl_fixed_t x, wl_fixed_t y)
>> +{
>> +   struct wayland_input *input = data;
>> +   int32_t fx, fy;
>> +
>> +   if (input->output->frame) {
>> +   frame_touch_down(input->output->frame, input, id,
>> +wl_fixed_to_int(x), wl_fixed_to_int(y));
>> +   frame_interior(input->output->frame, &fx, &fy, NULL,
>> NULL);
>> +   x -= wl_fixed_from_int(fx);
>> +   y -= wl_fixed_from_int(fy);
>>
>
> You probably want to handle FRAME_STATUS_MOVE here.  See also
> input_handle_button.
>
>
> I saw the code handling FRAME_STATUS_MOVE in input_handle_button returned
> after handling that. Do we need to return here?
> Also, I saw the pointer code notify the event only if the location is 
> THEME_LOCATION_CLIENT_AREA.
> Do we need to do the same thing? (I noticed the change of return type of
> frame_touch_down from void to theme_location in [2] but that patch hasn't
> been applied.
>

Actually, the return probably shouldn't be there in the pointer code.  None
of the frame states preempt any of the others.  Regarding notfying, that's
a more delicate question.  We probably want to do one of two things: a)
only call weston_touc_notify if the first touch point went down inside the
client area, otherwise only pass touch points to the frame or b) track
which touch points went down inside the client area and only notify for
those.  I think a) would be the simplest to implement and would probably
provide the best user experience.


>
>
>> +
>> +   if (frame_status(input->output->frame) &
>> FRAME_STATUS_REPAINT)
>> +
>> weston_output_schedule_repaint(&input->output->base);
>> +   }
>> +
>> +   weston_output_transform_coordinate(&input->output->base, x, y,
>> &x, &y);
>> +
>> +   notify_touch(&input->base, time, id, x, y, WL_TOUCH_DOWN);
>> +}
>> +
>> +static void
>> +input_handle_touch_up(void *data, struct wl_touch *touch, uint32_t
>> serial,
>> + uint32_t time, int32_t id)
>> +{
>> +   struct wayland_input *input = data;
>> +
>> +   if (input->output->frame) {
>> +   frame_touch_up(input->output->frame, input, id);
>> +
>>
>
> You need to handle FRAME_STATUS_CLOSE here.  See also input_handle_button.
>
> Perhaps, we want to add a wayland_output_handle_frame_status function to
> handle all of these so they can all be put in one place.  If we want to do
> that, then it should probably be in it's own patch.
>
>
>> +   if (frame_status(input->output->frame) &
>> FRAME_STATUS_REPAINT)
>> +
>> weston_output_schedule_repaint(&input->output->base);
>> +   }
>> +
>> +   notify_touch(&input->base, time, id, 0, 0, WL_TOUCH_UP);
>> +}
>> +
>> +static void
>> +input_handle_touch_motion(void *data, struct wl_touch *touch, uint32_t
>> time,
>> + int32_t id, wl_fixed_t x, wl_fixed_t y)
>> +{
>> +   struct wayland_input *input = data;
>> +   int32_t fx, fy;
>> +
>> +   if (input->output->frame) {
>> +   frame_touch_motion(input->output->frame, input, id,
>> +wl_fixed_to_int(x), wl_fixed_to_int(y));
>> +
>> +   frame_interior(input->output->frame, &fx, &fy, NULL,
>> NULL);
>> +   x -= wl_fixed_from_int(fx);
>> +   y -= wl_fixed_from_int(fy);
>> +
>> +   if (frame_status(input->output->frame) &
>> FRAME_STATUS_REPAINT)
>> +
>> weston_output_schedule_repaint(&input->output->base);
>> +   }
>> +
>> +   weston_output_transform_coordinate(&input->out

Re: [PATCH] Destroy resources when destroying input and output

2014-05-09 Thread Neil Roberts

Jason Ekstrand  writes:

> Most of the magic there is in allowing resources with no handler in
> libwayland-server. The patch would be about 4 lines. Right now,
> client-side wl_proxy objects are allowed to have a NULL implementation
> and there's no problem; server-side, this is not currently allowed. If
> we allowed this ten Neil's wl_resource_zombify would only have to set
> the destructor, and implementation to NULL. For that matter, we could
> just do that explicitly in weston and not add API for it.

Don't forget that we also want to ignore requests that have zombie
resources as arguments, not just if the resource is directly used as a
target of a request.

It looks like the client-side already has code to handle zombie objects
by putting a marker called WL_ZOMBIE_OBJECT in the ID map. Perhaps we
should use the same mechanism on the server side too.

If a zombie object is received in an event on the client side it looks
like wl_closure_lookup_objects just sets the proxy object to NULL. Is
that safe? Wouldn't the event handlers crash if they get a NULL resource
in an event?

If we can somehow make wl_closure_lookup_objects report when it finds a
zombie object we can make the upper layers just consume the event. We
could do this on both the client and side and the server side.

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

[PATCH wayland 3/6 v2] protocol: Fix order of wl_pointer, wl_keyboard and wl_touch messages

2014-05-09 Thread Jonas Ådahl

The "release" message of wl_pointer, wl_keyboard and wl_touch introduced
in version 3 was placed first in the respective interface XML element,
causing wayland-scanner to misbehave and set the version number of the
"release" message to all subsequent messages with no explicitly specified
"since" version.

Signed-off-by: Jonas Ådahl 
---

Fixed typo in commit subject and changed the wording of the commit
message a bit. The content of the patch is identical to v1.

Jonas


 protocol/wayland.xml | 31 +++
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/protocol/wayland.xml b/protocol/wayland.xml
index 330f8ab..22eb6e7 100644
--- a/protocol/wayland.xml
+++ b/protocol/wayland.xml
@@ -1384,10 +1384,6 @@
   
 
 
-
-  
-
-
 
   
Notification that this seat's pointer is focused on a certain
@@ -1485,6 +1481,13 @@
   
   
 
+
+
+
+
+  
+
+
   
 
   
@@ -1493,10 +1496,6 @@
   associated with a seat.
 
 
-
-  
-
-
 
   
This specifies the format of the keymap provided to the
@@ -1573,6 +1572,12 @@
   
   
 
+
+
+
+
+  
+
   
 
   
@@ -1587,10 +1592,6 @@
   contact point can be identified by the ID of the sequence.
 
 
-
-  
-
-
 
   
A new touch point has appeared on the surface. This touch point is
@@ -1643,6 +1644,12 @@
this surface may re-use the touch point ID.
   
 
+
+
+
+
+  
+
   
 
   
-- 
1.9.1

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

Re: Bug 78372 - create multiple windows with offset

2014-05-09 Thread Bill Spitzak


On 05/09/2014 12:34 AM, Pekka Paalanen wrote:


Possibly you are reading the words "save/restore" literally, in that you
are imagining some blob of data stored in the compositor that is
recognized to restore the layout. However this is NOT what is wanted.


Sure, that's the first thing comes to my mind. Let the client save a
cookie or an encrypted blob, that it can pass back to the server when
it creates the windows again. /handwaving/


That is what I was worried about. This will not work, as we need layouts 
that work the first time the client is run, that can be used by 
different compositors, and can port to non-Wayland systems.



The client has no
clue whatsoever on e.g. what positions on an output might be ok, what
else is on the screen, or which outputs would be ok to conquer. You
also might be on a tiling compositor, where your expectations simply
totally fail.

Your program would have to be a mind-reader to get the initial layout
right for any user.


I have no problem with the compositor moving the windows from the 
client's expected positions. It is a *HINT*. Clients can probably assume 
that if they reuse the positions from a previous run on the same 
compositor with the same outputs, they will get the same positions. I 
think also they can assume that if a window has a smaller x value than 
another it will be further to the left. That is all I am trying to get.


I think an x/y position in some global space will fulfill these 
requirements and anything more complex is unnecessary and confusing, as 
well as making it impossible to port these positions to other operating 
systems.


I think it is acceptable that the actual client api is "output+x+y" 
along with another api that says "the x+y of the corner of each output 
is this". Then the client can do the conversion from "global" x/y to 
per-output x/y.

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

Re: [PATCH] desktop-shell: Fix black edges on scaled desktop pattern

2014-05-09 Thread Bill Spitzak




On 05/09/2014 02:11 AM, Pekka Paalanen wrote:

On Thu, 08 May 2014 20:00:35 -0700
Bill Spitzak  wrote:


Filter sampling outside the source image can leak black into the edges
of the
desktop image. This is most easily seen by scaling the default tiled image
with this weston.ini:

# no background-image and no background-color
background-type=scale-crop
---
   clients/desktop-shell.c |2 ++
   1 file changed, 2 insertions(+)

diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
index 4880888..e121cc7 100644
--- a/clients/desktop-shell.c
+++ b/clients/desktop-shell.c
@@ -724,6 +724,7 @@ background_draw(struct widget *widget, void *data)
case BACKGROUND_SCALE:
cairo_matrix_init_scale(&matrix, sx, sy);
cairo_pattern_set_matrix(pattern, &matrix);
+   cairo_pattern_set_extend(pattern, CAIRO_EXTEND_PAD);
break;
case BACKGROUND_SCALE_CROP:
s = (sx < sy) ? sx : sy;
@@ -733,6 +734,7 @@ background_draw(struct widget *widget, void *data)
cairo_matrix_init_translate(&matrix, tx, ty);
cairo_matrix_scale(&matrix, s, s);
cairo_pattern_set_matrix(pattern, &matrix);
+   cairo_pattern_set_extend(pattern, CAIRO_EXTEND_PAD);
break;
case BACKGROUND_TILE:
cairo_pattern_set_extend(pattern, CAIRO_EXTEND_REPEAT);


Ok, I see the problem, and I see this fix should be good, but for some
reason this patch does not apply at all. I had to do the change
manually. Anyway, it works right and fixes the issue.

Hm, it does apply with --ignore-whitespace, though. When I save the raw
email, there seems to be extra spaces in the beginning of all context
lines. Maybe Format="flowed" messes it up?


git send-email did not work (as it wants a mta set up) so I tried to do 
it by hand. Fail.


If I set up a mta here at work it will look like the email came from 
work. That is not a problem for the mailing list, right? (assuming I 
change the from address to my gmail account).

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

[PATCH weston] libinput-seat: literal values for WESTON_LIBINPUT_LOG_PRIORITY

2014-05-09 Thread U. Artie Eoff

Only accept specific literal values from the environment variable
WESTON_LIBINPUT_LOG_PRIORITY... "debug", "info", or "error".

Signed-off-by: U. Artie Eoff 
---
 src/libinput-seat.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/libinput-seat.c b/src/libinput-seat.c
index a38d470..d59ae42 100644
--- a/src/libinput-seat.c
+++ b/src/libinput-seat.c
@@ -271,8 +271,15 @@ udev_input_init(struct udev_input *input, struct 
weston_compositor *c, struct ud
libinput_log_set_handler(&libinput_log_func, NULL);
 
log_priority = getenv("WESTON_LIBINPUT_LOG_PRIORITY");
+
if (log_priority) {
-   libinput_log_set_priority(strtol(log_priority, NULL, 10));
+   if (strcmp(log_priority, "debug") == 0) {
+   libinput_log_set_priority(LIBINPUT_LOG_PRIORITY_DEBUG);
+   } else if (strcmp(log_priority, "info") == 0) {
+   libinput_log_set_priority(LIBINPUT_LOG_PRIORITY_INFO);
+   } else if (strcmp(log_priority, "error") == 0) {
+   libinput_log_set_priority(LIBINPUT_LOG_PRIORITY_ERROR);
+   }
}
 
input->libinput = libinput_udev_create_for_seat(&libinput_interface, 
input,
-- 
1.9.0

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

Re: [PATCH] desktop-shell: Fix black edges on scaled desktop pattern

2014-05-09 Thread Jonas Ådahl

On Fri, May 09, 2014 at 11:13:54AM -0700, Bill Spitzak wrote:
> 
> 
> On 05/09/2014 02:11 AM, Pekka Paalanen wrote:
> >On Thu, 08 May 2014 20:00:35 -0700
> >Bill Spitzak  wrote:
> >
> >>Filter sampling outside the source image can leak black into the edges
> >>of the
> >>desktop image. This is most easily seen by scaling the default tiled image
> >>with this weston.ini:
> >>
> >># no background-image and no background-color
> >>background-type=scale-crop
> >>---
> >>   clients/desktop-shell.c |2 ++
> >>   1 file changed, 2 insertions(+)
> >>
> >>diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
> >>index 4880888..e121cc7 100644
> >>--- a/clients/desktop-shell.c
> >>+++ b/clients/desktop-shell.c
> >>@@ -724,6 +724,7 @@ background_draw(struct widget *widget, void *data)
> >>case BACKGROUND_SCALE:
> >>cairo_matrix_init_scale(&matrix, sx, sy);
> >>cairo_pattern_set_matrix(pattern, &matrix);
> >>+   cairo_pattern_set_extend(pattern, CAIRO_EXTEND_PAD);
> >>break;
> >>case BACKGROUND_SCALE_CROP:
> >>s = (sx < sy) ? sx : sy;
> >>@@ -733,6 +734,7 @@ background_draw(struct widget *widget, void *data)
> >>cairo_matrix_init_translate(&matrix, tx, ty);
> >>cairo_matrix_scale(&matrix, s, s);
> >>cairo_pattern_set_matrix(pattern, &matrix);
> >>+   cairo_pattern_set_extend(pattern, CAIRO_EXTEND_PAD);
> >>break;
> >>case BACKGROUND_TILE:
> >>cairo_pattern_set_extend(pattern, CAIRO_EXTEND_REPEAT);
> >
> >Ok, I see the problem, and I see this fix should be good, but for some
> >reason this patch does not apply at all. I had to do the change
> >manually. Anyway, it works right and fixes the issue.
> >
> >Hm, it does apply with --ignore-whitespace, though. When I save the raw
> >email, there seems to be extra spaces in the beginning of all context
> >lines. Maybe Format="flowed" messes it up?
> 
> git send-email did not work (as it wants a mta set up) so I tried to
> do it by hand. Fail.
> 
> If I set up a mta here at work it will look like the email came from
> work. That is not a problem for the mailing list, right? (assuming I
> change the from address to my gmail account).

If you are using gmail, you can just use Googles SMTP server directly.
The example configuration in the manual [0] even is a @gmail.com
address setup.

Jonas

[0] http://git-scm.com/docs/git-send-email

> ___
> 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] clients: Initialize label in keyboard handling code

2014-05-09 Thread Kristian Høgsberg

On Wed, May 07, 2014 at 01:11:07AM +, Bryce W. Harrington wrote:
> Quells warning:
>   clients/keyboard.c: In function ‘keyboard_handle_key.isra.5’:
>   clients/keyboard.c:556:11: warning: ‘label’ may be used uninitialized in
>   this function [-Wuninitialized]
> 
> Signed-off-by: Bryce Harrington 

Thanks Bryce, patch applied.

Kristian

> ---
>  clients/keyboard.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/clients/keyboard.c b/clients/keyboard.c
> index cd1ad58..7c11cec 100644
> --- a/clients/keyboard.c
> +++ b/clients/keyboard.c
> @@ -530,7 +530,7 @@ append(char *s1, const char *s2)
>  static void
>  keyboard_handle_key(struct keyboard *keyboard, uint32_t time, const struct 
> key *key, struct input *input, enum wl_pointer_button_state state)
>  {
> - const char *label;
> + const char *label = NULL;
>  
>   switch(keyboard->state) {
>   case KEYBOARD_STATE_DEFAULT :
> -- 
> 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

Re: [PATCH 3/5] clients: Use calloc instead of malloc/memset=0

2014-05-09 Thread Kristian Høgsberg

On Wed, May 07, 2014 at 02:13:11AM +, Bryce W. Harrington wrote:
> 
> Signed-off-by: Bryce Harrington 
> ---
>  clients/editor.c  |4 +---
>  clients/subsurfaces.c |8 ++--
>  clients/window.c  |   13 ++---
>  3 files changed, 5 insertions(+), 20 deletions(-)
> 
> diff --git a/clients/editor.c b/clients/editor.c
> index 6ed76d4..b439d9e 100644
> --- a/clients/editor.c
> +++ b/clients/editor.c
> @@ -559,9 +559,7 @@ text_entry_create(struct editor *editor, const char *text)
>  {
>   struct text_entry *entry;
>  
> - entry = xmalloc(sizeof *entry);
> - memset(entry, 0, sizeof *entry);
> -
> + entry = xcalloc(1, sizeof *entry);

We have zalloc for when we just want malloc+memset.

Kristian

>   entry->widget = widget_add_widget(editor->widget, entry);
>   entry->window = editor->window;
>   entry->text = strdup(text);
> diff --git a/clients/subsurfaces.c b/clients/subsurfaces.c
> index 15af9aa..a683787 100644
> --- a/clients/subsurfaces.c
> +++ b/clients/subsurfaces.c
> @@ -492,9 +492,7 @@ triangle_create(struct window *window, struct egl_state 
> *egl)
>  {
>   struct triangle *tri;
>  
> - tri = xmalloc(sizeof *tri);
> - memset(tri, 0, sizeof *tri);
> -
> + tri = xcalloc(1, sizeof *tri);
>   tri->egl = egl;
>   tri->widget = window_add_subsurface(window, tri,
>   int_to_mode(option_triangle_mode));
> @@ -714,9 +712,7 @@ demoapp_create(struct display *display)
>  {
>   struct demoapp *app;
>  
> - app = xmalloc(sizeof *app);
> - memset(app, 0, sizeof *app);
> -
> + app = xcalloc(1, sizeof *app);
>   app->egl = egl_state_create(display_get_display(display));
>  
>   app->display = display;
> diff --git a/clients/window.c b/clients/window.c
> index cfc1260..2212351 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -1139,12 +1139,7 @@ shm_surface_create(struct display *display, struct 
> wl_surface *wl_surface,
>   struct shm_surface *surface;
>   DBG_OBJ(wl_surface, "\n");
>  
> - surface = xmalloc(sizeof *surface);
> - memset(surface, 0, sizeof *surface);
> -
> - if (!surface)
> - return NULL;
> -
> + surface = xcalloc(1, sizeof *surface);
>   surface->base.prepare = shm_surface_prepare;
>   surface->base.swap = shm_surface_swap;
>   surface->base.acquire = shm_surface_acquire;
> @@ -4336,11 +4331,7 @@ surface_create(struct window *window)
>   struct display *display = window->display;
>   struct surface *surface;
>  
> - surface = xmalloc(sizeof *surface);
> - memset(surface, 0, sizeof *surface);
> - if (!surface)
> - return NULL;
> -
> + surface = xcalloc(1, sizeof *surface);
>   surface->window = window;
>   surface->surface = wl_compositor_create_surface(display->compositor);
>   surface->buffer_scale = 1;
> -- 
> 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

Re: [PATCH 4/5] clients: Use xzalloc instead of xcalloc when allocating single element

2014-05-09 Thread Kristian Høgsberg

On Wed, May 07, 2014 at 02:13:11AM +, Bryce W. Harrington wrote:
> 
> Signed-off-by: Bryce Harrington 
> ---
>  clients/desktop-shell.c |2 +-
>  clients/editor.c|2 +-
>  clients/fullscreen.c|2 +-
>  clients/subsurfaces.c   |6 +++---
>  clients/window.c|4 ++--
>  clients/wscreensaver.c  |2 +-
>  6 files changed, 9 insertions(+), 9 deletions(-)

Oh, didn't see this one when I replied to the earlier xcalloc patch.
I don't think we have a single place where we actually need xcalloc, so
I'd rather not add it at all.

Kristian

> diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
> index a3b2534..0b8d08b 100644
> --- a/clients/desktop-shell.c
> +++ b/clients/desktop-shell.c
> @@ -1217,7 +1217,7 @@ create_output(struct desktop *desktop, uint32_t id)
>  {
>   struct output *output;
>  
> - output = xcalloc(1, sizeof *output);
> + output = xzalloc(sizeof *output);
>   output->output =
>   display_bind(desktop->display, id, &wl_output_interface, 2);
>   output->server_output_id = id;
> diff --git a/clients/editor.c b/clients/editor.c
> index b439d9e..bda3e91 100644
> --- a/clients/editor.c
> +++ b/clients/editor.c
> @@ -559,7 +559,7 @@ text_entry_create(struct editor *editor, const char *text)
>  {
>   struct text_entry *entry;
>  
> - entry = xcalloc(1, sizeof *entry);
> + entry = xzalloc(sizeof *entry);
>   entry->widget = widget_add_widget(editor->widget, entry);
>   entry->window = editor->window;
>   entry->text = strdup(text);
> diff --git a/clients/fullscreen.c b/clients/fullscreen.c
> index ad7c703..8f41455 100644
> --- a/clients/fullscreen.c
> +++ b/clients/fullscreen.c
> @@ -480,7 +480,7 @@ output_handler(struct output *output, void *data)
>   if (fsout->output == output)
>   return;
>  
> - fsout = xcalloc(1, sizeof *fsout);
> + fsout = xzalloc(sizeof *fsout);
>   fsout->output = output;
>   wl_list_insert(&fullscreen->output_list, &fsout->link);
>  }
> diff --git a/clients/subsurfaces.c b/clients/subsurfaces.c
> index a683787..1ed698b 100644
> --- a/clients/subsurfaces.c
> +++ b/clients/subsurfaces.c
> @@ -212,7 +212,7 @@ egl_state_create(struct wl_display *display)
>   EGLint major, minor, n;
>   EGLBoolean ret;
>  
> - egl = xcalloc(1, sizeof *egl);
> + egl = xzalloc(sizeof *egl);
>   egl->dpy = eglGetDisplay(display);
>   assert(egl->dpy);
>  
> @@ -492,7 +492,7 @@ triangle_create(struct window *window, struct egl_state 
> *egl)
>  {
>   struct triangle *tri;
>  
> - tri = xcalloc(1, sizeof *tri);
> + tri = xzalloc(sizeof *tri);
>   tri->egl = egl;
>   tri->widget = window_add_subsurface(window, tri,
>   int_to_mode(option_triangle_mode));
> @@ -712,7 +712,7 @@ demoapp_create(struct display *display)
>  {
>   struct demoapp *app;
>  
> - app = xcalloc(1, sizeof *app);
> + app = xzalloc(sizeof *app);
>   app->egl = egl_state_create(display_get_display(display));
>  
>   app->display = display;
> diff --git a/clients/window.c b/clients/window.c
> index 2212351..a103530 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -1139,7 +1139,7 @@ shm_surface_create(struct display *display, struct 
> wl_surface *wl_surface,
>   struct shm_surface *surface;
>   DBG_OBJ(wl_surface, "\n");
>  
> - surface = xcalloc(1, sizeof *surface);
> + surface = xzalloc(sizeof *surface);
>   surface->base.prepare = shm_surface_prepare;
>   surface->base.swap = shm_surface_swap;
>   surface->base.acquire = shm_surface_acquire;
> @@ -4331,7 +4331,7 @@ surface_create(struct window *window)
>   struct display *display = window->display;
>   struct surface *surface;
>  
> - surface = xcalloc(1, sizeof *surface);
> + surface = xzalloc(sizeof *surface);
>   surface->window = window;
>   surface->surface = wl_compositor_create_surface(display->compositor);
>   surface->buffer_scale = 1;
> diff --git a/clients/wscreensaver.c b/clients/wscreensaver.c
> index 1070c07..d87d040 100644
> --- a/clients/wscreensaver.c
> +++ b/clients/wscreensaver.c
> @@ -175,7 +175,7 @@ create_wscreensaver_instance(struct wscreensaver 
> *screensaver,
>   struct ModeInfo *mi;
>   struct rectangle drawarea;
>  
> - mi = xcalloc(1, sizeof *mi);
> + mi = xzalloc(sizeof *mi);
>  
>   if (demo_mode)
>   mi->window = window_create(screensaver->display);
> -- 
> 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

Re: [PATCH 5/5] clients: Use xstrdup instead of strdup

2014-05-09 Thread Kristian Høgsberg

On Wed, May 07, 2014 at 02:13:11AM +, Bryce W. Harrington wrote:
> 
> Signed-off-by: Bryce Harrington 
> ---
>  clients/editor.c   |   12 ++--
>  clients/image.c|4 ++--
>  clients/keyboard.c |   12 ++--
>  clients/terminal.c |2 +-
>  4 files changed, 15 insertions(+), 15 deletions(-)
> 

This one looks good, but doesn't apply without the rest of the series.

Kristian

> diff --git a/clients/editor.c b/clients/editor.c
> index bda3e91..ece8b1d 100644
> --- a/clients/editor.c
> +++ b/clients/editor.c
> @@ -258,7 +258,7 @@ text_input_preedit_string(void *data,
>   }
>  
>   text_entry_set_preedit(entry, text, entry->preedit_info.cursor);
> - entry->preedit.commit = strdup(commit);
> + entry->preedit.commit = xstrdup(commit);
>   entry->preedit.attr_list = 
> pango_attr_list_ref(entry->preedit_info.attr_list);
>  
>   clear_pending_preedit(entry);
> @@ -562,7 +562,7 @@ text_entry_create(struct editor *editor, const char *text)
>   entry = xzalloc(sizeof *entry);
>   entry->widget = widget_add_widget(editor->widget, entry);
>   entry->window = editor->window;
> - entry->text = strdup(text);
> + entry->text = xstrdup(text);
>   entry->active = 0;
>   entry->cursor = strlen(text);
>   entry->anchor = entry->cursor;
> @@ -686,7 +686,7 @@ text_entry_update_layout(struct text_entry *entry)
>   strcpy(text + entry->cursor + strlen(entry->preedit.text),
>  entry->text + entry->cursor);
>   } else {
> - text = strdup(entry->text);
> + text = xstrdup(entry->text);
>   }
>  
>   if (entry->cursor != entry->anchor) {
> @@ -809,7 +809,7 @@ text_entry_commit_and_reset(struct text_entry *entry)
>   char *commit = NULL;
>  
>   if (entry->preedit.commit)
> - commit = strdup(entry->preedit.commit);
> + commit = xstrdup(entry->preedit.commit);
>  
>   text_entry_reset_preedit(entry);
>   if (commit) {
> @@ -832,7 +832,7 @@ text_entry_set_preedit(struct text_entry *entry,
>   if (!preedit_text)
>   return;
>  
> - entry->preedit.text = strdup(preedit_text);
> + entry->preedit.text = xstrdup(preedit_text);
>   entry->preedit.cursor = preedit_cursor;
>  
>   text_entry_update_layout(entry);
> @@ -1345,7 +1345,7 @@ main(int argc, char *argv[])
>   editor.entry = text_entry_create(&editor, "Entry");
>   editor.entry->click_to_show = click_to_show;
>   if (preferred_language)
> - editor.entry->preferred_language = strdup(preferred_language);
> + editor.entry->preferred_language = xstrdup(preferred_language);
>   editor.editor = text_entry_create(&editor, "Numeric");
>   editor.editor->content_purpose = WL_TEXT_INPUT_CONTENT_PURPOSE_NUMBER;
>   editor.editor->click_to_show = click_to_show;
> diff --git a/clients/image.c b/clients/image.c
> index cba68c5..b4a7bb8 100644
> --- a/clients/image.c
> +++ b/clients/image.c
> @@ -362,12 +362,12 @@ image_create(struct display *display, const char 
> *filename,
>  
>   image = xzalloc(sizeof *image);
>  
> - copy = strdup(filename);
> + copy = xstrdup(filename);
>   b = basename(copy);
>   snprintf(title, sizeof title, "Wayland Image - %s", b);
>   free(copy);
>  
> - image->filename = strdup(filename);
> + image->filename = xstrdup(filename);
>   image->image = load_cairo_surface(filename);
>  
>   if (!image->image) {
> diff --git a/clients/keyboard.c b/clients/keyboard.c
> index cd1ad58..6b1e7a0 100644
> --- a/clients/keyboard.c
> +++ b/clients/keyboard.c
> @@ -440,12 +440,12 @@ virtual_keyboard_commit_preedit(struct virtual_keyboard 
> *keyboard)
>   keyboard->surrounding_text = surrounding_text;
>   keyboard->surrounding_cursor += 
> strlen(keyboard->preedit_string);
>   } else {
> - keyboard->surrounding_text = strdup(keyboard->preedit_string);
> + keyboard->surrounding_text = xstrdup(keyboard->preedit_string);
>   keyboard->surrounding_cursor = strlen(keyboard->preedit_string);
>   }
>  
>   free(keyboard->preedit_string);
> - keyboard->preedit_string = strdup("");
> + keyboard->preedit_string = xstrdup("");
>  }
>  
>  static void
> @@ -757,7 +757,7 @@ handle_surrounding_text(void *data,
>   struct virtual_keyboard *keyboard = data;
>  
>   free(keyboard->surrounding_text);
> - keyboard->surrounding_text = strdup(text);
> + keyboard->surrounding_text = xstrdup(text);
>  
>   keyboard->surrounding_cursor = cursor;
>  }
> @@ -772,7 +772,7 @@ handle_reset(void *data,
>  
>   if (strlen(keyboard->preedit_string)) {
>   free(keyboard->preedit_string);
> - keyboard->preedit_string = strdup("");
> + keyboard->preedit_string = xstrdup("");
>   }
>  }
>  
> @@ -840,7 +840,7 @@ handle_preferred_language(void *data,
>   keyboard->preferred_la

Re: [PATCH v2] doc: Added API documentation for wl_display_create function.

2014-05-09 Thread Kristian Høgsberg

On Wed, May 07, 2014 at 09:37:45AM +0530, Srivardhan Hebbar wrote:
> Signed-off-by: Srivardhan Hebbar 
> ---
>  src/wayland-server.c |9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index f2b1b42..57b65ce 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -788,6 +788,15 @@ bind_display(struct wl_client *client,
>  destroy_client_display_resource);
>  }
>  
> +/** Create Wayland display object.
> + *
> + * \param None
> + * \return The Wayland display object. Null if failed to create
> + *
> + * This creates the wl_display object.
> + *
> + * \memberof wl_display
> + */

That's better thanks.  Patch applied.

Kristian

>  WL_EXPORT struct wl_display *
>  wl_display_create(void)
>  {
> -- 
> 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

Re: [PATCH weston] window.c: Set the input region of the tooltip to empty

2014-05-09 Thread Kristian Høgsberg

On Wed, May 07, 2014 at 11:47:08AM +0300, Pekka Paalanen wrote:
> On Tue, 6 May 2014 14:40:53 -0700
> Kristian Høgsberg  wrote:
> 
> > On Mon, May 05, 2014 at 05:02:15PM +0300, Ander Conselvan de Oliveira
> > wrote:
> > > From: Ander Conselvan de Oliveira
> > > 
> > > 
> > > Otherwise it might receive touch events.
> > 
> > I think a better approach is to just hide the tooltip if it (or the
> > panel) gets touch events.  I don't think I agree with Pekka that we
> > can't use subsurfaces for this, but I guess I'm not sure what the
> > problem he sees there is.
> 
> The panel is not a window, so sub-surfaces can be made to work there
> well enough.
> 
> It is for normal windows like registered with xdg_shell where
> sub-surfaces are not suitable for tooltips. A sub-surface is a part of
> the window, not another window. A tooltip is not expected to change the
> window geometry, but a sub-surface does count into the window geometry.
> Extruding sub-surfaces therefore affect the (parent) window geometry. It
> is in the protocol spec.

No, sure if no other geometry information is available.  For xdg-shell
windows, the window geometry overrides that though.  And I don't think
there's a clear distinction between being part of the window or being
a separate window here, and I don't see why it would useful...

Kristian

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

Re: [PATCH weston 3/3] shell: Fix crash when restoring focus state during workspace change

2014-05-09 Thread Kristian Høgsberg

On Wed, May 07, 2014 at 11:57:28AM +0300, Ander Conselvan de Oliveira wrote:
> From: Ander Conselvan de Oliveira 
> 
> The check to avoid calling weston_keyboard_set_focus() for a seat that
> didn't have a keyboard in restore_focus_state() was cheking the wrong
> seat (the one from the previous loop). That caused a crash when
> switching workspaces if there was an extra seat that didn't have a
> keyboard.
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=78349

Thanks Ander, all three committed.

Kristian

> ---
>  desktop-shell/shell.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index fac3120..ea7b3cd 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -731,7 +731,7 @@ restore_focus_state(struct desktop_shell *shell, struct 
> workspace *ws)
>   wl_list_for_each_safe(seat, next_seat, &pending_seat_list, link) {
>   wl_list_insert(&shell->compositor->seat_list, &seat->link);
>  
> - if (state->seat->keyboard == NULL)
> + if (seat->keyboard == NULL)
>   continue;
>  
>   weston_keyboard_set_focus(seat->keyboard, NULL);
> -- 
> 1.8.3.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 weston] shell: Don't allow maximized surfaces to be moved with touch

2014-05-09 Thread Kristian Høgsberg

On Wed, May 07, 2014 at 02:22:23PM +0300, Ander Conselvan de Oliveira wrote:
> From: Ander Conselvan de Oliveira 
> 
> Moving a maximized surface with the pointer is already not possible,
> so make the behavior with touch consistent.
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=78208

Thanks, committed.

Kristian

> ---
>  desktop-shell/shell.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index ea7b3cd..db55ea9 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -1453,7 +1453,7 @@ surface_touch_move(struct shell_surface *shsurf, struct 
> weston_seat *seat)
>   if (!shsurf)
>   return -1;
>  
> - if (shsurf->state.fullscreen)
> + if (shsurf->state.fullscreen || shsurf->state.maximized)
>   return 0;
>  
>   move = malloc(sizeof *move);
> -- 
> 1.8.3.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] desktop-shell: Fix black edges on scaled desktop pattern

2014-05-09 Thread Bill Spitzak

Filter sampling outside the source image can leak black into the edges of the
desktop image. This is most easily seen by scaling the default tiled image
with this weston.ini:

# no background-image and no background-color
background-type=scale-crop
---
 clients/desktop-shell.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
index 4880888..e121cc7 100644
--- a/clients/desktop-shell.c
+++ b/clients/desktop-shell.c
@@ -724,6 +724,7 @@ background_draw(struct widget *widget, void *data)
case BACKGROUND_SCALE:
cairo_matrix_init_scale(&matrix, sx, sy);
cairo_pattern_set_matrix(pattern, &matrix);
+   cairo_pattern_set_extend(pattern, CAIRO_EXTEND_PAD);
break;
case BACKGROUND_SCALE_CROP:
s = (sx < sy) ? sx : sy;
@@ -733,6 +734,7 @@ background_draw(struct widget *widget, void *data)
cairo_matrix_init_translate(&matrix, tx, ty);
cairo_matrix_scale(&matrix, s, s);
cairo_pattern_set_matrix(pattern, &matrix);
+   cairo_pattern_set_extend(pattern, CAIRO_EXTEND_PAD);
break;
case BACKGROUND_TILE:
cairo_pattern_set_extend(pattern, CAIRO_EXTEND_REPEAT);
-- 
1.7.9.5

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

Re: [PATCH] desktop-shell: Fix black edges on scaled desktop pattern

2014-05-09 Thread Bill Spitzak


Thanks, it looks like that setup worked, patch sent correctly now.

On 05/09/2014 11:52 AM, Jonas Ådahl wrote:

If you are using gmail, you can just use Googles SMTP server directly.
The example configuration in the manual [0] even is a @gmail.com
address setup.

Jonas

[0] http://git-scm.com/docs/git-send-email


___
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] desktop-shell: Fix black edges on scaled desktop pattern

2014-05-09 Thread Kristian Høgsberg

On Thu, May 08, 2014 at 08:00:35PM -0700, Bill Spitzak wrote:
> Filter sampling outside the source image can leak black into the edges of
> the
> desktop image. This is most easily seen by scaling the default tiled image
> with this weston.ini:
> 
>   # no background-image and no background-color
>   background-type=scale-crop

Thanks, that's a nice detail to get right.  Patch applied.

Kristian

> ---
>  clients/desktop-shell.c |2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
> index 4880888..e121cc7 100644
> --- a/clients/desktop-shell.c
> +++ b/clients/desktop-shell.c
> @@ -724,6 +724,7 @@ background_draw(struct widget *widget, void *data)
>   case BACKGROUND_SCALE:
>   cairo_matrix_init_scale(&matrix, sx, sy);
>   cairo_pattern_set_matrix(pattern, &matrix);
> + cairo_pattern_set_extend(pattern, CAIRO_EXTEND_PAD);
>   break;
>   case BACKGROUND_SCALE_CROP:
>   s = (sx < sy) ? sx : sy;
> @@ -733,6 +734,7 @@ background_draw(struct widget *widget, void *data)
>   cairo_matrix_init_translate(&matrix, tx, ty);
>   cairo_matrix_scale(&matrix, s, s);
>   cairo_pattern_set_matrix(pattern, &matrix);
> + cairo_pattern_set_extend(pattern, CAIRO_EXTEND_PAD);
>   break;
>   case BACKGROUND_TILE:
>   cairo_pattern_set_extend(pattern, CAIRO_EXTEND_REPEAT);
> -- 
> 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

Re: [PATCH] event: Cheking for NULL before dereferencing the pointer.

2014-05-09 Thread Kristian Høgsberg

On Fri, May 09, 2014 at 02:56:14PM +0530, Srivardhan wrote:
> 
> 
> > -Original Message-
> > From: wayland-devel [mailto:wayland-devel-
> > boun...@lists.freedesktop.org] On Behalf Of Hardening
> > Sent: Friday, May 09, 2014 1:08 PM
> > To: wayland-devel@lists.freedesktop.org
> > Subject: Re: [PATCH] event: Cheking for NULL before dereferencing the
> > pointer.
> > 
> > Le 09/05/2014 08:43, Srivardhan Hebbar a écrit :
> > > Checking for NULL before dereferencing the wl_event_source pointer so
> > > as to avoid crash.
> > >
> > > Signed-off-by: Srivardhan Hebbar 
> > > ---
> > >   src/event-loop.c |7 ++-
> > >   1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/event-loop.c b/src/event-loop.c index
> > > 9790cde..b62d16e 100644
> > > --- a/src/event-loop.c
> > > +++ b/src/event-loop.c
> > > @@ -312,7 +312,12 @@ wl_event_source_check(struct wl_event_source
> > *source)
> > >   WL_EXPORT int
> > >   wl_event_source_remove(struct wl_event_source *source)
> > >   {
> > > - struct wl_event_loop *loop = source->loop;
> > > + struct wl_event_loop *loop;
> > > +
> > > + if (source == NULL)
> > > + return 0;
> > > +
> > > + loop = source->loop;
> > >
> > >   /* We need to explicitly remove the fd, since closing the fd
> > >* isn't enough in case we've dup'ed the fd. */
> > >
> > 
> > Hello Srivardhan,
> > 
> > do you have a case where this check is hit ? I may be wrong but having no
> > loop associated with a source event is not supposed to happen. So my guess
> > is that a caller of wl_event_source_remove has forgotten to nullify the
> event
> > source after the call.
> 
> Hi,
> 
> I was going through the code of Weston. In the main function in compositor.c
> wl_event_loop_add_signal() is called to allocate the memory for
> signals[](Line no: 4196. struct wl_event_source *signals[4]). The function
> returns NULL if memory allocation failed. After that there is no NULL check
> for 'signals'. In the end while clearing up, this function is called. So if
> memory allocation failed then a NULL is passed to this function. Hence
> adding code to check for the NULL.

You caught a problem here, but the issue is that we don't check for NULL
where we allocate the source.  Passing a NULL pointer to
wl_event_source_remove() is a programmer error and we don't want to
silently fail.

Kristian


> Thank-you,
> Hebbar
> > 
> > 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
> 
> ___
> 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 weston] rpi: build fix for compute_rects debug

2014-05-09 Thread Kristian Høgsberg

On Fri, May 09, 2014 at 03:08:06PM +0300, Pekka Paalanen wrote:
> From: Pekka Paalanen 
> 
> See 918f2dd4cfb8b177f67b45653efbbe4325cbe9dc

Thanks Pekka, applied.

Kristian

> Signed-off-by: Pekka Paalanen 
> ---
>  src/rpi-renderer.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/rpi-renderer.c b/src/rpi-renderer.c
> index 3a7f65c..c222eb6 100644
> --- a/src/rpi-renderer.c
> +++ b/src/rpi-renderer.c
> @@ -858,8 +858,8 @@ rpir_view_compute_rects(struct rpir_view *view,
>   src_height = int_max(src_height, 0);
>  
>   DBG("rpir_view %p %dx%d: p1 %f, %f; p2 %f, %f\n", view,
> - view->view->geometry.width,
> - view->view->geometry.height,
> + view->view->surface->width,
> + view->view->surface->height,
>   p1.f[0], p1.f[1], p2.f[0], p2.f[1]);
>   DBG("src rect %d;%d, %d;%d, %d;%dx%d;%d\n",
>   src_x >> 16, src_x & 0x,
> -- 
> 1.8.5.5
> 
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] vaapi-recorder: Don't loop trying to write on out of space condition

2014-05-09 Thread Kristian Høgsberg

On Fri, May 09, 2014 at 03:57:38PM +0300, Ander Conselvan de Oliveira wrote:
> From: Ander Conselvan de Oliveira 
> 
> The error handling for the function that writes the encoded frame on
> the disk was bogus, always assuming the buffer supplied to the encoder
> was too small. That would cause a bigger buffer to be allocated and
> another attempt to encode the frame was done. In the case of a failure
> to write to disk (due to ENOSPC, for instance) that would cause an
> endless loop.

Thanks committed.  I added the bugzilla link to the commit message for
reference.

Kristian

> ---
> Possibly related to
> https://bugs.freedesktop.org/show_bug.cgi?id=69330
> ---
>  src/compositor-drm.c | 27 +++
>  src/vaapi-recorder.c | 42 +-
>  src/vaapi-recorder.h |  2 +-
>  3 files changed, 53 insertions(+), 18 deletions(-)
> 
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index 5f59789..7d514e4 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -2558,6 +2558,18 @@ planes_binding(struct weston_seat *seat, uint32_t 
> time, uint32_t key, void *data
>  
>  #ifdef BUILD_VAAPI_RECORDER
>  static void
> +recorder_destroy(struct drm_output *output)
> +{
> + vaapi_recorder_destroy(output->recorder);
> + output->recorder = NULL;
> +
> + output->base.disable_planes--;
> +
> + wl_list_remove(&output->recorder_frame_listener.link);
> + weston_log("[libva recorder] done\n");
> +}
> +
> +static void
>  recorder_frame_notify(struct wl_listener *listener, void *data)
>  {
>   struct drm_output *output;
> @@ -2579,7 +2591,12 @@ recorder_frame_notify(struct wl_listener *listener, 
> void *data)
>   return;
>   }
>  
> - vaapi_recorder_frame(output->recorder, fd, output->current->stride);
> + ret = vaapi_recorder_frame(output->recorder, fd,
> +output->current->stride);
> + if (ret < 0) {
> + weston_log("[libva recorder] aborted: %m\n");
> + recorder_destroy(output);
> + }
>  }
>  
>  static void *
> @@ -2637,13 +2654,7 @@ recorder_binding(struct weston_seat *seat, uint32_t 
> time, uint32_t key,
>  
>   weston_log("[libva recorder] initialized\n");
>   } else {
> - vaapi_recorder_destroy(output->recorder);
> - output->recorder = NULL;
> -
> - output->base.disable_planes--;
> -
> - wl_list_remove(&output->recorder_frame_listener.link);
> - weston_log("[libva recorder] done\n");
> + recorder_destroy(output);
>   }
>  }
>  #else
> diff --git a/src/vaapi-recorder.c b/src/vaapi-recorder.c
> index 0095a42..921494d 100644
> --- a/src/vaapi-recorder.c
> +++ b/src/vaapi-recorder.c
> @@ -50,6 +50,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -93,6 +94,7 @@ struct vaapi_recorder {
>   int width, height;
>   int frame_count;
>  
> + int error;
>   int destroying;
>   pthread_t worker_thread;
>   pthread_mutex_t mutex;
> @@ -761,7 +763,13 @@ encoder_create_output_buffer(struct vaapi_recorder *r)
>   return VA_INVALID_ID;
>  }
>  
> -static int
> +enum output_write_status {
> + OUTPUT_WRITE_SUCCESS,
> + OUTPUT_WRITE_OVERFLOW,
> + OUTPUT_WRITE_FATAL
> +};
> +
> +static enum output_write_status
>  encoder_write_output(struct vaapi_recorder *r, VABufferID output_buf)
>  {
>   VACodedBufferSegment *segment;
> @@ -770,19 +778,22 @@ encoder_write_output(struct vaapi_recorder *r, 
> VABufferID output_buf)
>  
>   status = vaMapBuffer(r->va_dpy, output_buf, (void **) &segment);
>   if (status != VA_STATUS_SUCCESS)
> - return -1;
> + return OUTPUT_WRITE_FATAL;
>  
>   if (segment->status & VA_CODED_BUF_STATUS_SLICE_OVERFLOW_MASK) {
>   r->encoder.output_size *= 2;
>   vaUnmapBuffer(r->va_dpy, output_buf);
> - return -1;
> + return OUTPUT_WRITE_OVERFLOW;
>   }
>  
>   count = write(r->output_fd, segment->buf, segment->size);
>  
>   vaUnmapBuffer(r->va_dpy, output_buf);
>  
> - return count;
> + if (count < 0)
> + return OUTPUT_WRITE_FATAL;
> +
> + return OUTPUT_WRITE_SUCCESS;
>  }
>  
>  static void
> @@ -792,9 +803,8 @@ encoder_encode(struct vaapi_recorder *r, VASurfaceID 
> input)
>  
>   VABufferID buffers[8];
>   int count = 0;
> -
> - int slice_type;
> - int ret, i;
> + int i, slice_type;
> + enum output_write_status ret;
>  
>   if ((r->frame_count % r->encoder.intra_period) == 0)
>   slice_type = SLICE_TYPE_I;
> @@ -829,7 +839,10 @@ encoder_encode(struct vaapi_recorder *r, VASurfaceID 
> input)
>   output_buf = VA_INVALID_ID;
>  
>   vaDestroyBuffer(r->va_dpy, buffers[--count]);
> - } while (ret < 0);
> + } while (ret == OUTPUT_WRITE_OVERFLOW);
> +
> + if (ret == OUTPUT_

Re: [PATCH 1/2] Set to NULL event source after a call to wl_event_source_remove

2014-05-09 Thread Kristian Høgsberg

On Fri, May 09, 2014 at 04:03:51PM +0200, Hardening wrote:
> This patch sets to NULL event sources after a call to wl_event_source_remove()
> has been made.

We don't generally set freed memory to NULL, unless we rely on testing that
to decide whether the pointer points to an object or not.

Kristian

> ---
>  src/clipboard.c  | 3 ++-
>  src/compositor-drm.c | 3 +++
>  src/compositor-rpi.c | 1 +
>  src/compositor-x11.c | 2 ++
>  src/compositor.c | 6 +-
>  src/evdev-touchpad.c | 1 +
>  src/evdev.c  | 4 +++-
>  src/libinput-seat.c  | 1 +
>  src/logind-util.c| 2 ++
>  xwayland/launcher.c  | 6 +-
>  xwayland/selection.c | 9 +++--
>  11 files changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/src/clipboard.c b/src/clipboard.c
> index 5a3a02d..0e25dc1 100644
> --- a/src/clipboard.c
> +++ b/src/clipboard.c
> @@ -61,6 +61,7 @@ clipboard_source_unref(struct clipboard_source *source)
>  
>   if (source->event_source) {
>   wl_event_source_remove(source->event_source);
> + source->event_source = NULL;
>   close(source->fd);
>   }
>   wl_signal_emit(&source->base.destroy_signal,
> @@ -90,8 +91,8 @@ clipboard_source_data(int fd, uint32_t mask, void *data)
>   len = read(fd, p, size);
>   if (len == 0) {
>   wl_event_source_remove(source->event_source);
> - close(fd);
>   source->event_source = NULL;
> + close(fd);
>   } else if (len < 0) {
>   clipboard_source_unref(source);
>   clipboard->source = NULL;
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index 5f59789..0110f41 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -2379,7 +2379,9 @@ drm_destroy(struct weston_compositor *ec)
>   udev_input_destroy(&d->input);
>  
>   wl_event_source_remove(d->udev_drm_source);
> + d->udev_drm_source = NULL;
>   wl_event_source_remove(d->drm_source);
> + d->drm_source = NULL;
>  
>   destroy_sprites(d);
>  
> @@ -2849,6 +2851,7 @@ drm_compositor_create(struct wl_display *display,
>  
>  err_udev_monitor:
>   wl_event_source_remove(ec->udev_drm_source);
> + ec->udev_drm_source = NULL;
>   udev_monitor_unref(ec->udev_monitor);
>  err_drm_source:
>   wl_event_source_remove(ec->drm_source);
> diff --git a/src/compositor-rpi.c b/src/compositor-rpi.c
> index 287451d..cbfb770 100644
> --- a/src/compositor-rpi.c
> +++ b/src/compositor-rpi.c
> @@ -213,6 +213,7 @@ static void
>  rpi_flippipe_release(struct rpi_flippipe *flippipe)
>  {
>   wl_event_source_remove(flippipe->source);
> + flippipe->source = NULL;
>   close(flippipe->readfd);
>   close(flippipe->writefd);
>  }
> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
> index 56b3228..9f171e7 100644
> --- a/src/compositor-x11.c
> +++ b/src/compositor-x11.c
> @@ -485,6 +485,7 @@ x11_output_destroy(struct weston_output *output_base)
>   (struct x11_compositor *)output->base.compositor;
>  
>   wl_event_source_remove(output->finish_frame_timer);
> + output->finish_frame_timer = NULL;
>  
>   if (compositor->use_pixman) {
>   pixman_renderer_output_destroy(output_base);
> @@ -1424,6 +1425,7 @@ x11_destroy(struct weston_compositor *ec)
>   struct x11_compositor *compositor = (struct x11_compositor *)ec;
>  
>   wl_event_source_remove(compositor->xcb_source);
> + compositor->xcb_source = NULL;
>   x11_input_destroy(compositor);
>  
>   weston_compositor_shutdown(ec); /* destroys outputs, too */
> diff --git a/src/compositor.c b/src/compositor.c
> index cd1ca9a..6ad3387 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -3736,8 +3736,12 @@ weston_compositor_shutdown(struct weston_compositor 
> *ec)
>   struct weston_output *output, *next;
>  
>   wl_event_source_remove(ec->idle_source);
> - if (ec->input_loop_source)
> + ec->idle_source = NULL;
> +
> + if (ec->input_loop_source) {
>   wl_event_source_remove(ec->input_loop_source);
> + ec->input_loop_source = NULL;
> + }
>  
>   /* Destroy all outputs associated with this compositor */
>   wl_list_for_each_safe(output, next, &ec->output_list, link)
> diff --git a/src/evdev-touchpad.c b/src/evdev-touchpad.c
> index 69f913a..360f87f 100644
> --- a/src/evdev-touchpad.c
> +++ b/src/evdev-touchpad.c
> @@ -663,6 +663,7 @@ touchpad_destroy(struct evdev_dispatch *dispatch)
>  
>   touchpad->filter->interface->destroy(touchpad->filter);
>   wl_event_source_remove(touchpad->fsm.timer_source);
> + touchpad->fsm.timer_source = NULL;
>   free(dispatch);
>  }
>  
> diff --git a/src/evdev.c b/src/evdev.c
> index 888dfbd..a1bce2c 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -697,8 +697,10 @@ evdev_device_destroy(struct evdev_device *device)
>   if (dispatch)
>   dispatch->interface->destroy(dispatch);
>  
> - if (device->source)
>

Re: [PATCH 2/2] Handle OOM with signal events

2014-05-09 Thread Kristian Høgsberg

On Fri, May 09, 2014 at 04:03:52PM +0200, Hardening wrote:
> This patch handles the case where a signal event source can not be created.
> ---
>  src/compositor.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index 6ad3387..047df8a 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -4197,6 +4197,7 @@ int main(int argc, char *argv[])
>   display = wl_display_create();
>  
>   loop = wl_display_get_event_loop(display);
> + memset(signals, 0, sizeof(signals));

We set all entries of signals[4], and they're going to be valid
signal source pointers or NULL if the allocation fails.  No need to memset.

>   signals[0] = wl_event_loop_add_signal(loop, SIGTERM, on_term_signal,
> display);
>   signals[1] = wl_event_loop_add_signal(loop, SIGINT, on_term_signal,
> @@ -4208,6 +4209,9 @@ int main(int argc, char *argv[])
>   signals[3] = wl_event_loop_add_signal(loop, SIGCHLD, sigchld_handler,
> NULL);
>  
> + if (!signals[0] || !signals[1] || !signals[2] || !signals[3])
> + goto out_signals;
> +
>   config = weston_config_parse("weston.ini");
>   if (config != NULL) {
>   weston_log("Using config file '%s'\n",
> @@ -4321,8 +4325,11 @@ int main(int argc, char *argv[])
>  
>   wl_signal_emit(&ec->destroy_signal, ec);
>  
> - for (i = ARRAY_LENGTH(signals); i;)
> - wl_event_source_remove(signals[--i]);
> + out_signals:
> + for (i = ARRAY_LENGTH(signals); i; i--) {
> + if (signals[i-1])

We can just add the if condition to the existing loop, no need to
iterate in reverse.

Kristian

> + wl_event_source_remove(signals[i-1]);
> + }
>  
>   weston_compositor_xkb_destroy(ec);
>  
> -- 
> 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 weston] libinput-seat: literal values for WESTON_LIBINPUT_LOG_PRIORITY

2014-05-09 Thread Kristian Høgsberg

On Fri, May 09, 2014 at 11:24:40AM -0700, U. Artie Eoff wrote:
> Only accept specific literal values from the environment variable
> WESTON_LIBINPUT_LOG_PRIORITY... "debug", "info", or "error".
> 
> Signed-off-by: U. Artie Eoff 

Thanks Artie, I think we can squeeze that in with the RC2.

Kristian

> ---
>  src/libinput-seat.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libinput-seat.c b/src/libinput-seat.c
> index a38d470..d59ae42 100644
> --- a/src/libinput-seat.c
> +++ b/src/libinput-seat.c
> @@ -271,8 +271,15 @@ udev_input_init(struct udev_input *input, struct 
> weston_compositor *c, struct ud
>   libinput_log_set_handler(&libinput_log_func, NULL);
>  
>   log_priority = getenv("WESTON_LIBINPUT_LOG_PRIORITY");
> +
>   if (log_priority) {
> - libinput_log_set_priority(strtol(log_priority, NULL, 10));
> + if (strcmp(log_priority, "debug") == 0) {
> + libinput_log_set_priority(LIBINPUT_LOG_PRIORITY_DEBUG);
> + } else if (strcmp(log_priority, "info") == 0) {
> + libinput_log_set_priority(LIBINPUT_LOG_PRIORITY_INFO);
> + } else if (strcmp(log_priority, "error") == 0) {
> + libinput_log_set_priority(LIBINPUT_LOG_PRIORITY_ERROR);
> + }
>   }
>  
>   input->libinput = libinput_udev_create_for_seat(&libinput_interface, 
> input,
> -- 
> 1.9.0
> 
> ___
> 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] desktop-shell: Fix black edges on scaled desktop pattern

2014-05-09 Thread Kristian Høgsberg

On Fri, May 09, 2014 at 01:52:33PM -0700, Bill Spitzak wrote:
> Thanks, it looks like that setup worked, patch sent correctly now.

Great, it's good to have git send-email working.  As it turns out, I
didn't have any problems applying your initial email, but I do have the
fix whitespace option Pekka mentioned in my git config.

Kristian

> On 05/09/2014 11:52 AM, Jonas Ådahl wrote:
> >If you are using gmail, you can just use Googles SMTP server directly.
> >The example configuration in the manual [0] even is a @gmail.com
> >address setup.
> >
> >Jonas
> >
> >[0] http://git-scm.com/docs/git-send-email
> >
> >>___
> >>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
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland 6/6] scanner: Generate macros for getting the 'since' version of an event

2014-05-09 Thread Kristian Høgsberg

On Thu, May 08, 2014 at 11:39:49PM +0200, Jonas Ådahl wrote:
> This could be useful for compositors who need to be able to not send
> events if the client bound a version lower than the newest provided.
> 
> Event version numbers are exposed as
> [INTERFACE_NAME]_[EVENT_NAME]_SINCE_VERSION for example wl_output.scale
> will have the version macro WL_OUTPUT_SCALE_SINCE_VERSION.

Yeah, that's a little more readable I guess.  This and previous patches
applied, thanks.

Kristian

> 
> Signed-off-by: Jonas Ådahl 
> ---
>  src/scanner.c | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/src/scanner.c b/src/scanner.c
> index 28fadb0..80c466e 100644
> --- a/src/scanner.c
> +++ b/src/scanner.c
> @@ -579,6 +579,18 @@ emit_opcodes(struct wl_list *message_list, struct 
> interface *interface)
>  }
>  
>  static void
> +emit_opcode_versions(struct wl_list *message_list, struct interface 
> *interface)
> +{
> + struct message *m;
> +
> + wl_list_for_each(m, message_list, link)
> + printf("#define %s_%s_SINCE_VERSION\t%d\n",
> +interface->uppercase_name, m->uppercase_name, m->since);
> +
> + printf("\n");
> +}
> +
> +static void
>  emit_type(struct arg *a)
>  {
>   switch (a->type) {
> @@ -1004,6 +1016,7 @@ emit_header(struct protocol *protocol, int server)
>   if (server) {
>   emit_structs(&i->request_list, i);
>   emit_opcodes(&i->event_list, i);
> + emit_opcode_versions(&i->event_list, i);
>   emit_event_wrappers(&i->event_list, i);
>   } else {
>   emit_structs(&i->event_list, i);
> -- 
> 1.9.1
> 
> ___
> 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 weston 5/5] tests: rename xwayland test

2014-05-09 Thread Kristian Høgsberg

On Wed, May 07, 2014 at 04:26:29PM +0300, Pekka Paalanen wrote:
> From: Pekka Paalanen 
> 
> If the test is named xwayland.weston, then the automake test harness
> keys it off xwayland.log. Making xwayland.log runs the test.
> The test harness has implicit rules to create a %.log from all of
> %$TEST_EXTENSIONS. So we have implicit rules to create %.log from %.la
> and %.log from %.weston.
> 
> We also build xwayland.so, which produces xwayland.la.
> 
> When the test harness goes running the xwayland test, it ends up using
> the %.la rule, which is wrong. It passes xwayland.la as the test name to
> weston-tests-env, which then loads it as a plugin into Weston and waits
> for Weston to exit. Which it never does.
> 
> Fix this by making the test have a different name than the Xwayland
> plugin.
> 
> Signed-off-by: Pekka Paalanen 

All applied, thanks Pekka.

Kristian

> ---
>  Makefile.am | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index a247c3d..177ce2e 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -927,10 +927,10 @@ buffer_count_weston_LDADD = libtest-client.la 
> $(EGL_TESTS_LIBS)
>  endif
>  
>  if ENABLE_XWAYLAND_TEST
> -weston_tests +=  xwayland.weston
> -xwayland_weston_SOURCES = tests/xwayland-test.c
> -xwayland_weston_CFLAGS = $(GCC_CFLAGS) $(XWAYLAND_TEST_CFLAGS)
> -xwayland_weston_LDADD = libtest-client.la $(XWAYLAND_TEST_LIBS)
> +weston_tests +=  xwayland-test.weston
> +xwayland_test_weston_SOURCES = tests/xwayland-test.c
> +xwayland_test_weston_CFLAGS = $(GCC_CFLAGS) $(XWAYLAND_TEST_CFLAGS)
> +xwayland_test_weston_LDADD = libtest-client.la $(XWAYLAND_TEST_LIBS)
>  endif
>  
>  matrix_test_SOURCES =\
> -- 
> 1.8.5.5
> 
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] editor: Fix cursor positioning with pointer and touch

2014-05-09 Thread Kristian Høgsberg

On Thu, May 08, 2014 at 02:55:50PM +0300, Ander Conselvan de Oliveira wrote:
> The calculation off the vertical offset between the widget coordinates
> and where the text was rendered was wrong. It was using the constant for
> horizontal offset for that too.
> ---
>  clients/editor.c | 33 +++--
>  1 file changed, 23 insertions(+), 10 deletions(-)

That fixes it here, thanks.  I added

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=78411

to the commit message.

Kristian

> 
> diff --git a/clients/editor.c b/clients/editor.c
> index 3b00833..f3f6141 100644
> --- a/clients/editor.c
> +++ b/clients/editor.c
> @@ -1011,7 +1011,17 @@ text_entry_draw_cursor(struct text_entry *entry, 
> cairo_t *cr)
>   cairo_stroke(cr);
>  }
>  
> -static const int text_offset_left = 10;
> +static int
> +text_offset_left(struct rectangle *allocation)
> +{
> + return 10;
> +}
> +
> +static int
> +text_offset_top(struct rectangle *allocation)
> +{
> + return allocation->height / 2;
> +}
>  
>  static void
>  text_entry_redraw_handler(struct widget *widget, void *data)
> @@ -1048,7 +1058,9 @@ text_entry_redraw_handler(struct widget *widget, void 
> *data)
>  
>   cairo_set_source_rgba(cr, 0, 0, 0, 1);
>  
> - cairo_translate(cr, text_offset_left, allocation.height / 2);
> + cairo_translate(cr,
> + text_offset_left(&allocation),
> + text_offset_top(&allocation));
>  
>   if (!entry->layout)
>   entry->layout = pango_cairo_create_layout(cr);
> @@ -1075,6 +1087,7 @@ text_entry_motion_handler(struct widget *widget,
>  {
>   struct text_entry *entry = data;
>   struct rectangle allocation;
> + int tx, ty;
>  
>   if (!entry->button_pressed) {
>   return CURSOR_IBEAM;
> @@ -1082,10 +1095,10 @@ text_entry_motion_handler(struct widget *widget,
>  
>   widget_get_allocation(entry->widget, &allocation);
>  
> - text_entry_set_cursor_position(entry,
> -x - allocation.x - text_offset_left,
> -y - allocation.y - text_offset_left,
> -false);
> + tx = x - allocation.x - text_offset_left(&allocation);
> + ty = y - allocation.y - text_offset_top(&allocation);
> +
> + text_entry_set_cursor_position(entry, tx, ty, false);
>  
>   return CURSOR_IBEAM;
>  }
> @@ -1105,8 +1118,8 @@ text_entry_button_handler(struct widget *widget,
>   widget_get_allocation(entry->widget, &allocation);
>   input_get_position(input, &x, &y);
>  
> - x -= allocation.x + text_offset_left;
> - y -= allocation.y + text_offset_left;
> + x -= allocation.x + text_offset_left(&allocation);
> + y -= allocation.y + text_offset_top(&allocation);
>  
>   editor = window_get_user_data(entry->window);
>  
> @@ -1149,8 +1162,8 @@ text_entry_touch_handler(struct widget *widget, struct 
> input *input,
>  
>   widget_get_allocation(entry->widget, &allocation);
>  
> - x = tx - (allocation.x + text_offset_left);
> - y = ty - (allocation.y + text_offset_left);
> + x = tx - (allocation.x + text_offset_left(&allocation));
> + y = ty - (allocation.y + text_offset_top(&allocation));
>  
>   editor = window_get_user_data(entry->window);
>   text_entry_activate(entry, seat);
> -- 
> 1.8.3.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 weston] window.c: Set the input region of the tooltip to empty

2014-05-09 Thread Jasper St. Pierre

This requires a bit of insider knowledge: we're planning on adding a
set_window_geometry request to xdg_shell explicitly override the window
geometry rectangle for a surface. I don't think we've talked about this on
the list before.

This makes subsurfaces viable for tooltips, with the caveat that the
compositor can't do things like fade them in or out for create/destroy
effects. The more I think about this, the more OK I am with it. The client
should probably fade in/out tooltips themselves if it really wants fancy
effects.


On Fri, May 9, 2014 at 3:48 PM, Kristian Høgsberg wrote:

> On Wed, May 07, 2014 at 11:47:08AM +0300, Pekka Paalanen wrote:
> > On Tue, 6 May 2014 14:40:53 -0700
> > Kristian Høgsberg  wrote:
> >
> > > On Mon, May 05, 2014 at 05:02:15PM +0300, Ander Conselvan de Oliveira
> > > wrote:
> > > > From: Ander Conselvan de Oliveira
> > > > 
> > > >
> > > > Otherwise it might receive touch events.
> > >
> > > I think a better approach is to just hide the tooltip if it (or the
> > > panel) gets touch events.  I don't think I agree with Pekka that we
> > > can't use subsurfaces for this, but I guess I'm not sure what the
> > > problem he sees there is.
> >
> > The panel is not a window, so sub-surfaces can be made to work there
> > well enough.
> >
> > It is for normal windows like registered with xdg_shell where
> > sub-surfaces are not suitable for tooltips. A sub-surface is a part of
> > the window, not another window. A tooltip is not expected to change the
> > window geometry, but a sub-surface does count into the window geometry.
> > Extruding sub-surfaces therefore affect the (parent) window geometry. It
> > is in the protocol spec.
>
> No, sure if no other geometry information is available.  For xdg-shell
> windows, the window geometry overrides that though.  And I don't think
> there's a clear distinction between being part of the window or being
> a separate window here, and I don't see why it would useful...
>
> Kristian
>
> >
> >
> > Thanks,
> > pq
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>



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

Re: [PATCH weston] window.c: Set the input region of the tooltip to empty

2014-05-09 Thread Pekka Paalanen

On Fri, 9 May 2014 20:09:43 -0400
"Jasper St. Pierre"  wrote:

> This requires a bit of insider knowledge: we're planning on
> adding a set_window_geometry request to xdg_shell explicitly
> override the window geometry rectangle for a surface. I don't
> think we've talked about this on the list before.

I may have seen a word fly by, maybe, but certainly I do not recall
seeing anything real. Thanks for the note.

> This makes subsurfaces viable for tooltips, with the caveat that
> the compositor can't do things like fade them in or out for
> create/destroy effects. The more I think about this, the more OK
> I am with it. The client should probably fade in/out tooltips
> themselves if it really wants fancy effects.

Right, if you say so.

> On Fri, May 9, 2014 at 3:48 PM, Kristian Høgsberg
> wrote:
> 
> > On Wed, May 07, 2014 at 11:47:08AM +0300, Pekka Paalanen wrote:
> > > On Tue, 6 May 2014 14:40:53 -0700
> > > Kristian Høgsberg  wrote:
> > >
> > > > On Mon, May 05, 2014 at 05:02:15PM +0300, Ander Conselvan
> > > > de Oliveira wrote:
> > > > > From: Ander Conselvan de Oliveira
> > > > > 
> > > > >
> > > > > Otherwise it might receive touch events.
> > > >
> > > > I think a better approach is to just hide the tooltip if it
> > > > (or the panel) gets touch events.  I don't think I agree
> > > > with Pekka that we can't use subsurfaces for this, but I
> > > > guess I'm not sure what the problem he sees there is.
> > >
> > > The panel is not a window, so sub-surfaces can be made to
> > > work there well enough.
> > >
> > > It is for normal windows like registered with xdg_shell where
> > > sub-surfaces are not suitable for tooltips. A sub-surface is
> > > a part of the window, not another window. A tooltip is not
> > > expected to change the window geometry, but a sub-surface
> > > does count into the window geometry. Extruding sub-surfaces
> > > therefore affect the (parent) window geometry. It is in the
> > > protocol spec.
> >
> > No, sure if no other geometry information is available.  For
> > xdg-shell windows, the window geometry overrides that though.
> > And I don't think there's a clear distinction between being
> > part of the window or being a separate window here, and I don't
> > see why it would useful...

If that removes the need for the compositor to ever identify tooltip
surfaces, then I suppose that is ok.

Would not even tiling window managers need to identify tooltips?
Otherwise they might accidentally clip away everything that goes
outside of the defined window geometry, especially if there is
another window next to it. Unless you add a special rule in the
window manager, that all sub-surfaces are drawn on top of all main
surfaces... but if the adjacent window also uses a sub-surfaces, the
tooltip might still get clipped by sub-surfaces but not the main
surface, which would look even funnier. Up to you, I've never dealt
with window management.

Hmm, I suppose the same question applies to also floating window
management. If you trigger a tooltip in a window that is not
top-most, would you expect that the tooltip may be obscured by
another window? If you use sub-surfaces instead of a specific
tooltip surface type, you cannot change this behaviour by a
compositor choice.

Don't forget to be explicit in the xdg_shell specification though,
that it overrides the wl_subcompositor specification, which is not
limited to desktop systems.

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

48 matches

Mail list logo