Peter Hutterer wrote: > On Mon, Jun 22, 2009 at 07:13:12PM -0400, Thomas Jaeger wrote: >>>> * Subpixel coordinates are not working for me. Glancing through the >>>> code, I think this is to be expected for devices like my tablet whose >>>> device resolution is higher than the screen resolution, but it seems >>>> that this should work for accelerated devices like my trackpoint. Once >>>> again, not really a big deal. >>> yes, those bits are missing. sorry. subpixels right now are only for accel >>> code. feel free to send me a patch for this though, rescaleValuatorAxis >>> should be it. >> I've attached a series of patches enabling subpixel coordinates for >> high-resolution devices along with a few minor refactorings and a >> bugfix. I haven't noticed any additional issues running this code for >> the last few days. >> >> I don't like how the pointer acceleration code currently handles >> subpixel coordinates: I think it'd be much cleaner if it didn't modify >> pDev->last.remainder[...] directly, but instead returned the new >> subpixel value, for example using the following interface. I'd be happy >> to supply a patch. >> >> /* pointer acceleration handling */ >> typedef void (*PointerAccelSchemeProc)( >> DeviceIntPtr /*pDev*/, >> int /*first_valuator*/, >> int /*num_valuators*/, >> int* /*valuators*/, >> float* /*x_frac*/, >> float* /*y_frac*/, >> int /*evtime*/ >> ); > > punting this to simon (cc'd) A good idea at first glance; I aligned to the scheme modifying valuators and remainder in-place simply because that is how it was done. But, aside for being cleaner in some way, what does it buy us? It's also harder to optimize the no-op case since you always need to copy back.
BTW, I've got API changes (mailed to list+peter) which would allow multiple accel contexts anyway (in case that's the targeted use case). >> static int >> -rescaleValuatorAxis(int coord, AxisInfoPtr from, AxisInfoPtr to, >> +rescaleValuatorAxis(int coord, float remainder, float *remainder_return, >> AxisInfoPtr from, AxisInfoPtr to, > > at this point I wonder whether we should just give in and work with floats. > the reason why floats are separate are mainly because subpixels were > tacked onto the current system, but keeping them separate everywhere seems > to overly complicate things. > would it make sense to just change the return value and coord to a float? > > this has a larger fallout, including switching dev->last.remainder over as > well. The question is, is it worth the effort? > AIUI floats are a problem on embedded platforms where they carry a > significant performance hit. I'm not sure if the current implementation > really avoids this issue. Most of the event processing deals with integers > only, but during event generation where we do the pointer accel anyway I > think splitting it up doesn't give us much benefit, does it? I'd stick to integers + remainder. If only because of this: http://www.mega-nerd.com/FPcast/ Of course, lrintf() and friends are POSIX.1-2001, which we can use. But we really should avoid not using them, then. The float-only is tempting of course, and if done carefully it may be superior. But really, as long as there is no 100% nailed-down explanation for bug#21456, it'd prefer not to. Also, for large ranges, floats may not have sufficient precision to represent adjacent integers without 'holes'. The int+remainder approach gets around this, because we know the int range well and can assume all ints are in fact representable. > the approach of the patch looks alright to me, but carrying that many > parameters around makes me cringe (as said above). Same here. Why not represent the value in a struct ValuatorAxisValueCoordinateTypeThing { int value; float remainder; } and ease the function signature bloat this way? (Name should be shorter to reach that goal :) One could use NaN as a marker for paths where remainder processing is not wanted. Cheers, Simon _______________________________________________ xorg-devel mailing list [email protected] http://lists.x.org/mailman/listinfo/xorg-devel
