From f604bedd4f3ab65ab3c7e38a218b1819cbca0fa4 Mon Sep 17 00:00:00 2001 From: Andrew Coffey Date: Sun, 30 Jun 2024 11:51:13 -0500 Subject: [PATCH 1/4] set_at now take a long long for y instead of int --- src_c/draw.c | 10 +++++++--- test/draw_test.py | 8 ++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src_c/draw.c b/src_c/draw.c index a5667b8c94..522ec49ef2 100644 --- a/src_c/draw.c +++ b/src_c/draw.c @@ -1425,8 +1425,12 @@ clip_line(SDL_Surface *surf, SDL_Rect surf_clip_rect, int *x1, int *y1, } static int -set_at(SDL_Surface *surf, SDL_Rect surf_clip_rect, int x, int y, Uint32 color) +set_at(SDL_Surface *surf, SDL_Rect surf_clip_rect, int x, long long y, + Uint32 color) { + // y should be long long so that y * surf->pitch doesn't overflow the int + // bounds in the case of very large surfaces and drawing on the edge of + // them Uint8 *pixels = (Uint8 *)surf->pixels; if (x < surf_clip_rect.x || x >= surf_clip_rect.x + surf_clip_rect.w || @@ -1459,7 +1463,7 @@ static void set_and_check_rect(SDL_Surface *surf, SDL_Rect surf_clip_rect, int x, int y, Uint32 color, int *drawn_area) { - if (set_at(surf, surf_clip_rect, x, y, color)) { + if (set_at(surf, surf_clip_rect, x, (long long)y, color)) { add_pixel_to_drawn_list(x, y, drawn_area); } } @@ -1788,7 +1792,7 @@ drawhorzlineclip(SDL_Surface *surf, SDL_Rect surf_clip_rect, Uint32 color, } if (x1 == x2) { - set_at(surf, surf_clip_rect, x1, y1, color); + set_at(surf, surf_clip_rect, x1, (long long)y1, color); return; } drawhorzline(surf, color, x1, y1, x2); diff --git a/test/draw_test.py b/test/draw_test.py index cb72452fdf..4e1de724b8 100644 --- a/test/draw_test.py +++ b/test/draw_test.py @@ -1846,6 +1846,14 @@ def test_line_clipping_with_thickness(self): "start={}, end={}".format(end_points[n], start_points[n]), ) + def test_line_draw_large_surf_regression(self): + """Regression test for https://github.com/pygame-community/pygame-ce/issues/2961""" + surface = pygame.Surface((43371, 43371)) + + point1 = [30021, 37135] + point2 = [30022, 37136] + pygame.draw.line(surface, (255, 255, 255), point1, point2, 1) + ### Lines Testing ############################################################# From 4691737d8dcd1e9a049de5dfe42cc3d9fc228a5e Mon Sep 17 00:00:00 2001 From: Andrew Coffey Date: Sun, 30 Jun 2024 12:43:41 -0500 Subject: [PATCH 2/4] Skip new unit test if CI is running or the machine running doesn't have the memory for it --- test/draw_test.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/test/draw_test.py b/test/draw_test.py index 4e1de724b8..d967039953 100644 --- a/test/draw_test.py +++ b/test/draw_test.py @@ -1,4 +1,5 @@ import math +import os import sys import unittest import warnings @@ -1846,12 +1847,16 @@ def test_line_clipping_with_thickness(self): "start={}, end={}".format(end_points[n], start_points[n]), ) + @unittest.skipIf("CI" in os.environ, "Surface is too large for CI to handle") def test_line_draw_large_surf_regression(self): """Regression test for https://github.com/pygame-community/pygame-ce/issues/2961""" - surface = pygame.Surface((43371, 43371)) + try: + surface = pygame.Surface((14457, 37200)) + except pygame.error: + self.skipTest("Surface too large to run this test in this environment") - point1 = [30021, 37135] - point2 = [30022, 37136] + point1 = [400, 37135] + point2 = [401, 37136] pygame.draw.line(surface, (255, 255, 255), point1, point2, 1) From 398d8abeff716217fb805863d891d38c0f031b37 Mon Sep 17 00:00:00 2001 From: Andrew Coffey Date: Tue, 24 Jun 2025 21:43:39 -0500 Subject: [PATCH 3/4] Test now skips based on total memory --- test/draw_test.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/draw_test.py b/test/draw_test.py index d967039953..1d6387af7c 100644 --- a/test/draw_test.py +++ b/test/draw_test.py @@ -1847,13 +1847,15 @@ def test_line_clipping_with_thickness(self): "start={}, end={}".format(end_points[n], start_points[n]), ) - @unittest.skipIf("CI" in os.environ, "Surface is too large for CI to handle") + @unittest.skipIf(pygame.system.get_total_ram() < 4096, "Surface is too large") def test_line_draw_large_surf_regression(self): """Regression test for https://github.com/pygame-community/pygame-ce/issues/2961""" try: surface = pygame.Surface((14457, 37200)) except pygame.error: - self.skipTest("Surface too large to run this test in this environment") + self.skipTest( + "Surface too large to run this test in this environment, or too many background tasks" + ) point1 = [400, 37135] point2 = [401, 37136] From 9aba3c6149c89738428582cc2651cf6a7dfc062a Mon Sep 17 00:00:00 2001 From: Andrew Coffey Date: Sun, 29 Jun 2025 13:16:07 -0500 Subject: [PATCH 4/4] use intptr_t instead of long long and add 32-bit architecture guard --- src_c/draw.c | 43 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/src_c/draw.c b/src_c/draw.c index 522ec49ef2..4accefdef1 100644 --- a/src_c/draw.c +++ b/src_c/draw.c @@ -1425,12 +1425,12 @@ clip_line(SDL_Surface *surf, SDL_Rect surf_clip_rect, int *x1, int *y1, } static int -set_at(SDL_Surface *surf, SDL_Rect surf_clip_rect, int x, long long y, +set_at(SDL_Surface *surf, SDL_Rect surf_clip_rect, intptr_t x, intptr_t y, Uint32 color) { - // y should be long long so that y * surf->pitch doesn't overflow the int - // bounds in the case of very large surfaces and drawing on the edge of - // them + // x and y should be intptr_t so that (y * surf->pitch) + x doesn't + // overflow the int bounds in the case of very large surfaces and + // drawing on the edge of them Uint8 *pixels = (Uint8 *)surf->pixels; if (x < surf_clip_rect.x || x >= surf_clip_rect.x + surf_clip_rect.w || @@ -1438,6 +1438,37 @@ set_at(SDL_Surface *surf, SDL_Rect surf_clip_rect, int x, long long y, return 0; } +#if INTPTR_MAX == INT32_MAX + // Need to bounds check operation on 32-bit systems + // first check y * surf->pitch doesn't overflow INT32_MAX + // or underflow INT32_MIN + // a * b > c if and only if a > c / b || a * b < c if and only if a < c / b + if ((y > (INT32_MAX / surf->pitch)) || (y < (INT32_MIN / surf->pitch))) { + return 0; + } + + intptr_t product = y * surf->pitch; + + // now that the multiplication is fine, check that the + // addition is fine + // this is complicated, based on the sign of x + if (x >= 0) { + // product + x > INT32_MAX if and only if product > INT32_MAX - x + // product + x < INT32_MIN cannot happen since we have something that + // is no smaller than INT32_MIN added to something no smaller than 0 + if (product > (INT32_MAX - x)) { + return 0; + } + } + else { + // product + x > INT32_MAX cannot happen for a similar reason + // product + x < INT32_MIN if and only if product < INT32_MIN - x + if (product < (INT32_MIN - x)) { + return 0; + } + } +#endif + switch (PG_SURF_BytesPerPixel(surf)) { case 1: *((Uint8 *)pixels + y * surf->pitch + x) = (Uint8)color; @@ -1463,7 +1494,7 @@ static void set_and_check_rect(SDL_Surface *surf, SDL_Rect surf_clip_rect, int x, int y, Uint32 color, int *drawn_area) { - if (set_at(surf, surf_clip_rect, x, (long long)y, color)) { + if (set_at(surf, surf_clip_rect, (intptr_t)x, (intptr_t)y, color)) { add_pixel_to_drawn_list(x, y, drawn_area); } } @@ -1792,7 +1823,7 @@ drawhorzlineclip(SDL_Surface *surf, SDL_Rect surf_clip_rect, Uint32 color, } if (x1 == x2) { - set_at(surf, surf_clip_rect, x1, (long long)y1, color); + set_at(surf, surf_clip_rect, (intptr_t)x1, (intptr_t)y1, color); return; } drawhorzline(surf, color, x1, y1, x2);