[PATCH 2/2] damageext: Xineramify (v4)
Aaron Plattner
aplattner at nvidia.com
Tue Oct 8 16:43:28 PDT 2013
On 10/08/2013 03:28 PM, Aaron Plattner wrote:
> On 10/02/2013 08:56 AM, Adam Jackson wrote:
>> v4: Use the new PostDispatchCallback, to avoid recursing through
>> FlushCallback -> WriteToClient -> FlushCallback -> ...
>>
>> Screen 0 holds the "real" damage for all drawable types; the window
>> report hooks for other screens look up screen 0 and pile on. Therefore
>> we don't need to wrap Subtract, though we do have to be careful how we
>> subtract since the internal DamageSubtract clips to the per-screen root
>> window. The real compexity is the cleverness required for deferring
>> writing the events, but there's no getting around that.
>>
>> Add is probably (still) somewhat broken since it will only hit screen 0,
>> but Add really only exists for DRI1's sake, and DRI1 disables itself
>> with Xinerama enabled anyway. In the absence of a use case, I'm leaving
>> it unwrapped under Xinerama; if someone wants to define how it ought to
>> work, be my guest.
>>
>> Signed-off-by: Adam Jackson <ajax at redhat.com>
>> ---
>> Xext/panoramiX.c | 3 +
>> Xext/panoramiX.h | 3 +
>> damageext/damageext.c | 311 +++++++++++++++++++++++++++++++++++++++++------
>> damageext/damageextint.h | 4 +
>> 4 files changed, 283 insertions(+), 38 deletions(-)
>>
>> diff --git a/Xext/panoramiX.c b/Xext/panoramiX.c
>> index 7f888e3..35f5ff6 100644
>> --- a/Xext/panoramiX.c
>> +++ b/Xext/panoramiX.c
>> @@ -56,6 +56,7 @@ Equipment Corporation.
>> #ifdef XFIXES
>> #include "xfixesint.h"
>> #endif
>> +#include "damageextint.h"
>> #ifdef COMPOSITE
>> #include "compint.h"
>> #endif
>> @@ -586,6 +587,7 @@ PanoramiXExtensionInit(void)
>> #ifdef XFIXES
>> PanoramiXFixesInit();
>> #endif
>> + PanoramiXDamageInit();
>> #ifdef COMPOSITE
>> PanoramiXCompositeInit();
>> #endif
>> @@ -893,6 +895,7 @@ PanoramiXResetProc(ExtensionEntry * extEntry)
>> #ifdef XFIXES
>> PanoramiXFixesReset();
>> #endif
>> + PanoramiXDamageReset();
>> #ifdef COMPOSITE
>> PanoramiXCompositeReset ();
>> #endif
>> diff --git a/Xext/panoramiX.h b/Xext/panoramiX.h
>> index 6578dfa..b06fce4 100644
>> --- a/Xext/panoramiX.h
>> +++ b/Xext/panoramiX.h
>> @@ -64,6 +64,9 @@ typedef struct {
>> struct {
>> Bool root;
>> } pict;
>> + struct {
>> + Bool queued;
>> + } damage;
>> char raw_data[4];
>> } u;
>> } PanoramiXRes;
>> diff --git a/damageext/damageext.c b/damageext/damageext.c
>> index cf6b63b..bd7aea4 100644
>> --- a/damageext/damageext.c
>> +++ b/damageext/damageext.c
>> @@ -1,5 +1,6 @@
>> /*
>> * Copyright © 2002 Keith Packard
>> + * Copyright 2013 Red Hat, Inc.
>> *
>> * Permission to use, copy, modify, distribute, and sell this software and its
>> * documentation for any purpose is hereby granted without fee, provided that
>> @@ -28,6 +29,15 @@
>> #include "protocol-versions.h"
>> #include "extinit.h"
>>
>> +#ifdef PANORAMIX
>> +#include "panoramiX.h"
>> +#include "panoramiXsrv.h"
>> +
>> +static RESTYPE XRT_DAMAGE;
>> +static int (*PanoramiXSaveDamageVector[XDamageNumberRequests]) (ClientPtr);
>> +
>> +#endif
>> +
>> static unsigned char DamageReqCode;
>> static int DamageEventBase;
>> static RESTYPE DamageExtType;
>> @@ -38,10 +48,21 @@ static DevPrivateKeyRec DamageClientPrivateKeyRec;
>> #define DamageClientPrivateKey (&DamageClientPrivateKeyRec)
>>
>> static void
>> +DamageNoteCritical(ClientPtr pClient)
>> +{
>> + DamageClientPtr pDamageClient = GetDamageClient(pClient);
>> +
>> + /* Composite extension marks clients with manual Subwindows as critical */
>> + if (pDamageClient->critical > 0) {
>> + SetCriticalOutputPending();
>> + pClient->smart_priority = SMART_MAX_PRIORITY;
>> + }
>> +}
>> +
>> +static void
>> DamageExtNotify(DamageExtPtr pDamageExt, BoxPtr pBoxes, int nBoxes)
>> {
>> ClientPtr pClient = pDamageExt->pClient;
>> - DamageClientPtr pDamageClient = GetDamageClient(pClient);
>> DrawablePtr pDrawable = pDamageExt->pDrawable;
>> xDamageNotifyEvent ev;
>> int i;
>> @@ -51,7 +72,7 @@ DamageExtNotify(DamageExtPtr pDamageExt, BoxPtr pBoxes, int nBoxes)
>> .type = DamageEventBase + XDamageNotify,
>> .level = pDamageExt->level,
>> .drawable = pDamageExt->drawable,
>> - .damage = pDamageExt->id,
>> + .damage = pDamageExt->report_id,
>> .timestamp = currentTime.milliseconds,
>> .geometry.x = pDrawable->x,
>> .geometry.y = pDrawable->y,
>> @@ -77,11 +98,8 @@ DamageExtNotify(DamageExtPtr pDamageExt, BoxPtr pBoxes, int nBoxes)
>> ev.area.height = pDrawable->height;
>> WriteEventsToClient(pClient, 1, (xEvent *) &ev);
>> }
>> - /* Composite extension marks clients with manual Subwindows as critical */
>> - if (pDamageClient->critical > 0) {
>> - SetCriticalOutputPending();
>> - pClient->smart_priority = SMART_MAX_PRIORITY;
>> - }
>> +
>> + DamageNoteCritical(pClient);
>> }
>>
>> static void
>> @@ -163,19 +181,56 @@ ProcDamageQueryVersion(ClientPtr client)
>> return Success;
>> }
>>
>> +static DamageExtPtr
>> +DamageExtCreate(DrawablePtr pDrawable, DamageReportLevel level,
>> + ClientPtr client, XID id, XID drawable, XID report_id,
>> + DamageReportFunc reportFunc)
>> +{
>> + DamageExtPtr pDamageExt = malloc(sizeof(DamageExtRec));
>> + if (!pDamageExt)
>> + return NULL;
>> +
>> + pDamageExt->id = id;
>> + pDamageExt->report_id = report_id;
>> + pDamageExt->drawable = drawable;
>> + pDamageExt->pDrawable = pDrawable;
>> + pDamageExt->level = level;
>> + pDamageExt->pClient = client;
>> + pDamageExt->pDamage = DamageCreate(reportFunc, DamageExtDestroy, level,
>> + FALSE, pDrawable->pScreen, pDamageExt);
>> + if (!pDamageExt->pDamage) {
>> + free(pDamageExt);
>> + return NULL;
>> + }
>> +
>> + if (!AddResource(id, DamageExtType, (pointer) pDamageExt))
>> + return NULL;
>> +
>> + DamageSetReportAfterOp(pDamageExt->pDamage, TRUE);
>> + DamageRegister(pDrawable, pDamageExt->pDamage);
>> +
>> + if (pDrawable->type == DRAWABLE_WINDOW) {
>> + RegionPtr pRegion = &((WindowPtr) pDrawable)->borderClip;
>> + RegionTranslate(pRegion, -pDrawable->x, -pDrawable->y);
>> + DamageReportDamage(pDamageExt->pDamage, pRegion);
>> + RegionTranslate(pRegion, pDrawable->x, pDrawable->y);
>> + }
>> +
>> + return pDamageExt;
>> +}
>> +
>> static int
>> -ProcDamageCreate(ClientPtr client)
>> +doDamageCreate(ClientPtr client, XID reportDrawable, XID reportDamage,
>> + DamageReportFunc reportFunc)
>> {
>> DrawablePtr pDrawable;
>> DamageExtPtr pDamageExt;
>> DamageReportLevel level;
>> - RegionPtr pRegion;
>> int rc;
>>
>> REQUEST(xDamageCreateReq);
>> -
>> REQUEST_SIZE_MATCH(xDamageCreateReq);
>
> Doesn't hurt, but the callers have to do this anyway making this
> redundant, don't they?
>
>> - LEGAL_NEW_RESOURCE(stuff->damage, client);
>> +
>> rc = dixLookupDrawable(&pDrawable, stuff->drawable, client, 0,
>> DixGetAttrAccess | DixReadAccess);
>> if (rc != Success)
>> @@ -199,39 +254,25 @@ ProcDamageCreate(ClientPtr client)
>> return BadValue;
>> }
>>
>> - pDamageExt = malloc(sizeof(DamageExtRec));
>> + pDamageExt = DamageExtCreate(pDrawable, level, client, stuff->damage,
>> + reportDrawable, reportDamage, reportFunc);
>> if (!pDamageExt)
>> return BadAlloc;
>> - pDamageExt->id = stuff->damage;
>> - pDamageExt->drawable = stuff->drawable;
>> - pDamageExt->pDrawable = pDrawable;
>> - pDamageExt->level = level;
>> - pDamageExt->pClient = client;
>> - pDamageExt->pDamage = DamageCreate(DamageExtReport,
>> - DamageExtDestroy,
>> - level,
>> - FALSE, pDrawable->pScreen, pDamageExt);
>> - if (!pDamageExt->pDamage) {
>> - free(pDamageExt);
>> - return BadAlloc;
>> - }
>> - if (!AddResource(stuff->damage, DamageExtType, (pointer) pDamageExt))
>> - return BadAlloc;
>> -
>> - DamageSetReportAfterOp(pDamageExt->pDamage, TRUE);
>> - DamageRegister(pDamageExt->pDrawable, pDamageExt->pDamage);
>> -
>> - if (pDrawable->type == DRAWABLE_WINDOW) {
>> - pRegion = &((WindowPtr) pDrawable)->borderClip;
>> - RegionTranslate(pRegion, -pDrawable->x, -pDrawable->y);
>> - DamageReportDamage(pDamageExt->pDamage, pRegion);
>> - RegionTranslate(pRegion, pDrawable->x, pDrawable->y);
>> - }
>>
>> return Success;
>> }
>>
>> static int
>> +ProcDamageCreate(ClientPtr client)
>> +{
>> + REQUEST(xDamageCreateReq);
>> + REQUEST_SIZE_MATCH(xDamageCreateReq);
>> + LEGAL_NEW_RESOURCE(stuff->damage, client);
>> + return doDamageCreate(client, stuff->drawable, stuff->damage,
>> + DamageExtReport);
>> +}
>> +
>> +static int
>> ProcDamageDestroy(ClientPtr client)
>> {
>> REQUEST(xDamageDestroyReq);
>> @@ -243,6 +284,24 @@ ProcDamageDestroy(ClientPtr client)
>> return Success;
>> }
>>
>> +/*
>> + * DamageSubtract clips to the window border, which is wrong for Xinerama
>> + * since that clips you to the ScreenRec.
>
> damageproto.txt doesn't seem to say anything about the region being
> clipped so this seems fine to me, but Keith should probably be the final
> arbiter on that.
>
>> + */
>> +static Bool
>> +DamageExtSubtract(DamagePtr pDamage, const RegionPtr pRegion)
>> +{
>> +#ifdef PANORAMIX
>> + if (!noPanoramiXExtension) {
>> + RegionPtr damageRegion = DamageRegion(pDamage);
>> + RegionSubtract(damageRegion, damageRegion, pRegion);
>> + return RegionNotEmpty(damageRegion);
>> + }
>> +#endif
>> +
>> + return DamageSubtract(pDamage, pRegion);
>> +}
>> +
>> static int
>> ProcDamageSubtract(ClientPtr client)
>> {
>> @@ -262,7 +321,7 @@ ProcDamageSubtract(ClientPtr client)
>> if (pRepair) {
>> if (pParts)
>> RegionIntersect(pParts, DamageRegion(pDamage), pRepair);
>> - if (DamageSubtract(pDamage, pRepair))
>> + if (DamageExtSubtract(pDamage, pRepair))
>> DamageExtReport(pDamage, DamageRegion(pDamage),
>> (void *) pDamageExt);
>> }
>> @@ -272,6 +331,7 @@ ProcDamageSubtract(ClientPtr client)
>> DamageEmpty(pDamage);
>> }
>> }
>> +
>> return Success;
>> }
>>
>> @@ -468,6 +528,176 @@ SDamageNotifyEvent(xDamageNotifyEvent * from, xDamageNotifyEvent * to)
>> cpswaps(from->geometry.height, to->geometry.height);
>> }
>>
>> +#ifdef PANORAMIX
>> +
>> +static void
>> +damageDispatchCallback(CallbackListPtr *cbl, void *closure, void *unused)
>> +{
>> + DamageExtPtr pDamageExt = closure;
>> + RegionPtr pRegion = DamageRegion(pDamageExt->pDamage);
>> + PanoramiXRes *damage = NULL;
>> +
>> + DamageExtReport(pDamageExt->pDamage, pRegion, pDamageExt);
>> + DeleteCallback(&PostDispatchCallback, damageDispatchCallback, pDamageExt);
>> +
>> + dixLookupResourceByType((void **)&damage, pDamageExt->id, XRT_DAMAGE,
>> + serverClient, DixWriteAccess);
>> +
>> + if (damage)
>> + damage->u.damage.queued = FALSE;
>> +}
>> +
>> +/* for screen 0 */
>> +static void
>> +PanoramiXDamageQueue(DamagePtr pDamage, RegionPtr pRegion, void *closure)
>> +{
>> + DamageExtPtr pDamageExt = closure;
>> + PanoramiXRes *damage = NULL;
>> +
>> + /* happens on unmap? sigh xinerama */
>> + if (RegionNil(pRegion))
>> + return;
>> +
>> + dixLookupResourceByType((void **)&damage, pDamageExt->report_id, XRT_DAMAGE,
>> + serverClient, DixWriteAccess);
>> +
>> + if (damage) {
>> + if (!damage->u.damage.queued) {
>> + AddCallback(&PostDispatchCallback, damageDispatchCallback,
>> + pDamageExt);
>> + damage->u.damage.queued = TRUE;
>> + }
>> + }
>> +
>> + DamageNoteCritical(pDamageExt->pClient);
>> +}
>> +
>> +/* for screens 1 to n */
>> +static void
>> +PanoramiXDamageAccumulate(DamagePtr pDamage, RegionPtr pRegion, void *closure)
>> +{
>> + DamageExtPtr pDamageExt = closure, pDamageExt0 = NULL;
>> + PanoramiXRes *damage = NULL;
>> +
>> + /* happens on unmap? sigh xinerama */
>> + if (RegionNil(pRegion))
>> + return;
>> +
>> + dixLookupResourceByType((void **)&damage, pDamageExt->report_id, XRT_DAMAGE,
>> + serverClient, DixWriteAccess);
>> +
>> + if (damage) {
>> + dixLookupResourceByType((void **)&pDamageExt0, damage->info[0].id,
>> + DamageExtType, serverClient, DixWriteAccess);
>> +
>> + if (pDamageExt0) {
>> + DamageReportDamage(pDamageExt0->pDamage, pRegion);
>> + DamageEmpty(pDamageExt->pDamage);
>> + }
>> + }
>> +}
>> +
>> +static int
>> +PanoramiXDamageCreate(ClientPtr client)
>> +{
>> + PanoramiXRes *draw, *damage;
>> + int i, rc;
>> +
>> + REQUEST(xDamageCreateReq);
>> +
>> + REQUEST_SIZE_MATCH(xDamageCreateReq);
>> + LEGAL_NEW_RESOURCE(stuff->damage, client);
>> + rc = dixLookupResourceByClass((void **)&draw, stuff->drawable, XRC_DRAWABLE,
>> + client, DixGetAttrAccess | DixReadAccess);
>> + if (rc != Success)
>> + return rc;
>> +
>> + if (!(damage = calloc(1, sizeof(PanoramiXRes))))
>> + return BadAlloc;
>> +
>> + damage->type = XRT_DAMAGE;
>> + if (!AddResource(stuff->damage, XRT_DAMAGE, damage))
>> + return BadAlloc;
>> +
>> + /* pixmaps exist on all screens, so just watching screen 0 works */
>> + if (draw->type == XRT_PIXMAP) {
>> + damage->info[0].id = stuff->damage;
>> +
>> + rc = PanoramiXSaveDamageVector[X_DamageCreate](client);
>> + if (rc != Success) {
>> + FreeResource(damage->info[0].id, None);
>> + return rc;
>> + }
>> + } else {
>> + rc = doDamageCreate(client, stuff->drawable, stuff->damage,
>> + PanoramiXDamageQueue);
>> + if (rc == Success) {
>> + panoramix_setup_ids(damage, client, stuff->damage);
>> +
>> + FOR_NSCREENS_FORWARD_SKIP(i) {
>> + stuff->damage = damage->info[i].id;
>> + stuff->drawable = draw->info[i].id;
>> + rc = doDamageCreate(client, draw->info[0].id,
>> + damage->info[0].id,
>> + PanoramiXDamageAccumulate);
>> + if (rc != Success)
>> + FreeResource(damage->info[0].id, None);
>> + }
>> + } else {
>> + FreeResource(damage->info[0].id, None);
>> + }
>> + }
>> +
>> + return rc;
>> +}
>> +
>> +static int
>> +PanoramiXDamageDestroy(ClientPtr client)
>> +{
>> + REQUEST(xDamageDestroyReq);
>> + PanoramiXRes *damage;
>> + int i, rc = Success;
>> +
>> + REQUEST_SIZE_MATCH(xDamageDestroyReq);
>> +
>> + rc = dixLookupResourceByType((void **)&damage, stuff->damage, XRT_DAMAGE,
>> + client, DixDestroyAccess);
>> + if (rc != Success)
>> + return rc;
>> +
>> + FOR_NSCREENS_BACKWARD(i) {
>> + if ((stuff->damage = damage->info[i].id)) {
>
> Clever, which means I had to read it a few times. How horrible would
>
> stuff->damage = damage->info[i].id;
> if (stuff->damage) {
> ...
> }
>
> be, to make it clearer?
>
>> + rc = PanoramiXSaveDamageVector[X_DamageDestroy](client);
>> + if (rc != Success)
>> + break;
>> + }
>> + }
>> +
>> + return rc;
>> +}
>> +
>> +void
>> +PanoramiXDamageInit(void)
>> +{
>> + XRT_DAMAGE = CreateNewResourceType(XineramaDeleteResource,
>> + "XineramaDamage");
>
> Maybe FatalError if this fails?
>
>> + memcpy(PanoramiXSaveDamageVector, ProcDamageVector,
>> + sizeof(ProcDamageVector));
>> +
>> + ProcDamageVector[X_DamageCreate] = PanoramiXDamageCreate;
>> + ProcDamageVector[X_DamageDestroy] = PanoramiXDamageDestroy;
>> +}
>> +
>> +void
>> +PanoramiXDamageReset(void)
>> +{
>> + memcpy(ProcDamageVector, PanoramiXSaveDamageVector,
>> + sizeof(ProcDamageVector));
>> +}
>> +
>> +#endif /* PANORAMIX */
>> +
>> void
>> DamageExtensionInit(void)
>> {
>> @@ -502,5 +732,10 @@ DamageExtensionInit(void)
>> (EventSwapPtr) SDamageNotifyEvent;
>> SetResourceTypeErrorValue(DamageExtType,
>> extEntry->errorBase + BadDamage);
>> +#ifdef PANORAMIX
>> + if (XRT_DAMAGE)
>> + SetResourceTypeErrorValue(XRT_DAMAGE,
>> + extEntry->errorBase + BadDamage);
>> +#endif
>> }
>> }
>> diff --git a/damageext/damageextint.h b/damageext/damageextint.h
>> index 2723379..7319a1d 100644
>> --- a/damageext/damageextint.h
>> +++ b/damageext/damageextint.h
>> @@ -54,6 +54,7 @@ typedef struct _DamageExt {
>> DamageReportLevel level;
>> ClientPtr pClient;
>> XID id;
>> + XID report_id;
>> XID drawable;
>> } DamageExtRec, *DamageExtPtr;
>>
>> @@ -67,4 +68,7 @@ typedef struct _DamageExt {
>> void
>> DamageExtSetCritical(ClientPtr pClient, Bool critical);
>>
>> +void PanoramiXDamageInit(void);
>> +void PanoramiXDamageReset(void);
>> +
>> #endif /* _DAMAGEEXTINT_H_ */
>> --
>> 1.8.3.1
>
> Reviewed-by: Aaron Plattner <aplattner at nvidia.com>
> (assuming that Keith acks the DamageSubtract thing)
>
> I'm getting by build tree updated so I can try to provide a Tested-by too.
Never mind, it looks like this still isn't properly translating the
damage by the root window's position. Was it intentional that v3 of the
patch dropped the "Properly translate coordinates for window reports in
Xinerama mode" part of v2?
Also, the initial damage doesn't cover the whole Xineramified root window:
$ xdpyinfo | grep dimensions
dimensions: 4480x1440 pixels (1222x385 millimeters)
$ ./rootdamage
Damage 2560x1440 @ (0, 0)
--
Aaron
More information about the xorg-devel
mailing list