[PATCH] data-device: Clean up the logic in start_drag

[Pekka Paalanen]

On Wed, 25 Jun 2014 09:09:33 -0700
Jason Ekstrand <jason at jlekstrand.net> wrote:

> On Wed, Jun 25, 2014 at 6:16 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> 
> > On Tue, 24 Jun 2014 21:23:53 -0700
> > Jason Ekstrand <jason at jlekstrand.net> wrote:
> >
> > > Previoiusly, we had a mess of logic that was repeated with one of the
> > > repeats negated.  Not only was this unnecisaraly confusing, but it
> > > segfaulted and one of the negations was wrong.  This cleans the whole
> > mess
> > > up and should fix bug #79725.
> > > ---
> > >  src/data-device.c | 30 ++++++++++++++++--------------
> > >  1 file changed, 16 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/src/data-device.c b/src/data-device.c
> > > index 6a81bc8..092eb0c 100644
> > > --- a/src/data-device.c
> > > +++ b/src/data-device.c
> > > @@ -648,15 +648,22 @@ data_device_start_drag(struct wl_client *client,
> > struct wl_resource *resource,
> > >       struct weston_seat *seat = wl_resource_get_user_data(resource);
> > >       struct weston_data_source *source = NULL;
> > >       struct weston_surface *icon = NULL;
> > > +     int is_pointer_grab, is_touch_grab;
> > >       int32_t ret = 0;
> >
> > Adding a temporary variable for
> > wl_resource_get_user_data(origin_resource) would make the lines below much
> > shorter.
> >
> 
> Good idea.  Done.
> 
> 
> >
> > >
> > > -     if ((seat->pointer->button_count == 0 ||
> > > -         seat->pointer->grab_serial != serial ||
> > > -         !seat->pointer->focus ||
> > > -         seat->pointer->focus->surface !=
> > wl_resource_get_user_data(origin_resource)) &&
> > > -             (seat->touch->grab_serial != serial ||
> > > -             !seat->touch->focus ||
> > > -             seat->touch->focus->surface !=
> > wl_resource_get_user_data(origin_resource)))
> > > +     is_pointer_grab = seat->pointer &&
> > > +                       seat->pointer->button_count == 1 &&
> >
> > The negation of seat->pointer->button_count == 0 would be to
> > compare !=0 or >0. Is ==1 always correct?
> > Can't we start a drag with two buttons down?
> >
> 
> Let's keep with one for now.  Multitouch is hard to get right and I have
> yet to see a particularly good framework for handling it in a
> non-fullscreen scenario.  Also, I don't think weston allows for per-finger
> dragging right now, I think it has to be per wl_touch.

This comment was about mouse buttons, but ok. ;-)


Thanks,
pq