Allow erasing pixels in pygame.Surface.scroll and add repeat functionality

buildconfig/stubs/pygame/surface.pyi

@@ -104,7 +104,9 @@ class Surface:
         rect: Optional[RectValue] = None,
         special_flags: int = 0,
     ) -> Rect: ...
-    def scroll(self, dx: int = 0, dy: int = 0, /) -> None: ...
+    def scroll(
+        self, dx: int = 0, dy: int = 0, erase: bool = False, repeat: bool = False, /
+    ) -> None: ...
     @overload
     def set_colorkey(self, color: ColorValue, flags: int = 0, /) -> None: ...
     @overload

docs/reST/ref/surface.rst

@@ -311,16 +311,26 @@
    .. method:: scroll
 
       | :sl:`Shift the surface image in place`
-      | :sg:`scroll(dx=0, dy=0, /) -> None`
+      | :sg:`scroll(dx=0, dy=0, erase=False, repeat=False, /) -> None`
 
       Move the image by dx pixels right and dy pixels down. dx and dy may be
-      negative for left and up scrolls respectively. Areas of the surface that
-      are not overwritten retain their original pixel values. Scrolling is
-      contained by the Surface clip area. It is safe to have dx and dy values
-      that exceed the surface size.
+      negative for left and up scrolls respectively.
+
+      If the erase argument is ``True`` the space created by the shifting pixels
+      is filled with black, otherwise it is left unchanged.
+
+      Scrolling is contained by the Surface clip area. It is safe to have dx
+      and dy values that exceed the surface size.
+
+      If the repeat argument is ``True`` the pixels that disappear out of the surface
+      or clip bounds are brought back on the opposite side resulting in an infinitely
+      scrolling and repeating surface.
 
       .. versionaddedold:: 1.9
 
+      .. versionchanged:: 2.5.0
+         Add repeating scroll and allow erasing pixels
+
       .. ## Surface.scroll ##
 
    .. method:: set_colorkey

src_c/doc/surface_doc.h

@@ -7,7 +7,7 @@
 #define DOC_SURFACE_CONVERTALPHA "convert_alpha() -> Surface\nchange the pixel format of an image including per pixel alphas"
 #define DOC_SURFACE_COPY "copy() -> Surface\ncreate a new copy of a Surface"
 #define DOC_SURFACE_FILL "fill(color, rect=None, special_flags=0) -> Rect\nfill Surface with a solid color"
-#define DOC_SURFACE_SCROLL "scroll(dx=0, dy=0, /) -> None\nShift the surface image in place"
+#define DOC_SURFACE_SCROLL "scroll(dx=0, dy=0, erase=False, repeat=False, /) -> None\nShift the surface image in place"
 #define DOC_SURFACE_SETCOLORKEY "set_colorkey(color, flags=0, /) -> None\nset_colorkey(None) -> None\nSet the transparent colorkey"
 #define DOC_SURFACE_GETCOLORKEY "get_colorkey() -> RGB or None\nGet the current transparent colorkey"
 #define DOC_SURFACE_SETALPHA "set_alpha(value, flags=0, /) -> None\nset_alpha(None) -> None\nset the alpha value for the full Surface image"

src_c/surface.c

@@ -117,9 +117,6 @@ static void
 surface_dealloc(PyObject *self);
 static void
 surface_cleanup(pgSurfaceObject *self);
-static void
-surface_move(Uint8 *src, Uint8 *dst, int h, int span, int srcpitch,
-             int dstpitch);
 
 static PyObject *
 surf_get_at(PyObject *self, PyObject *args);
@@ -2315,16 +2312,17 @@ surf_fblits(pgSurfaceObject *self, PyObject *const *args, Py_ssize_t nargs)
 static PyObject *
 surf_scroll(PyObject *self, PyObject *args, PyObject *keywds)
 {
-    int dx = 0, dy = 0;
+    int dx = 0, dy = 0, erase = 0, repeat = 0;
     SDL_Surface *surf;
-    int bpp;
-    int pitch;
     SDL_Rect *clip_rect;
-    int w, h;
-    Uint8 *src, *dst;
-
-    static char *kwids[] = {"dx", "dy", NULL};
-    if (!PyArg_ParseTupleAndKeywords(args, keywds, "|ii", kwids, &dx, &dy)) {
+    int w = 0, h = 0, x = 0, y = 0;
+    int bpp = 0, pitch = 0, span = 0;
+    int xoffset = 0;
+    Uint8 *linesrc, *startsrc, *endsrc;
+
+    static char *kwids[] = {"dx", "dy", "erase", "repeat", NULL};
+    if (!PyArg_ParseTupleAndKeywords(args, keywds, "|iipp", kwids, &dx, &dy,
+                                     &erase, &repeat)) {
         return NULL;
     }
 
@@ -2338,43 +2336,214 @@ surf_scroll(PyObject *self, PyObject *args, PyObject *keywds)
     clip_rect = &surf->clip_rect;
     w = clip_rect->w;
     h = clip_rect->h;
-    if (dx >= w || dx <= -w || dy >= h || dy <= -h) {
+    x = clip_rect->x;
+    y = clip_rect->y;
+    if (x > surf->w || x + w < 0 || y > surf->h || y + h < 0) {
         Py_RETURN_NONE;
     }
+    /* Get the intersection between the clip rect and the
+       surface absolute rect to avoid segfaults */
+    if (x < 0) {
+        w += x;
+        x = 0;
+    }
+    if (y < 0) {
+        h += y;
+        y = 0;
+    }
+    if (x + w > surf->w) {
+        w -= (x + w) - surf->w;
+    }
+    if (y + h > surf->h) {
+        h -= (y + h) - surf->h;
+    }
+    /* If the clip rect is outside the surface fill and return
+      for scrolls without repeat. Only fill when erase is true */
+    if (!repeat) {
+        if (dx >= w || dx <= -w || dy >= h || dy <= -h) {
+            if (erase) {
+                if (SDL_FillRect(surf, NULL, 0) == -1) {
+                    PyErr_SetString(pgExc_SDLError, SDL_GetError());
+                    return NULL;
+                }
+            }
+            Py_RETURN_NONE;
+        }
+    }
+    // Repeated scrolls are periodic so we can delete the exceeding value
+    if (dx >= w || dx <= -w) {
+        dx = dx % w;
+    }
+    if (dy >= h || dy <= -h) {
+        dy = dy % h;
+    }
 
     if (!pgSurface_Lock((pgSurfaceObject *)self)) {
         return NULL;
     }
 
     bpp = PG_SURF_BytesPerPixel(surf);
     pitch = surf->pitch;
-    src = dst =
-        (Uint8 *)surf->pixels + clip_rect->y * pitch + clip_rect->x * bpp;
-    if (dx >= 0) {
-        w -= dx;
-        if (dy > 0) {
-            h -= dy;
-            dst += dy * pitch + dx * bpp;
+    span = w * bpp;
+    linesrc = (Uint8 *)surf->pixels + pitch * y + bpp * x;
+    startsrc = endsrc = linesrc;
+    xoffset = dx * bpp;
+    if (dy > 0) {
+        endsrc = linesrc + pitch * (h - 1);
+        linesrc = endsrc;
+    }
+
+    if (repeat) {
+        if (dy != 0) {
+            int yincrease = dy > 0 ? -pitch : pitch;
+            int spanincrease = dy > 0 ? -span : span;
+            int templen = (dy > 0 ? dy * span : -dy * span);
+            int tempheight = (dy > 0 ? dy : -dy);
+            /* Create a temporary buffer to store the pixels that
+               are disappearing from the surface */
+            Uint8 *tempbuf = (Uint8 *)malloc(templen);
+            if (tempbuf == NULL) {
+                if (!pgSurface_Unlock((pgSurfaceObject *)self)) {
+                    return NULL;
+                }
+                return PyErr_NoMemory();
+            }
+            memset(tempbuf, 0, templen);
+            Uint8 *templine = tempbuf;
+            Uint8 *tempend =
+                templine + (dy > 0 ? (dy - 1) * span : -(dy + 1) * span);
+            if (dy > 0) {
+                templine = tempend;
+            }
+            int looph = h;
+            while (looph--) {
+                // If the current line should disappear copy it to the
+                // temporary buffer
+                if ((templine <= tempend && dy < 0) ||
+                    (templine >= tempbuf && dy > 0)) {
+                    if (dx > 0) {
+                        memcpy(templine, linesrc + span - xoffset, xoffset);
+                        memcpy(templine + xoffset, linesrc, span - xoffset);
+                    }
+                    else if (dx < 0) {
+                        memcpy(templine + span + xoffset, linesrc, -xoffset);
+                        memcpy(templine, linesrc - xoffset, span + xoffset);
+                    }
+                    else {
+                        memcpy(templine, linesrc, span);
+                    }
+                    memset(linesrc, 0, span);
+                    templine += spanincrease;
+                }
+                else {
+                    Uint8 *pastesrc = linesrc + pitch * dy;
+                    if ((dy < 0 && pastesrc >= startsrc) ||
+                        (dy > 0 && pastesrc <= endsrc)) {
+                        if (dx > 0) {
+                            memcpy(pastesrc, linesrc + span - xoffset,
+                                   xoffset);
+                            memcpy(pastesrc + xoffset, linesrc,
+                                   span - xoffset);
+                        }
+                        else if (dx < 0) {
+                            memcpy(pastesrc + span + xoffset, linesrc,
+                                   -xoffset);
+                            memcpy(pastesrc, linesrc - xoffset,
+                                   span + xoffset);
+                        }
+                        else {
+                            memcpy(pastesrc, linesrc, span);
+                        }
+                    }
+                }
+                linesrc += yincrease;
+            }
+            // Copy the data of the buffer back to the original pixels to
+            // repeat
+            templine = tempbuf;
+            if (dy < 0) {
+                linesrc = startsrc + pitch * (h - tempheight);
+            }
+            else {
+                linesrc = startsrc;
+            }
+            while (tempheight--) {
+                memcpy(linesrc, templine, span);
+                linesrc += pitch;
+                templine += span;
+            }
+            free(tempbuf);
         }
         else {
-            h += dy;
-            src -= dy * pitch;
-            dst += dx * bpp;
+            // No y-shifting, the temporary buffer should only store the x loss
+            Uint8 *tempbuf = (Uint8 *)malloc((dx > 0 ? xoffset : -xoffset));
+            if (tempbuf == NULL) {
+                if (!pgSurface_Unlock((pgSurfaceObject *)self)) {
+                    return NULL;
+                }
+                return PyErr_NoMemory();
+            }
+            while (h--) {
+                if (dx > 0) {
+                    memcpy(tempbuf, linesrc + span - xoffset, xoffset);
+                    memcpy(linesrc + xoffset, linesrc, span - xoffset);
+                    memcpy(linesrc, tempbuf, xoffset);
+                }
+                else if (dx < 0) {
+                    memcpy(tempbuf, linesrc, -xoffset);
+                    memcpy(linesrc, linesrc - xoffset, span + xoffset);
+                    memcpy(linesrc + span + xoffset, tempbuf, -xoffset);
+                }
+                linesrc += pitch;
+            }
+            free(tempbuf);
         }
     }
     else {
-        w += dx;
-        if (dy > 0) {
-            h -= dy;
-            src -= dx * bpp;
-            dst += dy * pitch;
+        if (dy != 0) {
+            /* Copy the current line to a before or after position if it's
+               valid with consideration of x offset and memset to avoid
+               artifacts */
+            int yincrease = dy > 0 ? -pitch : pitch;
+            while (h--) {
+                Uint8 *pastesrc = linesrc + pitch * dy;
+                if ((dy < 0 && pastesrc >= startsrc) ||
+                    (dy > 0 && pastesrc <= endsrc)) {
+                    if (dx > 0) {
+                        memcpy(pastesrc + xoffset, linesrc, span - xoffset);
+                    }
+                    else if (dx < 0) {
+                        memcpy(pastesrc, linesrc - xoffset, span + xoffset);
+                    }
+                    else {
+                        memcpy(pastesrc, linesrc, span);
+                    }
+                    if (erase) {
+                        memset(linesrc, 0, span);
+                    }
+                }
+                linesrc += yincrease;
+            }
         }
         else {
-            h += dy;
-            src -= dy * pitch + dx * bpp;
+            // No y-shifting, we only need to move pixels on the same line
+            while (h--) {
+                if (dx > 0) {
+                    memcpy(linesrc + xoffset, linesrc, span - xoffset);
+                    if (erase) {
+                        memset(linesrc, 0, xoffset);
+                    }
+                }
+                else if (dx < 0) {
+                    memcpy(linesrc, linesrc - xoffset, span + xoffset);
+                    if (erase) {
+                        memset(linesrc + span + xoffset, 0, -xoffset);
+                    }
+                }
+                linesrc += pitch;
+            }
         }
     }
-    surface_move(src, dst, h, w * bpp, pitch, pitch);
 
     if (!pgSurface_Unlock((pgSurfaceObject *)self)) {
         return NULL;
@@ -3717,23 +3886,6 @@ surf_get_pixels_address(PyObject *self, PyObject *closure)
 #endif
 }
 
-static void
-surface_move(Uint8 *src, Uint8 *dst, int h, int span, int srcpitch,
-             int dstpitch)
-{
-    if (src < dst) {
-        src += (h - 1) * srcpitch;
-        dst += (h - 1) * dstpitch;
-        srcpitch = -srcpitch;
-        dstpitch = -dstpitch;
-    }
-    while (h--) {
-        memmove(dst, src, span);
-        src += srcpitch;
-        dst += dstpitch;
-    }
-}
-
 static int
 surface_do_overlap(SDL_Surface *src, SDL_Rect *srcrect, SDL_Surface *dst,
                    SDL_Rect *dstrect)