Hi all, I noticed a small glitch in current Xi2 sub-pixel handling. Right now, root coordinates may become negative by half a pixel. Essentially, sub-pixels need to be respected in clipping. Below is a incomplete fix I cooked up.
However, it occurred to me there is a broader issue beneath: that of a proper pixel model. Whatever we cook up here shouldn't be at odds with core rendering(?), probably XRender, maybe composite, you name it. Obvious choices (to me) are: 1) let a hypothetical 1x1 screen have [0..1[ coordinate space. 2) let a hypothetical 2x2 screen have [0..1] coordinate space, neglecting the border half-pixel. This maintains the relation between screen resolution and input coordinate space distance. 3) <fill in> To me 1 sounds sane, 2 sounds compatible. 2) is codified below, but only for valuators; I guess we need to properly confine screen limits as well. And it's untested, merely for discussion. I'm also unsure about it since it modifies clipValuators(), which is called every now and then. Cheers, Simon Peter Hutterer wrote: > On Sat, Sep 12, 2009 at 07:18:35PM +0200, Simon Thum wrote: >> Hi Peter, >> >> this is just a little reminder about a small issue I've noticed, since >> 7.5 is on its way. >> >> I've just checked against latest build and I still see that phenomenon: >> >> EVENT type 6 >> device: 6 (6) >> detail: 0 >>>>> root: -0.36/82.33 >> event: 7.64/64.33 >> buttons: >> modifiers: locked 0 latched 0 base 0 effective: 0 >> group: locked 0 latched 0 base 0 effective: 0 >> valuators: 0.64 81.33 >> windows: root 0x6c event 0x2a00001 child 0x0 >> >> Of course, event coords may also become negative due to subpixel >> handling. If this could happen before, then ignore this email, but I >> guess that will cause some headaches. > > event coords are allowed negative, they are relative to the window's origin > after all. Root window coordinates must never be negative, AFAIK. So you're > correct - this is a bug. > >> Basic problem: A hypothetical 1x1 window, which only contains the >> integer coordinate (0,0) could now get everything from (-0.5, -0.5) to >> (0.5, 0.5). In a sense, it got bigger. >> >> I've searched the Xi spec a bit but I couldn't find any clarification >> about the issue (the one in the X.org repo). I think it should at least >> be stated explicitly in specs. If you somehow wanna fix it before 7.5, >> I've attached a small but incomplete patch which lays out what I think >> is proper clipping wrt subpixels. > > Patch looks good, what's the incomplete part about it? I'm happy to push > this once you claim you have a complete patch :) > > Cheers, > Peter >
>From b94b7033a830d67de76bdcf6187e13d599df599c Mon Sep 17 00:00:00 2001 From: Simon Thum <[email protected]> Date: Mon, 15 Jun 2009 20:23:18 +0200 Subject: [PATCH] dix: make clipping respect subpixels Make clipping of coordinates optionally respect subpixels. This is needed for XI2. The clipping rules intend to avoid enlarging the screen by one pixel. --- dix/getevents.c | 45 ++++++++++++++++++++++++++++++++------------- 1 files changed, 32 insertions(+), 13 deletions(-) diff --git a/dix/getevents.c b/dix/getevents.c index 46e5080..9605af3 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -576,7 +576,7 @@ GetMaximumEventsNum(void) { * InitValuatorAxisClassStruct. */ static void -clipAxis(DeviceIntPtr pDev, int axisNum, int *val) +clipAxis(DeviceIntPtr pDev, int axisNum, int *val, float* remainder) { AxisInfoPtr axis = pDev->valuator->axes + axisNum; /* InitValuatoraAxisStruct ensures that (min < max). */ @@ -586,12 +586,28 @@ clipAxis(DeviceIntPtr pDev, int axisNum, int *val) /* If a value range is defined, clip. If not, do nothing */ if (axis->max_value <= axis->min_value) - return; + return; + + if (*val < axis->min_value) { + *val = axis->min_value; + if (remainder) + *remainder = 0.0f; + return; + } + if (*val == axis->min_value && remainder && (*remainder < 0.0f)) { + *remainder = 0.0f; + return; + } - if (*val < axis->min_value) - *val = axis->min_value; - if (*val > axis->max_value) - *val = axis->max_value; + if (*val > axis->max_value) { + *val = axis->max_value; + if (remainder) + *remainder = 0.0f; + return; + } + if (*val == axis->max_value && remainder && (*remainder > 0.0f)) { + *remainder = 0.0f; + } } /** @@ -604,7 +620,8 @@ clipValuators(DeviceIntPtr pDev, int first_valuator, int num_valuators, int i; for (i = 0; i < num_valuators; i++) - clipAxis(pDev, i + first_valuator, &(valuators[i])); + clipAxis(pDev, i + first_valuator, &(valuators[i]), + &pDev->last.remainder[i]); } /** @@ -666,14 +683,15 @@ moveAbsolute(DeviceIntPtr dev, int *x, int *y, else *y = dev->last.valuators[1]; - clipAxis(dev, 0, x); - clipAxis(dev, 1, y); + clipAxis(dev, 0, x, &dev->last.remainder[0]); + clipAxis(dev, 1, y, &dev->last.remainder[1]); i = (first > 2) ? 0 : 2; for (; i < num; i++) { dev->last.valuators[i + first] = valuators[i]; - clipAxis(dev, i, &dev->last.valuators[i + first]); + clipAxis(dev, i, &dev->last.valuators[i + first], + &dev->last.remainder[i + first]); } } @@ -707,8 +725,8 @@ moveRelative(DeviceIntPtr dev, int *x, int *y, * co-ord space limit). If it is attached, we need x/y to go over the * limits to be able to change screens. */ if(dev->u.master) { - clipAxis(dev, 0, x); - clipAxis(dev, 1, y); + clipAxis(dev, 0, x, &dev->last.remainder[0]); + clipAxis(dev, 1, y, &dev->last.remainder[1]); } /* calc other axes, clip, drop back into valuators */ @@ -716,7 +734,8 @@ moveRelative(DeviceIntPtr dev, int *x, int *y, for (; i < num; i++) { dev->last.valuators[i + first] += valuators[i]; - clipAxis(dev, i, &dev->last.valuators[i + first]); + clipAxis(dev, i, &dev->last.valuators[i + first], + &dev->last.remainder[i + first]); valuators[i] = dev->last.valuators[i + first]; } } -- 1.6.3.3
_______________________________________________ xorg-devel mailing list [email protected] http://lists.x.org/mailman/listinfo/xorg-devel
