From ec92294fead3655827d9fb8edee91d4c8ba9701f Mon Sep 17 00:00:00 2001 From: Marko Zivic Date: Thu, 10 Oct 2024 01:03:59 +0200 Subject: [PATCH 1/5] Added warning to ellipse Added warning in docs Added test for this warning --- docs/reST/ref/draw.rst | 4 +++- src_c/draw.c | 11 +++++++++++ test/draw_test.py | 20 ++++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/docs/reST/ref/draw.rst b/docs/reST/ref/draw.rst index dfe4b669d3..f0d028edcc 100644 --- a/docs/reST/ref/draw.rst +++ b/docs/reST/ref/draw.rst @@ -276,7 +276,8 @@ object around the draw calls (see :func:`pygame.Surface.lock` and :type color: Color or string (for :doc:`color_list`) or int or tuple(int, int, int, [int]) :param Rect rect: rectangle to indicate the position and dimensions of the ellipse, the ellipse will be centered inside the rectangle and bounded - by it + by it. Negative dimension and position values smaller than negated dimension are deprecated, + and will raise an exception in future versions on pygame-ce. :param int width: (optional) used for line thickness or to indicate that the ellipse is to be filled (not to be confused with the width value of the ``rect`` parameter) @@ -296,6 +297,7 @@ object around the draw calls (see :func:`pygame.Surface.lock` and :rtype: Rect .. versionchangedold:: 2.0.0 Added support for keyword arguments. + .. versionchanged:: 2.5.2 Negative values in rect will raise a deprecation warning .. ## pygame.draw.ellipse ## diff --git a/src_c/draw.c b/src_c/draw.c index 84138bc077..f8d4953f3d 100644 --- a/src_c/draw.c +++ b/src_c/draw.c @@ -706,6 +706,17 @@ ellipse(PyObject *self, PyObject *arg, PyObject *kwargs) PG_SURF_BytesPerPixel(surf)); } + if (drawn_area[0] - drawn_area[2] < 0 || drawn_area[2] < 0 || + drawn_area[1] - drawn_area[3] < 0 || drawn_area[3] < 0) { + if (PyErr_WarnEx(PyExc_DeprecationWarning, + "Negative values in position and dimension rect are " + "deprecated and have no functionality. This will " + "cause an error in a future version of pygame-ce.", + 1) == -1) { + return pgRect_New4(0, 0, 0, 0); + } + } + CHECK_LOAD_COLOR(colorobj) if (width < 0) { diff --git a/test/draw_test.py b/test/draw_test.py index 872e72ee58..732f8ea52c 100644 --- a/test/draw_test.py +++ b/test/draw_test.py @@ -408,6 +408,26 @@ def test_ellipse__valid_width_values(self): self.assertEqual(surface.get_at(pos), expected_color) self.assertIsInstance(bounds_rect, pygame.Rect) + def test_ellipse__negative_rect_warning(self): + """Ensures draw ellipse shows DeprecationWarning for rect with negative values""" + # Generate few faulty rects. + faulty_rects = [] + for i in range(4): + faulty_rect = [0, 0, 0, 0] + faulty_rect[i] = -15 + if i < 2: + faulty_rect[i + 2] = -5 + faulty_rects.append(faulty_rect) + with warnings.catch_warnings(record=True) as w: + for count, rect in enumerate(faulty_rects): + # Cause all warnings to always be triggered. + warnings.simplefilter("always") + # Trigger DeprecationWarning. + self.draw_ellipse(pygame.Surface((6, 6)), (255, 255, 255), rect) + # Check if there is only one warning and is a DeprecationWarning. + self.assertEqual(len(w), count + 1) + self.assertTrue(issubclass(w[-1].category, DeprecationWarning)) + def test_ellipse__valid_rect_formats(self): """Ensures draw ellipse accepts different rect formats.""" pos = (1, 1) From 5e99a6b4d538e4f35ede201ad2a00a045832ba79 Mon Sep 17 00:00:00 2001 From: Marko Zivic Date: Thu, 10 Oct 2024 01:27:30 +0200 Subject: [PATCH 2/5] Only check rect dimensions --- docs/reST/ref/draw.rst | 6 +++--- src_c/draw.c | 34 +++++++++++++++++----------------- test/draw_test.py | 8 +------- 3 files changed, 21 insertions(+), 27 deletions(-) diff --git a/docs/reST/ref/draw.rst b/docs/reST/ref/draw.rst index f0d028edcc..6c9728d702 100644 --- a/docs/reST/ref/draw.rst +++ b/docs/reST/ref/draw.rst @@ -276,8 +276,8 @@ object around the draw calls (see :func:`pygame.Surface.lock` and :type color: Color or string (for :doc:`color_list`) or int or tuple(int, int, int, [int]) :param Rect rect: rectangle to indicate the position and dimensions of the ellipse, the ellipse will be centered inside the rectangle and bounded - by it. Negative dimension and position values smaller than negated dimension are deprecated, - and will raise an exception in future versions on pygame-ce. + by it. Negative rect dimension values are deprecated, and will raise an exception + in a future versions on pygame-ce. :param int width: (optional) used for line thickness or to indicate that the ellipse is to be filled (not to be confused with the width value of the ``rect`` parameter) @@ -297,7 +297,7 @@ object around the draw calls (see :func:`pygame.Surface.lock` and :rtype: Rect .. versionchangedold:: 2.0.0 Added support for keyword arguments. - .. versionchanged:: 2.5.2 Negative values in rect will raise a deprecation warning + .. versionchanged:: 2.5.2 Negative rect dimension values will raise a deprecation warning .. ## pygame.draw.ellipse ## diff --git a/src_c/draw.c b/src_c/draw.c index f8d4953f3d..57a3e212f5 100644 --- a/src_c/draw.c +++ b/src_c/draw.c @@ -706,17 +706,6 @@ ellipse(PyObject *self, PyObject *arg, PyObject *kwargs) PG_SURF_BytesPerPixel(surf)); } - if (drawn_area[0] - drawn_area[2] < 0 || drawn_area[2] < 0 || - drawn_area[1] - drawn_area[3] < 0 || drawn_area[3] < 0) { - if (PyErr_WarnEx(PyExc_DeprecationWarning, - "Negative values in position and dimension rect are " - "deprecated and have no functionality. This will " - "cause an error in a future version of pygame-ce.", - 1) == -1) { - return pgRect_New4(0, 0, 0, 0); - } - } - CHECK_LOAD_COLOR(colorobj) if (width < 0) { @@ -727,14 +716,25 @@ ellipse(PyObject *self, PyObject *arg, PyObject *kwargs) return RAISE(PyExc_RuntimeError, "error locking surface"); } - if (!width || - width >= MIN(rect->w / 2 + rect->w % 2, rect->h / 2 + rect->h % 2)) { - draw_ellipse_filled(surf, rect->x, rect->y, rect->w, rect->h, color, - drawn_area); + if (rect->w < 0 || rect->h < 0) { + if (PyErr_WarnEx(PyExc_DeprecationWarning, + "Negative rect dimension values are deprecated and " + "have no functionality. This will cause an error in " + "a future version of pygame-ce.", + 1) == -1) { + return pgRect_New4(0, 0, 0, 0); + } } else { - draw_ellipse_thickness(surf, rect->x, rect->y, rect->w, rect->h, - width - 1, color, drawn_area); + if (!width || width >= MIN(rect->w / 2 + rect->w % 2, + rect->h / 2 + rect->h % 2)) { + draw_ellipse_filled(surf, rect->x, rect->y, rect->w, rect->h, + color, drawn_area); + } + else { + draw_ellipse_thickness(surf, rect->x, rect->y, rect->w, rect->h, + width - 1, color, drawn_area); + } } if (!pgSurface_Unlock(surfobj)) { diff --git a/test/draw_test.py b/test/draw_test.py index 732f8ea52c..d6238c59c1 100644 --- a/test/draw_test.py +++ b/test/draw_test.py @@ -411,13 +411,7 @@ def test_ellipse__valid_width_values(self): def test_ellipse__negative_rect_warning(self): """Ensures draw ellipse shows DeprecationWarning for rect with negative values""" # Generate few faulty rects. - faulty_rects = [] - for i in range(4): - faulty_rect = [0, 0, 0, 0] - faulty_rect[i] = -15 - if i < 2: - faulty_rect[i + 2] = -5 - faulty_rects.append(faulty_rect) + faulty_rects = ((10, 10, -5, 3), (10, 10, 5, -3)) with warnings.catch_warnings(record=True) as w: for count, rect in enumerate(faulty_rects): # Cause all warnings to always be triggered. From 42df11762ab014e2fabc3c710d0d32e9825b1107 Mon Sep 17 00:00:00 2001 From: Marko Zivic Date: Thu, 10 Oct 2024 11:50:26 +0200 Subject: [PATCH 3/5] Return NULL --- src_c/draw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src_c/draw.c b/src_c/draw.c index 57a3e212f5..9732137779 100644 --- a/src_c/draw.c +++ b/src_c/draw.c @@ -722,7 +722,7 @@ ellipse(PyObject *self, PyObject *arg, PyObject *kwargs) "have no functionality. This will cause an error in " "a future version of pygame-ce.", 1) == -1) { - return pgRect_New4(0, 0, 0, 0); + return NULL; } } else { From e6d7998e48c4c6fda9c24fc17a7cde8116794def Mon Sep 17 00:00:00 2001 From: Marko Zivic Date: Sat, 12 Oct 2024 02:22:38 +0200 Subject: [PATCH 4/5] Added to arc and rect --- docs/reST/ref/draw.rst | 8 ++- src_c/draw.c | 134 ++++++++++++++++++++++++----------------- test/draw_test.py | 28 +++++++++ 3 files changed, 114 insertions(+), 56 deletions(-) diff --git a/docs/reST/ref/draw.rst b/docs/reST/ref/draw.rst index 6c9728d702..64422ed4f6 100644 --- a/docs/reST/ref/draw.rst +++ b/docs/reST/ref/draw.rst @@ -53,7 +53,8 @@ object around the draw calls (see :func:`pygame.Surface.lock` and :param color: color to draw with, the alpha value is optional if using a tuple ``(RGB[A])`` :type color: Color or string (for :doc:`color_list`) or int or tuple(int, int, int, [int]) - :param Rect rect: rectangle to draw, position and dimensions + :param Rect rect: rectangle to draw, position and dimensions. Negative rect dimension + values are deprecated, and will raise an exception in a future versions on pygame-ce. :param int width: (optional) used for line thickness or to indicate that the rectangle is to be filled (not to be confused with the width value of the ``rect`` parameter) @@ -96,6 +97,7 @@ object around the draw calls (see :func:`pygame.Surface.lock` and .. versionchangedold:: 2.0.0 Added support for keyword arguments. .. versionchangedold:: 2.0.0.dev8 Added support for border radius. + .. versionchanged:: 2.5.2 Negative rect dimension values will raise a deprecation warning .. ## pygame.draw.rect ## @@ -319,7 +321,8 @@ object around the draw calls (see :func:`pygame.Surface.lock` and :type color: Color or string (for :doc:`color_list`) or int or tuple(int, int, int, [int]) :param Rect rect: rectangle to indicate the position and dimensions of the ellipse which the arc will be based on, the ellipse will be centered - inside the rectangle + inside the rectangle. Negative rect dimension values are deprecated, + and will raise an exception in a future versions on pygame-ce. :param float start_angle: start angle of the arc in radians :param float stop_angle: stop angle of the arc in radians @@ -351,6 +354,7 @@ object around the draw calls (see :func:`pygame.Surface.lock` and :rtype: Rect .. versionchangedold:: 2.0.0 Added support for keyword arguments. + .. versionchanged:: 2.5.2 Negative rect dimension values will raise a deprecation warning .. ## pygame.draw.arc ## diff --git a/src_c/draw.c b/src_c/draw.c index 9732137779..46b5ca0e49 100644 --- a/src_c/draw.c +++ b/src_c/draw.c @@ -655,8 +655,20 @@ arc(PyObject *self, PyObject *arg, PyObject *kwargs) width = MIN(width, MIN(rect->w, rect->h) / 2); - draw_arc(surf, rect->x + rect->w / 2, rect->y + rect->h / 2, rect->w / 2, - rect->h / 2, width, angle_start, angle_stop, color, drawn_area); + if (rect->w < 0 || rect->h < 0) { + if (PyErr_WarnEx(PyExc_DeprecationWarning, + "Negative rect dimension values are deprecated and " + "have no functionality. This will cause an error in " + "a future version of pygame-ce.", + 1) == -1) { + return NULL; + } + } + else { + draw_arc(surf, rect->x + rect->w / 2, rect->y + rect->h / 2, + rect->w / 2, rect->h / 2, width, angle_start, angle_stop, + color, drawn_area); + } if (!pgSurface_Unlock(surfobj)) { return RAISE(PyExc_RuntimeError, "error unlocking surface"); @@ -1146,65 +1158,79 @@ rect(PyObject *self, PyObject *args, PyObject *kwargs) return pgRect_New4(rect->x, rect->y, 0, 0); } - /* If there isn't any rounded rect-ness OR the rect is really thin in one - direction. The "really thin in one direction" check is necessary because - draw_round_rect fails (draws something bad) on rects with a dimension - that is 0 or 1 pixels across.*/ - if ((radius <= 0 && top_left_radius <= 0 && top_right_radius <= 0 && - bottom_left_radius <= 0 && bottom_right_radius <= 0) || - abs(rect->w) < 2 || abs(rect->h) < 2) { - sdlrect.x = rect->x; - sdlrect.y = rect->y; - sdlrect.w = rect->w; - sdlrect.h = rect->h; - SDL_GetClipRect(surf, &cliprect); - /* SDL_FillRect respects the clip rect already, but in order to - return the drawn area, we need to do this here, and keep the - pointer to the result in clipped */ - if (!SDL_IntersectRect(&sdlrect, &cliprect, &clipped)) { - return pgRect_New4(rect->x, rect->y, 0, 0); - } - if (width > 0 && (width * 2) < clipped.w && (width * 2) < clipped.h) { - draw_rect(surf, sdlrect.x, sdlrect.y, sdlrect.x + sdlrect.w - 1, - sdlrect.y + sdlrect.h - 1, width, color); - } - else { - pgSurface_Prep(surfobj); - pgSurface_Lock(surfobj); - result = SDL_FillRect(surf, &clipped, color); - pgSurface_Unlock(surfobj); - pgSurface_Unprep(surfobj); - if (result != 0) - return RAISE(pgExc_SDLError, SDL_GetError()); + if (rect->w < 0 || rect->h < 0) { + if (PyErr_WarnEx(PyExc_DeprecationWarning, + "Negative rect dimension values are deprecated and " + "have no functionality. This will cause an error in " + "a future version of pygame-ce.", + 1) == -1) { + return NULL; } - return pgRect_New(&clipped); } else { - if (!pgSurface_Lock(surfobj)) { - return RAISE(PyExc_RuntimeError, "error locking surface"); + /* If there isn't any rounded rect-ness OR the rect is really thin in + one direction. The "really thin in one direction" check is necessary + because draw_round_rect fails (draws something bad) on rects with a + dimension that is 0 or 1 pixels across.*/ + if ((radius <= 0 && top_left_radius <= 0 && top_right_radius <= 0 && + bottom_left_radius <= 0 && bottom_right_radius <= 0) || + abs(rect->w) < 2 || abs(rect->h) < 2) { + sdlrect.x = rect->x; + sdlrect.y = rect->y; + sdlrect.w = rect->w; + sdlrect.h = rect->h; + SDL_GetClipRect(surf, &cliprect); + /* SDL_FillRect respects the clip rect already, but in order to + return the drawn area, we need to do this here, and keep the + pointer to the result in clipped */ + if (!SDL_IntersectRect(&sdlrect, &cliprect, &clipped)) { + return pgRect_New4(rect->x, rect->y, 0, 0); + } + if (width > 0 && (width * 2) < clipped.w && + (width * 2) < clipped.h) { + draw_rect(surf, sdlrect.x, sdlrect.y, + sdlrect.x + sdlrect.w - 1, sdlrect.y + sdlrect.h - 1, + width, color); + } + else { + pgSurface_Prep(surfobj); + pgSurface_Lock(surfobj); + result = SDL_FillRect(surf, &clipped, color); + pgSurface_Unlock(surfobj); + pgSurface_Unprep(surfobj); + if (result != 0) + return RAISE(pgExc_SDLError, SDL_GetError()); + } + return pgRect_New(&clipped); } + else { + if (!pgSurface_Lock(surfobj)) { + return RAISE(PyExc_RuntimeError, "error locking surface"); + } - /* Little bit to normalize the rect: this matters for the rounded - rects, despite not mattering for the normal rects. */ - if (rect->w < 0) { - rect->x += rect->w; - rect->w = -rect->w; - } - if (rect->h < 0) { - rect->y += rect->h; - rect->h = -rect->h; - } + /* Little bit to normalize the rect: this matters for the rounded + rects, despite not mattering for the normal rects. */ + if (rect->w < 0) { + rect->x += rect->w; + rect->w = -rect->w; + } + if (rect->h < 0) { + rect->y += rect->h; + rect->h = -rect->h; + } - if (width > rect->w / 2 || width > rect->h / 2) { - width = MAX(rect->w / 2, rect->h / 2); - } + if (width > rect->w / 2 || width > rect->h / 2) { + width = MAX(rect->w / 2, rect->h / 2); + } - draw_round_rect(surf, rect->x, rect->y, rect->x + rect->w - 1, - rect->y + rect->h - 1, radius, width, color, - top_left_radius, top_right_radius, bottom_left_radius, - bottom_right_radius, drawn_area); - if (!pgSurface_Unlock(surfobj)) { - return RAISE(PyExc_RuntimeError, "error unlocking surface"); + draw_round_rect(surf, rect->x, rect->y, rect->x + rect->w - 1, + rect->y + rect->h - 1, radius, width, color, + top_left_radius, top_right_radius, + bottom_left_radius, bottom_right_radius, + drawn_area); + if (!pgSurface_Unlock(surfobj)) { + return RAISE(PyExc_RuntimeError, "error unlocking surface"); + } } } diff --git a/test/draw_test.py b/test/draw_test.py index d6238c59c1..0d7695f396 100644 --- a/test/draw_test.py +++ b/test/draw_test.py @@ -4792,6 +4792,20 @@ def test_rect__valid_width_values(self): self.assertEqual(surface.get_at(pos), expected_color) self.assertIsInstance(bounds_rect, pygame.Rect) + def test_rect__negative_rect_warning(self): + """Ensures draw rect shows DeprecationWarning for rect with negative values""" + # Generate few faulty rects. + faulty_rects = ((10, 10, -5, 3), (10, 10, 5, -3)) + with warnings.catch_warnings(record=True) as w: + for count, rect in enumerate(faulty_rects): + # Cause all warnings to always be triggered. + warnings.simplefilter("always") + # Trigger DeprecationWarning. + self.draw_rect(pygame.Surface((6, 6)), (255, 255, 255), rect) + # Check if there is only one warning and is a DeprecationWarning. + self.assertEqual(len(w), count + 1) + self.assertTrue(issubclass(w[-1].category, DeprecationWarning)) + def test_rect__valid_rect_formats(self): """Ensures draw rect accepts different rect formats.""" pos = (1, 1) @@ -7052,6 +7066,20 @@ def test_arc__valid_start_angle_values(self): self.assertEqual(surface.get_at(pos), expected_color, msg) self.assertIsInstance(bounds_rect, pygame.Rect, msg) + def test_arc__negative_rect_warning(self): + """Ensures draw arc shows DeprecationWarning for rect with negative values""" + # Generate few faulty rects. + faulty_rects = ((10, 10, -5, 3), (10, 10, 5, -3)) + with warnings.catch_warnings(record=True) as w: + for count, rect in enumerate(faulty_rects): + # Cause all warnings to always be triggered. + warnings.simplefilter("always") + # Trigger DeprecationWarning. + self.draw_arc(pygame.Surface((6, 6)), (255, 255, 255), rect, 0, 7) + # Check if there is only one warning and is a DeprecationWarning. + self.assertEqual(len(w), count + 1) + self.assertTrue(issubclass(w[-1].category, DeprecationWarning)) + def test_arc__valid_rect_formats(self): """Ensures draw arc accepts different rect formats.""" expected_color = pygame.Color("red") From 9e3ee00f8c0df04e292aee66d22ffb5006367b60 Mon Sep 17 00:00:00 2001 From: Marko Zivic Date: Sat, 2 Nov 2024 14:56:25 +0100 Subject: [PATCH 5/5] Change version in docs --- docs/reST/ref/draw.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/reST/ref/draw.rst b/docs/reST/ref/draw.rst index 360199ba98..110b247b87 100644 --- a/docs/reST/ref/draw.rst +++ b/docs/reST/ref/draw.rst @@ -92,7 +92,7 @@ object around the draw calls (see :func:`pygame.Surface.lock` and .. versionchangedold:: 2.0.0 Added support for keyword arguments. .. versionchangedold:: 2.0.0.dev8 Added support for border radius. - .. versionchanged:: 2.5.2 Negative rect dimension values will raise a deprecation warning + .. versionchanged:: 2.5.3 Negative rect dimension values will raise a deprecation warning .. ## pygame.draw.rect ## @@ -294,7 +294,7 @@ object around the draw calls (see :func:`pygame.Surface.lock` and :rtype: Rect .. versionchangedold:: 2.0.0 Added support for keyword arguments. - .. versionchanged:: 2.5.2 Negative rect dimension values will raise a deprecation warning + .. versionchanged:: 2.5.3 Negative rect dimension values will raise a deprecation warning .. ## pygame.draw.ellipse ## @@ -349,7 +349,7 @@ object around the draw calls (see :func:`pygame.Surface.lock` and :rtype: Rect .. versionchangedold:: 2.0.0 Added support for keyword arguments. - .. versionchanged:: 2.5.2 Negative rect dimension values will raise a deprecation warning + .. versionchanged:: 2.5.3 Negative rect dimension values will raise a deprecation warning .. ## pygame.draw.arc ##