On Son, 2009-03-01 at 01:36 +0100, Maarten Maathuis wrote:
>
> @@ -73,8 +62,9 @@ unsigned long
> exaGetPixmapOffset(PixmapPtr pPix)
> {
> ExaScreenPriv (pPix->drawable.pScreen);
> + ExaPixmapPriv (pPix);
>
> - return (CARD8 *)ExaGetPixmapAddress(pPix) - pExaScr->info->memoryBase;
> + return (CARD8 *)pExaPixmap->fb_ptr - pExaScr->info->memoryBase;
> }
>
> void *
This looks good, might want to put it in the same commit which sets
pExaPixmap->fb_ptr for the screen pixmap though.
> @@ -449,15 +439,20 @@ exaModifyPixmapHeader(PixmapPtr pPixmap, int width, int
> height, int depth,
> }
> }
>
> -
> if (pExaScr->info->ModifyPixmapHeader) {
> ret = pExaScr->info->ModifyPixmapHeader(pPixmap, width, height, depth,
> bitsPerPixel, devKind,
> pPixData);
> if (ret == TRUE)
> - return ret;
> + goto out;
> }
> - return pExaScr->SavedModifyPixmapHeader(pPixmap, width, height, depth,
> + ret = pExaScr->SavedModifyPixmapHeader(pPixmap, width, height, depth,
> bitsPerPixel, devKind, pPixData);
> +
> +out:
> + /* Always NULL this, we don't want lingering pointers. */
> + pPixmap->devPrivate.ptr = NULL;
> +
> + return ret;
> }
This looks like part of my patch from
http://bugs.freedesktop.org/show_bug.cgi?id=20212 . I'd prefer keeping
that as a separate change.
> @@ -520,14 +515,26 @@ exaGetOffscreenPixmap (DrawablePtr pDrawable, int *xp,
> int *yp)
> Bool
> ExaDoPrepareAccess(DrawablePtr pDrawable, int index)
> {
> - ScreenPtr pScreen = pDrawable->pScreen;
> - ExaScreenPriv (pScreen);
> - PixmapPtr pPixmap = exaGetDrawablePixmap (pDrawable);
> - Bool offscreen = exaPixmapIsOffscreen(pPixmap);
> + ScreenPtr pScreen = pDrawable->pScreen;
> + ExaScreenPriv (pScreen);
> + PixmapPtr pPixmap = exaGetDrawablePixmap (pDrawable);
> + ExaPixmapPriv(pPixmap);
> + Bool offscreen;
> +
> + if (!pExaPixmap)
> + FatalError("Calling PrepareAccess on a pixmap not known to exa.\n");
>
> - /* Unhide pixmap pointer */
> - if (pPixmap->devPrivate.ptr == NULL && !(pExaScr->info->flags &
> EXA_HANDLES_PIXMAPS)) {
> - pPixmap->devPrivate.ptr = ExaGetPixmapAddress(pPixmap);
> + if (pPixmap->devPrivate.ptr != NULL)
> + FatalError("Unbalanced Prepare/FinishAccess or data corruption."
> + "pPixmap->devPrivate.ptr is %p.\n", pPixmap->devPrivate.ptr);
I'm not sure I like the proliferation of FatalErrors; like assert()s
they should only be used to catch conditions which 'can never happen'.
Is that true for all the cases where you want to add FatalError calls?
--
Earthling Michel Dänzer | http://www.vmware.com
Libre software enthusiast | Debian, X and DRI developer
_______________________________________________
xorg-devel mailing list
[email protected]
http://lists.x.org/mailman/listinfo/xorg-devel
