Merge pull request #2067 from Starbuck5/destination-alpha0

Enforce dest_alpha->0 in opaque dest blitter

Author: Starbuck5

src_c/simd_blitters_avx2.c

@@ -201,6 +201,11 @@ alphablit_alpha_avx2_argb_no_surf_alpha_opaque_dst(SDL_BlitInfo *info)
 
     __m256i src_alpha, temp;
 
+    /* The location of the destination's missing alpha channel can be masked
+     * out by composing all the other channels' masks. */
+    __m256i mask_out_alpha = _mm256_set1_epi32(
+        (info->dst->Rmask | info->dst->Gmask | info->dst->Bmask));
+
     /* Original 'Straight Alpha' blending equation:
         --------------------------------------------
         dstRGB = (srcRGB * srcA) + (dstRGB * (1-srcA))
@@ -237,14 +242,21 @@ alphablit_alpha_avx2_argb_no_surf_alpha_opaque_dst(SDL_BlitInfo *info)
             dstRGBA >>= 8
         */
 
-    RUN_AVX2_BLITTER(RUN_16BIT_SHUFFLE_OUT(
-        src_alpha = _mm256_shuffle_epi8(shuff_src, shuff_out_alpha);
-        temp = _mm256_sub_epi16(shuff_src, shuff_dst);
-        temp = _mm256_mullo_epi16(temp, src_alpha);
-        shuff_dst = _mm256_slli_epi16(shuff_dst, 8);
-        shuff_dst = _mm256_add_epi16(shuff_dst, temp);
-        shuff_dst = _mm256_add_epi16(shuff_dst, shuff_src);
-        shuff_dst = _mm256_srli_epi16(shuff_dst, 8);))
+    RUN_AVX2_BLITTER(
+        RUN_16BIT_SHUFFLE_OUT(
+            src_alpha = _mm256_shuffle_epi8(shuff_src, shuff_out_alpha);
+            temp = _mm256_sub_epi16(shuff_src, shuff_dst);
+            temp = _mm256_mullo_epi16(temp, src_alpha);
+            shuff_dst = _mm256_slli_epi16(shuff_dst, 8);
+            shuff_dst = _mm256_add_epi16(shuff_dst, temp);
+            shuff_dst = _mm256_add_epi16(shuff_dst, shuff_src);
+            shuff_dst = _mm256_srli_epi16(shuff_dst, 8););
+
+        /* It seems like the destination pixel alpha shouldn't matter, since
+         * it is RGBX, but it has to be zero. I suspect this is a weak spot
+         * in the other blitting routines, that they need alpha 0 from these
+         * surfaces. */
+        pixels_dst = _mm256_and_si256(pixels_dst, mask_out_alpha);)
 }
 #else
 void

test/surface_test.py

@@ -18,6 +18,7 @@
 import gc
 import weakref
 import ctypes
+import itertools
 
 IS_PYPY = "PyPy" == platform.python_implementation()
 
@@ -1084,6 +1085,41 @@ def test_blit__SRCALPHA32_to_8(self):
         source.set_at((0, 0), test_color)
         target.blit(source, (0, 0))
 
+    def test_blit__SCALPHA32_TO_OPAQUE32(self):
+        # Apparently these types of blits need to write 0 to the destination's
+        # alpha channel (it's not really an alpha channel but it's treated like one)
+        # See https://github.com/pygame-community/pygame-ce/pull/2067
+
+        alphas = [0, 10, 50, 122, 240, 255]
+
+        combinations = list(itertools.combinations_with_replacement(alphas, 2))
+        width = len(combinations)
+
+        # masks explicitly specified so direct pixel access of bytes below is
+        # gauranteed to be stable
+        surf1 = pygame.Surface((width, 1), depth=32, masks=(0xFF0000, 0xFF00, 0xFF, 0))
+        surf2 = pygame.Surface(
+            (width, 1),
+            pygame.SRCALPHA,
+            depth=32,
+            masks=(0xFF0000, 0xFF00, 0xFF, 0xFF000000),
+        )
+
+        for i in range(width):
+            alpha1, alpha2 = combinations[i]
+            surf1.set_at((i, 0), (0, 0, 0, alpha1))
+            surf2.set_at((i, 0), (0, 0, 0, alpha2))
+
+        surf1.blit(surf2, (0, 0))
+
+        # Why use get_buffer?
+        # get_at for RGBX surfaces seems to always think A=255, regardless
+        # of bytes in surface, which makes sense.
+        surf1_bytes = surf1.get_buffer().raw
+        for i in range(width):
+            # +3 gets the "alpha channel" in this pixel format
+            assert surf1_bytes[i * 4 + 3] == 0
+
 
 class GeneralSurfaceTests(unittest.TestCase):
     @unittest.skipIf(