Commit b7ede6cc47ffdb5e0bb253181595472ea520f985

Sam Lantinga 2015-06-19T23:20:43

Fixed bug 1550 - SDL_RenderCopy/CopyEx in software should optionally render 8bit alpha Adam M. There are three problems in the code that I see. 1. SW_RenderCopyEx enables a color key on surface_scaled even if the source surface didn't have a color key. 2. SW_RenderCopyEx doesn't copy blend mode, color mod, or alpha mod from src to surface_scaled. 3. When SDL_BlitScaled(src, srcrect, surface_scaled, &tmp_rect) is called, it blends the src pixels into surface_scaled instead of overwriting them (if src has blending, etc. enabled). I've attached a patch that 1) fixes the three problems that I mentioned, 2) adds the requested performance improvement of using the regular blit function if no rotation or flipping is needed, 3) avoids cloning the source surface if no stretching is required, and simplifies the rotation code slightly.

diff --git a/src/render/SDL_render.c b/src/render/SDL_render.c
index 1ddb623..088fe7c 100644
--- a/src/render/SDL_render.c
+++ b/src/render/SDL_render.c
@@ -1734,6 +1734,10 @@ SDL_RenderCopyEx(SDL_Renderer * renderer, SDL_Texture * texture,
     SDL_FRect frect;
     SDL_FPoint fcenter;
 
+    if (flip == SDL_FLIP_NONE && angle == 0) { /* fast path when we don't need rotation or flipping */
+        return SDL_RenderCopy(renderer, texture, srcrect, dstrect);
+    }
+
     CHECK_RENDERER_MAGIC(renderer, -1);
     CHECK_TEXTURE_MAGIC(texture, -1);
 
diff --git a/src/render/software/SDL_render_sw.c b/src/render/software/SDL_render_sw.c
index 918f32b..119b200 100644
--- a/src/render/software/SDL_render_sw.c
+++ b/src/render/software/SDL_render_sw.c
@@ -600,7 +600,6 @@ SW_RenderCopyEx(SDL_Renderer * renderer, SDL_Texture * texture,
     SDL_Surface *src = (SDL_Surface *) texture->driverdata;
     SDL_Rect final_rect, tmp_rect;
     SDL_Surface *surface_rotated, *surface_scaled;
-    Uint32 colorkey;
     int retval, dstwidth, dstheight, abscenterx, abscentery;
     double cangle, sangle, px, py, p1x, p1y, p2x, p2y, p3x, p3y, p4x, p4y;
 
@@ -618,66 +617,113 @@ SW_RenderCopyEx(SDL_Renderer * renderer, SDL_Texture * texture,
     final_rect.w = (int)dstrect->w;
     final_rect.h = (int)dstrect->h;
 
-    surface_scaled = SDL_CreateRGBSurface(SDL_SWSURFACE, final_rect.w, final_rect.h, src->format->BitsPerPixel,
-                                          src->format->Rmask, src->format->Gmask,
-                                          src->format->Bmask, src->format->Amask );
-    if (surface_scaled) {
-        SDL_GetColorKey(src, &colorkey);
-        SDL_SetColorKey(surface_scaled, SDL_TRUE, colorkey);
-        tmp_rect = final_rect;
-        tmp_rect.x = 0;
-        tmp_rect.y = 0;
-
-        retval = SDL_BlitScaled(src, srcrect, surface_scaled, &tmp_rect);
-        if (!retval) {
-            SDLgfx_rotozoomSurfaceSizeTrig(tmp_rect.w, tmp_rect.h, -angle, &dstwidth, &dstheight, &cangle, &sangle);
-            surface_rotated = SDLgfx_rotateSurface(surface_scaled, -angle, dstwidth/2, dstheight/2, GetScaleQuality(), flip & SDL_FLIP_HORIZONTAL, flip & SDL_FLIP_VERTICAL, dstwidth, dstheight, cangle, sangle);
-            if(surface_rotated) {
-                /* Find out where the new origin is by rotating the four final_rect points around the center and then taking the extremes */
-                abscenterx = final_rect.x + (int)center->x;
-                abscentery = final_rect.y + (int)center->y;
-                /* Compensate the angle inversion to match the behaviour of the other backends */
-                sangle = -sangle;
-
-                /* Top Left */
-                px = final_rect.x - abscenterx;
-                py = final_rect.y - abscentery;
-                p1x = px * cangle - py * sangle + abscenterx;
-                p1y = px * sangle + py * cangle + abscentery;
-
-                /* Top Right */
-                px = final_rect.x + final_rect.w - abscenterx;
-                py = final_rect.y - abscentery;
-                p2x = px * cangle - py * sangle + abscenterx;
-                p2y = px * sangle + py * cangle + abscentery;
-
-                /* Bottom Left */
-                px = final_rect.x - abscenterx;
-                py = final_rect.y + final_rect.h - abscentery;
-                p3x = px * cangle - py * sangle + abscenterx;
-                p3y = px * sangle + py * cangle + abscentery;
-
-                /* Bottom Right */
-                px = final_rect.x + final_rect.w - abscenterx;
-                py = final_rect.y + final_rect.h - abscentery;
-                p4x = px * cangle - py * sangle + abscenterx;
-                p4y = px * sangle + py * cangle + abscentery;
-
-                tmp_rect.x = (int)MIN(MIN(p1x, p2x), MIN(p3x, p4x));
-                tmp_rect.y = (int)MIN(MIN(p1y, p2y), MIN(p3y, p4y));
-                tmp_rect.w = dstwidth;
-                tmp_rect.h = dstheight;
-
-                retval = SDL_BlitSurface(surface_rotated, NULL, surface, &tmp_rect);
+    /* SDLgfx_rotateSurface doesn't accept a source rectangle, so crop and scale if we need to */
+    tmp_rect = final_rect;
+    tmp_rect.x = 0;
+    tmp_rect.y = 0;
+    if (srcrect->w == final_rect.w && srcrect->h == final_rect.h && srcrect->x == 0 && srcrect->y == 0) {
+        surface_scaled = src; /* but if we don't need to, just use the original */
+        retval = 0;
+    } else {
+        SDL_Surface *blit_src = src;
+        Uint32 colorkey;
+        SDL_BlendMode blendMode;
+        Uint8 alphaMod, r, g, b;
+        SDL_bool cloneSource = SDL_FALSE;
+
+        surface_scaled = SDL_CreateRGBSurface(SDL_SWSURFACE, final_rect.w, final_rect.h, src->format->BitsPerPixel,
+                                              src->format->Rmask, src->format->Gmask,
+                                              src->format->Bmask, src->format->Amask );
+        if (!surface_scaled) {
+            return -1;
+        }
+
+        /* copy the color key, alpha mod, blend mode, and color mod so the scaled surface behaves like the source */
+        if (SDL_GetColorKey(src, &colorkey) == 0) {
+            SDL_SetColorKey(surface_scaled, SDL_TRUE, colorkey);
+            cloneSource = SDL_TRUE;
+        }
+        SDL_GetSurfaceAlphaMod(src, &alphaMod); /* these will be copied to surface_scaled below if necessary */
+        SDL_GetSurfaceBlendMode(src, &blendMode);
+        SDL_GetSurfaceColorMod(src, &r, &g, &b);
+
+        /* now we need to blit the src into surface_scaled. since we want to copy the colors from the source to
+         * surface_scaled rather than blend them, etc. we'll need to disable the blend mode, alpha mod, etc.
+         * but we don't want to modify src (in case it's being used on other threads), so we'll need to clone it
+         * before changing the blend options
+         */
+        cloneSource |= blendMode != SDL_BLENDMODE_NONE || (alphaMod & r & g & b) != 255;
+        if (cloneSource) {
+            blit_src = SDL_ConvertSurface(src, src->format, src->flags); /* clone src */
+            if (!blit_src) {
                 SDL_FreeSurface(surface_scaled);
-                SDL_FreeSurface(surface_rotated);
-                return retval;
+                return -1;
             }
+            SDL_SetSurfaceAlphaMod(blit_src, 255); /* disable all blending options in blit_src */
+            SDL_SetSurfaceBlendMode(blit_src, SDL_BLENDMODE_NONE);
+            SDL_SetColorKey(blit_src, 0, 0);
+            SDL_SetSurfaceColorMod(blit_src, 255, 255, 255);
+            SDL_SetSurfaceRLE(blit_src, 0); /* don't RLE encode a surface we'll only use once */
+
+            SDL_SetSurfaceAlphaMod(surface_scaled, alphaMod); /* copy blending options to surface_scaled */
+            SDL_SetSurfaceBlendMode(surface_scaled, blendMode);
+            SDL_SetSurfaceColorMod(surface_scaled, r, g, b);
+        }
+
+        retval = SDL_BlitScaled(blit_src, srcrect, surface_scaled, &tmp_rect);
+        if (blit_src != src) {
+            SDL_FreeSurface(blit_src);
         }
-        return retval;
     }
 
-    return -1;
+    if (!retval) {
+        SDLgfx_rotozoomSurfaceSizeTrig(tmp_rect.w, tmp_rect.h, -angle, &dstwidth, &dstheight, &cangle, &sangle);
+        surface_rotated = SDLgfx_rotateSurface(surface_scaled, -angle, dstwidth/2, dstheight/2, GetScaleQuality(), flip & SDL_FLIP_HORIZONTAL, flip & SDL_FLIP_VERTICAL, dstwidth, dstheight, cangle, sangle);
+        if(surface_rotated) {
+            /* Find out where the new origin is by rotating the four final_rect points around the center and then taking the extremes */
+            abscenterx = final_rect.x + (int)center->x;
+            abscentery = final_rect.y + (int)center->y;
+            /* Compensate the angle inversion to match the behaviour of the other backends */
+            sangle = -sangle;
+
+            /* Top Left */
+            px = final_rect.x - abscenterx;
+            py = final_rect.y - abscentery;
+            p1x = px * cangle - py * sangle + abscenterx;
+            p1y = px * sangle + py * cangle + abscentery;
+
+            /* Top Right */
+            px = final_rect.x + final_rect.w - abscenterx;
+            py = final_rect.y - abscentery;
+            p2x = px * cangle - py * sangle + abscenterx;
+            p2y = px * sangle + py * cangle + abscentery;
+
+            /* Bottom Left */
+            px = final_rect.x - abscenterx;
+            py = final_rect.y + final_rect.h - abscentery;
+            p3x = px * cangle - py * sangle + abscenterx;
+            p3y = px * sangle + py * cangle + abscentery;
+
+            /* Bottom Right */
+            px = final_rect.x + final_rect.w - abscenterx;
+            py = final_rect.y + final_rect.h - abscentery;
+            p4x = px * cangle - py * sangle + abscenterx;
+            p4y = px * sangle + py * cangle + abscentery;
+
+            tmp_rect.x = (int)MIN(MIN(p1x, p2x), MIN(p3x, p4x));
+            tmp_rect.y = (int)MIN(MIN(p1y, p2y), MIN(p3y, p4y));
+            tmp_rect.w = dstwidth;
+            tmp_rect.h = dstheight;
+
+            retval = SDL_BlitSurface(surface_rotated, NULL, surface, &tmp_rect);
+            SDL_FreeSurface(surface_rotated);
+        }
+    }
+
+    if (surface_scaled != src) {
+        SDL_FreeSurface(surface_scaled);
+    }
+    return retval;
 }
 
 static int
diff --git a/src/render/software/SDL_rotate.c b/src/render/software/SDL_rotate.c
index 0edf784..3a8bc21 100644
--- a/src/render/software/SDL_rotate.c
+++ b/src/render/software/SDL_rotate.c
@@ -188,10 +188,8 @@ _transformSurfaceRGBA(SDL_Surface * src, SDL_Surface * dst, int cx, int cy, int 
                 dy = (sdy >> 16);
                 if (flipx) dx = sw - dx;
                 if (flipy) dy = sh - dy;
-                if ((dx > -1) && (dy > -1) && (dx < (src->w-1)) && (dy < (src->h-1))) {
-                    sp = (tColorRGBA *)src->pixels;
-                    sp += ((src->pitch/4) * dy);
-                    sp += dx;
+                if ((unsigned)dx < (unsigned)sw && (unsigned)dy < (unsigned)sh) {
+                    sp = (tColorRGBA *) ((Uint8 *) src->pixels + src->pitch * dy) + dx;
                     c00 = *sp;
                     sp += 1;
                     c01 = *sp;
@@ -237,14 +235,12 @@ _transformSurfaceRGBA(SDL_Surface * src, SDL_Surface * dst, int cx, int cy, int 
             sdx = (ax + (isin * dy)) + xd;
             sdy = (ay - (icos * dy)) + yd;
             for (x = 0; x < dst->w; x++) {
-                dx = (short) (sdx >> 16);
-                dy = (short) (sdy >> 16);
-                if (flipx) dx = (src->w-1)-dx;
-                if (flipy) dy = (src->h-1)-dy;
-                if ((dx >= 0) && (dy >= 0) && (dx < src->w) && (dy < src->h)) {
-                    sp = (tColorRGBA *) ((Uint8 *) src->pixels + src->pitch * dy);
-                    sp += dx;
-                    *pc = *sp;
+                dx = (sdx >> 16);
+                dy = (sdy >> 16);
+                if ((unsigned)dx < (unsigned)src->w && (unsigned)dy < (unsigned)src->h) {
+                    if(flipx) dx = sw - dx;
+                    if(flipy) dy = sh - dy;
+                    *pc = *((tColorRGBA *)((Uint8 *)src->pixels + src->pitch * dy) + dx);
                 }
                 sdx += icos;
                 sdy += isin;
@@ -277,7 +273,7 @@ static void
 transformSurfaceY(SDL_Surface * src, SDL_Surface * dst, int cx, int cy, int isin, int icos, int flipx, int flipy)
 {
     int x, y, dx, dy, xd, yd, sdx, sdy, ax, ay;
-    tColorY *pc, *sp;
+    tColorY *pc;
     int gap;
 
     /*
@@ -301,14 +297,12 @@ transformSurfaceY(SDL_Surface * src, SDL_Surface * dst, int cx, int cy, int isin
         sdx = (ax + (isin * dy)) + xd;
         sdy = (ay - (icos * dy)) + yd;
         for (x = 0; x < dst->w; x++) {
-            dx = (short) (sdx >> 16);
-            dy = (short) (sdy >> 16);
-            if (flipx) dx = (src->w-1)-dx;
-            if (flipy) dy = (src->h-1)-dy;
-            if ((dx >= 0) && (dy >= 0) && (dx < src->w) && (dy < src->h)) {
-                sp = (tColorY *) (src->pixels);
-                sp += (src->pitch * dy + dx);
-                *pc = *sp;
+            dx = (sdx >> 16);
+            dy = (sdy >> 16);
+            if ((unsigned)dx < (unsigned)src->w && (unsigned)dy < (unsigned)src->h) {
+                if (flipx) dx = (src->w-1)-dx;
+                if (flipy) dy = (src->h-1)-dy;
+                *pc = *((tColorY *)src->pixels + src->pitch * dy + dx);
             }
             sdx += icos;
             sdy += isin;