[PATCHv10 4/5] dri2: Support the DRI2InvalidateBuffers event.

[Peter Hutterer]

On Fri, Mar 05, 2010 at 12:47:31PM +0100, Francisco Jerez wrote:
> Peter Hutterer <peter.hutterer at who-t.net> writes:
> > [...]
> >   
> >> +static void
> >> +DRI2ConfigNotify(WindowPtr pWin, int x, int y, int w, int h, int bw,
> >> +		 WindowPtr pSib)
> >> +{
> >> +    DrawablePtr pDraw = (DrawablePtr)pWin;
> >> +    ScreenPtr pScreen = pDraw->pScreen;
> >> +    DRI2ScreenPtr ds = DRI2GetScreen(pScreen);
> >> +    DRI2DrawablePtr dd = DRI2GetDrawable(pDraw);
> >> +
> >> +    if (ds->ConfigNotify) {
> >> +	UNWRAP(pScreen, ds, ConfigNotify);
> >> +	(*pScreen->ConfigNotify)(pWin, x, y, w, h, bw, pSib);
> >> +	WRAP(pScreen, ds, ConfigNotify, DRI2ConfigNotify);
> >> +    }
> >
> > I don't think this is correct just yet. You're always forcing
> > DRI2ConfigNotify back after the wrap. What you should be doing though is
> > popping back whatever was there before. Otherwise, having a different
> > nesting order will screw up the wrapping code.
> >
> > See 664ac92d8bbe956dd6fd80fac5dc3161028803b2 for a case where this has
> > bitten us once already. this may not be possible with the current code, but
> > better be safe than sorry.
> >
> 
> I thought the whole point of this unwrap-call-and-rewrap model was that
> each link in the chain can safely assume that it'll be at the top of the
> stack whenever it's called.

I thought the point of the wrapping is that when the code is called, the
struct is restored to whatever it was for this particular layer that's to be
called next. that is sort-of the same as what you say, except that (as you
point out) if you have cross-callers interesting things happen.

> If we cannot rely on this assumption anyway, could you explain me how
> the change you're proposing is better than a plain:
> +    if (ds->ConfigNotify)
> +	(*ds->ConfigNotify)(pWin, x, y, w, h, bw, pSib);
> +

if you call into ds->ConfigNotify without wrapping the screen you can be
exposed to a loop. if anyone in the next layer calls pScreen->ConfigNotify,
you now jump up a layer and loop around again (?)

anyway, I've just checked the cursor rendering code in misprite.c and that
also hardcodes the wrapping part. so I guess you're right - you'll be ok if
you avoid the cross calls without the chain.

Cheers,
  Peter

> IIUC, the roots of the problem fixed by 664ac92d8 were functions like
> ProcXFixesHideCursor that were calling CursorDisplayCursor directly
> instead of invoking the whole chain.