[PATCH 20/20] dix: reduce scope of tmp and mult.

[Simon Thum]

On 04/20/2011 10:21 AM, Daniel Stone wrote:
> Hi,
> 
> On Wed, Apr 20, 2011 at 04:28:29PM +1000, Peter Hutterer wrote:
>> index 0741604..4901d6a 100644
>> --- a/dix/ptrveloc.c
>> +++ b/dix/ptrveloc.c
>> @@ -1122,7 +1122,6 @@ acceleratePointerPredictable(
>>      ValuatorMask* val,
>>      CARD32 evtime)
>>  {
>> -    float tmp, mult; /* no need to init */
>>      int dx = 0, dy = 0, tmpi;
>>      DeviceVelocityPtr velocitydata = GetDevicePredictableAccelData(dev);
>>      Bool soften = TRUE;
>> @@ -1150,6 +1149,8 @@ acceleratePointerPredictable(
>>          }
>>  
>>          if (dev->ptrfeed && dev->ptrfeed->ctrl.num) {
>> +            float mult;
> 
> Couldn't you just put tmp here as well, rather than put it twice below:
> 
>> @@ -1167,6 +1168,7 @@ acceleratePointerPredictable(
>>                  /* Calculate the new delta (with accel) and drop it back
>>                   * into the valuator masks */
>>                  if (dx) {
>> +                    float tmp;
>>                      tmp = mult * fdx + dev->last.remainder[0];
>>                      /* Since it may not be apparent: lrintf() does not offer
>>                       * strong statements about rounding; however because we
>> @@ -1178,6 +1180,7 @@ acceleratePointerPredictable(
>>                      dev->last.remainder[0] = tmp - (float)tmpi;
>>                  }
>>                  if (dy) {
>> +                    float tmp;
>>                      tmp = mult * fdy + dev->last.remainder[1];
>>                      tmpi = lrintf(tmp);
>>                      valuator_mask_set(val, 1, tmpi);
> 
> But it doesn't matter really.  With or without that, for the series:
> Reviewed-by: Daniel Stone <daniel at fooishbar.org>
> 
> I'd also found and written 01/20 independently (it's sitting in another
> branch) while having a similar 'wtf is all this?' excursion through the
> pointer code, and had wondered how it worked!
Well, beyond a certain point there's simply no way I can make it better
understandable for anyone but me in my current (at the time) mindset.
Yes, it's complicated, but it's on par with the problem. And if I had
gotten requests to clarify things in time, that might have been avoidable.

After all, the development process and review density surely improved.
Today, some of peter's changes might have surfaced before even hitting
master. But the real point is that if the issue is complex, the code
will be too or it's too simple, meaning plain wrong.

I predict that X input will have to go some way towards more complexity
to deliver the features one might naturally expect from a window system.
There simply isn't much to argue about touchscreens bound to their
output screen. X knows both. It's natural, it just requires some amount
of in-server complexity - not some helper program floating around and
fighting against other helper programs trying to achieve something else,
piling up complexity on the system boundaries.

What can be argued about is reducible complexity, which means the scheme
code here in accel. But this is the most straightforward part if you
haven't been wading through mickeys before.

We can remove some of the settings and the scheme code without impairing
functionality. We can make it simpler and better documented.

I'm happy to review patches.

Regards,

Simon