← Back to team overview

multi-touch-dev team mailing list archive

Re: Package review for xserver-xorg-input-evdev

 

On Fri, 2010-08-20 at 18:46 +0200, Henrik Rydberg wrote:
> On 08/20/2010 05:18 PM, Chase Douglas wrote:
> > Normally we do all our packaging and reviewing in lp and bzr. However,
> > the xserver-xorg-input-evdev package is maintained in git at
> > git.debian.org. I have pushed a new version of evdev with gesture
> > changes to git://kernel.ubuntu.com/cndougla/xserver-xorg-input-evdev.git
> > branch ubuntu.
> > 
> > I would like to do a packaging review just like we do in lp, so I'd like
> > to do it over this list if that's alright with everyone. If a better
> > mechanism comes about, feel free to propose it :).
> 
> How about sending the actual patches ala lkml? Definitely easier to review :-)

I would if they were normal patches. But the commits end up being diffs
of diffs. And really long diffs at that. Maybe when things settle down
and we're only making small changes we can do that.

> Ok, here comes comments.
> 
> configure.ac:
> 
> We actually depend on xserver being built with gesture support here... if
> somebody on non-ubuntu takes this evdev, compiles it with --enable-grail, what
> will happen?

I'm not really sure what we can do. We should add a runtime depends on
the xserver with gesture support, so I'll fix that up. Beyond that,
anyone running with --enable-grail should know what they're doing :).
They'll have to have grail installed on their system, so one would hope
they understand that the server needs the extension too. All we can do
is provide a proper dependency chain for Ubuntu.

> GrailGesture:
> 
> Still have a problem here with mixed device/screen coordinates based on pad/screen.

I think the disconnect is that master device valuators are in screen
coords, while slave device valuators are in device coords. We use the
master device valuators. I have added a comment to the patch to note
this.

> EvdevProcessEvent:
> 
> abs_mt_x_min etc aren't really used anymore.

Ahh yes, thanks for catching this. No functionality change, but the
patch is a few lines smaller after removal :).

> 101-gestures.patch:
> 
> still contains references to gevdev

Ahh, yes, forgot to strip out one file from the original evdev->gevdev
patch. I've removed it now.

I've pushed the changes up to the git repo. Please review again.

Thanks,

-- Chase




Follow ups

References