Add warning when using negative rect dimensions in draw module

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.2 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.2 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.2 Negative rect dimension values will raise a deprecation warning
 
    .. ## pygame.draw.arc ##
 

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");
+            }
         }
     }
 

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")