Re: [PATCH wayland v4] protocol: Add wl_surface.damage_buffer

2015-11-29 Thread Pekka Paalanen
On Fri, 27 Nov 2015 09:33:41 -0800
Jason Ekstrand  wrote:

> On Nov 27, 2015 1:35 AM, "Pekka Paalanen"  wrote:
> >
> > On Fri, 27 Nov 2015 16:47:19 +0800
> > Jonas Ådahl  wrote:
> >  
> > > On Thu, Nov 26, 2015 at 03:44:12PM -0600, Derek Foreman wrote:  
> > > > wl_surface.damage uses surface local co-ordinates.
> > > >
> > > > Buffer scale and buffer transforms came along, and EGL surfaces
> > > > have no understanding of them.
> > > >
> > > > Theoretically, clients pass damage rectangles - in Y-inverted surface
> > > > co-ordinates) to EGLSwapBuffersWithDamage, and the EGL implementation
> > > > passed them on to wayland.  However, for this to work the EGL
> > > > implementation must be able to flip those rectangles into the space
> > > > the compositor is expecting, but it's unable to do so because it
> > > > doesn't know the height of the transformed buffer.
> > > >
> > > > So, currently, EGLSwapBuffersWithDamage is unusable and EGLSwapBuffers
> > > > has to pass (0,0) - (INT32_MAX, INT32_MAX) damage to function.
> > > >
> > > > wl_surface.damage_buffer allows damage to be registered on a surface
> > > > in buffer co-ordinates, avoiding this problem.
> > > >
> > > > Credit where it's due, these ideas are not entirely my own:
> > > > Over a year ago the idea of changing damage co-ordinates to buffer
> > > > co-ordinates was suggested (by Jason Ekstrand), and it was at least
> > > > partially rejected and abandoned.  At the time it was also suggested
> > > > (by Pekka Paalanen) that adding a new wl_surface.damage_buffer request
> > > > was another option.
> > > >
> > > > This will eventually resolve:
> > > > https://bugs.freedesktop.org/show_bug.cgi?id=78190
> > > > by making the problem irrelevant.
> > > >
> > > > Signed-off-by: Derek Foreman   
> > >
> > > Reviewed-by: Jonas Ådahl , with a couple of comments
> > > below.  
> >
> > Reviewed-by: Pekka Paalanen 
> >
> > ...
> >  
> > > > ---
> > > >
> > > > Changes from v3:
> > > >  replaced the last paragraph with Pekka's version
> > > >   (with one tiny grammar change)
> > > >
> > > >  protocol/wayland.xml | 49  
> +++--
> > > >  1 file changed, 47 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> > > > index f9e6d76..4e97bb9 100644
> > > > --- a/protocol/wayland.xml
> > > > +++ b/protocol/wayland.xml
> > > > @@ -176,7 +176,7 @@
> > > >  
> > > >
> > > >
> > > > -  
> > > > +  
> > > >  
> > > >A compositor.  This object is a singleton global.  The
> > > >compositor is in charge of combining the contents of multiple
> > > > @@ -986,7 +986,7 @@
> > > >  
> > > >
> > > >
> > > > -  
> > > > +  
> > > >  
> > > >A surface is a rectangular area that is displayed on the  
> screen.
> > > >It has a location, size and pixel contents.
> > > > @@ -1109,6 +1109,10 @@
> > > > wl_surface.commit assigns pending damage as the current damage,
> > > > and clears pending damage. The server will clear the current
> > > > damage as it repaints the surface.
> > > > +
> > > > +   Alternatively, damage can be posted with wl_surface.damage_buffer
> > > > +   which uses buffer co-ordinates instead of surface co-ordinates,
> > > > +   and is probably the preferred and intuitive way of doing this.
> > > >
> > > >
> > > >
> > > > @@ -1325,6 +1329,47 @@
> > > >
> > > >
> > > >  
> > > > +
> > > > +  
> > >
> > > Bikeshed mode enabled: missing empty line between "Version N additions"
> > > and actual additions.
> > >  
> > > > +
> > > > +  
> > > > +   This request is used to describe the regions where the pending
> > > > +   buffer is different from the current surface contents, and where
> > > > +   the surface therefore needs to be repainted. The compositor
> > > > +   ignores the parts of the damage that fall outside of the surface.
> > > > +
> > > > +   Damage is double-buffered state, see wl_surface.commit.
> > > > +
> > > > +   The damage rectangle is specified in buffer coordinates.
> > > > +
> > > > +   The initial value for pending damage is empty: no damage.
> > > > +   wl_surface.damage_buffer adds pending damage: the new pending
> > > > +   damage is the union of old pending damage and the given rectangle.
> > > > +
> > > > +   wl_surface.commit assigns pending damage as the current damage,
> > > > +   and clears pending damage. The server will clear the current
> > > > +   damage as it repaints the surface.
> > > > +
> > > > +   This request differs from wl_surface.damage in only one way - it
> > > > +   takes damage in buffer co-ordinates instead of surface local
> > > > +   co-ordinates.  While this generally is more intuitive than surface
> > > > +   co-ordinates, it is especially desirable when using wp_viewport
> > > > +   or when a drawing library (like EGL) is unaware of buffer scale
> > > > +   and buffer transform.
> > > > +
> > > > +   Since it is

Re: [PATCH weston 11/11] simple-damage: Add --use-buffer-damage flag

2015-11-29 Thread Pekka Paalanen
On Fri, 27 Nov 2015 14:55:13 -0600
Derek Foreman  wrote:

> On 27/11/15 07:47 AM, Pekka Paalanen wrote:
> > On Wed, 18 Nov 2015 16:32:34 -0600
> > Derek Foreman  wrote:
> >   
> >> Add a new flag for testing damage in buffer co-ordinates
> >>
> >> Signed-off-by: Derek Foreman 
> >> ---
> >>  clients/simple-damage.c | 62 
> >> +
> >>  1 file changed, 47 insertions(+), 15 deletions(-)
> >>  
> > 
> > Hi Derek,
> > 
> > even without your series, running simple-damage --use-viewport
> > --scale=2 will leave trails. It's not a damage issue, because
> > compositor repaints do not clear it. It seems to be a rendering issue
> > in the app. Using viewport with any scale > 1 will trigger that.  
> 
> Yes, I was seeing this too.  Doesn't happen with the pixman renderer.
> 
> Figuring it out is on my TODO list somewhere, right next to sorting out
> the damage issues when zoomed and when changing transforms between
> commits. :)
> 
> If I mess up the gl renderer to do a full texture upload with ever
> damage commit the problem goes away.

Ha! You're right, I forgot the compositor has a copy of the surface
contents and uses damage to update that. D'oh.

> Checking the damage co-ordinates in gl_renderer_flush_damage(),
> sometimes thin areas are getting shrunk to 0 (when they look like they
> should be getting expanded by the transform)
> 
> I guess the thing damage regions come from the two (erase, replace)
> damage regions being bashed together and overlapping when the ball is
> moving slowly...
> 
> weston_surface_to_buffer_rect() looks like it could be part of the
> problem, since it looks like it does the viewport stuff first, then
> truncates to integer, then transforms the resulting rect.  Could be
> losing thing rectangles in the middle?

You are on to something there. On quick look, I'd blame the rounding of
x2,y2 being in the wrong direction. But then users need to ensure not
crossing over image boundaries.

> Appears to not happen with my transforms branch that replaced all sorts
> of this stuff with matrix mults with no conversion to integer in
> between.  Likely because it doesn't have to round off stuff to ints
> between steps?

Quite likely, and maybe the final rounding is also done towards
enlarging the damage region there?

> I think that's as far as I'm going to go in debugging this one.

If simply fixing the rounding in weston_surface_to_buffer_rect() fixes
it, maybe it could be landed fast?

> > Anyway, I didn't see this patch changing any behaviour that I tried.

> > Otherwise this patch looks fine, but I have hard time understanding how
> > everything works here. Especially the rotations made my brain fall off.
> > That's why just a  
> 
> We're only really rotating a circle, and it's orientation independent,
> right? ;)

Yeeeah... except when you are rounding to integer pixels and if the
rounding direction makes the circle not centered and...

Actually I was more trying to understand the code that was already in
redraw(), that you clarified to do things like off_x = bwidth - tx; and
then it does paint_box(buffer->shm_data, bpitch, off_x, off_y, bwidth,
bborder, 0x);, so would that not paint up to off_x + bwidth =
bwidth - tx + bwidth which will be greater than bwidth, i.e. drawing
beyond the image border? But when I looked at the rendering, I didn't
see any evidence of that, so there is something I don't understand.
After all, tx > bwidth cannot be, right? Ooh, but bwidth is only half
of the actual bwidth... that's probably the thing I missed. :-P

Yeah, hard time following all that.


Thanks,
pq

> 
> > Acked-by: Pekka Paalanen 
> > 
> > Whether to use damage_buffer by default when available, I don't think
> > it matters. It depends on your toolkit which one is more
> > straightforward to use, otherwise they are equally good.  
> 
> Ok, I'll leave it alone out of sheer laziness then.


pgpKQwxpbLP67.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland] protocol: specify behavior of get_pointer when capabilities change

2015-11-29 Thread Peter Hutterer
Also applies to touch/keyboard

Signed-off-by: Peter Hutterer 
---
 protocol/wayland.xml | 30 +++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/protocol/wayland.xml b/protocol/wayland.xml
index f9e6d76..370fafc 100644
--- a/protocol/wayland.xml
+++ b/protocol/wayland.xml
@@ -1350,6 +1350,24 @@
This is emitted whenever a seat gains or loses the pointer,
keyboard or touch capabilities.  The argument is a capability
enum containing the complete set of capabilities this seat has.
+
+   When the pointer capability is added, a client may create a
+   wl_pointer object using the wl_seat.get_pointer request. This object
+   will receive pointer events until the capability is removed in the
+   future.
+
+   When the pointer capability is removed, a client should destroy the
+   wl_pointer objects associated with the seat where the capability was
+   removed, using the wl_pointer.release request. No further pointer
+   events will be received on these objects.
+
+   If a seat regains the pointer capability and a client has a pointer
+   object obtained previously, that object may start sending pointer
+   events. This behavior is implementation-dependent and must not be
+   relied upon.
+
+   The above behavior also applies to wl_keyboard and wl_touch with the
+   keyboard and touch capabilities, respectively.
   
   
 
@@ -1360,7 +1378,9 @@
for this seat.
 
This request only takes effect if the seat has the pointer
-   capability.
+   capability, or has had the pointer capability in the past.
+   It is a protocol violation to issue this request on a seat that has
+   never had the pointer capability.
   
   
 
@@ -1371,7 +1391,9 @@
for this seat.
 
This request only takes effect if the seat has the keyboard
-   capability.
+   capability, or has had the keyboard capability in the past.
+   It is a protocol violation to issue this request on a seat that has
+   never had the keyboard capability.
   
   
 
@@ -1382,7 +1404,9 @@
for this seat.
 
This request only takes effect if the seat has the touch
-   capability.
+   capability, or has had the touch capability in the past.
+   It is a protocol violation to issue this request on a seat that has
+   never had the touch capability.
   
   
 
-- 
2.5.0

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


[PATCH v2 wayland] protocol: add support for cross-interface enum attributes

2015-11-29 Thread Auke Booij
The enum attribute, for which scanner support was introduced in
1771299, can be used to link message arguments to s. However,
some arguments refer to s in a different .

This adds scanner support for referring to an  in a different
 using dot notation. It also sets the attributes in this
style in the wayland XML protocol (wl_shm_pool::create_buffer::format
to wl_shm::format, and wl_surface::set_buffer_transform::transform to
wl_output::transform), and updates the documentation XSL so that this
new style is supported.

Changes since v1:
 - several implementation bugs fixed

Signed-off-by: Auke Booij 
---
 doc/publican/protocol-to-docbook.xsl | 17 +--
 protocol/wayland.xml |  4 +--
 src/scanner.c| 59 +++-
 3 files changed, 61 insertions(+), 19 deletions(-)

diff --git a/doc/publican/protocol-to-docbook.xsl 
b/doc/publican/protocol-to-docbook.xsl
index fad207a..1fa066d 100644
--- a/doc/publican/protocol-to-docbook.xsl
+++ b/doc/publican/protocol-to-docbook.xsl
@@ -103,9 +103,20 @@
 
 
   
-
-  
-
+
+  
+
+  
+  ::
+  
+
+  
+  
+
+  
+
+  
+
 
   
   
diff --git a/protocol/wayland.xml b/protocol/wayland.xml
index f9e6d76..0873553 100644
--- a/protocol/wayland.xml
+++ b/protocol/wayland.xml
@@ -229,7 +229,7 @@
   
   
   
-  
+  
 
 
 
@@ -1292,7 +1292,7 @@
wl_output.transform enum the invalid_transform protocol error
is raised.
   
-  
+  
 
 
 
diff --git a/src/scanner.c b/src/scanner.c
index 406519f..e69bd2a 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -785,32 +785,62 @@ start_element(void *data, const char *element_name, const 
char **atts)
}
 }
 
+static struct enumeration *
+find_enumeration(struct protocol *protocol, struct interface *interface, char 
*enum_attribute)
+{
+   struct interface *i;
+   struct enumeration *e, *f = NULL;
+   char *enum_name;
+   int idx = 0, j;
+
+for (j = 0; j + 1 < (int)strlen(enum_attribute); j++) {
+   if (enum_attribute[j] == '.') {
+   idx = j;
+   }
+   }
+
+   if (idx > 0) {
+   enum_name = enum_attribute + idx + 1;
+
+   wl_list_for_each(i, &protocol->interface_list, link)
+   if (strncmp(i->name, enum_attribute, idx) == 0)
+   wl_list_for_each(e, &i->enumeration_list, link)
+   if(strcmp(e->name, enum_name) == 0)
+   f = e;
+   } else if (interface) {
+   enum_name = enum_attribute;
+
+   wl_list_for_each(e, &interface->enumeration_list, link)
+   if(strcmp(e->name, enum_name) == 0)
+   f = e;
+   }
+
+   return f;
+}
+
 static void
-verify_arguments(struct parse_context *ctx, struct wl_list *messages, struct 
wl_list *enumerations)
+verify_arguments(struct parse_context *ctx, struct interface *interface, 
struct wl_list *messages, struct wl_list *enumerations)
 {
struct message *m;
wl_list_for_each(m, messages, link) {
struct arg *a;
wl_list_for_each(a, &m->arg_list, link) {
-   struct enumeration *e, *f;
+   struct enumeration *e;
 
if (!a->enumeration_name)
continue;
 
-   f = NULL;
-   wl_list_for_each(e, enumerations, link) {
-   if(strcmp(e->name, a->enumeration_name) == 0)
-   f = e;
-   }
 
-   if (f == NULL)
+   e = find_enumeration(ctx->protocol, interface, 
a->enumeration_name);
+
+   if (e == NULL)
fail(&ctx->loc,
 "could not find enumeration %s",
 a->enumeration_name);
 
switch (a->type) {
case INT:
-   if (f->bitfield)
+   if (e->bitfield)
fail(&ctx->loc,
 "bitfield-style enum must only be 
referenced by uint");
break;
@@ -848,12 +878,13 @@ end_element(void *data, const XML_Char *name)
 ctx->enumeration->name);
}
ctx->enumeration = NULL;
-   } else if (strcmp(name, "interface") == 0) {
-   struct in

Re: [PATCH wayland] Add support for cross-interface enum attributes

2015-11-29 Thread Derek Foreman
On 29/11/15 05:22 AM, Auke Booij wrote:
> On 29 November 2015 at 01:27, Auke Booij  wrote:
>> Signed-off-by: Auke Booij 
> 
> I realized this is the wrong search algorithm. I will fix it later today.

It also seems like it could use a much more elaborate commit log. :)

Thanks,
Derek

> ___
> 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] Add support for cross-interface enum attributes

2015-11-29 Thread Auke Booij
On 29 November 2015 at 01:27, Auke Booij  wrote:
> Signed-off-by: Auke Booij 

I realized this is the wrong search algorithm. I will fix it later today.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel