[RFC PATCH] Let XineramaGetImageData look up drawables itself.
Jamey Sharp
jamey at minilop.net
Fri Apr 23 18:22:58 PDT 2010
Advantages:
- Spares callers from allocating a MAXSCREENS-sized temporary array.
- Hides implementation details of the PanoramiXRes info array.
- Avoids looking up drawables on screens that are entirely clipped out
of the image bounds.
Disadvantages:
When called in a loop, may look up the same drawable multiple times.
This only affects XYPixmaps and images larger than 256kB, so it should
have minimal real impact.
Validates Xinerama-allocated XIDs late, so if the Xinerama wrapper
didn't initialize the PanoramiXRes info array correctly, that error
can't always be reported correctly. It seems to me that if we have a
valid PanoramiXRes that this client is allowed access to, we ought to be
able to get all the per-screen drawables that go with it too, so I don't
think this error case should be possible.
Signed-off-by: Jamey Sharp <jamey at minilop.net>
---
Have I analyzed this correctly? And especially, can I assume that
per-screen drawables are accessible if the PanoramiXRes is? If so,
perhaps errors should just be ignored here?
Xext/panoramiX.c | 28 ++++++++++++++++++++--------
Xext/panoramiXprocs.c | 38 ++++++++++++++++----------------------
Xext/panoramiXsrv.h | 6 ++++--
Xext/shm.c | 31 ++++++++++---------------------
4 files changed, 50 insertions(+), 53 deletions(-)
diff --git a/Xext/panoramiX.c b/Xext/panoramiX.c
index 96eb8f9..50fa970 100644
--- a/Xext/panoramiX.c
+++ b/Xext/panoramiX.c
@@ -1135,9 +1135,11 @@ CopyBits(char *dst, int shiftL, char *src, int bytes)
1 bpp and planar data to be already cleared when presented
to this function */
-void
+int
XineramaGetImageData(
- DrawablePtr *pDrawables,
+ PanoramiXRes *xr_drawable,
+ ClientPtr client,
+ Mask access_mode,
int left,
int top,
int width,
@@ -1150,10 +1152,14 @@ XineramaGetImageData(
){
RegionRec SrcRegion, GrabRegion;
BoxRec SrcBox, *pbox;
- int x, y, w, h, i, j, nbox, size, sizeNeeded, ScratchPitch, inOut, depth;
- DrawablePtr pDraw = pDrawables[0];
+ int x, y, w, h, i, j, nbox, size, sizeNeeded, ScratchPitch, inOut, depth, rc;
+ DrawablePtr pDraw;
char *ScratchMem = NULL;
+ rc = dixLookupDrawable(&pDraw, xr_drawable->info[0].id, client, 0, access_mode);
+ if (rc != Success)
+ return rc;
+
size = 0;
/* find box in logical screen space */
@@ -1172,9 +1178,15 @@ XineramaGetImageData(
depth = (format == XYPixmap) ? 1 : pDraw->depth;
for(i = 0; i < PanoramiXNumScreens; i++) {
- pDraw = pDrawables[i];
-
inOut = RECT_IN_REGION(pScreen,&XineramaScreenRegions[i],&SrcBox);
+ if (inOut == rgnOUT)
+ continue;
+
+ if (i != 0) { /* already found first drawable outside the loop */
+ rc = dixLookupDrawable(&pDraw, xr_drawable->info[i].id, client, 0, access_mode);
+ if (rc != Success)
+ break;
+ }
if(inOut == rgnIN) {
(*pDraw->pScreen->GetImage)(pDraw,
@@ -1182,8 +1194,7 @@ XineramaGetImageData(
SrcBox.y1 - pDraw->y - panoramiXdataPtr[i].y,
width, height, format, planemask, data);
break;
- } else if (inOut == rgnOUT)
- continue;
+ }
REGION_INTERSECT(pScreen, &GrabRegion, &SrcRegion,
&XineramaScreenRegions[i]);
@@ -1280,4 +1291,5 @@ XineramaGetImageData(
REGION_UNINIT(pScreen, &SrcRegion);
REGION_UNINIT(pScreen, &GrabRegion);
+ return rc;
}
diff --git a/Xext/panoramiXprocs.c b/Xext/panoramiXprocs.c
index 6834efb..3baa8f9 100644
--- a/Xext/panoramiXprocs.c
+++ b/Xext/panoramiXprocs.c
@@ -1050,31 +1050,30 @@ int PanoramiXCopyArea(ClientPtr client)
srcx = stuff->srcX; srcy = stuff->srcY;
dstx = stuff->dstX; dsty = stuff->dstY;
if((dst->type == XRT_PIXMAP) && (src->type == XRT_WINDOW)) {
- DrawablePtr drawables[MAXSCREENS];
- DrawablePtr pDst;
+ DrawablePtr pSrc, pDst;
GCPtr pGC;
char *data;
int pitch, rc;
- FOR_NSCREENS(j) {
- rc = dixLookupDrawable(drawables+j, src->info[j].id, client, 0,
+ rc = dixLookupDrawable(&pSrc, src->info[0].id, client, 0,
DixGetAttrAccess);
- if (rc != Success)
- return rc;
- }
+ if (rc != Success)
+ return rc;
- pitch = PixmapBytePad(stuff->width, drawables[0]->depth);
+ pitch = PixmapBytePad(stuff->width, pSrc->depth);
if(!(data = xcalloc(1, stuff->height * pitch)))
return BadAlloc;
- XineramaGetImageData(drawables, srcx, srcy,
+ rc = XineramaGetImageData(src, client, DixGetAttrAccess, srcx, srcy,
stuff->width, stuff->height, ZPixmap, ~0, data, pitch,
srcIsRoot);
+ if (rc != Success)
+ return rc;
FOR_NSCREENS_BACKWARD(j) {
stuff->gc = gc->info[j].id;
VALIDATE_DRAWABLE_AND_GC(dst->info[j].id, pDst, DixWriteAccess);
- if(drawables[0]->depth != pDst->depth) {
+ if(pSrc->depth != pDst->depth) {
client->errorValue = stuff->dstDrawable;
xfree(data);
return (BadMatch);
@@ -1805,13 +1804,12 @@ int PanoramiXPutImage(ClientPtr client)
int PanoramiXGetImage(ClientPtr client)
{
- DrawablePtr drawables[MAXSCREENS];
DrawablePtr pDraw;
PanoramiXRes *draw;
xGetImageReply xgi;
Bool isRoot;
char *pBuf;
- int i, x, y, w, h, format, rc;
+ int x, y, w, h, format, rc;
Mask plane = 0, planemask;
int linesDone, nlines, linesPerBuf;
long widthBytesLine, length;
@@ -1869,14 +1867,6 @@ int PanoramiXGetImage(ClientPtr client)
return(BadMatch);
}
- drawables[0] = pDraw;
- for(i = 1; i < PanoramiXNumScreens; i++) {
- rc = dixLookupDrawable(drawables+i, draw->info[i].id, client, 0,
- DixGetAttrAccess);
- if (rc != Success)
- return rc;
- }
-
xgi.visual = wVisual (((WindowPtr) pDraw));
xgi.type = X_Reply;
xgi.sequenceNumber = client->sequence;
@@ -1923,8 +1913,10 @@ int PanoramiXGetImage(ClientPtr client)
if(pDraw->depth == 1)
bzero(pBuf, nlines * widthBytesLine);
- XineramaGetImageData(drawables, x, y + linesDone, w, nlines,
+ rc = XineramaGetImageData(draw, client, DixGetAttrAccess, x, y + linesDone, w, nlines,
format, planemask, pBuf, widthBytesLine, isRoot);
+ if (rc != Success)
+ ErrorF("tried to read from unknown Xinerama drawables\n");
(void)WriteToClient(client,
(int)(nlines * widthBytesLine),
@@ -1940,9 +1932,11 @@ int PanoramiXGetImage(ClientPtr client)
bzero(pBuf, nlines * widthBytesLine);
- XineramaGetImageData(drawables, x, y + linesDone, w,
+ rc = XineramaGetImageData(draw, client, DixGetAttrAccess, x, y + linesDone, w,
nlines, format, plane, pBuf,
widthBytesLine, isRoot);
+ if (rc != Success)
+ ErrorF("tried to read from unknown Xinerama drawables\n");
(void)WriteToClient(client,
(int)(nlines * widthBytesLine),
diff --git a/Xext/panoramiXsrv.h b/Xext/panoramiXsrv.h
index c77b119..f4e0c61 100644
--- a/Xext/panoramiXsrv.h
+++ b/Xext/panoramiXsrv.h
@@ -40,8 +40,10 @@ extern _X_EXPORT unsigned long XRT_COLORMAP;
typedef Bool (*XineramaVisualsEqualProcPtr)(VisualPtr, ScreenPtr, VisualPtr);
extern _X_EXPORT XineramaVisualsEqualProcPtr XineramaVisualsEqualPtr;
-extern _X_EXPORT void XineramaGetImageData(
- DrawablePtr *pDrawables,
+extern _X_EXPORT int XineramaGetImageData(
+ PanoramiXRes *xr_drawable,
+ ClientPtr client,
+ Mask access_mode,
int left,
int top,
int width,
diff --git a/Xext/shm.c b/Xext/shm.c
index ab58c27..7fc6675 100644
--- a/Xext/shm.c
+++ b/Xext/shm.c
@@ -610,11 +610,10 @@ static int
ProcPanoramiXShmGetImage(ClientPtr client)
{
PanoramiXRes *draw;
- DrawablePtr *drawables;
DrawablePtr pDraw;
xShmGetImageReply xgi;
ShmDescPtr shmdesc;
- int i, x, y, w, h, format, rc;
+ int x, y, w, h, format, rc;
Mask plane = 0, planemask;
long lenPer = 0, length, widthBytesLine;
Bool isRoot;
@@ -671,21 +670,6 @@ ProcPanoramiXShmGetImage(ClientPtr client)
return(BadMatch);
}
- drawables = xcalloc(PanoramiXNumScreens, sizeof(DrawablePtr));
- if(!drawables)
- return(BadAlloc);
-
- drawables[0] = pDraw;
- for(i = 1; i < PanoramiXNumScreens; i++) {
- rc = dixLookupDrawable(drawables+i, draw->info[i].id, client, 0,
- DixReadAccess);
- if (rc != Success)
- {
- xfree(drawables);
- return rc;
- }
- }
-
xgi.visual = wVisual(((WindowPtr)pDraw));
xgi.type = X_Reply;
xgi.length = 0;
@@ -707,22 +691,27 @@ ProcPanoramiXShmGetImage(ClientPtr client)
if (length == 0) {/* nothing to do */ }
else if (format == ZPixmap) {
- XineramaGetImageData(drawables, x, y, w, h, format, planemask,
+ rc = XineramaGetImageData(draw, client, DixReadAccess,
+ x, y, w, h, format, planemask,
shmdesc->addr + stuff->offset,
widthBytesLine, isRoot);
+ if (rc != Success)
+ return rc;
} else {
length = stuff->offset;
for (; plane; plane >>= 1) {
if (planemask & plane) {
- XineramaGetImageData(drawables, x, y, w, h,
- format, plane, shmdesc->addr + length,
+ rc = XineramaGetImageData(draw, client, DixReadAccess,
+ x, y, w, h, format, plane,
+ shmdesc->addr + length,
widthBytesLine, isRoot);
+ if (rc != Success)
+ return rc;
length += lenPer;
}
}
}
- xfree(drawables);
if (client->swapped) {
int n;
--
1.7.0
More information about the xorg-devel
mailing list