[PATCHv10 4/5] dri2: Support the DRI2InvalidateBuffers event.
Francisco Jerez
currojerez at riseup.net
Fri Mar 5 03:47:31 PST 2010
Peter Hutterer <peter.hutterer at who-t.net> writes:
> On Mon, Mar 01, 2010 at 08:19:44PM +0100, Francisco Jerez wrote:
>> Bumps the supported DRI2 protocol version.
>>
>> Signed-off-by: Francisco Jerez <currojerez at riseup.net>
>> ---
>> v10: Move resource allocation to dri2ext.c.
>>
>> configure.ac | 2 +-
>> hw/xfree86/dri2/dri2.c | 122 +++++++++++++++++++++++++++++++++++++++++++
>> hw/xfree86/dri2/dri2.h | 6 ++
>> hw/xfree86/dri2/dri2ext.c | 75 ++++++++++++++++++++++++++-
>> include/protocol-versions.h | 2 +-
>> 5 files changed, 204 insertions(+), 3 deletions(-)
>>
>
> [...]
>
>> +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.
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);
+
IIUC, the roots of the problem fixed by 664ac92d8 were functions like
ProcXFixesHideCursor that were calling CursorDisplayCursor directly
instead of invoking the whole chain.
>> +
>> + if (dd && (dd->width != w || dd->height != h))
>> + DRI2InvalidateDrawable(pDraw);
>> +}
>> +
>> Bool
>> DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
>> {
>> @@ -869,6 +989,8 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
>>
>> dixSetPrivate(&pScreen->devPrivates, dri2ScreenPrivateKey, ds);
>>
>> + WRAP(pScreen, ds, ConfigNotify, DRI2ConfigNotify);
>
> this one is fine, since you're initializing it here.
>
> Cheers,
> Peter
>
>> +
>> xf86DrvMsg(pScreen->myNum, X_INFO, "[DRI2] Setup complete\n");
>> for (i = 0; i < sizeof(driverTypeNames) / sizeof(driverTypeNames[0]); i++) {
>> if (i < ds->numDrivers && ds->driverNames[i]) {
>> diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
>> index 1c8626b..1d5190f 100644
>> --- a/hw/xfree86/dri2/dri2.h
>> +++ b/hw/xfree86/dri2/dri2.h
>> @@ -265,4 +265,10 @@ extern _X_EXPORT void DRI2WaitMSCComplete(ClientPtr client, DrawablePtr pDraw,
>> int frame, unsigned int tv_sec,
>> unsigned int tv_usec);
>>
>> +extern _X_EXPORT int DRI2TrackClient(DrawablePtr pDraw, int id,
>> + void (*invalidate)(DrawablePtr, void *),
>> + void (*destroy)(DrawablePtr, void *),
>> + void *priv);
>> +extern _X_EXPORT void DRI2UntrackClient(DrawablePtr pDraw, int id);
>> +
>> #endif
>> diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c
>> index bd92fd3..5644fe5 100644
>> --- a/hw/xfree86/dri2/dri2ext.c
>> +++ b/hw/xfree86/dri2/dri2ext.c
>> @@ -52,6 +52,7 @@
>>
>> static ExtensionEntry *dri2Extension;
>> static RESTYPE dri2DrawableRes;
>> +static RESTYPE dri2ClientRefRes;
>>
>> static Bool
>> validDrawable(ClientPtr client, XID drawable, Mask access_mode,
>> @@ -197,6 +198,60 @@ ProcDRI2DestroyDrawable(ClientPtr client)
>> return client->noClientException;
>> }
>>
>> +static void
>> +DRI2InvalidateEvent(DrawablePtr pDraw, void *priv)
>> +{
>> + XID *id = priv;
>> + ClientPtr client = clients[CLIENT_ID(*id)];
>> + xDRI2InvalidateBuffers event;
>> +
>> + if (!client || client->clientGone)
>> + return;
>> +
>> + event.type = DRI2EventBase + DRI2_InvalidateBuffers;
>> + event.sequenceNumber = client->sequence;
>> + event.drawable = pDraw->id;
>> +
>> + WriteEventsToClient(client, 1, (xEvent *)&event);
>> +}
>> +
>> +static void
>> +DRI2InvalidateDestroy(DrawablePtr pDraw, void *priv)
>> +{
>> + XID *id = priv;
>> +
>> + FreeResource(*id, dri2ClientRefRes);
>> + xfree(id);
>> +}
>> +
>> +static int
>> +track_client(ClientPtr client, DrawablePtr pDraw)
>> +{
>> + XID *id;
>> + int ret = BadAlloc;
>> +
>> + id = xalloc(sizeof(*id));
>> + if (!id)
>> + goto fail;
>> +
>> + /* Allocate a new resource for the client. */
>> + *id = FakeClientID(client->index);
>> + if (!AddResource(*id, dri2ClientRefRes, pDraw))
>> + goto fail;
>> +
>> + ret = DRI2TrackClient(pDraw, client->index, DRI2InvalidateEvent,
>> + DRI2InvalidateDestroy, id);
>> + if (ret)
>> + goto fail;
>> +
>> + return Success;
>> +
>> +fail:
>> + if (id)
>> + DRI2InvalidateDestroy(pDraw, id);
>> +
>> + return ret;
>> +}
>>
>> static void
>> send_buffers_reply(ClientPtr client, DrawablePtr pDrawable,
>> @@ -266,6 +321,9 @@ ProcDRI2GetBuffers(ClientPtr client)
>> buffers = DRI2GetBuffers(pDrawable, &width, &height,
>> attachments, stuff->count, &count);
>>
>> + status = track_client(client, pDrawable);
>> + if (status)
>> + return status;
>>
>> send_buffers_reply(client, pDrawable, buffers, count, width, height);
>>
>> @@ -293,6 +351,10 @@ ProcDRI2GetBuffersWithFormat(ClientPtr client)
>> buffers = DRI2GetBuffersWithFormat(pDrawable, &width, &height,
>> attachments, stuff->count, &count);
>>
>> + status = track_client(client, pDrawable);
>> + if (status)
>> + return status;
>> +
>> send_buffers_reply(client, pDrawable, buffers, count, width, height);
>>
>> return client->noClientException;
>> @@ -629,16 +691,27 @@ static int DRI2DrawableGone(pointer p, XID id)
>> return Success;
>> }
>>
>> +static int
>> +DRI2ClientRefGone(pointer p, XID id)
>> +{
>> + DRI2UntrackClient(p, CLIENT_ID(id));
>> + return Success;
>> +}
>> +
>> int DRI2EventBase;
>>
>> static void
>> DRI2ExtensionInit(void)
>> {
>> dri2DrawableRes = CreateNewResourceType(DRI2DrawableGone, "DRI2Drawable");
>> -
>> if (!dri2DrawableRes)
>> return;
>>
>> + dri2ClientRefRes =
>> + CreateNewResourceType(DRI2ClientRefGone, "DRI2ClientRef");
>> + if (!dri2ClientRefRes)
>> + return;
>> +
>> dri2Extension = AddExtension(DRI2_NAME,
>> DRI2NumberEvents,
>> DRI2NumberErrors,
>> diff --git a/include/protocol-versions.h b/include/protocol-versions.h
>> index c74b7fa..c425eef 100644
>> --- a/include/protocol-versions.h
>> +++ b/include/protocol-versions.h
>> @@ -53,7 +53,7 @@
>>
>> /* DRI2 */
>> #define SERVER_DRI2_MAJOR_VERSION 1
>> -#define SERVER_DRI2_MINOR_VERSION 2
>> +#define SERVER_DRI2_MINOR_VERSION 3
>>
>> /* Generic event extension */
>> #define SERVER_GE_MAJOR_VERSION 1
>> --
>> 1.6.4.4
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20100305/9ee31e9e/attachment-0001.pgp>
More information about the xorg-devel
mailing list