[PATCH] dix/dispatch: DoGetImage: Use a single buffer
Daniel Kurtz
djkurtz at chromium.org
Sat Mar 29 02:53:47 PDT 2014
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 <kaleb at 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 <kaleb at 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 at 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 <djkurtz at 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
More information about the xorg-devel
mailing list