SDL3: surface+pixelarray+font+draw: runtime fixes

Author: ankith26

docs/reST/c_api/surface.rst

@@ -56,4 +56,4 @@ Header file: src_c/include/pygame.h
    by the blit.
 
    The C version of the :py:meth:`pygame.Surface.blit` method.
-   Return ``1`` on success, ``0`` on an exception.
+   Return ``0`` on success, ``1`` on an exception.

src_c/_pygame.h

@@ -77,9 +77,17 @@
 
 #define PG_CreateSurface SDL_CreateSurface
 #define PG_CreateSurfaceFrom SDL_CreateSurfaceFrom
-#define PG_ConvertSurface SDL_ConvertSurface
 #define PG_ConvertSurfaceFormat SDL_ConvertSurface
 
+/* Convert surface using palette and format of dst */
+static inline SDL_Surface *
+PG_ConvertSurface(SDL_Surface *surface, SDL_Surface *dst)
+{
+    return SDL_ConvertSurfaceAndColorspace(surface, dst->format,
+                                           SDL_GetSurfacePalette(dst),
+                                           SDL_GetSurfaceColorspace(dst), 0);
+}
+
 #define PG_PixelFormatEnum SDL_PixelFormat
 
 #define PG_SurfaceHasRLE SDL_SurfaceHasRLE
@@ -144,11 +152,23 @@ PG_SURF_BitsPerPixel(SDL_Surface *surf)
 
 #define PG_PixelFormat const SDL_PixelFormatDetails
 
+static inline SDL_Palette *
+PG_GetSurfacePalette(SDL_Surface *surf)
+{
+    SDL_Palette *ret = SDL_GetSurfacePalette(surf);
+    if (!ret && SDL_ISPIXELFORMAT_INDEXED(surf->format)) {
+        /* Palette doesn't exist but is expected, so create it.
+         * SDL will associate the newly created palette with the surface */
+        ret = SDL_CreateSurfacePalette(surf);
+    }
+    return ret;
+}
+
 static inline bool
 PG_GetSurfaceDetails(SDL_Surface *surf, PG_PixelFormat **format_p,
                      SDL_Palette **palette_p)
 {
-    *palette_p = SDL_GetSurfacePalette(surf);
+    *palette_p = PG_GetSurfacePalette(surf);
     *format_p = SDL_GetPixelFormatDetails(surf->format);
     return *format_p != NULL;
 }
@@ -159,7 +179,6 @@ PG_GetSurfaceFormat(SDL_Surface *surf)
     return SDL_GetPixelFormatDetails(surf->format);
 }
 
-#define PG_GetSurfacePalette SDL_GetSurfacePalette
 #define PG_SetPaletteColors SDL_SetPaletteColors
 #define PG_SetSurfacePalette SDL_SetSurfacePalette
 #define PG_SetSurfaceColorKey SDL_SetSurfaceColorKey
@@ -215,10 +234,15 @@ PG_GetSurfaceFormat(SDL_Surface *surf)
     SDL_CreateRGBSurfaceWithFormat(0, width, height, 0, format)
 #define PG_CreateSurfaceFrom(width, height, format, pixels, pitch) \
     SDL_CreateRGBSurfaceWithFormatFrom(pixels, width, height, 0, pitch, format)
-#define PG_ConvertSurface(src, fmt) SDL_ConvertSurface(src, fmt, 0)
 #define PG_ConvertSurfaceFormat(src, pixel_format) \
     SDL_ConvertSurfaceFormat(src, pixel_format, 0)
 
+static inline SDL_Surface *
+PG_ConvertSurface(SDL_Surface *surface, SDL_Surface *dst)
+{
+    return SDL_ConvertSurface(surface, dst->format, 0);
+}
+
 #define PG_PixelFormatEnum SDL_PixelFormatEnum
 
 #define PG_SoftStretchNearest(src, srcrect, dst, dstrect) \

src_c/draw.c

@@ -1334,7 +1334,7 @@ flood_fill(PyObject *self, PyObject *arg, PyObject *kwargs)
     if (pgSurface_Check(colorobj)) {
         pat_surfobj = ((pgSurfaceObject *)colorobj);
 
-        pattern = PG_ConvertSurface(pat_surfobj->surf, surf->format);
+        pattern = PG_ConvertSurface(pat_surfobj->surf, surf);
 
         if (pattern == NULL) {
             return RAISE(PyExc_RuntimeError, "error converting pattern surf");

src_c/font.c

@@ -717,11 +717,7 @@ font_render(PyObject *self, PyObject *args, PyObject *kwds)
             resolve to Render_Solid, that needs to be explicitly handled. */
             if (surf != NULL && bg_rgba_obj != Py_None) {
                 SDL_SetColorKey(surf, 0, 0);
-#if SDL_TTF_VERSION_ATLEAST(3, 0, 0)
-                SDL_Palette *palette = SDL_GetSurfacePalette(surf);
-#else
-                SDL_Palette *palette = surf->format->palette;
-#endif
+                SDL_Palette *palette = PG_GetSurfacePalette(surf);
                 if (palette) {
                     palette->colors[0].r = backg.r;
                     palette->colors[0].g = backg.g;

src_c/pixelarray_methods.c

@@ -137,7 +137,7 @@ _make_surface(pgPixelArrayObject *array, PyObject *args)
     }
 
     /* Ensure the new surface has the same format as the original */
-    new_surf = PG_ConvertSurface(temp_surf, surf->format);
+    new_surf = PG_ConvertSurface(temp_surf, surf);
     if (temp_surf != surf) {
         SDL_FreeSurface(temp_surf);
     }

src_c/pixelcopy.c

@@ -1197,11 +1197,7 @@ make_surface(PyObject *self, PyObject *arg)
         pgBuffer_Release(&pg_view);
         return RAISE(pgExc_SDLError, SDL_GetError());
     }
-#if SDL_VERSION_ATLEAST(3, 0, 0)
-    SDL_Palette *palette = SDL_GetSurfacePalette(surf);
-#else
-    SDL_Palette *palette = surf->format->palette;
-#endif
+    SDL_Palette *palette = PG_GetSurfacePalette(surf);
     if (SDL_ISPIXELFORMAT_INDEXED(PG_SURF_FORMATENUM(surf))) {
         /* Give the surface something other than an all white palette.
          *          */

src_c/surface.c

@@ -1544,24 +1544,21 @@ surf_set_alpha(pgSurfaceObject *self, PyObject *args)
     bool success =
         PG_SetSurfaceRLE(surf, (flags & PGS_RLEACCEL) ? SDL_TRUE : SDL_FALSE);
     /* HACK HACK HACK */
-    // TODO SDL3: figure out how to port this or if it's relevant to SDL3.
-#if !SDL_VERSION_ATLEAST(3, 0, 0)
-    if ((surf->flags & SDL_RLEACCEL) && (!(flags & PGS_RLEACCEL))) {
+    if (SDL_MUSTLOCK(surf) && (!(flags & PGS_RLEACCEL))) {
         /* hack to strip SDL_RLEACCEL flag off surface immediately when
            it is not requested */
         SDL_Rect sdlrect;
         sdlrect.x = 0;
         sdlrect.y = 0;
-        sdlrect.h = 0;
-        sdlrect.w = 0;
+        sdlrect.h = 1;
+        sdlrect.w = 1;
 
         SDL_Surface *surface =
             PG_CreateSurface(1, 1, PG_SURF_FORMATENUM(surf));
 
         SDL_LowerBlit(surf, &sdlrect, surface, &sdlrect);
         SDL_FreeSurface(surface);
     }
-#endif
     /* HACK HACK HACK */
     if (success) {
         success = PG_SetSurfaceAlphaMod(surf, alpha);
@@ -1623,7 +1620,7 @@ surf_copy(pgSurfaceObject *self, PyObject *_null)
     SURF_INIT_CHECK(surf)
 
     pgSurface_Prep(self);
-    newsurf = PG_ConvertSurface(surf, surf->format);
+    newsurf = PG_ConvertSurface(surf, surf);
     pgSurface_Unprep(self);
 
     final = surf_subtype_new(Py_TYPE(self), newsurf, 1);
@@ -1683,7 +1680,7 @@ surf_convert(pgSurfaceObject *self, PyObject *args)
     if (argobject) {
         if (pgSurface_Check(argobject)) {
             src = pgSurface_AsSurface(argobject);
-            newsurf = PG_ConvertSurface(surf, src->format);
+            newsurf = PG_ConvertSurface(surf, src);
         }
         else {
             /* will be updated later, initialize to make static analyzer happy
@@ -1837,7 +1834,7 @@ surf_convert(pgSurfaceObject *self, PyObject *args)
     if (argobject) {
         if (pgSurface_Check(argobject)) {
             src = pgSurface_AsSurface(argobject);
-            newsurf = PG_ConvertSurface(surf, src->format);
+            newsurf = PG_ConvertSurface(surf, src);
         }
         else {
             /* will be updated later, initialize to make static analyzer happy
@@ -1961,7 +1958,7 @@ surf_convert(pgSurfaceObject *self, PyObject *args)
                     SDL_SetPixelFormatPalette(&format, palette);
                 }
             }
-            newsurf = PG_ConvertSurface(surf, &format);
+            newsurf = SDL_ConvertSurface(surf, &format, 0);
             SDL_SetSurfaceBlendMode(newsurf, SDL_BLENDMODE_NONE);
             SDL_FreePalette(palette);
         }
@@ -3019,10 +3016,8 @@ surf_get_flags(PyObject *self, PyObject *_null)
     if (PG_SurfaceHasRLE(surf)) {
         flags |= PGS_RLEACCELOK;
     }
-    // TODO SDL3: figure out how to properly emulate SDL2 check/relevance
-    // Current implementation is just a placeholder.
 #if SDL_VERSION_ATLEAST(3, 0, 0)
-    if (SDL_SurfaceHasRLE(surf)) {
+    if (SDL_MUSTLOCK(surf)) {
         flags |= PGS_RLEACCEL;
     }
 #else
@@ -3767,7 +3762,7 @@ surf_premul_alpha(pgSurfaceObject *self, PyObject *_null)
 
     pgSurface_Prep(self);
     // Make a copy of the surface first
-    newsurf = PG_ConvertSurface(surf, surf->format);
+    newsurf = PG_ConvertSurface(surf, surf);
 
     if ((surf->w > 0 && surf->h > 0)) {
         // If the surface has no pixels we don't need to premul
@@ -4461,6 +4456,76 @@ surface_do_overlap(SDL_Surface *src, SDL_Rect *srcrect, SDL_Surface *dst,
     return dstoffset < span || dstoffset > src->pitch - span;
 }
 
+int
+PG_BlitSurface(SDL_Surface *src, const SDL_Rect *srcrect, SDL_Surface *dst,
+               SDL_Rect *dstrect)
+{
+#if SDL_VERSION_ATLEAST(3, 0, 0)
+    /* SDL3 doesn't modify dstrect, so compat for that.
+     * Below logic taken from SDL2 source with slight modifications */
+    SDL_Rect r_src, r_dst;
+
+    r_src.x = 0;
+    r_src.y = 0;
+    r_src.w = src->w;
+    r_src.h = src->h;
+
+    if (dstrect) {
+        r_dst.x = dstrect->x;
+        r_dst.y = dstrect->y;
+    }
+    else {
+        r_dst.x = 0;
+        r_dst.y = 0;
+    }
+
+    /* clip the source rectangle to the source surface */
+    if (srcrect) {
+        SDL_Rect tmp;
+        if (SDL_IntersectRect(srcrect, &r_src, &tmp) == SDL_FALSE) {
+            goto end;
+        }
+
+        /* Shift dstrect, if srcrect origin has changed */
+        r_dst.x += tmp.x - srcrect->x;
+        r_dst.y += tmp.y - srcrect->y;
+
+        /* Update srcrect */
+        r_src = tmp;
+    }
+
+    /* There're no dstrect.w/h parameters. It's the same as srcrect */
+    r_dst.w = r_src.w;
+    r_dst.h = r_src.h;
+
+    /* clip the destination rectangle against the clip rectangle */
+    {
+        SDL_Rect tmp, clip_rect;
+        SDL_GetSurfaceClipRect(dst, &clip_rect);
+        if (SDL_IntersectRect(&r_dst, &clip_rect, &tmp) == SDL_FALSE) {
+            goto end;
+        }
+
+        /* Update dstrect */
+        r_dst = tmp;
+    }
+
+    if (r_dst.w > 0 && r_dst.h > 0) {
+        if (dstrect) { /* update output parameter */
+            *dstrect = r_dst;
+        }
+        return SDL_BlitSurface(src, srcrect, dst, dstrect) ? 0 : -1;
+    }
+end:
+    if (dstrect) {
+        dstrect->w = dstrect->h = 0;
+    }
+    return 0;
+#else
+    return SDL_BlitSurface(src, srcrect, dst, dstrect);
+#endif
+}
+
 /*this internal blit function is accessible through the C api*/
 int
 pgSurface_Blit(pgSurfaceObject *dstobj, pgSurfaceObject *srcobj,
@@ -4471,13 +4536,11 @@ pgSurface_Blit(pgSurfaceObject *dstobj, pgSurfaceObject *srcobj,
     SDL_Surface *subsurface = NULL;
     int result, suboffsetx = 0, suboffsety = 0;
     SDL_Rect orig_clip, sub_clip, dstclip;
-#if !SDL_VERSION_ATLEAST(3, 0, 0)
     Uint8 alpha;
-#endif
 
     if (!PG_GetSurfaceClipRect(dst, &dstclip)) {
         PyErr_SetString(pgExc_SDLError, SDL_GetError());
-        return 0;
+        return 1;
     }
 
     /* passthrough blits to the real surface */
@@ -4528,8 +4591,6 @@ pgSurface_Blit(pgSurfaceObject *dstobj, pgSurfaceObject *srcobj,
         result = pygame_Blit(src, srcrect, dst, dstrect, blend_flags);
         /* Py_END_ALLOW_THREADS */
     }
-// TODO SDL3: port the below bit of code. Skipping for initial surface port.
-#if !SDL_VERSION_ATLEAST(3, 0, 0)
     /* can't blit alpha to 8bit, crashes SDL */
     else if (PG_SURF_BytesPerPixel(dst) == 1 &&
              (SDL_ISPIXELFORMAT_ALPHA(PG_SURF_FORMATENUM(src)) ||
@@ -4539,17 +4600,22 @@ pgSurface_Blit(pgSurfaceObject *dstobj, pgSurfaceObject *srcobj,
             result = pygame_Blit(src, srcrect, dst, dstrect, 0);
         }
         else {
+#if SDL_VERSION_ATLEAST(3, 0, 0)
+            const SDL_PixelFormatDetails *fmt =
+                SDL_GetPixelFormatDetails(src->format);
+            src = fmt ? SDL_ConvertSurface(src,
+                                           SDL_GetPixelFormatForMasks(
+                                               fmt->bits_per_pixel, fmt->Rmask,
+                                               fmt->Gmask, fmt->Bmask, 0))
+                      : NULL;
+
+#else
             SDL_PixelFormat *fmt = src->format;
             SDL_PixelFormat newfmt;
 
             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;
@@ -4562,13 +4628,10 @@ pgSurface_Blit(pgSurfaceObject *dstobj, pgSurfaceObject *srcobj,
             newfmt.Rloss = fmt->Rloss;
             newfmt.Gloss = fmt->Gloss;
             newfmt.Bloss = fmt->Bloss;
-            src = PG_ConvertSurface(src, &newfmt);
-            if (src) {
-#if SDL_VERSION_ATLEAST(3, 0, 0)
-                result = SDL_BlitSurface(src, srcrect, dst, dstrect) ? 0 : -1;
-#else
-                result = SDL_BlitSurface(src, srcrect, dst, dstrect);
+            src = SDL_ConvertSurface(src, &newfmt, 0);
 #endif
+            if (src) {
+                result = PG_BlitSurface(src, srcrect, dst, dstrect);
                 SDL_FreeSurface(src);
             }
             else {
@@ -4577,7 +4640,6 @@ pgSurface_Blit(pgSurfaceObject *dstobj, pgSurfaceObject *srcobj,
         }
         /* Py_END_ALLOW_THREADS */
     }
-#endif
     else if (blend_flags != PYGAME_BLEND_ALPHA_SDL2 &&
              !(pg_EnvShouldBlendAlphaSDL2()) && !SDL_HasColorKey(src) &&
              (PG_SURF_BytesPerPixel(dst) == 4 ||
@@ -4598,11 +4660,7 @@ pgSurface_Blit(pgSurfaceObject *dstobj, pgSurfaceObject *srcobj,
     }
     else {
         /* Py_BEGIN_ALLOW_THREADS */
-#if SDL_VERSION_ATLEAST(3, 0, 0)
-        result = SDL_BlitSurface(src, srcrect, dst, dstrect) ? 0 : -1;
-#else
-        result = SDL_BlitSurface(src, srcrect, dst, dstrect);
-#endif
+        result = PG_BlitSurface(src, srcrect, dst, dstrect);
         /* Py_END_ALLOW_THREADS */
     }
 

src_c/surface_fill.c

@@ -1143,7 +1143,8 @@ surface_fill_blend(SDL_Surface *surface, SDL_Rect *rect, Uint32 color,
         }
 
         default: {
-            result = SDL_SetError("invalid blend flag for this operation");
+            SDL_SetError("invalid blend flag for this operation");
+            result = -1;
             break;
         }
     }