From a9afa0d9d4c66cda8fe7e6c1a72765e704e94985 Mon Sep 17 00:00:00 2001
From: Kevin Matocha <kmatocha@icloud.com>
Date: Thu, 11 Mar 2021 16:18:17 -0600
Subject: [PATCH] Move input checks to shared-module, update docstrings

---
 shared-bindings/bitmaptools/__init__.c | 38 ++----------------------
 shared-module/bitmaptools/__init__.c   | 40 +++++++++++++++++++-------
 2 files changed, 31 insertions(+), 47 deletions(-)

diff --git a/shared-bindings/bitmaptools/__init__.c b/shared-bindings/bitmaptools/__init__.c
index 6804889081..118564ff85 100644
--- a/shared-bindings/bitmaptools/__init__.c
+++ b/shared-bindings/bitmaptools/__init__.c
@@ -255,8 +255,8 @@ MP_DEFINE_CONST_FUN_OBJ_KW(bitmaptools_rotozoom_obj, 0, bitmaptools_obj_rotozoom
 //|      :param bitmap dest_bitmap: Destination bitmap that will be written into
 //|      :param int x1: x-pixel position of the first corner of the rectangular fill region
 //|      :param int y1: y-pixel position of the first corner of the rectangular fill region
-//|      :param int x2: x-pixel position of the second corner of the rectangular fill region
-//|      :param int y2: y-pixel position of the second corner of the rectangular fill region
+//|      :param int x2: x-pixel position of the second corner of the rectangular fill region (exclusive)
+//|      :param int y2: y-pixel position of the second corner of the rectangular fill region (exclusive)
 //|      :param int value: Bitmap palette index that will be written into the rectangular
 //|             fill region in the destination bitmap"""
 //|      ...
@@ -289,40 +289,6 @@ STATIC mp_obj_t bitmaptools_obj_fill_region(size_t n_args, const mp_obj_t *pos_a
     int16_t x2 = args[ARG_x2].u_int;
     int16_t y2 = args[ARG_y2].u_int;
 
-    // Ensure x1 < x2 and y1 < y2
-    if (x1 > x2) {
-        int16_t temp=x2;
-        x2=x1;
-        x1=temp;
-    }
-    if (y1 > y2) {
-        int16_t temp=y2;
-        y2=y1;
-        y1=temp;
-    }
-
-    // constrain to bitmap dimensions
-    if (x1 < 0) {
-        x1 = 0;
-    } else if (x1 > destination->width) {
-        x1 = destination->width;
-    }
-    if (x2 < 0) {
-        x2 = 0;
-    } else if (x2 > destination->width) {
-        x2 = destination->width;
-    }
-    if (y1 < 0) {
-        y1 = 0;
-    } else if (y1 > destination->height) {
-        y1 = destination->height;
-    }
-    if (y2 < 0) {
-        y2 = 0;
-    } else if (y2 > destination->height) {
-        y2 = destination->height;
-    }
-
     common_hal_bitmaptools_fill_region(destination, x1, y1, x2, y2, value);
 
     return mp_const_none;
diff --git a/shared-module/bitmaptools/__init__.c b/shared-module/bitmaptools/__init__.c
index 72db55c242..6f168b5552 100644
--- a/shared-module/bitmaptools/__init__.c
+++ b/shared-module/bitmaptools/__init__.c
@@ -175,6 +175,17 @@ void common_hal_bitmaptools_rotozoom(displayio_bitmap_t *self, int16_t ox, int16
     }
 }
 
+int16_t constrain(int16_t input, int16_t min, int16_t max) {
+    // constrain the input between the min and max values
+    if (input < min) {
+        return min;
+    }
+    if (input > max) {
+        return max;
+    }
+    return input;
+}
+
 void common_hal_bitmaptools_fill_region(displayio_bitmap_t *destination,
                                         int16_t x1, int16_t y1,
                                         int16_t x2, int16_t y2,
@@ -187,6 +198,24 @@ void common_hal_bitmaptools_fill_region(displayio_bitmap_t *destination,
         mp_raise_RuntimeError(translate("Read-only object"));
     }
 
+    // Ensure x1 < x2 and y1 < y2
+    if (x1 > x2) {
+        int16_t temp=x2;
+        x2=x1;
+        x1=temp;
+    }
+    if (y1 > y2) {
+        int16_t temp=y2;
+        y2=y1;
+        y1=temp;
+    }
+
+    // constrain to bitmap dimensions
+    x1 = constrain(x1, 0, destination->width);
+    x2 = constrain(x2, 0, destination->width);
+    y1 = constrain(y1, 0, destination->height);
+    y2 = constrain(y2, 0, destination->height);
+
     // update the dirty rectangle
     displayio_bitmap_set_dirty_area(destination, x1, y1, x2, y2);
 
@@ -198,17 +227,6 @@ void common_hal_bitmaptools_fill_region(displayio_bitmap_t *destination,
     }
 }
 
-int16_t constrain(int16_t input, int16_t min, int16_t max) {
-    // constrain the input between the min and max values
-    if (input < min) {
-        return min;
-    }
-    if (input > max) {
-        return max;
-    }
-    return input;
-}
-
 void common_hal_bitmaptools_draw_line(displayio_bitmap_t *destination,
                                  int16_t x0, int16_t y0,
                                  int16_t x1, int16_t y1,