[PATCH 3/3] exa: properly wrap Create/DestroyPixmap in ExaCheckPolyArc and fix GC (un)wrapping.
Maarten Maathuis
madman2003 at gmail.com
Sat Nov 7 04:32:41 PST 2009
- Fix the system that is required for it to actually work.
- The comment near the new macros explains the problem that existed before.
- Why it didn't cause issues before i don't know.
- We cannot unwrap the GC funcs, because they do prepare/finish access too.
- Protect against setting CreatePixmap twice (which would break swapping) in GC
functions.
- Remove one (now invalid) use of ExaDoPrepareAccess.
Signed-off-by: Maarten Maathuis <madman2003 at gmail.com>
---
exa/exa.c | 99 ++++++++++++++++++++--------------------------------
exa/exa_priv.h | 100 ++++++++++++++++++++++++++++++++++++++--------------
exa/exa_unaccel.c | 9 +++++
3 files changed, 120 insertions(+), 88 deletions(-)
diff --git a/exa/exa.c b/exa/exa.c
index 46e9182..3ff7f22 100644
--- a/exa/exa.c
+++ b/exa/exa.c
@@ -481,11 +481,10 @@ const GCFuncs exaGCFuncs = {
};
/*
- * This wrapper exists to allow fbValidateGC to work.
* Note that we no longer assume newly created pixmaps to be in normal ram.
* This assumption is certainly not garuanteed with driver allocated pixmaps.
*/
-static PixmapPtr
+PixmapPtr
exaCreatePixmapWithPrepare(ScreenPtr pScreen, int w, int h, int depth,
unsigned usage_hint)
{
@@ -495,24 +494,19 @@ exaCreatePixmapWithPrepare(ScreenPtr pScreen, int w, int h, int depth,
/* This swaps between this function and the real upper layer function.
* Normally this would swap to the fb layer pointer, this is a very special case.
*/
- swap(pExaScr, pScreen, CreatePixmap);
+ swap_special(pExaScr, pScreen, CreatePixmap);
pPixmap = pScreen->CreatePixmap(pScreen, w, h, depth, usage_hint);
- swap(pExaScr, pScreen, CreatePixmap);
+ swap_special(pExaScr, pScreen, CreatePixmap);
if (!pPixmap)
return NULL;
- /* Note the usage of ExaDoPrepareAccess, this allowed because:
- * The pixmap is new, so not offscreen in the classic exa case.
- * For EXA_HANDLES_PIXMAPS the driver will handle whatever is needed.
- * We want to signal that the pixmaps will be used as destination.
- */
- ExaDoPrepareAccess(pPixmap, EXA_PREPARE_AUX_DEST);
+ exaPrepareAccess(&pPixmap->drawable, EXA_PREPARE_AUX_DEST);
return pPixmap;
}
-static Bool
+Bool
exaDestroyPixmapWithFinish(PixmapPtr pPixmap)
{
ScreenPtr pScreen = pPixmap->drawable.pScreen;
@@ -524,9 +518,9 @@ exaDestroyPixmapWithFinish(PixmapPtr pPixmap)
/* This swaps between this function and the real upper layer function.
* Normally this would swap to the fb layer pointer, this is a very special case.
*/
- swap(pExaScr, pScreen, DestroyPixmap);
+ swap_special(pExaScr, pScreen, DestroyPixmap);
ret = pScreen->DestroyPixmap(pPixmap);
- swap(pExaScr, pScreen, DestroyPixmap);
+ swap_special(pExaScr, pScreen, DestroyPixmap);
return ret;
}
@@ -542,20 +536,17 @@ exaValidateGC(GCPtr pGC,
ScreenPtr pScreen = pDrawable->pScreen;
ExaScreenPriv(pScreen);
- CreatePixmapProcPtr old_ptr = NULL;
- DestroyPixmapProcPtr old_ptr2 = NULL;
PixmapPtr pTile = NULL;
- EXA_GC_PROLOGUE(pGC);
-
- /* save the "fb" pointer. */
- old_ptr = pExaScr->SavedCreatePixmap;
- /* create a new upper layer pointer. */
- wrap(pExaScr, pScreen, CreatePixmap, exaCreatePixmapWithPrepare);
-
- /* save the "fb" pointer. */
- old_ptr2 = pExaScr->SavedDestroyPixmap;
- /* create a new upper layer pointer. */
- wrap(pExaScr, pScreen, DestroyPixmap, exaDestroyPixmapWithFinish);
+ Bool restore = FALSE;
+ EXA_GC_PROLOGUE_FUNC(pGC);
+
+ /* create a new upper layer pointer. Warning, CreatePixmap may already be
+ wrapped from another place. */
+ if (pScreen->CreatePixmap != exaCreatePixmapWithPrepare) {
+ restore = TRUE;
+ wrap_special(pExaScr, pScreen, CreatePixmap, exaCreatePixmapWithPrepare);
+ wrap_special(pExaScr, pScreen, DestroyPixmap, exaDestroyPixmapWithFinish);
+ }
/* Either of these conditions is enough to trigger access to a tile pixmap. */
/* With pGC->tileIsPixel == 1, you run the risk of dereferencing an invalid tile pixmap pointer. */
@@ -587,34 +578,30 @@ exaValidateGC(GCPtr pGC,
exaFinishAccess(&pGC->stipple->drawable, EXA_PREPARE_MASK);
/* switch back to the normal upper layer. */
- unwrap(pExaScr, pScreen, CreatePixmap);
- /* restore copy of fb layer pointer. */
- pExaScr->SavedCreatePixmap = old_ptr;
-
- /* switch back to the normal upper layer. */
- unwrap(pExaScr, pScreen, DestroyPixmap);
- /* restore copy of fb layer pointer. */
- pExaScr->SavedDestroyPixmap = old_ptr2;
+ if (restore) {
+ unwrap_special(pExaScr, pScreen, CreatePixmap);
+ unwrap_special(pExaScr, pScreen, DestroyPixmap);
+ }
- EXA_GC_EPILOGUE(pGC);
+ EXA_GC_EPILOGUE_FUNC(pGC);
}
/* Is exaPrepareAccessGC() needed? */
static void
exaDestroyGC(GCPtr pGC)
{
- EXA_GC_PROLOGUE (pGC);
+ EXA_GC_PROLOGUE_FUNC (pGC);
(*pGC->funcs->DestroyGC)(pGC);
- EXA_GC_EPILOGUE (pGC);
+ EXA_GC_EPILOGUE_FUNC (pGC);
}
static void
exaChangeGC (GCPtr pGC,
unsigned long mask)
{
- EXA_GC_PROLOGUE (pGC);
+ EXA_GC_PROLOGUE_FUNC (pGC);
(*pGC->funcs->ChangeGC) (pGC, mask);
- EXA_GC_EPILOGUE (pGC);
+ EXA_GC_EPILOGUE_FUNC (pGC);
}
static void
@@ -622,9 +609,9 @@ exaCopyGC (GCPtr pGCSrc,
unsigned long mask,
GCPtr pGCDst)
{
- EXA_GC_PROLOGUE (pGCDst);
+ EXA_GC_PROLOGUE_FUNC (pGCDst);
(*pGCDst->funcs->CopyGC) (pGCSrc, mask, pGCDst);
- EXA_GC_EPILOGUE (pGCDst);
+ EXA_GC_EPILOGUE_FUNC (pGCDst);
}
static void
@@ -633,25 +620,25 @@ exaChangeClip (GCPtr pGC,
pointer pvalue,
int nrects)
{
- EXA_GC_PROLOGUE (pGC);
+ EXA_GC_PROLOGUE_FUNC (pGC);
(*pGC->funcs->ChangeClip) (pGC, type, pvalue, nrects);
- EXA_GC_EPILOGUE (pGC);
+ EXA_GC_EPILOGUE_FUNC (pGC);
}
static void
exaCopyClip(GCPtr pGCDst, GCPtr pGCSrc)
{
- EXA_GC_PROLOGUE (pGCDst);
+ EXA_GC_PROLOGUE_FUNC (pGCDst);
(*pGCDst->funcs->CopyClip)(pGCDst, pGCSrc);
- EXA_GC_EPILOGUE (pGCDst);
+ EXA_GC_EPILOGUE_FUNC(pGCDst);
}
static void
exaDestroyClip(GCPtr pGC)
{
- EXA_GC_PROLOGUE (pGC);
+ EXA_GC_PROLOGUE_FUNC (pGC);
(*pGC->funcs->DestroyClip)(pGC);
- EXA_GC_EPILOGUE (pGC);
+ EXA_GC_EPILOGUE_FUNC (pGC);
}
/**
@@ -682,18 +669,12 @@ exaChangeWindowAttributes(WindowPtr pWin, unsigned long mask)
Bool ret;
ScreenPtr pScreen = pWin->drawable.pScreen;
ExaScreenPriv(pScreen);
- CreatePixmapProcPtr old_ptr = NULL;
- DestroyPixmapProcPtr old_ptr2 = NULL;
- /* save the "fb" pointer. */
- old_ptr = pExaScr->SavedCreatePixmap;
/* create a new upper layer pointer. */
- wrap(pExaScr, pScreen, CreatePixmap, exaCreatePixmapWithPrepare);
+ wrap_special(pExaScr, pScreen, CreatePixmap, exaCreatePixmapWithPrepare);
- /* save the "fb" pointer. */
- old_ptr2 = pExaScr->SavedDestroyPixmap;
/* create a new upper layer pointer. */
- wrap(pExaScr, pScreen, DestroyPixmap, exaDestroyPixmapWithFinish);
+ wrap_special(pExaScr, pScreen, DestroyPixmap, exaDestroyPixmapWithFinish);
if ((mask & CWBackPixmap) && pWin->backgroundState == BackgroundPixmap)
exaPrepareAccess(&pWin->background.pixmap->drawable, EXA_PREPARE_SRC);
@@ -711,14 +692,10 @@ exaChangeWindowAttributes(WindowPtr pWin, unsigned long mask)
exaFinishAccess(&pWin->border.pixmap->drawable, EXA_PREPARE_MASK);
/* switch back to the normal upper layer. */
- unwrap(pExaScr, pScreen, CreatePixmap);
- /* restore copy of fb layer pointer. */
- pExaScr->SavedCreatePixmap = old_ptr;
+ unwrap_special(pExaScr, pScreen, CreatePixmap);
/* switch back to the normal upper layer. */
- unwrap(pExaScr, pScreen, DestroyPixmap);
- /* restore copy of fb layer pointer. */
- pExaScr->SavedDestroyPixmap = old_ptr2;
+ unwrap_special(pExaScr, pScreen, DestroyPixmap);
return ret;
}
diff --git a/exa/exa_priv.h b/exa/exa_priv.h
index 5b056da..d267c1a 100644
--- a/exa/exa_priv.h
+++ b/exa/exa_priv.h
@@ -152,25 +152,25 @@ typedef struct _ExaMigrationRec {
typedef void (*EnableDisableFBAccessProcPtr)(int, Bool);
typedef struct {
ExaDriverPtr info;
- ScreenBlockHandlerProcPtr SavedBlockHandler;
- ScreenWakeupHandlerProcPtr SavedWakeupHandler;
- CreateGCProcPtr SavedCreateGC;
- CloseScreenProcPtr SavedCloseScreen;
- GetImageProcPtr SavedGetImage;
- GetSpansProcPtr SavedGetSpans;
- CreatePixmapProcPtr SavedCreatePixmap;
- DestroyPixmapProcPtr SavedDestroyPixmap;
- CopyWindowProcPtr SavedCopyWindow;
- ChangeWindowAttributesProcPtr SavedChangeWindowAttributes;
- BitmapToRegionProcPtr SavedBitmapToRegion;
- CreateScreenResourcesProcPtr SavedCreateScreenResources;
- ModifyPixmapHeaderProcPtr SavedModifyPixmapHeader;
+ ScreenBlockHandlerProcPtr SavedBlockHandler[2];
+ ScreenWakeupHandlerProcPtr SavedWakeupHandler[2];
+ CreateGCProcPtr SavedCreateGC[2];
+ CloseScreenProcPtr SavedCloseScreen[2];
+ GetImageProcPtr SavedGetImage[2];
+ GetSpansProcPtr SavedGetSpans[2];
+ CreatePixmapProcPtr SavedCreatePixmap[2];
+ DestroyPixmapProcPtr SavedDestroyPixmap[2];
+ CopyWindowProcPtr SavedCopyWindow[2];
+ ChangeWindowAttributesProcPtr SavedChangeWindowAttributes[2];
+ BitmapToRegionProcPtr SavedBitmapToRegion[2];
+ CreateScreenResourcesProcPtr SavedCreateScreenResources[2];
+ ModifyPixmapHeaderProcPtr SavedModifyPixmapHeader[2];
#ifdef RENDER
- CompositeProcPtr SavedComposite;
- TrianglesProcPtr SavedTriangles;
- GlyphsProcPtr SavedGlyphs;
- TrapezoidsProcPtr SavedTrapezoids;
- AddTrapsProcPtr SavedAddTraps;
+ CompositeProcPtr SavedComposite[2];
+ TrianglesProcPtr SavedTriangles[2];
+ GlyphsProcPtr SavedGlyphs[2];
+ TrapezoidsProcPtr SavedTrapezoids[2];
+ AddTrapsProcPtr SavedAddTraps[2];
#endif
void (*do_migration) (ExaMigrationPtr pixmaps, int npixmaps, Bool can_accel);
Bool (*pixmap_is_offscreen) (PixmapPtr pPixmap);
@@ -224,31 +224,70 @@ extern DevPrivateKey exaGCPrivateKey;
#define ExaGCPriv(gc) ExaGCPrivPtr pExaGC = ExaGetGCPriv(gc)
/*
+ * Special wrapping macros for overiding top level pointers.
+ * They are needed because without them:
+ * damageDestroyPixmap is swapped with exaDestroyPixmapWithFinish
+ * exaDestroyPixmapWithFinish does it's thing and swaps back to damageDestroyPixmap
+ * exaDestroyPixmap then swaps to exaDestroyPixmapWithFinish, instead of
+ * (w)fbDestroyPixmap, which is bad. So add a 2nd SavedDestroyPixmap to avoid this.
+ */
+
+#define wrap_special(priv, real, mem, func) {\
+ priv->Saved##mem[1] = real->mem; \
+ real->mem = func; \
+}
+
+#define unwrap_special(priv, real, mem) {\
+ real->mem = priv->Saved##mem[1]; \
+}
+
+#define swap_special(priv, real, mem) {\
+ void *tmp = priv->Saved##mem[1]; \
+ priv->Saved##mem[1] = real->mem; \
+ real->mem = tmp; \
+}
+
+/*
* Some macros to deal with function wrapping.
*/
#define wrap(priv, real, mem, func) {\
- priv->Saved##mem = real->mem; \
+ priv->Saved##mem[0] = real->mem; \
real->mem = func; \
}
#define unwrap(priv, real, mem) {\
- real->mem = priv->Saved##mem; \
+ real->mem = priv->Saved##mem[0]; \
}
#define swap(priv, real, mem) {\
- void *tmp = priv->Saved##mem; \
- priv->Saved##mem = real->mem; \
+ void *tmp = priv->Saved##mem[0]; \
+ priv->Saved##mem[0] = real->mem; \
real->mem = tmp; \
}
#define EXA_GC_PROLOGUE(_gc_) \
ExaGCPriv(_gc_); \
- swap(pExaGC, _gc_, funcs); \
swap(pExaGC, _gc_, ops);
#define EXA_GC_EPILOGUE(_gc_) \
- swap(pExaGC, _gc_, funcs); \
- swap(pExaGC, _gc_, ops);
+ swap(pExaGC, _gc_, ops); \
+
+/* GC functions can be called from fallback GC ops,
+ ensure that ops are not swapped twice. */
+#define EXA_GC_PROLOGUE_FUNC(_gc_) \
+ ExaGCPriv(_gc_); \
+ Bool restore_gc_ops = FALSE; \
+ if (_gc_->ops == &exaOps) { \
+ restore_gc_ops = TRUE; \
+ swap(pExaGC, _gc_, ops); \
+ } \
+ swap(pExaGC, _gc_, funcs);
+
+#define EXA_GC_EPILOGUE_FUNC(_gc_) \
+ if (restore_gc_ops) {\
+ swap(pExaGC, _gc_, ops); \
+ } \
+ swap(pExaGC, _gc_, funcs);
/** Align an offset to an arbitrary alignment */
#define EXA_ALIGN(offset, align) (((offset) + (align) - 1) - \
@@ -313,8 +352,8 @@ typedef struct {
typedef struct {
/* GC values from the layer below. */
- GCOps *Savedops;
- GCFuncs *Savedfuncs;
+ GCOps *Savedops[2];
+ GCFuncs *Savedfuncs[2];
} ExaGCPrivRec, *ExaGCPrivPtr;
typedef struct {
@@ -552,6 +591,13 @@ exaDoMigration (ExaMigrationPtr pixmaps, int npixmaps, Bool can_accel);
Bool
exaPixmapIsPinned (PixmapPtr pPix);
+PixmapPtr
+exaCreatePixmapWithPrepare(ScreenPtr pScreen, int w, int h, int depth,
+ unsigned usage_hint);
+
+Bool
+exaDestroyPixmapWithFinish(PixmapPtr pPixmap);
+
extern const GCFuncs exaGCFuncs;
/* exa_classic.c */
diff --git a/exa/exa_unaccel.c b/exa/exa_unaccel.c
index c8f0172..d7b248b 100644
--- a/exa/exa_unaccel.c
+++ b/exa/exa_unaccel.c
@@ -229,12 +229,21 @@ void
ExaCheckPolyArc (DrawablePtr pDrawable, GCPtr pGC,
int narcs, xArc *pArcs)
{
+ ScreenPtr pScreen = pDrawable->pScreen;
+ ExaScreenPriv(pScreen);
EXA_GC_PROLOGUE(pGC);
EXA_FALLBACK(("to %p (%c)\n", pDrawable, exaDrawableLocation(pDrawable)));
exaPrepareAccess (pDrawable, EXA_PREPARE_DEST);
exaPrepareAccessGC (pGC);
+ /* Install a new top level pointer that deals with the coming CreatePixmap
+ * call as well as the following DestroyPixmap call.
+ */
+ wrap_special(pExaScr, pScreen, CreatePixmap, exaCreatePixmapWithPrepare);
+ wrap_special(pExaScr, pScreen, DestroyPixmap, exaDestroyPixmapWithFinish);
pGC->ops->PolyArc (pDrawable, pGC, narcs, pArcs);
+ unwrap_special(pExaScr, pScreen, CreatePixmap);
+ unwrap_special(pExaScr, pScreen, DestroyPixmap);
exaFinishAccessGC (pGC);
exaFinishAccess (pDrawable, EXA_PREPARE_DEST);
EXA_GC_EPILOGUE(pGC);
--
1.6.5.1
More information about the xorg-devel
mailing list