multi-touch-dev team mailing list archive

[Henrik Rydberg <rydberg@xxxxxxxxxxx>]

On 08/20/2010 07:24 PM, Chase Douglas wrote:

> 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.


Ok.

>> 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.


All fine, my bad.

>> 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 :).


Looks ok now.


>> 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.


Looks ok now.


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


The patch is easier to grasp now, that's good! We need to test this one a bit
more, so no real ack yet. :-)

Cheers,
Henrik