[PATCH 1/2] Replace miSpriteCursorFuncRec with direct calls to midispcur.c.

Peter Hutterer peter.hutterer at who-t.net
Wed May 19 15:14:22 PDT 2010


On Wed, May 19, 2010 at 09:45:23AM -0700, Jamey Sharp wrote:
> On Wed, May 19, 2010 at 5:06 AM, Peter Hutterer
> <peter.hutterer at who-t.net> wrote:
> >> @@ -861,7 +850,7 @@ miSpriteSetCursor (DeviceIntPtr pDev, ScreenPtr pScreen,
> >>               pointer->saved.y1 -= dy;
> >>               pointer->saved.x2 -= dx;
> >>               pointer->saved.y2 -= dy;
> >> -             (void) (*pScreenPriv->funcs->ChangeSave) (pScreen,
> >> +             (void) miDCChangeSave(pScreen,
> >>                               pointer->saved.x1,
> >>                               pointer->saved.y1,
> >>                                  pointer->saved.x2 -
> >> @@ -870,7 +859,7 @@ miSpriteSetCursor (DeviceIntPtr pDev, ScreenPtr pScreen,
> >>                                  pointer->saved.y1,
> >>                               dx, dy);
> >>           }
> >> -         (void) (*pScreenPriv->funcs->MoveCursor) (pScreen, pCursor,
> >> +         (void) miDCMoveCursor(pScreen, pCursor,
> >
> > I'd say this and the one cast above are superfluous.
> 
> Worse, they're in an "#if 0" block: "FIXME: Disabled for MPX, should
> be rewritten". :-) It's also full of bad indentation.

I wonder who'd do things like that... ;)

for the archives:
The old code had some special case for if the cursor moved _within_ the
bounds of the saved backing store. In that case, the server wouldn't
undisplay and redisplay, it would simply shift the cursor image within that
backing store and overwrite.
That's fine for one cursor but gave me some headaches for multiple cursors,
hence the if 0.

> I'd rather not touch that. I just wanted to record what the intent of
> those calls was once the function pointers are gone.
> 
> >> @@ -954,13 +940,7 @@ static void
> >>  miSpriteDeviceCursorCleanup(DeviceIntPtr pDev, ScreenPtr pScreen)
> >>  {
> >>      if (DevHasCursor(pDev))
> >> -    {
> >> -        miSpriteScreenPtr pScreenPriv;
> >> -        pScreenPriv = dixLookupPrivate(&pScreen->devPrivates,
> >> -                                       miSpriteScreenKey);
> >> -
> >> -        (*pScreenPriv->funcs->DeviceCursorCleanup)(pDev, pScreen);
> >> -    }
> >> +        miDCDeviceCleanup(pDev, pScreen);
> >
> > indentation
> 
> I don't know what rule you want me to follow there. This file has
> all-spaces in some functions, and mixed spaces/tabs in others. I just
> preserved the indentation style that line had already. Anyway, it
> looks like spaces-only dominates in this file, in which case I think
> this hunk is correct?

yes, looks correct. I think I might have started writing here by accident
for the message below (fixing up the indentation). Either that, or I was
already cross-eyed last night. so just ignore this bit.

Cheers,
  Peter



More information about the xorg-devel mailing list