DoGetImage chops up an image into IMAGE_BUFSIZE strips. This predates commit [0] (a pull from XFree86 to X.Org), at which time IMAGE_BUFSIZE was 8k. Back then the immediate buffers were allocated with ALLOCATE_LOCAL(), so it made sense to try to keep them small since at least on some systems, these buffers were allocated on the stack.
[0] commit ded6147bfb5d75ff1e67c858040a628b61bc17d1 Author: Kaleb Keithley <ka...@freedesktop.org> Date: Fri Nov 14 15:54:54 2003 +0000 R6.6 is the Xorg base-line 8k would fetch an 1024x864 monochrome screen w/ 32-bit word in just 14 buffers. But, this was soon deemed too small, and the buffer size was increased to 64 kB in commit [1], although it was still using ALLOCATE_LOCAL(). [1] commit d568221710959cf7d783e6ff0fb80fb43a231124 Author: Kaleb Keithley <ka...@freedesktop.org> Date: Fri Nov 14 16:49:22 2003 +0000 XFree86 4.3.0.1 Eventually people realized that alloca() had no error detection, and could overrun the stack, so and ALLOCATE_LOCAL was killed, replaced by a succession of heap allocators starting with commit [2]: xalloc() -> xcalloc -> calloc(). [2] commit 6e4f5cf83f35ffebb51633ab30b1826e63e37223 Author: Ben Byer <bbyer@bbyer.local> Date: Mon Nov 5 05:53:34 2007 -0800 changing ALLOCATE_LOCAL to xalloc to prevent stack overflow Since we are now allocating from the heap, there should no longer be necessary, or desirable to chop up the DoGetImage() buffer. In theory, we *could* still chop it up as a fallback if the big buffer allocation fails. But, if the system is that close to running out of heap space, it is might be best to just fail quickly. When copying from a EXA pixmap, this can save a bit of overhead from multiple EXA prepare/finish access calls. Signed-off-by: Daniel Kurtz <djku...@chromium.org> --- dix/dispatch.c | 109 +++++++++++++++++------------------------------------ include/servermd.h | 8 ---- 2 files changed, 35 insertions(+), 82 deletions(-) diff --git a/dix/dispatch.c b/dix/dispatch.c index 4f830f7..a0b1bc0 100644 --- a/dix/dispatch.c +++ b/dix/dispatch.c @@ -1977,8 +1977,7 @@ DoGetImage(ClientPtr client, int format, Drawable drawable, Mask planemask) { DrawablePtr pDraw, pBoundingDraw; - int nlines, linesPerBuf, rc; - int linesDone; + int rc; /* coordinates relative to the bounding drawable */ int relx, rely; @@ -2072,31 +2071,7 @@ DoGetImage(ClientPtr client, int format, Drawable drawable, } - xgi.length = length; - - xgi.length = bytes_to_int32(xgi.length); - if (widthBytesLine == 0 || height == 0) - linesPerBuf = 0; - else if (widthBytesLine >= IMAGE_BUFSIZE) - linesPerBuf = 1; - else { - linesPerBuf = IMAGE_BUFSIZE / widthBytesLine; - if (linesPerBuf > height) - linesPerBuf = height; - } - length = linesPerBuf * widthBytesLine; - if (linesPerBuf < height) { - /* we have to make sure intermediate buffers don't need padding */ - while ((linesPerBuf > 1) && - (length & ((1L << LOG2_BYTES_PER_SCANLINE_PAD) - 1))) { - linesPerBuf--; - length -= widthBytesLine; - } - while (length & ((1L << LOG2_BYTES_PER_SCANLINE_PAD) - 1)) { - linesPerBuf++; - length += widthBytesLine; - } - } + xgi.length = bytes_to_int32(length); if (!(pBuf = calloc(1, length))) return BadAlloc; WriteReplyToClient(client, sizeof(xGetImageReply), &xgi); @@ -2108,60 +2083,46 @@ DoGetImage(ClientPtr client, int format, Drawable drawable, } } - if (linesPerBuf == 0) { + if (length == 0) { /* nothing to do */ } else if (format == ZPixmap) { - linesDone = 0; - while (height - linesDone > 0) { - nlines = min(linesPerBuf, height - linesDone); - (*pDraw->pScreen->GetImage) (pDraw, - x, - y + linesDone, - width, - nlines, - format, planemask, (void *) pBuf); - if (pVisibleRegion) - XaceCensorImage(client, pVisibleRegion, widthBytesLine, - pDraw, x, y + linesDone, width, - nlines, format, pBuf); - - /* Note that this is NOT a call to WriteSwappedDataToClient, - as we do NOT byte swap */ - ReformatImage(pBuf, (int) (nlines * widthBytesLine), - BitsPerPixel(pDraw->depth), ClientOrder(client)); - - WriteToClient(client, (int) (nlines * widthBytesLine), pBuf); - linesDone += nlines; - } + (*pDraw->pScreen->GetImage) (pDraw, + x, + y, + width, + height, + format, planemask, (void *) pBuf); + if (pVisibleRegion) + XaceCensorImage(client, pVisibleRegion, widthBytesLine, + pDraw, x, y, width, height, format, pBuf); + + /* Note that this is NOT a call to WriteSwappedDataToClient, + as we do NOT byte swap */ + ReformatImage(pBuf, length, + BitsPerPixel(pDraw->depth), ClientOrder(client)); + + WriteToClient(client, length, pBuf); } else { /* XYPixmap */ - + int plane_size = height * widthBytesLine; for (; plane; plane >>= 1) { if (planemask & plane) { - linesDone = 0; - while (height - linesDone > 0) { - nlines = min(linesPerBuf, height - linesDone); - (*pDraw->pScreen->GetImage) (pDraw, - x, - y + linesDone, - width, - nlines, - format, plane, (void *) pBuf); - if (pVisibleRegion) - XaceCensorImage(client, pVisibleRegion, - widthBytesLine, - pDraw, x, y + linesDone, width, - nlines, format, pBuf); - - /* Note: NOT a call to WriteSwappedDataToClient, - as we do NOT byte swap */ - ReformatImage(pBuf, (int) (nlines * widthBytesLine), - 1, ClientOrder(client)); - - WriteToClient(client, (int)(nlines * widthBytesLine), pBuf); - linesDone += nlines; - } + (*pDraw->pScreen->GetImage) (pDraw, + x, + y, + width, + height, + format, plane, (void *) pBuf); + if (pVisibleRegion) + XaceCensorImage(client, pVisibleRegion, widthBytesLine, + pDraw, x, y, width, height, format, pBuf); + + /* Note: NOT a call to WriteSwappedDataToClient, + as we do NOT byte swap */ + ReformatImage(pBuf, plane_size, 1, ClientOrder(client)); + + WriteToClient(client, plane_size, pBuf); } } } diff --git a/include/servermd.h b/include/servermd.h index 11f6c10..42a5e74 100644 --- a/include/servermd.h +++ b/include/servermd.h @@ -300,14 +300,6 @@ SOFTWARE. #endif /* __aarch64__ */ -/* size of buffer to use with GetImage, measured in bytes. There's obviously - * a trade-off between the amount of heap used and the number of times the - * ddx routine has to be called. - */ -#ifndef IMAGE_BUFSIZE -#define IMAGE_BUFSIZE (64*1024) -#endif - /* pad scanline to a longword */ #ifndef BITMAP_SCANLINE_UNIT #define BITMAP_SCANLINE_UNIT 32 -- 1.9.1.423.g4596e3a _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel