Unverified Commit 1d9d83e0 authored by Grimmauld's avatar Grimmauld Committed by GitHub
Browse files

python3Packages.pygame{,-ce}: fix tests with new sdl2-compat (#460025)

parents 2803f914 0fa3aaa7
Loading
Loading
Loading
Loading
+151 −0
Original line number Diff line number Diff line
From 3e3fbdc11ab00c4c04eb68c40b23979245c987fa Mon Sep 17 00:00:00 2001
From: Marcin Serwin <marcin@serwin.dev>
Date: Sat, 8 Nov 2025 19:47:41 +0100
Subject: [PATCH] Use SDL_AllocFormat instead of creating it manually

According to the docs, `SDL_PixelFormat` is a read-only structure.
Creating it manually leaves out some important fields like `format` and
`next` pointer to be undefined.

Signed-off-by: Marcin Serwin <marcin@serwin.dev>
---
 src_c/surface.c | 80 ++++++++++++++-----------------------------------
 1 file changed, 23 insertions(+), 57 deletions(-)

diff --git a/src_c/surface.c b/src_c/surface.c
index f118a4db4..e51e80554 100644
--- a/src_c/surface.c
+++ b/src_c/surface.c
@@ -1844,9 +1844,8 @@ surf_convert(pgSurfaceObject *self, PyObject *args)
              */
             int bpp = 0;
             SDL_Palette *palette = SDL_AllocPalette(default_palette_size);
-            SDL_PixelFormat format;
+            Uint32 format_enum = 0;
 
-            memcpy(&format, surf->format, sizeof(format));
             if (pg_IntFromObj(argobject, &bpp)) {
                 Uint32 Rmask, Gmask, Bmask, Amask;
 
@@ -1904,30 +1903,23 @@ surf_convert(pgSurfaceObject *self, PyObject *args)
                                          "nonstandard bit depth given");
                     }
                 }
-                format.Rmask = Rmask;
-                format.Gmask = Gmask;
-                format.Bmask = Bmask;
-                format.Amask = Amask;
+                format_enum = SDL_MasksToPixelFormatEnum(bpp, Rmask, Gmask,
+                                                         Bmask, Amask);
             }
             else if (PySequence_Check(argobject) &&
                      PySequence_Size(argobject) == 4) {
-                Uint32 mask;
+                Uint32 Rmask, Gmask, Bmask, Amask;
 
-                if (!pg_UintFromObjIndex(argobject, 0, &format.Rmask) ||
-                    !pg_UintFromObjIndex(argobject, 1, &format.Gmask) ||
-                    !pg_UintFromObjIndex(argobject, 2, &format.Bmask) ||
-                    !pg_UintFromObjIndex(argobject, 3, &format.Amask)) {
+                if (!pg_UintFromObjIndex(argobject, 0, &Rmask) ||
+                    !pg_UintFromObjIndex(argobject, 1, &Gmask) ||
+                    !pg_UintFromObjIndex(argobject, 2, &Bmask) ||
+                    !pg_UintFromObjIndex(argobject, 3, &Amask)) {
                     pgSurface_Unprep(self);
                     return RAISE(PyExc_ValueError,
                                  "invalid color masks given");
                 }
-                mask =
-                    format.Rmask | format.Gmask | format.Bmask | format.Amask;
-                for (bpp = 0; bpp < 32; ++bpp) {
-                    if (!(mask >> bpp)) {
-                        break;
-                    }
-                }
+                format_enum = SDL_MasksToPixelFormatEnum(bpp, Rmask, Gmask,
+                                                         Bmask, Amask);
             }
             else {
                 pgSurface_Unprep(self);
@@ -1935,22 +1927,11 @@ surf_convert(pgSurfaceObject *self, PyObject *args)
                     PyExc_ValueError,
                     "invalid argument specifying new format to convert to");
             }
-            format.BitsPerPixel = (Uint8)bpp;
-            format.BytesPerPixel = (bpp + 7) / 8;
-            if (PG_FORMAT_BitsPerPixel((&format)) > 8) {
-                /* Allow a 8 bit source surface with an empty palette to be
-                 * converted to a format without a palette (pygame-ce issue
-                 * #146). If the target format has a non-NULL palette pointer
-                 * then SDL_ConvertSurface checks that the palette is not
-                 * empty-- that at least one entry is not black.
-                 */
-                format.palette = NULL;
-            }
-            if (SDL_ISPIXELFORMAT_INDEXED(SDL_MasksToPixelFormatEnum(
-                    PG_FORMAT_BitsPerPixel((&format)), format.Rmask,
-                    format.Gmask, format.Bmask, format.Amask))) {
+            SDL_PixelFormat *format = SDL_AllocFormat(format_enum);
+
+            if (SDL_ISPIXELFORMAT_INDEXED(format_enum)) {
                 if (SDL_ISPIXELFORMAT_INDEXED(PG_SURF_FORMATENUM(surf))) {
-                    SDL_SetPixelFormatPalette(&format, surf->format->palette);
+                    SDL_SetPixelFormatPalette(format, surf->format->palette);
                 }
                 else {
                     /* Give the surface something other than an all white
@@ -1958,12 +1939,13 @@ surf_convert(pgSurfaceObject *self, PyObject *args)
                      */
                     SDL_SetPaletteColors(palette, default_palette_colors, 0,
                                          default_palette_size);
-                    SDL_SetPixelFormatPalette(&format, palette);
+                    SDL_SetPixelFormatPalette(format, palette);
                 }
             }
-            newsurf = PG_ConvertSurface(surf, &format);
+            newsurf = PG_ConvertSurface(surf, format);
             SDL_SetSurfaceBlendMode(newsurf, SDL_BLENDMODE_NONE);
             SDL_FreePalette(palette);
+            SDL_FreeFormat(format);
         }
     }
     else {
@@ -4540,29 +4522,13 @@ pgSurface_Blit(pgSurfaceObject *dstobj, pgSurfaceObject *srcobj,
         }
         else {
             SDL_PixelFormat *fmt = src->format;
-            SDL_PixelFormat newfmt;
+            SDL_PixelFormat *newfmt =
+                SDL_AllocFormat(SDL_MasksToPixelFormatEnum(
+                    fmt->BitsPerPixel, fmt->Rmask, fmt->Gmask, fmt->Bmask, 0));
 
-            newfmt.palette = 0; /* Set NULL (or SDL gets confused) */
-#if SDL_VERSION_ATLEAST(3, 0, 0)
-            newfmt.bits_per_pixel = fmt->bits_per_pixel;
-            newfmt.bytes_per_pixel = fmt->bytes_per_pixel;
-#else
-            newfmt.BitsPerPixel = fmt->BitsPerPixel;
-            newfmt.BytesPerPixel = fmt->BytesPerPixel;
-#endif
-            newfmt.Amask = 0;
-            newfmt.Rmask = fmt->Rmask;
-            newfmt.Gmask = fmt->Gmask;
-            newfmt.Bmask = fmt->Bmask;
-            newfmt.Ashift = 0;
-            newfmt.Rshift = fmt->Rshift;
-            newfmt.Gshift = fmt->Gshift;
-            newfmt.Bshift = fmt->Bshift;
-            newfmt.Aloss = 0;
-            newfmt.Rloss = fmt->Rloss;
-            newfmt.Gloss = fmt->Gloss;
-            newfmt.Bloss = fmt->Bloss;
-            src = PG_ConvertSurface(src, &newfmt);
+            src = PG_ConvertSurface(src, newfmt);
+
+            SDL_FreeFormat(newfmt);
             if (src) {
 #if SDL_VERSION_ATLEAST(3, 0, 0)
                 result = SDL_BlitSurface(src, srcrect, dst, dstrect) ? 0 : -1;
-- 
2.51.0
+5 −1
Original line number Diff line number Diff line
@@ -61,8 +61,12 @@ buildPythonPackage rec {
        ]) buildInputs
      );
    })
    # https://github.com/libsdl-org/sdl2-compat/issues/476

    # Can be removed with the next SDL3 bump.
    ./skip-rle-tests.patch

    # https://github.com/pygame-community/pygame-ce/pull/3639
    ./0001-Use-SDL_AllocFormat-instead-of-creating-it-manually.patch
  ];

  postPatch = ''
+11 −11
Original line number Diff line number Diff line
diff --git a/test/surface_test.py b/test/surface_test.py
index 4e4b5d4..ffc7ffb 100644
index c2c91f4f5..58d916de8 100644
--- a/test/surface_test.py
+++ b/test/surface_test.py
@@ -375,6 +375,7 @@ class SurfaceTypeTest(unittest.TestCase):
         self.assertTrue(s1.get_flags() & pygame.RLEACCELOK)
         self.assertTrue(not s2.get_flags() & pygame.RLEACCELOK)
 
+    @unittest.skipIf(True, "https://github.com/libsdl-org/sdl2-compat/issues/476")
     def test_solarwolf_rle_usage(self):
         """Test for error/crash when calling set_colorkey() followed
         by convert twice in succession. Code originally taken
@@ -403,6 +404,7 @@ class SurfaceTypeTest(unittest.TestCase):
@@ -404,6 +404,7 @@ class SurfaceTypeTest(unittest.TestCase):
         finally:
             pygame.display.quit()
 
+    @unittest.skipIf(True, "https://github.com/libsdl-org/sdl2-compat/issues/476")
+    @unittest.skipIf(True, "https://github.com/libsdl-org/SDL/pull/14429")
     def test_solarwolf_rle_usage_2(self):
         """Test for RLE status after setting alpha"""
 
@@ -435,6 +436,7 @@ class SurfaceTypeTest(unittest.TestCase):
         finally:
             pygame.display.quit()
 
+    @unittest.skipIf(True, "https://github.com/libsdl-org/SDL/issues/14424")
     def test_set_alpha__set_colorkey_rle(self):
         pygame.display.init()
         try:
+53 −0
Original line number Diff line number Diff line
From bc1a52062adfacc94661883157dd4ef58e00e031 Mon Sep 17 00:00:00 2001
From: Marcin Serwin <marcin@serwin.dev>
Date: Sat, 8 Nov 2025 14:48:04 +0100
Subject: [PATCH] Use SDL_AllocFormat instead of creating it manually

According to the docs, `SDL_PixelFormat` is a read-only structure.
Creating it manually leaves out some important fields like `format` and
`next` pointer to be undefined.

Signed-off-by: Marcin Serwin <marcin@serwin.dev>
---
 src_c/surface.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/src_c/surface.c b/src_c/surface.c
index 958ce43f..9be3d8fb 100644
--- a/src_c/surface.c
+++ b/src_c/surface.c
@@ -3730,24 +3730,13 @@ pgSurface_Blit(pgSurfaceObject *dstobj, pgSurfaceObject *srcobj,
         }
         else {
             SDL_PixelFormat *fmt = src->format;
-            SDL_PixelFormat newfmt;
-
-            newfmt.palette = 0; /* Set NULL (or SDL gets confused) */
-            newfmt.BitsPerPixel = fmt->BitsPerPixel;
-            newfmt.BytesPerPixel = fmt->BytesPerPixel;
-            newfmt.Amask = 0;
-            newfmt.Rmask = fmt->Rmask;
-            newfmt.Gmask = fmt->Gmask;
-            newfmt.Bmask = fmt->Bmask;
-            newfmt.Ashift = 0;
-            newfmt.Rshift = fmt->Rshift;
-            newfmt.Gshift = fmt->Gshift;
-            newfmt.Bshift = fmt->Bshift;
-            newfmt.Aloss = 0;
-            newfmt.Rloss = fmt->Rloss;
-            newfmt.Gloss = fmt->Gloss;
-            newfmt.Bloss = fmt->Bloss;
-            src = SDL_ConvertSurface(src, &newfmt, 0);
+            SDL_PixelFormat *newfmt =
+                SDL_AllocFormat(SDL_MasksToPixelFormatEnum(
+                    fmt->BitsPerPixel, fmt->Rmask, fmt->Gmask, fmt->Bmask, 0));
+
+            src = SDL_ConvertSurface(src, newfmt, 0);
+
+            SDL_FreeFormat(newfmt);
             if (src) {
                 result = SDL_BlitSurface(src, srcrect, dst, dstrect);
                 SDL_FreeSurface(src);
-- 
2.51.0
+0 −14
Original line number Diff line number Diff line
diff --git a/src_c/surface.c b/src_c/surface.c
index ee9991fb..32c007bd 100644
--- a/src_c/surface.c
+++ b/src_c/surface.c
@@ -733,7 +733,8 @@ _raise_create_surface_error(void)
 {
     const char *msg = SDL_GetError();
 
-    if (strcmp(msg, "Unknown pixel format") == 0)
+    if (strcmp(msg, "Unknown pixel format") == 0 ||
+        strcmp(msg, "Parameter 'format' is invalid") == 0)
         return RAISE(PyExc_ValueError, "Invalid mask values");
     return RAISE(pgExc_SDLError, msg);
 }
Loading