[PATCH 1/2] render/input: set anicursor hooks to control rendering

[Vignatti Tiago (Nokia-D/Helsinki)]

Hi,

On Wed, Aug 26, 2009 at 03:16:46AM +0200, ext Peter Hutterer wrote:
> On Tue, Aug 25, 2009 at 05:03:15PM +0300, Tiago Vignatti wrote:
> > +void
> > +AnimCursorStart(ScreenPtr pScreen)
> > +{
> > +    AnimCurScreenPtr    as;
> > +
> > +    as = (AnimCurScreenPtr) xalloc (sizeof (AnimCurScreenRec));
> > +    if (!as)
> > +        FatalError("AnimCursor: failed to allocate memory\n");
> > +
> > +    /* AnimCursorStart can be stacked without problems. For this we do a
> > +     * comparision on BlockHandler function, guaranteeing that such function
> > +     * will not point to ifself. Address function comparision is not the best
> > +     * way, but eh... */
> 
> any code that is "... not the best way, but eh..." usually guarantees that
> sometime, someone will spend a significant portion of time hating you. at
> least mark spots with a FIXME or XXX so they are immediately obvious.

Agreed. A simply counter solve this.

 
> > +    if (pScreen->BlockHandler != AnimCurScreenBlockHandler) {
> > +        Wrap(as, pScreen, CloseScreen, AnimCurCloseScreen);
> > +        Wrap(as, pScreen, BlockHandler, AnimCurScreenBlockHandler);
> > +        Wrap(as, pScreen, CursorLimits, AnimCurCursorLimits);
> > +        Wrap(as, pScreen, DisplayCursor, AnimCurDisplayCursor);
> > +        Wrap(as, pScreen, SetCursorPosition, AnimCurSetCursorPosition);
> > +        Wrap(as, pScreen, RealizeCursor, AnimCurRealizeCursor);
> > +        Wrap(as, pScreen, UnrealizeCursor, AnimCurUnrealizeCursor);
> > +        Wrap(as, pScreen, RecolorCursor, AnimCurRecolorCursor);
> > +        SetAnimCurScreen(pScreen,as);
> > +    }
> > +}
> 
> This seems like overkill. The issue at hand are the Block- and
> WakeupHandlers, the rest shouldn't have much impact. I see two simpler
> solutions:
> - just removing the block handler from the stack so it doesn't get called if
>   the cursor is invisible.

that's exactly what I've done. 

At first it seems overkill to unwrap the functions when the cursor is
invisible. But this is the only way that I see to remove the handler, given
that we're already doing a confusing stack of functions there. 


> - using Start/Stop to set a flag to skip the loop inside the block handler
>   and basically make the block handler a noop. I think this would be much
>   more readable, debuggable and has the same effect.

I really prefer to just turn off all this rendering functions. It doesn't make
sense to call them if the cursor is invisible. So I'd go for the first
solution.


> no _X_EXPORTS please. Init and CursorCreate shouldn't be _X_EXPORT either,
> while we're at it.

agreed.


I'm waiting you comments, Peter. Thanks,

            Tiago