[PATCH weston 1/2] simple-touch: Use damage_buffer if available

[Giulio Camuffo]

2016-02-20 5:20 GMT+02:00 Bryce Harrington <bryce at osg.samsung.com>:
> On Fri, Feb 12, 2016 at 09:03:16AM +0200, Giulio Camuffo wrote:
>> 2016-02-12 6:26 GMT+02:00 Bryce Harrington <bryce at osg.samsung.com>:
>> > On Fri, Jan 08, 2016 at 03:00:55PM -0600, Derek Foreman wrote:
>> >> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
>> >> ---
>> >>  clients/simple-touch.c | 18 +++++++++++++++---
>> >>  1 file changed, 15 insertions(+), 3 deletions(-)
>> >
>> > Given that the demo clients exist to show the functionality of the
>> > weston they're included with, damage_buffer is always going to be
>> > available, isn't it?
>>
>> But it can be used to test other compositors too. I think demo clients
>> should strive to be compatible with as many compositors as possible.
>
> Yes, they can be used to test other compositors, but I would argue their
> ultimate purpose is more educational oriented, and in service of that
> aim, keeping the code concise is beneficial.
>
> On the topic of testing, if you were in fact using this to test your
> compositor implementation, and your compositor lacks the damage_buffer
> feature, the error generated in that case would be a valid testing
> result, I should think.

I don't quite agree. While damage_buffer is better damage didn't go
anywere and compositors are supposed to implement it correctly. We can
maybe make it complain if buffer_damage is not available, if we want
to hint at compositors they should add that.
And i don't think the code this patch adds makes it difficult to
understand the code, it remains pretty straightforward. I would also
argue that if we want the code to be educational it should show how to
to remain backward compatible when new features are being used.


Thanks,
Giulio

>
> Bryce
>
>> > Maybe just keep the client concise and just use the new API?
>> >
>> > If we do want to demonstrate both routines, then IMHO the code should
>> > either show or explain why one would prefer one over the other.  I
>> > imagine a new wayland coder would be looking at simple-touch as a simple
>> > example of how to do touch, and might get confused seeing two ways to do
>> > it.
>> >
>> > Bryce
>> >
>> >> diff --git a/clients/simple-touch.c b/clients/simple-touch.c
>> >> index 446f2ca..383d802 100644
>> >> --- a/clients/simple-touch.c
>> >> +++ b/clients/simple-touch.c
>> >> @@ -56,6 +56,7 @@ struct touch {
>> >>       int has_argb;
>> >>       int width, height;
>> >>       void *data;
>> >> +     bool use_damage_buffer;
>> >>  };
>> >>
>> >>  static void
>> >> @@ -148,7 +149,10 @@ touch_paint(struct touch *touch, int32_t x, int32_t y, int32_t id)
>> >>       p[2] = c;
>> >>
>> >>       wl_surface_attach(touch->surface, touch->buffer, 0, 0);
>> >> -     wl_surface_damage(touch->surface, x - 2, y - 2, 5, 5);
>> >> +     if (touch->use_damage_buffer)
>> >> +             wl_surface_damage_buffer(touch->surface, x - 2, y - 2, 5, 5);
>> >> +     else
>> >> +             wl_surface_damage(touch->surface, x - 2, y - 2, 5, 5);
>> >>       /* todo: We could queue up more damage before committing, if there
>> >>        * are more input events to handle.
>> >>        */
>> >> @@ -269,9 +273,13 @@ handle_global(void *data, struct wl_registry *registry,
>> >>       struct touch *touch = data;
>> >>
>> >>       if (strcmp(interface, "wl_compositor") == 0) {
>> >> +             int cv = MIN(WL_SURFACE_DAMAGE_BUFFER_SINCE_VERSION, version);
>> >> +
>> >>               touch->compositor =
>> >>                       wl_registry_bind(registry, name,
>> >> -                                      &wl_compositor_interface, 1);
>> >> +                                      &wl_compositor_interface, cv);
>> >> +             if (cv >= WL_SURFACE_DAMAGE_BUFFER_SINCE_VERSION)
>> >> +                     touch->use_damage_buffer = true;
>> >>       } else if (strcmp(interface, "wl_shell") == 0) {
>> >>               touch->shell =
>> >>                       wl_registry_bind(registry, name,
>> >> @@ -308,6 +316,7 @@ touch_create(int width, int height)
>> >>       touch->display = wl_display_connect(NULL);
>> >>       assert(touch->display);
>> >>
>> >> +     touch->use_damage_buffer = false;
>> >>       touch->has_argb = 0;
>> >>       touch->registry = wl_display_get_registry(touch->display);
>> >>       wl_registry_add_listener(touch->registry, &registry_listener, touch);
>> >> @@ -337,7 +346,10 @@ touch_create(int width, int height)
>> >>
>> >>       memset(touch->data, 64, width * height * 4);
>> >>       wl_surface_attach(touch->surface, touch->buffer, 0, 0);
>> >> -     wl_surface_damage(touch->surface, 0, 0, width, height);
>> >> +     if (touch->use_damage_buffer)
>> >> +             wl_surface_damage_buffer(touch->surface, 0, 0, width, height);
>> >> +     else
>> >> +             wl_surface_damage(touch->surface, 0, 0, width, height);
>> >>       wl_surface_commit(touch->surface);
>> >>
>> >>       return touch;
>> >> --
>> >> 2.6.4
>> >>
>> >> _______________________________________________
>> >> wayland-devel mailing list
>> >> wayland-devel at lists.freedesktop.org
>> >> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>> > _______________________________________________
>> > wayland-devel mailing list
>> > wayland-devel at lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/wayland-devel