[PATCH] dix: make input clipping respect subpixels (#24187)

[Peter Hutterer]

On Thu, Oct 01, 2009 at 12:53:02PM +0200, Simon Thum wrote:
> Peter Hutterer wrote:
> > @@ -587,16 +587,41 @@ clipAxis(DeviceIntPtr pDev, int axisNum, int *val)
> >  
> >      /* If a value range is defined, clip. If not, do nothing */
> A nit: This is no longer true then.
> >      if (axis->max_value <= axis->min_value)
> > -        return;
> > +    {
> > +        /* Special case: for relative devices with no valid axis range, we
> > +         * still need to clip off the remainder to avoid negative root
> > +         * coordinates. We do NOT set val to 0, otherwise we can't cross
> > +         * screens anymore. */
> > +        if (*val <= axis->min_value && remainder && (*remainder < 0.0f))
> > +            *remainder = 0.0f;
> Errrm... that's a bug. You're killing the remainder based on an
> undefined axis range. That can't really be it?
> 
> Supposed the axis is screen (which we first checked it can't be, since
> screen axis would have a valid range) it would be sensible to me to add
> the analog line:
> 
> if (*val >= axis->max_value && remainder && (*remainder > 0.0f))
>     *remainder = 0.0f;
> 
> so we actually hold what's intended:
> "The clipping rules intend to avoid enlarging the
> valuator range by the remainder range, so assumptions
> one may have made stay valid."
> 
> Second, it should really be:
> 
> if (*val >= axis->max_value && remainder && (*remainder > 0.0f || *val >
> axis->min_value))
> 
> and vice versa, because if we're bigger/smaller (i.e. not just equal)
> the remainder always must be zeroed. That's basically why the patch is
> so complicated compared to what it does.
> 
> But I think the real solution must involve screen limits, which seems to
> happen somewhere else.

christ, what was I thinking. Thanks for the review.

/me goes and hides in the corner.
 
Cheers,
  Peter