Sorry, I forgot the attachment. Thomas Jaeger wrote: > Peter Hutterer wrote: >> On Mon, Jun 22, 2009 at 07:13:12PM -0400, Thomas Jaeger wrote: >> >>> From 67076a1404cad4042e5cfc667d28f150e1663115 Mon Sep 17 00:00:00 2001 >>> From: Thomas Jaeger <[email protected]> >>> Date: Sat, 20 Jun 2009 20:17:41 -0400 >>> Subject: [PATCH 4/4] dix: report subpixel coordinates for high-resolution >>> devices >>> >>> --- >>> dix/getevents.c | 93 >>> +++++++++++++++++++++++++++++++++++++----------------- >>> 1 files changed, 64 insertions(+), 29 deletions(-) >>> >>> diff --git a/dix/getevents.c b/dix/getevents.c >>> index 25dd187..a64a0c7 100644 >>> --- a/dix/getevents.c >>> +++ b/dix/getevents.c >>> @@ -240,10 +240,11 @@ CreateClassesChangedEvent(EventList* event, >>> * Rescale the coord between the two axis ranges. >>> */ >>> 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've been playing with the idea of changing the types of various > parameter, but it never seems to work out very well: The X server uses > many different formats to represent valuator information internally, so > you end up converting back and forth no matter what format you choose. > The fact that every other function in dix/getevents.c modifiers the > valuator array and/or dev->last.valuator[i] in-place doesn't exactly > help either, this code could use a good refactoring. So I think dealing > with long parameter lists is the lesser evil, and I'm resubmitting the > patch with a few minor style cleanups and a fixed rescaleValuatorEvents > that actually takes the fractional value into account. > >>> @@ -745,26 +759,36 @@ accelPointer(DeviceIntPtr dev, int first, int num, >>> int *valuators, CARD32 ms) >>> * @param dev The device to be moved. >>> * @param x Pointer to current x-axis value, may be modified. >>> * @param y Pointer to current y-axis value, may be modified. >>> + * @param x_frac >>> + * @param y_frac >> ^^ missing doc. I know it's just a few words, but.. >>> * @param scr Screen the device's sprite is currently on. >>> * @param screenx Screen x coordinate the sprite is on after the update. >>> * @param screeny Screen y coordinate the sprite is on after the update. >>> + * @param screenx_frac >>> + * @param screeny_frac >> ^^ same thing here > This is what happens when you decide to come back later to fill in the > details and then never do, sorry. > >>> } else { >>> - if (flags & POINTER_ACCELERATE) >>> + if (flags & POINTER_ACCELERATE) { >>> accelPointer(pDev, first_valuator, num_valuators, valuators, >>> ms); >>> + /* The pointer acceleration code modifies the fractional part >>> + * in-place, so we need to extract this information first */ >> ^^^ stray tab > Sorry, I haven't managed to configure vim in such a way that it adopts > the indenting and space conventions of the code that it is looking at... > > Thanks, > Tom
>From fd863877e0667e2691fa7cadec65ac4cd5758502 Mon Sep 17 00:00:00 2001 From: Thomas Jaeger <[email protected]> Date: Sat, 20 Jun 2009 20:17:41 -0400 Subject: [PATCH] dix: report subpixel coordinates for high-resolution devices --- dix/getevents.c | 92 +++++++++++++++++++++++++++++++++++++----------------- 1 files changed, 63 insertions(+), 29 deletions(-) diff --git a/dix/getevents.c b/dix/getevents.c index c510122..fcac056 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -240,10 +240,11 @@ CreateClassesChangedEvent(EventList* event, * Rescale the coord between the two axis ranges. */ static int -rescaleValuatorAxis(int coord, AxisInfoPtr from, AxisInfoPtr to, +rescaleValuatorAxis(int coord, float remainder, float *remainder_return, AxisInfoPtr from, AxisInfoPtr to, int defmax) { - int fmin = 0, tmin = 0, fmax = defmax, tmax = defmax; + int fmin = 0, tmin = 0, fmax = defmax, tmax = defmax, coord_return; + float value; if(from && from->min_value < from->max_value) { fmin = from->min_value; @@ -254,14 +255,23 @@ rescaleValuatorAxis(int coord, AxisInfoPtr from, AxisInfoPtr to, tmax = to->max_value; } - if(fmin == tmin && fmax == tmax) + if(fmin == tmin && fmax == tmax) { + if (remainder_return) + *remainder_return = remainder; return coord; + } - if(fmax == fmin) /* avoid division by 0 */ + if(fmax == fmin) { /* avoid division by 0 */ + if (remainder_return) + *remainder_return = 0.0; return 0; + } - return lroundf(((float)(coord - fmin)) * (tmax - tmin) / - (fmax - fmin)) + tmin; + value = (coord + remainder - fmin) * (tmax - tmin) / (fmax - fmin) + tmin; + coord_return = lroundf(value); + if (remainder_return) + *remainder_return = value - coord_return; + return coord_return; } /** @@ -289,9 +299,11 @@ updateSlaveDeviceCoords(DeviceIntPtr master, DeviceIntPtr pDev) /* scale back to device coordinates */ if(pDev->valuator->numAxes > 0) - pDev->last.valuators[0] = rescaleValuatorAxis(pDev->last.valuators[0], NULL, pDev->valuator->axes + 0, scr->width); + pDev->last.valuators[0] = rescaleValuatorAxis(pDev->last.valuators[0], pDev->last.remainder[0], + &pDev->last.remainder[0], NULL, pDev->valuator->axes + 0, scr->width); if(pDev->valuator->numAxes > 1) - pDev->last.valuators[1] = rescaleValuatorAxis(pDev->last.valuators[1], NULL, pDev->valuator->axes + 1, scr->height); + pDev->last.valuators[1] = rescaleValuatorAxis(pDev->last.valuators[1], pDev->last.remainder[1], + &pDev->last.remainder[0], NULL, pDev->valuator->axes + 1, scr->height); /* calculate the other axis as well based on info from the old * slave-device. If the old slave had less axes than this one, @@ -304,6 +316,8 @@ updateSlaveDeviceCoords(DeviceIntPtr master, DeviceIntPtr pDev) else pDev->last.valuators[i] = rescaleValuatorAxis(pDev->last.valuators[i], + pDev->last.remainder[i], + &pDev->last.remainder[i], lastSlave->valuator->axes + i, pDev->valuator->axes + i, 0); } @@ -410,7 +424,7 @@ GetMotionHistory(DeviceIntPtr pDev, xTimecoord **buff, unsigned long start, /* scale to screen coords */ to = &core_axis; to->max_value = pScreen->width; - coord = rescaleValuatorAxis(coord, &from, to, pScreen->width); + coord = rescaleValuatorAxis(coord, 0.0, NULL, &from, to, pScreen->width); memcpy(corebuf, &coord, sizeof(INT16)); corebuf++; @@ -421,7 +435,7 @@ GetMotionHistory(DeviceIntPtr pDev, xTimecoord **buff, unsigned long start, memcpy(&coord, icbuf++, sizeof(INT32)); to->max_value = pScreen->height; - coord = rescaleValuatorAxis(coord, &from, to, pScreen->height); + coord = rescaleValuatorAxis(coord, 0.0, NULL, &from, to, pScreen->height); memcpy(corebuf, &coord, sizeof(INT16)); } else if (IsMaster(pDev)) @@ -456,7 +470,7 @@ GetMotionHistory(DeviceIntPtr pDev, xTimecoord **buff, unsigned long start, dflt = 0; /* scale from stored range into current range */ - coord = rescaleValuatorAxis(coord, &from, to, 0); + coord = rescaleValuatorAxis(coord, 0.0, NULL, &from, to, 0); memcpy(ocbuf, &coord, sizeof(INT32)); ocbuf++; } @@ -745,26 +759,36 @@ accelPointer(DeviceIntPtr dev, int first, int num, int *valuators, CARD32 ms) * @param dev The device to be moved. * @param x Pointer to current x-axis value, may be modified. * @param y Pointer to current y-axis value, may be modified. + * @param x_frac Fractional part of current x-axis value, may be modified. + * @param y_frac Fractional part of current y-axis value, may be modified. * @param scr Screen the device's sprite is currently on. * @param screenx Screen x coordinate the sprite is on after the update. * @param screeny Screen y coordinate the sprite is on after the update. + * @param screenx_frac Fractional part of screen x coordinate, as above. + * @param screeny_frac Fractional part of screen y coordinate, as above. */ static void -positionSprite(DeviceIntPtr dev, int *x, int *y, - ScreenPtr scr, int *screenx, int *screeny) +positionSprite(DeviceIntPtr dev, int *x, int *y, float x_frac, float y_frac, + ScreenPtr scr, int *screenx, int *screeny, float *screenx_frac, float *screeny_frac) { int old_screenx, old_screeny; /* scale x&y to screen */ - if (dev->valuator->numAxes > 0) - *screenx = rescaleValuatorAxis(*x, dev->valuator->axes + 0, NULL, scr->width); - else + if (dev->valuator->numAxes > 0) { + *screenx = rescaleValuatorAxis(*x, x_frac, screenx_frac, + dev->valuator->axes + 0, NULL, scr->width); + } else { *screenx = dev->last.valuators[0]; + *screenx_frac = dev->last.remainder[0]; + } - if (dev->valuator->numAxes > 1 ) - *screeny = rescaleValuatorAxis(*y, dev->valuator->axes + 1, NULL, scr->height); - else + if (dev->valuator->numAxes > 1) { + *screeny = rescaleValuatorAxis(*y, y_frac, screeny_frac, + dev->valuator->axes + 1, NULL, scr->height); + } else { *screeny = dev->last.valuators[1]; + *screeny_frac = dev->last.remainder[1]; + } old_screenx = *screenx; old_screeny = *screeny; @@ -773,27 +797,31 @@ positionSprite(DeviceIntPtr dev, int *x, int *y, miPointerSetPosition(dev, screenx, screeny); if (dev->u.master) { - dev->u.master->last.valuators[0] = dev->last.valuators[0]; - dev->u.master->last.valuators[1] = dev->last.valuators[1]; + dev->u.master->last.valuators[0] = *screenx; + dev->u.master->last.valuators[1] = *screeny; + dev->u.master->last.remainder[0] = *screenx_frac; + dev->u.master->last.remainder[1] = *screeny_frac; } /* Crossed screen? Scale back to device coordiantes */ if(*screenx != old_screenx) { scr = miPointerGetScreen(dev); - *x = rescaleValuatorAxis(*screenx, NULL, + *x = rescaleValuatorAxis(*screenx, *screenx_frac, &x_frac, NULL, dev->valuator->axes + 0, scr->width); } if(*screeny != old_screeny) { scr = miPointerGetScreen(dev); - *y = rescaleValuatorAxis(*screeny, NULL, + *y = rescaleValuatorAxis(*screeny, *screeny_frac, &y_frac, NULL, dev->valuator->axes + 1, scr->height); } /* dropy x/y (device coordinates) back into valuators for next event */ dev->last.valuators[0] = *x; dev->last.valuators[1] = *y; + dev->last.remainder[0] = x_frac; + dev->last.remainder[1] = y_frac; } /** @@ -1018,6 +1046,7 @@ GetPointerEvents(EventList *events, DeviceIntPtr pDev, int type, int buttons, RawDeviceEvent *raw; int x = 0, y = 0, /* device coords */ cx, cy; /* only screen coordinates */ + float x_frac = 0.0, y_frac = 0.0, cx_frac, cy_frac; ScreenPtr scr = miPointerGetScreen(pDev); /* refuse events from disabled devices */ @@ -1050,26 +1079,31 @@ GetPointerEvents(EventList *events, DeviceIntPtr pDev, int type, int buttons, { if (num_valuators >= 1 && first_valuator == 0) - valuators[0] = rescaleValuatorAxis(valuators[0], NULL, + valuators[0] = rescaleValuatorAxis(valuators[0], 0.0, &x_frac, NULL, pDev->valuator->axes + 0, scr->width); if (first_valuator <= 1 && num_valuators >= (2 - first_valuator)) - valuators[1 - first_valuator] = rescaleValuatorAxis(valuators[1 - first_valuator], NULL, + valuators[1 - first_valuator] = rescaleValuatorAxis(valuators[1 - first_valuator], 0.0, &y_frac, NULL, pDev->valuator->axes + 1, scr->height); } moveAbsolute(pDev, &x, &y, first_valuator, num_valuators, valuators); } else { - if (flags & POINTER_ACCELERATE) + if (flags & POINTER_ACCELERATE) { accelPointer(pDev, first_valuator, num_valuators, valuators, ms); + /* The pointer acceleration code modifies the fractional part + * in-place, so we need to extract this information first */ + x_frac = pDev->last.remainder[0]; + y_frac = pDev->last.remainder[1]; + } moveRelative(pDev, &x, &y, first_valuator, num_valuators, valuators); } set_raw_valuators(raw, first_valuator, num_valuators, valuators, raw->valuators.data); - positionSprite(pDev, &x, &y, scr, &cx, &cy); + positionSprite(pDev, &x, &y, x_frac, y_frac, scr, &cx, &cy, &cx_frac, &cy_frac); updateHistory(pDev, first_valuator, num_valuators, ms); /* Update the valuators with the true value sent to the client*/ @@ -1102,8 +1136,8 @@ GetPointerEvents(EventList *events, DeviceIntPtr pDev, int type, int buttons, event->root_x = cx; /* root_x/y always in screen coords */ event->root_y = cy; - event->root_x_frac = pDev->last.remainder[0]; - event->root_y_frac = pDev->last.remainder[1]; + event->root_x_frac = cx_frac; + event->root_y_frac = cy_frac; set_valuators(pDev, event, first_valuator, num_valuators, valuators); -- 1.6.3.1
_______________________________________________ xorg-devel mailing list [email protected] http://lists.x.org/mailman/listinfo/xorg-devel
