Hi Chris, I read your commit comments again. It clearly spelled out the rational behind the patch. I am convinced. Sorry for making you explain it again.
Reviewed-by: Ping Cheng <pingli...@gmail.com> Ping On Sat, Feb 27, 2010 at 8:56 PM, Chris Bagwell <ch...@cnpbagwell.com> wrote: > On Sat, Feb 27, 2010 at 10:23 PM, Ping Cheng <pingli...@gmail.com> wrote: >> On Sat, Feb 27, 2010 at 7:10 PM, Chris Bagwell <ch...@cnpbagwell.com> wrote: >>> Ping, was this a comment about the patch or a general statement? The >>> patch limits special logic to Bamboo Touch series when invoked with >>> pad type. >> >> A general statement. >> >>> But in general, we need to support calling GetRanges() with pads for >>> cases when user defines devices in xorg.conf. Most devices with pad >>> support also share same /dev/input/event? with stylus/eraser. >>> Depending on order specified in xorg.conf, it can be the init of pad >>> that will get the MaxX/MaxY values for other types and store in common >>> struct for them to reference later. >> >> Only Bamboo touch reports pad with touch data. All the other models >> don't have this issue (at least not yet :). >> >>> And the patch still allows for that. >> >> I know the patch doesn't "cause harm" :). But it doesn't add anything >> either. Do you see any problem without the patch? >> >> Ping >> > > Yes, there are problems without the patch specific to Bamboo Touch's. > Let say touch and button data comes in on /dev/input/event7 (specific > to bamboo touch's as you mention). Standard behaviour with HAL is > that pad opens up /dev/input/event7 before touch opens up > /dev/input/event7. Current behaviour that prevents opening the same > /dev/input/event7 multiple times also prevents wcmUSBGetRanges() from > being called more then once. That means for /dev/input/event7, it > only calls wcmUSBGetRanges() with IsPad(priv) == true and never with > IsTouch(priv) == true. > > Knowing that fact, you can see how in my patch the MaxTouch* and > MaxResol* variables were not previously being set correctly. It > sticks at a default of 1024 for MaxTouchX/MaxTouchY. This value is to > big but makes touch usable with some resolution related issues. > > Now look at current git's xdrv/wcmConfig.c:xf86WcmMatchDevice() and > were it finds tablet_id == bamboo touchs. That whole logic inside > that if(bamboo) is only to work around this exact issue and is > duplicating logic inside wcmUSBGetRanges(). > > Its better to get wcmUSBGetRanges() working in all cases and then > remove work around logic. > > Chris > ------------------------------------------------------------------------------ Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev _______________________________________________ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel