From: "Mattias Engdeg�rd" <f91-men@nada.kth.se>

To: slouken@devolution.com
Subject: Re: [SDL] Question about SDL_FillRect()

I benchmarked with and without clipping UpdateRects and was unable to find
any difference on my moderately slow machine. Anyway, I haven't added
clipping in this patch, but fixed a couple of bugs and generally cleaned
up some of the X11 image code. Most importantly, UpdateRects now checks
for both zero height and width. Also, I eliminated the entire code to
byteswap X11 images since X11 can do that automatically if you ask it
nicely :-)

--HG--
extra : convert_revision : svn%3Ac70aab31-4412-0410-b14c-859654838e24/trunk%40271

src/video/x11/SDL_x11image.c

@@ -44,109 +44,92 @@ static int (*X_handler)(Display *, XErrorEvent *) = NULL;
 static int shm_errhandler(Display *d, XErrorEvent *e)
 {
         if ( e->error_code == BadAccess ) {
-        	++shm_error;
+        	shm_error = 1;
         	return(0);
         } else
 		return(X_handler(d,e));
 }
-#endif /* ! NO_SHARED_MEMORY */
-
-/* Various screen update functions available */
-static void X11_NormalUpdate(_THIS, int numrects, SDL_Rect *rects);
-static void X11_MITSHMUpdate(_THIS, int numrects, SDL_Rect *rects);
 
-int X11_SetupImage(_THIS, SDL_Surface *screen)
+static void try_mitshm(_THIS, SDL_Surface *screen)
 {
-#ifdef NO_SHARED_MEMORY
-	screen->pixels = malloc(screen->h*screen->pitch);
-#else
-	/* Allocate shared memory if possible */
-	if ( use_mitshm ) {
-		shminfo.shmid = shmget(IPC_PRIVATE, screen->h*screen->pitch,
-								IPC_CREAT|0777);
-		if ( shminfo.shmid >= 0 ) {
-			shminfo.shmaddr = (char *)shmat(shminfo.shmid, 0, 0);
-			shminfo.readOnly = False;
-			if ( shminfo.shmaddr != (char *)-1 ) {
-				shm_error = False;
-				X_handler = XSetErrorHandler(shm_errhandler);
-				XShmAttach(SDL_Display, &shminfo);
-				XSync(SDL_Display, True);
-				XSetErrorHandler(X_handler);
-				if ( shm_error == True )
-					shmdt(shminfo.shmaddr);
-			} else {
-				shm_error = True;
-			}
-			shmctl(shminfo.shmid, IPC_RMID, NULL);
+	if(!use_mitshm)
+		return;
+	shminfo.shmid = shmget(IPC_PRIVATE, screen->h*screen->pitch,
+			       IPC_CREAT | 0777);
+	if ( shminfo.shmid >= 0 ) {
+		shminfo.shmaddr = (char *)shmat(shminfo.shmid, 0, 0);
+		shminfo.readOnly = False;
+		if ( shminfo.shmaddr != (char *)-1 ) {
+			shm_error = False;
+			X_handler = XSetErrorHandler(shm_errhandler);
+			XShmAttach(SDL_Display, &shminfo);
+			XSync(SDL_Display, True);
+			XSetErrorHandler(X_handler);
+			if (shm_error)
+				shmdt(shminfo.shmaddr);
 		} else {
 			shm_error = True;
 		}
-		if ( shm_error == True )
-			use_mitshm = 0;
-	}
-	if ( use_mitshm ) {
-		screen->pixels = shminfo.shmaddr;
+		shmctl(shminfo.shmid, IPC_RMID, NULL);
 	} else {
-		screen->pixels = malloc(screen->h*screen->pitch);
-	}
-#endif /* NO_SHARED_MEMORY */
-	if ( screen->pixels == NULL ) {
-		SDL_OutOfMemory();
-		return(-1);
+		shm_error = True;
 	}
+	if ( shm_error )
+		use_mitshm = 0;
+	if ( use_mitshm )
+		screen->pixels = shminfo.shmaddr;
+}
+#endif /* ! NO_SHARED_MEMORY */
 
-#ifdef NO_SHARED_MEMORY
-	{
- 	        int bpp = screen->format->BytesPerPixel;
-		SDL_Ximage = XCreateImage(SDL_Display, SDL_Visual,
-					  this->hidden->depth, ZPixmap, 0,
-					  (char *)screen->pixels, 
-					  screen->w, screen->h,
-					  (bpp == 3) ? 32 : bpp * 8,
-					  0);
-	}
-#else
-	if ( use_mitshm ) {
+/* Various screen update functions available */
+static void X11_NormalUpdate(_THIS, int numrects, SDL_Rect *rects);
+static void X11_MITSHMUpdate(_THIS, int numrects, SDL_Rect *rects);
+
+int X11_SetupImage(_THIS, SDL_Surface *screen)
+{
+#ifndef NO_SHARED_MEMORY
+	try_mitshm(this, screen);
+	if(use_mitshm) {
 		SDL_Ximage = XShmCreateImage(SDL_Display, SDL_Visual,
 					     this->hidden->depth, ZPixmap,
 					     shminfo.shmaddr, &shminfo, 
 					     screen->w, screen->h);
-	} else {
- 	        int bpp = screen->format->BytesPerPixel;
-		SDL_Ximage = XCreateImage(SDL_Display, SDL_Visual,
-					  this->hidden->depth, ZPixmap, 0,
-					  (char *)screen->pixels, 
-					  screen->w, screen->h,
-					  (bpp == 3) ? 32 : bpp * 8,
-					  0);
-	}
-#endif /* NO_SHARED_MEMORY */
-	if ( SDL_Ximage == NULL ) {
-		SDL_SetError("Couldn't create XImage");
-#ifndef NO_SHARED_MEMORY
-		if ( use_mitshm ) {
+		if(!SDL_Ximage) {
 			XShmDetach(SDL_Display, &shminfo);
 			XSync(SDL_Display, False);
 			shmdt(shminfo.shmaddr);
 			screen->pixels = NULL;
+			goto error;
 		}
-#endif /* ! NO_SHARED_MEMORY */
-		return(-1);
-	}
-	screen->pitch = SDL_Ximage->bytes_per_line;
-
-	/* Determine what blit function to use */
-#ifdef NO_SHARED_MEMORY
-	this->UpdateRects = X11_NormalUpdate;
-#else
-	if ( use_mitshm ) {
 		this->UpdateRects = X11_MITSHMUpdate;
-	} else {
+	}
+#endif /* not NO_SHARED_MEMORY */
+	if(!use_mitshm) {
+		int bpp;
+		screen->pixels = malloc(screen->h*screen->pitch);
+		if ( screen->pixels == NULL ) {
+			SDL_OutOfMemory();
+			return -1;
+		}
+ 	        bpp = screen->format->BytesPerPixel;
+		SDL_Ximage = XCreateImage(SDL_Display, SDL_Visual,
+					  this->hidden->depth, ZPixmap, 0,
+					  (char *)screen->pixels, 
+					  screen->w, screen->h,
+					  32, 0);
+		if ( SDL_Ximage == NULL )
+			goto error;
+		/* XPutImage will convert byte sex automatically */
+		SDL_Ximage->byte_order = (SDL_BYTEORDER == SDL_BIG_ENDIAN)
+			                 ? MSBFirst : LSBFirst;
 		this->UpdateRects = X11_NormalUpdate;
 	}
-#endif
+	screen->pitch = SDL_Ximage->bytes_per_line;
 	return(0);
+
+error:
+	SDL_SetError("Couldn't create XImage");
+	return 1;
 }
 
 void X11_DestroyImage(_THIS, SDL_Surface *screen)
@@ -250,115 +233,21 @@ int X11_FlipHWSurface(_THIS, SDL_Surface *surface)
 	return(0);
 }
 
-/* Byte-swap the pixels in the display image */
-static void X11_SwapAllPixels(SDL_Surface *screen)
-{
-	int x, y;
-
-	switch (screen->format->BytesPerPixel) {
-	    case 2: {
-		Uint16 *spot;
-		for ( y=0; y<screen->h; ++y ) {
-			spot = (Uint16 *) ((Uint8 *)screen->pixels +
-						y * screen->pitch);
-			for ( x=0; x<screen->w; ++x, ++spot ) {
-				*spot = SDL_Swap16(*spot);
-			}
-		}
-	    }
-	    break;
-
-	    case 4: {
-		Uint32 *spot;
-		for ( y=0; y<screen->h; ++y ) {
-			spot = (Uint32 *) ((Uint8 *)screen->pixels +
-						y * screen->pitch);
-			for ( x=0; x<screen->w; ++x, ++spot ) {
-				*spot = SDL_Swap32(*spot);
-			}
-		}
-	    }
-	    break;
-
-	    default:
-		/* should never get here */
-		break;
-	}
-}
-static void X11_SwapPixels(SDL_Surface *screen, SDL_Rect *rect)
-{
-	int x, minx, maxx;
-	int y, miny, maxy;
-
-	switch (screen->format->BytesPerPixel) {
-	    case 2: {
-		Uint16 *spot;
-		minx = rect->x;
-		maxx = rect->x + rect->w;
-		miny = rect->y;
-		maxy = rect->y + rect->h;
-		for ( y=miny; y<maxy; ++y ) {
-		    spot = (Uint16 *) ((Uint8 *)screen->pixels +
-				       y * screen->pitch + minx * 2);
-		    for ( x=minx; x<maxx; ++x, ++spot ) {
-			*spot = SDL_Swap16(*spot);
-		    }
-		}
-	    }
-	    break;
-
-	    case 4: {
-		Uint32 *spot;
-		minx = rect->x;
-		maxx = rect->x + rect->w;
-		miny = rect->y;
-		maxy = rect->y + rect->h;
-		for ( y=miny; y<maxy; ++y ) {
-		    spot = (Uint32 *) ((Uint8 *)screen->pixels +
-				       y * screen->pitch + minx * 4);
-		    for ( x=minx; x<maxx; ++x, ++spot ) {
-			*spot = SDL_Swap32(*spot);
-		    }
-		}
-	    }
-	    break;
-
-	    default:
-		/* should never get here */
-		break;
-	}
-}
-
 static void X11_NormalUpdate(_THIS, int numrects, SDL_Rect *rects)
 {
 	int i;
-
-	/* Check for endian-swapped X server, swap if necessary (VERY slow!) */
-	if ( swap_pixels &&
-	     ((this->screen->format->BytesPerPixel%2) == 0) ) {
-		for ( i=0; i<numrects; ++i ) {
-			if ( ! rects[i].w ) { /* Clipped? */
-				continue;
-			}
-		        X11_SwapPixels(this->screen, rects + i);
-			XPutImage(GFX_Display, SDL_Window, SDL_GC, SDL_Ximage,
-				rects[i].x, rects[i].y,
-				rects[i].x, rects[i].y, rects[i].w, rects[i].h);
-			X11_SwapPixels(this->screen, rects + i);
-		}
-	} else {
-		for ( i=0; i<numrects; ++i ) {
-			if ( ! rects[i].w ) { /* Clipped? */
-				continue;
-			}
-			XPutImage(GFX_Display, SDL_Window, SDL_GC, SDL_Ximage,
-				rects[i].x, rects[i].y,
-				rects[i].x, rects[i].y, rects[i].w, rects[i].h);
+	
+	for (i = 0; i < numrects; ++i) {
+		if ( rects[i].w == 0 || rects[i].h == 0 ) { /* Clipped? */
+			continue;
 		}
+		XPutImage(GFX_Display, SDL_Window, SDL_GC, SDL_Ximage,
+			  rects[i].x, rects[i].y,
+			  rects[i].x, rects[i].y, rects[i].w, rects[i].h);
 	}
 	if ( SDL_VideoSurface->flags & SDL_ASYNCBLIT ) {
 		XFlush(GFX_Display);
-		++blit_queued;
+		blit_queued = 1;
 	} else {
 		XSync(GFX_Display, False);
 	}
@@ -370,7 +259,7 @@ static void X11_MITSHMUpdate(_THIS, int numrects, SDL_Rect *rects)
 	int i;
 
 	for ( i=0; i<numrects; ++i ) {
-		if ( ! rects[i].w ) { /* Clipped? */
+		if ( rects[i].w == 0 || rects[i].h == 0 ) { /* Clipped? */
 			continue;
 		}
 		XShmPutImage(GFX_Display, SDL_Window, SDL_GC, SDL_Ximage,
@@ -380,7 +269,7 @@ static void X11_MITSHMUpdate(_THIS, int numrects, SDL_Rect *rects)
 	}
 	if ( SDL_VideoSurface->flags & SDL_ASYNCBLIT ) {
 		XFlush(GFX_Display);
-		++blit_queued;
+		blit_queued = 1;
 	} else {
 		XSync(GFX_Display, False);
 	}
@@ -419,21 +308,11 @@ void X11_RefreshDisplay(_THIS)
 		XShmPutImage(SDL_Display, SDL_Window, SDL_GC, SDL_Ximage,
 				0, 0, 0, 0, this->screen->w, this->screen->h,
 				False);
-	} else {
-#else
-	{
+	} else
 #endif /* ! NO_SHARED_MEMORY */
-		/* Check for endian-swapped X server, swap if necessary */
-		if ( swap_pixels &&
-		     ((this->screen->format->BytesPerPixel%2) == 0) ) {
-			X11_SwapAllPixels(this->screen);
-			XPutImage(SDL_Display, SDL_Window, SDL_GC, SDL_Ximage,
-				0, 0, 0, 0, this->screen->w, this->screen->h);
-			X11_SwapAllPixels(this->screen);
-		} else {
-			XPutImage(SDL_Display, SDL_Window, SDL_GC, SDL_Ximage,
-				0, 0, 0, 0, this->screen->w, this->screen->h);
-		}
+	{
+		XPutImage(SDL_Display, SDL_Window, SDL_GC, SDL_Ximage,
+			  0, 0, 0, 0, this->screen->w, this->screen->h);
 	}
 	XSync(SDL_Display, False);
 }

src/video/x11/SDL_x11video.c

@@ -404,26 +404,14 @@ static int X11_VideoInit(_THIS, SDL_PixelFormat *vformat)
 	/* use default screen (from $DISPLAY) */
 	SDL_Screen = DefaultScreen(SDL_Display);
 
+	use_mitshm = 0;
 #ifndef NO_SHARED_MEMORY
 	/* Check for MIT shared memory extension */
-	use_mitshm = 0;
 	if ( local_X11 ) {
 		use_mitshm = XShmQueryExtension(SDL_Display);
 	}
 #endif /* NO_SHARED_MEMORY */
 
-	/* See whether or not we need to swap pixels */
-	swap_pixels = 0;
-	if ( SDL_BYTEORDER == SDL_LIL_ENDIAN ) {
-		if ( XImageByteOrder(SDL_Display) == MSBFirst ) {
-			swap_pixels = 1;
-		}
-	} else {
-		if ( XImageByteOrder(SDL_Display) == LSBFirst ) {
-			swap_pixels = 1;
-		}
-	}
-
 	/* Get the available video modes */
 	if(X11_GetVideoModes(this) < 0)
 	    return -1;

src/video/x11/SDL_x11video.h

@@ -85,7 +85,6 @@ struct SDL_PrivateVideoData {
     /* The variables used for displaying graphics */
     XImage *Ximage;		/* The X image for our window */
     GC	gc;			/* The graphic context for drawing */
-    int swap_pixels;		/* Flag: true if display is swapped endian */
 
     /* The current width and height of the fullscreen mode */
     int current_w;
@@ -171,7 +170,6 @@ struct SDL_PrivateVideoData {
 #define shminfo			(this->hidden->shminfo)
 #define SDL_Ximage		(this->hidden->Ximage)
 #define SDL_GC			(this->hidden->gc)
-#define swap_pixels		(this->hidden->swap_pixels)
 #define current_w		(this->hidden->current_w)
 #define current_h		(this->hidden->current_h)
 #define mouse_last		(this->hidden->mouse_last)