diff --git a/docs/reST/ref/draw.rst b/docs/reST/ref/draw.rst index 44ac398a3c..110b247b87 100644 --- a/docs/reST/ref/draw.rst +++ b/docs/reST/ref/draw.rst @@ -48,7 +48,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: :data:`pygame.typing.ColorLike` - :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) @@ -91,6 +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.3 Negative rect dimension values will raise a deprecation warning .. ## pygame.draw.rect ## @@ -271,7 +273,8 @@ object around the draw calls (see :func:`pygame.Surface.lock` and :type color: :data:`pygame.typing.ColorLike` :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 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) @@ -291,6 +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.3 Negative rect dimension values will raise a deprecation warning .. ## pygame.draw.ellipse ## @@ -312,7 +316,8 @@ object around the draw calls (see :func:`pygame.Surface.lock` and :type color: :data:`pygame.typing.ColorLike` :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 @@ -344,6 +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.3 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 0d6b9fc8a8..81fa4b5099 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"); @@ -716,14 +728,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 NULL; + } } 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)) { @@ -1135,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 872e72ee58..0d7695f396 100644 --- a/test/draw_test.py +++ b/test/draw_test.py @@ -408,6 +408,20 @@ 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 = ((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_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) @@ -4778,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) @@ -7038,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")