Commits

Lenard Lindstrom committed eb0a75b

fix bug involving blit blends and 32 bit surfaces having non-standard masks

  • Participants
  • Parent commits d30a1c6

Comments (0)

Files changed (3)

File src/alphablit.c

     int             srcppa = (info->src_flags & SDL_SRCALPHA && srcfmt->Amask);
     int             dstppa = (info->dst_flags & SDL_SRCALPHA && dstfmt->Amask);
 
-    if (srcbpp == 4 && dstbpp == 4 &&
-	srcfmt->Rmask == dstfmt->Rmask &&
-	srcfmt->Gmask == dstfmt->Gmask &&
-	srcfmt->Bmask == dstfmt->Bmask &&
-	srcfmt->Amask == dstfmt->Amask &&
-	!(info->src_flags & SDL_SRCALPHA))
+    if (srcbpp >= 3 && dstbpp >= 3 && !(info->src_flags & SDL_SRCALPHA))
     {
+	size_t srcoffsetR, srcoffsetG, srcoffsetB;
+	size_t dstoffsetR, dstoffsetG, dstoffsetB;
+	if (srcbpp == 3)
+	{
+	    SET_OFFSETS_24 (srcoffsetR, srcoffsetG, srcoffsetB, srcfmt);
+	}
+	else
+	{
+	    SET_OFFSETS_32 (srcoffsetR, srcoffsetG, srcoffsetB, srcfmt);
+	}
+	if (dstbpp == 3)
+	{
+	    SET_OFFSETS_24 (dstoffsetR, dstoffsetG, dstoffsetB, dstfmt);
+	}
+	else
+	{
+	    SET_OFFSETS_32 (dstoffsetR, dstoffsetG, dstoffsetB, dstfmt);
+	}
         while (height--)
         {
             LOOP_UNROLLED4(
             {
-                REPEAT_3(
-                {
-                    tmp = (*dst) + (*src);
-                    (*dst) = (tmp <= 255 ? tmp : 255);
-                    src++;
-                    dst++;
-                });
-                src++;
-                dst++;
+		tmp = dst[dstoffsetR] + src[srcoffsetR];
+		dst[dstoffsetR] = (tmp <= 255 ? tmp : 255);
+		tmp = dst[dstoffsetG] + src[srcoffsetG];
+		dst[dstoffsetG] = (tmp <= 255 ? tmp : 255);
+		tmp = dst[dstoffsetB] + src[srcoffsetB];
+		dst[dstoffsetB] = (tmp <= 255 ? tmp : 255);
+                src += srcbpp;
+                dst += dstbpp;
             }, n, width);
             src += srcskip;
             dst += dstskip;
     int             srcppa = (info->src_flags & SDL_SRCALPHA && srcfmt->Amask);
     int             dstppa = (info->dst_flags & SDL_SRCALPHA && dstfmt->Amask);
 
-    if (srcbpp == 4 && dstbpp == 4 &&
-	srcfmt->Rmask == dstfmt->Rmask &&
-	srcfmt->Gmask == dstfmt->Gmask &&
-	srcfmt->Bmask == dstfmt->Bmask &&
-	srcfmt->Amask == dstfmt->Amask &&
-	!(info->src_flags & SDL_SRCALPHA))
+    if (srcbpp >= 3 && dstbpp >= 3 && !(info->src_flags & SDL_SRCALPHA))
     {
+	size_t srcoffsetR, srcoffsetG, srcoffsetB;
+	size_t dstoffsetR, dstoffsetG, dstoffsetB;
+	if (srcbpp == 3)
+	{
+	    SET_OFFSETS_24 (srcoffsetR, srcoffsetG, srcoffsetB, srcfmt);
+	}
+	else
+	{
+	    SET_OFFSETS_32 (srcoffsetR, srcoffsetG, srcoffsetB, srcfmt);
+	}
+	if (dstbpp == 3)
+	{
+	    SET_OFFSETS_24 (dstoffsetR, dstoffsetG, dstoffsetB, dstfmt);
+	}
+	else
+	{
+	    SET_OFFSETS_32 (dstoffsetR, dstoffsetG, dstoffsetB, dstfmt);
+	}
         while (height--)
         {
             LOOP_UNROLLED4(
             {
-                REPEAT_3(
-                {
-                    tmp2 = (*dst) - (*src);
-                    (*dst) = (tmp2 >= 0 ? tmp2 : 0);
-                    src++;
-                    dst++;
-                });
-                src++;
-                dst++;
+		tmp2 = dst[dstoffsetR] - src[srcoffsetR];
+		dst[dstoffsetR] = (tmp2 >= 0 ? tmp2 : 0);
+		tmp2 = dst[dstoffsetG] - src[srcoffsetG];
+		dst[dstoffsetG] = (tmp2 >= 0 ? tmp2 : 0);
+		tmp2 = dst[dstoffsetB] - src[srcoffsetB];
+		dst[dstoffsetB] = (tmp2 >= 0 ? tmp2 : 0);
+                src += srcbpp;
+                dst += dstbpp;
             }, n, width);
             src += srcskip;
             dst += dstskip;
     int             srcppa = (info->src_flags & SDL_SRCALPHA && srcfmt->Amask);
     int             dstppa = (info->dst_flags & SDL_SRCALPHA && dstfmt->Amask);
 
-    if (srcbpp == 4 && dstbpp == 4 &&
-	srcfmt->Rmask == dstfmt->Rmask &&
-	srcfmt->Gmask == dstfmt->Gmask &&
-	srcfmt->Bmask == dstfmt->Bmask &&
-	srcfmt->Amask == dstfmt->Amask &&
-	!(info->src_flags & SDL_SRCALPHA))
+    if (srcbpp >= 3 && dstbpp >= 3 && !(info->src_flags & SDL_SRCALPHA))
     {
+	size_t srcoffsetR, srcoffsetG, srcoffsetB;
+	size_t dstoffsetR, dstoffsetG, dstoffsetB;
+	if (srcbpp == 3)
+	{
+	    SET_OFFSETS_24 (srcoffsetR, srcoffsetG, srcoffsetB, srcfmt);
+	}
+	else
+	{
+	    SET_OFFSETS_32 (srcoffsetR, srcoffsetG, srcoffsetB, srcfmt);
+	}
+	if (dstbpp == 3)
+	{
+	    SET_OFFSETS_24 (dstoffsetR, dstoffsetG, dstoffsetB, dstfmt);
+	}
+	else
+	{
+	    SET_OFFSETS_32 (dstoffsetR, dstoffsetG, dstoffsetB, dstfmt);
+	}
         while (height--)
         {
             LOOP_UNROLLED4(
             {
-                REPEAT_3(
-                {
-                    tmp = ((*dst) && (*src)) ? ((*dst) * (*src)) >> 8 : 0;
-                    (*dst) = (tmp <= 255 ? tmp : 255);
-                    src++;
-                    dst++;
-                });
-                src++;
-                dst++;
+		tmp = ((dst[dstoffsetR] && src[srcoffsetR]) ?
+		       (dst[dstoffsetR] * src[srcoffsetR]) >> 8 : 0);
+		dst[dstoffsetR] = (tmp <= 255 ? tmp : 255);
+		tmp = ((dst[dstoffsetG] && src[srcoffsetG]) ?
+		       (dst[dstoffsetG] * src[srcoffsetG]) >> 8 : 0);
+		dst[dstoffsetG] = (tmp <= 255 ? tmp : 255);
+		tmp = ((dst[dstoffsetB] && src[srcoffsetB]) ?
+		       (dst[dstoffsetB] * src[srcoffsetB]) >> 8 : 0);
+		dst[dstoffsetB] = (tmp <= 255 ? tmp : 255);
+                src += srcbpp;
+                dst += dstbpp;
             }, n, width);
             src += srcskip;
             dst += dstskip;
     int             srcppa = (info->src_flags & SDL_SRCALPHA && srcfmt->Amask);
     int             dstppa = (info->dst_flags & SDL_SRCALPHA && dstfmt->Amask);
 
-    if (srcbpp == 4 && dstbpp == 4 &&
-	srcfmt->Rmask == dstfmt->Rmask &&
-	srcfmt->Gmask == dstfmt->Gmask &&
-	srcfmt->Bmask == dstfmt->Bmask &&
-	srcfmt->Amask == dstfmt->Amask &&
-	!(info->src_flags & SDL_SRCALPHA))
+    if (srcbpp >= 3 && dstbpp >= 3 && !(info->src_flags & SDL_SRCALPHA))
     {
+	size_t srcoffsetR, srcoffsetG, srcoffsetB;
+	size_t dstoffsetR, dstoffsetG, dstoffsetB;
+	if (srcbpp == 3)
+	{
+	    SET_OFFSETS_24 (srcoffsetR, srcoffsetG, srcoffsetB, srcfmt);
+	}
+	else
+	{
+	    SET_OFFSETS_32 (srcoffsetR, srcoffsetG, srcoffsetB, srcfmt);
+	}
+	if (dstbpp == 3)
+	{
+	    SET_OFFSETS_24 (dstoffsetR, dstoffsetG, dstoffsetB, dstfmt);
+	}
+	else
+	{
+	    SET_OFFSETS_32 (dstoffsetR, dstoffsetG, dstoffsetB, dstfmt);
+	}
         while (height--)
         {
             LOOP_UNROLLED4(
             {
-                REPEAT_3(
-                {
-                    if ((*src) < (*dst))
-                        (*dst) = (*src);
-                    src++;
-                    dst++;
-                });
-                src++;
-                dst++;
+		if (src[srcoffsetR] < dst[dstoffsetR])
+		{
+		    dst[dstoffsetR] = src[srcoffsetR];
+		}
+		if (src[srcoffsetG] < dst[dstoffsetG])
+		{
+		    dst[dstoffsetG] = src[srcoffsetG];
+		}
+		if (src[srcoffsetB] < dst[dstoffsetB])
+		{
+		    dst[dstoffsetB] = src[srcoffsetB];
+		}
+                src += srcbpp;
+                dst += dstbpp;
             }, n, width);
             src += srcskip;
             dst += dstskip;
     int             srcppa = (info->src_flags & SDL_SRCALPHA && srcfmt->Amask);
     int             dstppa = (info->dst_flags & SDL_SRCALPHA && dstfmt->Amask);
 
-    if (srcbpp == 4 && dstbpp == 4 &&
-	srcfmt->Rmask == dstfmt->Rmask &&
-	srcfmt->Gmask == dstfmt->Gmask &&
-	srcfmt->Bmask == dstfmt->Bmask &&
-	srcfmt->Amask == dstfmt->Amask &&
-	!(info->src_flags & SDL_SRCALPHA))
+    if (srcbpp >= 3 && dstbpp >= 3 && !(info->src_flags & SDL_SRCALPHA))
     {
+	size_t srcoffsetR, srcoffsetG, srcoffsetB;
+	size_t dstoffsetR, dstoffsetG, dstoffsetB;
+	if (srcbpp == 3)
+	{
+	    SET_OFFSETS_24 (srcoffsetR, srcoffsetG, srcoffsetB, srcfmt);
+	}
+	else
+	{
+	    SET_OFFSETS_32 (srcoffsetR, srcoffsetG, srcoffsetB, srcfmt);
+	}
+	if (dstbpp == 3)
+	{
+	    SET_OFFSETS_24 (dstoffsetR, dstoffsetG, dstoffsetB, dstfmt);
+	}
+	else
+	{
+	    SET_OFFSETS_32 (dstoffsetR, dstoffsetG, dstoffsetB, dstfmt);
+	}
         while (height--)
         {
             LOOP_UNROLLED4(
             {
-                REPEAT_3(
-                {
-                    if ((*src) > (*dst))
-                        (*dst) = (*src);
-                    src++;
-                    dst++;
-                });
-                src++;
-                dst++;
+		if (src[srcoffsetR] > dst[dstoffsetR])
+		{
+		    dst[dstoffsetR] = src[srcoffsetR];
+		}
+		if (src[srcoffsetG] > dst[dstoffsetG])
+		{
+		    dst[dstoffsetG] = src[srcoffsetG];
+		}
+		if (src[srcoffsetB] > dst[dstoffsetB])
+		{
+		    dst[dstoffsetB] = src[srcoffsetB];
+		}
+                src += srcbpp;
+                dst += dstbpp;
             }, n, width);
             src += srcskip;
             dst += dstskip;
         return;
     }
 
-
     if (srcbpp == 1)
     {
         if (dstbpp == 1)

File src/surface.h

           fmt->Bshift == 8 ? 1 :                  \
 	                     2   );               \
     }
+
+#define SET_OFFSETS_32(or, og, ob, fmt)           \
+    {                                             \
+    or = (fmt->Rshift == 0  ? 0 :                 \
+          fmt->Rshift == 8  ? 1 :                 \
+          fmt->Rshift == 16 ? 2 :                 \
+	                      3   );              \
+    og = (fmt->Gshift == 0  ? 0 :                 \
+          fmt->Gshift == 8  ? 1 :                 \
+          fmt->Gshift == 16 ? 2 :                 \
+	                      3   );              \
+    ob = (fmt->Bshift == 0  ? 0 :                 \
+          fmt->Bshift == 8  ? 1 :                 \
+          fmt->Bshift == 16 ? 2 :                 \
+	                      3   );              \
+    }
 #else
 #define SET_OFFSETS_24(or, og, ob, fmt)           \
     {                                             \
           fmt->Bshift == 8 ? 1 :                  \
 	                     0   );               \
     }
+
+#define SET_OFFSETS_32(or, og, ob, fmt)           \
+    {                                             \
+    or = (fmt->Rshift == 0  ? 3 :                 \
+          fmt->Rshift == 8  ? 2 :                 \
+          fmt->Rshift == 16 ? 1 :                 \
+	                      0   );              \
+    og = (fmt->Gshift == 0  ? 3 :                 \
+          fmt->Gshift == 8  ? 2 :                 \
+          fmt->Gshift == 16 ? 1 :                 \
+	                      0   );              \
+    ob = (fmt->Bshift == 0  ? 3 :                 \
+          fmt->Bshift == 8  ? 2 :                 \
+          fmt->Bshift == 16 ? 1 :                 \
+	                      0   );              \
+    }
 #endif
 
 

File test/surface_test.py

 import pygame
 from pygame.locals import *
 
+def intify(i):
+    """If i is a long, cast to an int while preserving the bits"""
+    if 0x10000000 & i:
+        return int(~(0xFFFFFFFF ^ i))
+    return i
+
+def longify(i):
+    """If i in an int, cast to a long while preserving the bits"""
+    if i < 0:
+        return 0xFFFFFFFF & i
+    return long(i)
+
 class SurfaceTypeTest(unittest.TestCase):
     def test_set_clip( self ):
         """ see if surface.set_clip(None) works correctly.
     test_palette = [(0, 0, 0, 255),
                     (10, 30, 60, 0),
                     (25, 75, 100, 128),
-                    (100, 150, 200, 200),
+                    (200, 150, 100, 200),
                     (0, 100, 200, 255)]
     surf_size = (10, 12)
     test_points = [((0, 0), 1), ((4, 5), 1), ((9, 0), 2),
                         self._make_surface(32, srcalpha=True)]
         blend = [('BLEND_ADD', (0, 25, 100, 255),
                   lambda a, b: min(a + b, 255)),
-                 ('BLEND_SUB', (0, 25, 100, 100),
+                 ('BLEND_SUB', (100, 25, 0, 100),
                   lambda a, b: max(a - b, 0)),
-                 ('BLEND_MULT', (0, 7, 100, 0),
+                 ('BLEND_MULT', (100, 200, 0, 0),
                   lambda a, b: (a * b) // 256),
-                 ('BLEND_MIN', (0, 255, 0, 255), min),
+                 ('BLEND_MIN', (255, 0, 0, 255), min),
                  ('BLEND_MAX', (0, 255, 0, 255), max)]
 
         for src in sources:
                      special_flags=getattr(pygame, blend_name))
             self._assert_surface(dst, p, ", %s" % blend_name)
 
+        # Blend blits are special cased for 32 to 32 bit surfaces.
+        #
+        # Confirm that it works when the rgb bytes are not the
+        # least significant bytes.
+        pat = self._make_src_surface(32)
+        masks = pat.get_masks()
+        if min(masks) == intify(0xFF000000):
+            masks = [longify(m) >> 8 for m in masks]
+        else:
+            masks = [intify(m << 8) for m in masks]
+        src = pygame.Surface(pat.get_size(), 0, 32, masks)
+        self._fill_surface(src)
+        dst = pygame.Surface(src.get_size(), 0, 32, masks)
+        for blend_name, dst_color, op in blend:
+            p = []
+            for src_color in self.test_palette:
+                c = [op(dst_color[i], src_color[i]) for i in range(3)]
+                c.append(255)
+                p.append(tuple(c))
+            dst.fill(dst_color)
+            dst.blit(src,
+                     (0, 0),
+                     special_flags=getattr(pygame, blend_name))
+            self._assert_surface(dst, p, ", %s" % blend_name)
+
     def test_blit_blend_rgba(self):
         sources = [self._make_src_surface(8),
                    self._make_src_surface(16),
                                            src.get_bitsize(),
                                            src.get_flags())))
 
+        # Blend blits are special cased for 32 to 32 bit surfaces
+        # with per-pixel alpha.
+        #
+        # Confirm the general case is used instead when the formats differ.
         src = self._make_src_surface(32, srcalpha=True)
         masks = src.get_masks()
         dst = pygame.Surface(src.get_size(), SRCALPHA, 32,
                      special_flags=getattr(pygame, blend_name))
             self._assert_surface(dst, p, ", %s" % blend_name)
 
+        # Confirm this special case handles subsurfaces.
+        src = pygame.Surface((8, 10), SRCALPHA, 32)
+        dst = pygame.Surface((8, 10), SRCALPHA, 32)
+        tst = pygame.Surface((8, 10), SRCALPHA, 32)
+        src.fill((1, 2, 3, 4))
+        dst.fill((40, 30, 20, 10))
+        subsrc = src.subsurface((2, 3, 4, 4))
+        try:
+            subdst = dst.subsurface((2, 3, 4, 4))
+            try:
+                subdst.blit(subsrc, (0, 0), special_flags=BLEND_RGBA_ADD)
+            finally:
+                del subdst
+        finally:
+            del subsrc
+        tst.fill((40, 30, 20, 10))
+        tst.fill((41, 32, 23, 14), (2, 3, 4, 4))
+        for x in range(8):
+            for y in range(10):
+                self.failUnlessEqual(dst.get_at((x, y)), tst.get_at((x, y)),
+                                     "%s != %s at (%i, %i)" %
+                                     (dst.get_at((x, y)), tst.get_at((x, y)),
+                                      x, y))
+
     def test_GET_PIXELVALS(self):
         # surface.h GET_PIXELVALS bug regarding whether of not
         # a surface has per-pixel alpha. Looking at the Amask
         # is not enough. The surface's SRCALPHA flag must also
-        # be considered.
+        # be considered. Fix rev. 1923.
         src = self._make_surface(32, srcalpha=True)
         src.fill((0, 0, 0, 128))
         src.set_alpha(None)  # Clear SRCALPHA flag.