diff --git a/buildconfig/stubs/pygame/__init__.pyi b/buildconfig/stubs/pygame/__init__.pyi index a8ecdd7f25..ffb58f4cdb 100644 --- a/buildconfig/stubs/pygame/__init__.pyi +++ b/buildconfig/stubs/pygame/__init__.pyi @@ -605,6 +605,8 @@ from .constants import ( SCRAP_PPM as SCRAP_PPM, SCRAP_SELECTION as SCRAP_SELECTION, SCRAP_TEXT as SCRAP_TEXT, + SCROLL_ERASE as SCROLL_ERASE, + SCROLL_REPEAT as SCROLL_REPEAT, SHOWN as SHOWN, SRCALPHA as SRCALPHA, SRCCOLORKEY as SRCCOLORKEY, diff --git a/buildconfig/stubs/pygame/constants.pyi b/buildconfig/stubs/pygame/constants.pyi index 3d793e3c50..463f2509c8 100644 --- a/buildconfig/stubs/pygame/constants.pyi +++ b/buildconfig/stubs/pygame/constants.pyi @@ -528,6 +528,8 @@ SCRAP_PBM: str SCRAP_PPM: str SCRAP_SELECTION: int SCRAP_TEXT: str +SCROLL_ERASE: int +SCROLL_REPEAT: int SHOWN: int SRCALPHA: int SRCCOLORKEY: int diff --git a/buildconfig/stubs/pygame/locals.pyi b/buildconfig/stubs/pygame/locals.pyi index ef822c62d6..e596355311 100644 --- a/buildconfig/stubs/pygame/locals.pyi +++ b/buildconfig/stubs/pygame/locals.pyi @@ -530,6 +530,8 @@ SCRAP_PBM: str SCRAP_PPM: str SCRAP_SELECTION: int SCRAP_TEXT: str +SCROLL_ERASE: int +SCROLL_REPEAT: int SHOWN: int SRCALPHA: int SRCCOLORKEY: int diff --git a/buildconfig/stubs/pygame/surface.pyi b/buildconfig/stubs/pygame/surface.pyi index 535f5d776e..a32c840eba 100644 --- a/buildconfig/stubs/pygame/surface.pyi +++ b/buildconfig/stubs/pygame/surface.pyi @@ -103,7 +103,9 @@ class Surface: rect: Optional[RectLike] = 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, scroll_flag: int = 0, / + ) -> None: ... @overload def set_colorkey(self, color: ColorLike, flags: int = 0, /) -> None: ... @overload diff --git a/docs/reST/ref/surface.rst b/docs/reST/ref/surface.rst index 188587b9f3..859acf075e 100644 --- a/docs/reST/ref/surface.rst +++ b/docs/reST/ref/surface.rst @@ -317,17 +317,30 @@ .. method:: scroll - | :sl:`shift the surface image in place` - | :sg:`scroll(dx=0, dy=0, /) -> None` + | :sl:`shift the Surface pixels in place` + | :sg:`scroll(dx=0, dy=0, scroll_flag=0, /) -> 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. + Move the Surface by dx pixels right and dy pixels down. dx and dy may be + negative for left and up scrolls respectively. + + Scrolling is contained by the Surface clip area. It is safe to have dx + and dy values that exceed the surface size. + + The scroll flag can be: + * ``0`` (default): the pixels are shifted but previous pixels are + not modified. + + * ``pygame.SCROLL_ERASE``: the space created by the shifting pixels + is filled with black or transparency. + + * ``pygame.SCROLL_REPEAT``: 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.3 Add repeating scroll and allow erasing pixels + .. ## Surface.scroll ## .. method:: set_colorkey diff --git a/src_c/_pygame.h b/src_c/_pygame.h index e87986d776..b2e837e117 100644 --- a/src_c/_pygame.h +++ b/src_c/_pygame.h @@ -426,6 +426,12 @@ typedef enum { PGS_PREALLOC = 0x01000000 } PygameSurfaceFlags; +typedef enum { + PGS_SCROLL_DEFAULT = 0x00000000, + PGS_SCROLL_REPEAT = 0x00000001, + PGS_SCROLL_ERASE = 0x00000004 +} PygameScrollSurfaceFlags; + #define RAISE(x, y) (PyErr_SetString((x), (y)), NULL) #define RAISERETURN(x, y, r) \ PyErr_SetString((x), (y)); \ diff --git a/src_c/constants.c b/src_c/constants.c index 6856db5568..f9eb244007 100644 --- a/src_c/constants.c +++ b/src_c/constants.c @@ -116,6 +116,9 @@ MODINIT_DEFINE(constants) DEC_CONSTSF(SHOWN); DEC_CONSTSF(HIDDEN); + DEC_CONSTSF(SCROLL_ERASE); + DEC_CONSTSF(SCROLL_REPEAT); + DEC_CONSTSF(SCALED); DEC_CONST(GL_RED_SIZE); diff --git a/src_c/doc/surface_doc.h b/src_c/doc/surface_doc.h index 38a6fb7120..528031a4d9 100644 --- a/src_c/doc/surface_doc.h +++ b/src_c/doc/surface_doc.h @@ -7,7 +7,7 @@ #define DOC_SURFACE_CONVERTALPHA "convert_alpha() -> Surface\nchange the pixel format of a surface 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, scroll_flag=0, /) -> None\nshift the Surface pixels 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" diff --git a/src_c/surface.c b/src_c/surface.c index 072aa09b5c..fea4ad88be 100644 --- a/src_c/surface.c +++ b/src_c/surface.c @@ -82,9 +82,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); @@ -2273,19 +2270,204 @@ surf_fblits(pgSurfaceObject *self, PyObject *const *args, Py_ssize_t nargs) return RAISE(PyExc_TypeError, "Unknown error"); } +static int +scroll_repeat(int h, int dx, int dy, int pitch, int span, int xoffset, + Uint8 *startsrc, Uint8 *endsrc, Uint8 *linesrc) +{ + 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) { + PyErr_NoMemory(); + return -1; + } + 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 { + // No y-shifting, the temporary buffer should only store the x loss + Uint8 *tempbuf = (Uint8 *)malloc((dx > 0 ? xoffset : -xoffset)); + if (tempbuf == NULL) { + PyErr_NoMemory(); + return -1; + } + 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); + } + return 0; +} + +static int +scroll_default(int h, int dx, int dy, int pitch, int span, int xoffset, + Uint8 *startsrc, Uint8 *endsrc, Uint8 *linesrc, int erase) +{ + 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); + // Fix the missing pixel bug + if (dx < 0) { + memset(pastesrc + span + xoffset, 0, -xoffset); + } + else if (dx > 0) { + memset(pastesrc, 0, xoffset); + } + } + } + linesrc += yincrease; + } + } + else { + // 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; + } + } + return 0; +} + +static int +scroll(SDL_Surface *surf, int dx, int dy, int x, int y, int w, int h, + int repeat, int erase) +{ + int bpp = PG_SURF_BytesPerPixel(surf); + int pitch = surf->pitch; + int span = w * bpp; + Uint8 *linesrc = (Uint8 *)surf->pixels + pitch * y + bpp * x; + Uint8 *startsrc = linesrc; + int xoffset = dx * bpp; + Uint8 *endsrc = linesrc; + if (dy > 0) { + endsrc = linesrc + pitch * (h - 1); + linesrc = endsrc; + } + + if (repeat) { + return scroll_repeat(h, dx, dy, pitch, span, xoffset, startsrc, endsrc, + linesrc); + } + else { + return scroll_default(h, dx, dy, pitch, span, xoffset, startsrc, + endsrc, linesrc, erase); + } +} + static PyObject * surf_scroll(PyObject *self, PyObject *args, PyObject *keywds) { - int dx = 0, dy = 0; + int dx = 0, dy = 0, scroll_flag = PGS_SCROLL_DEFAULT; + int erase = 0, repeat = 0; SDL_Surface *surf; - int bpp; - int pitch; - SDL_Rect *clip_rect; - int w, h; - Uint8 *src, *dst; + SDL_Rect *clip_rect, work_rect; + int w = 0, h = 0, x = 0, y = 0; - static char *kwids[] = {"dx", "dy", NULL}; - if (!PyArg_ParseTupleAndKeywords(args, keywds, "|ii", kwids, &dx, &dy)) { + static char *kwids[] = {"dx", "dy", "scroll_flag", NULL}; + if (!PyArg_ParseTupleAndKeywords(args, keywds, "|iii", kwids, &dx, &dy, + &scroll_flag)) { return NULL; } @@ -2296,46 +2478,60 @@ surf_scroll(PyObject *self, PyObject *args, PyObject *keywds) Py_RETURN_NONE; } + switch (scroll_flag) { + case PGS_SCROLL_REPEAT: { + repeat = 1; + break; + } + case PGS_SCROLL_ERASE: { + erase = 1; + break; + } + default: { + if (scroll_flag != PGS_SCROLL_DEFAULT) { + return RAISE(PyExc_ValueError, "Invalid scroll flag"); + } + } + } + clip_rect = &surf->clip_rect; - w = clip_rect->w; - h = clip_rect->h; - if (dx >= w || dx <= -w || dy >= h || dy <= -h) { + SDL_Rect surf_rect = {0, 0, surf->w, surf->h}; + + // In SDL3, SDL_IntersectRect is renamed to SDL_GetRectIntersection + if (!SDL_IntersectRect(clip_rect, &surf_rect, &work_rect)) { Py_RETURN_NONE; } + w = work_rect.w; + h = work_rect.h; + x = work_rect.x; + y = work_rect.y; + + /* 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 + dx = dx % w; + 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; - } - else { - h += dy; - src -= dy * pitch; - dst += dx * bpp; - } - } - else { - w += dx; - if (dy > 0) { - h -= dy; - src -= dx * bpp; - dst += dy * pitch; - } - else { - h += dy; - src -= dy * pitch + dx * bpp; - } + if (scroll(surf, dx, dy, x, y, w, h, repeat, erase) < 0) { + pgSurface_Unlock((pgSurfaceObject *)self); + return NULL; } - surface_move(src, dst, h, w * bpp, pitch, pitch); if (!pgSurface_Unlock((pgSurfaceObject *)self)) { return NULL; @@ -3685,23 +3881,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) diff --git a/test/surface_test.py b/test/surface_test.py index 0e6a7b6de7..4e4b5d40ca 100644 --- a/test/surface_test.py +++ b/test/surface_test.py @@ -2679,70 +2679,31 @@ def test_unmap_rgb(self): self.assertIsInstance(unmapped_c, pygame.Color) def test_scroll(self): - scrolls = [ - (8, 2, 3), - (16, 2, 3), - (24, 2, 3), - (32, 2, 3), - (32, -1, -3), - (32, 0, 0), - (32, 11, 0), - (32, 0, 11), - (32, -11, 0), - (32, 0, -11), - (32, -11, 2), - (32, 2, -11), - ] - for bitsize, dx, dy in scrolls: - surf = pygame.Surface((10, 10), 0, bitsize) - surf.fill((255, 0, 0)) - surf.fill((0, 255, 0), (2, 2, 2, 2)) - comp = surf.copy() - comp.blit(surf, (dx, dy)) - surf.scroll(dx, dy) - w, h = surf.get_size() - for x in range(w): - for y in range(h): - with self.subTest(x=x, y=y): - self.assertEqual( - surf.get_at((x, y)), - comp.get_at((x, y)), - "%s != %s, bpp:, %i, x: %i, y: %i" - % ( - surf.get_at((x, y)), - comp.get_at((x, y)), - bitsize, - dx, - dy, - ), - ) - # Confirm clip rect containment - surf = pygame.Surface((20, 13), 0, 32) - surf.fill((255, 0, 0)) - surf.fill((0, 255, 0), (7, 1, 6, 6)) - comp = surf.copy() - clip = Rect(3, 1, 8, 14) - surf.set_clip(clip) - comp.set_clip(clip) - comp.blit(surf, (clip.x + 2, clip.y + 3), surf.get_clip()) - surf.scroll(2, 3) - w, h = surf.get_size() - for x in range(w): - for y in range(h): - self.assertEqual(surf.get_at((x, y)), comp.get_at((x, y))) - # Confirm keyword arguments and per-pixel alpha - spot_color = (0, 255, 0, 128) - surf = pygame.Surface((4, 4), pygame.SRCALPHA, 32) - surf.fill((255, 0, 0, 255)) - surf.set_at((1, 1), spot_color) - surf.scroll(dx=1) - self.assertEqual(surf.get_at((2, 1)), spot_color) - surf.scroll(dy=1) - self.assertEqual(surf.get_at((2, 2)), spot_color) - surf.scroll(dy=1, dx=1) - self.assertEqual(surf.get_at((3, 3)), spot_color) - surf.scroll(dx=-3, dy=-3) - self.assertEqual(surf.get_at((0, 0)), spot_color) + # Check segfaults for any direction or bits or mode + for bits in [8, 16, 24, 32]: + for flag in [0, pygame.SCROLL_ERASE, pygame.SCROLL_REPEAT]: + for dx in range(-1, 1): + for dy in range(-1, 1): + surface = pygame.Surface((20, 20), 0, bits) + surface.scroll(dx * 2, dy * 2, flag) + # Check non repeating mode working + surface = pygame.Surface((20, 20)) + surface.fill("green") + surface.scroll(10, 0) + self.assertEqual(surface.get_at((0, 0)), pygame.Color("green")) + # Check non repeating, erasing mode working + surface = pygame.Surface((20, 20)) + surface.fill("green") + surface.scroll(2, 2, pygame.SCROLL_ERASE) + self.assertEqual(surface.get_at((0, 0)), pygame.Color("black")) + # Check repeating mode working + surface = pygame.Surface((20, 20)) + surface.fill("green") + pygame.draw.rect(surface, "red", (0, 0, 10, 20)) + surface.scroll(4, 0, pygame.SCROLL_REPEAT) + self.assertEqual(surface.get_at((0, 0)), pygame.Color("green")) + # kwargs + surface.scroll(dx=1, dy=1, scroll_flag=0) class SurfaceSubtypeTest(unittest.TestCase):