Rework of changes to bounds checking of location in VectorShape, moving most of the code into shared-module.

lesamouraipourpre · lesamouraipourpre · commit 2bc260a102fd · 2021-09-10T21:48:01.000+01:00
diff --git a/shared-bindings/vectorio/VectorShape.c b/shared-bindings/vectorio/VectorShape.c
@@ -99,10 +99,7 @@ STATIC mp_obj_t vectorio_vector_shape_obj_set_x(mp_obj_t wrapper_shape, mp_obj_t
     vectorio_vector_shape_t *self = MP_OBJ_TO_PTR(draw_protocol->draw_get_protocol_self(wrapper_shape));
 
     mp_int_t x = mp_obj_get_int(x_obj);
-    bool dirty = common_hal_vectorio_vector_shape_set_x(self, x);
-    if (dirty) {
-        common_hal_vectorio_vector_shape_set_dirty(self);
-    }
+    common_hal_vectorio_vector_shape_set_x(self, x);
     return mp_const_none;
 }
 MP_DEFINE_CONST_FUN_OBJ_2(vectorio_vector_shape_set_x_obj, vectorio_vector_shape_obj_set_x);
@@ -133,10 +130,7 @@ STATIC mp_obj_t vectorio_vector_shape_obj_set_y(mp_obj_t wrapper_shape, mp_obj_t
     vectorio_vector_shape_t *self = MP_OBJ_TO_PTR(draw_protocol->draw_get_protocol_self(wrapper_shape));
 
     mp_int_t y = mp_obj_get_int(y_obj);
-    bool dirty = common_hal_vectorio_vector_shape_set_y(self, y);
-    if (dirty) {
-        common_hal_vectorio_vector_shape_set_dirty(self);
-    }
+    common_hal_vectorio_vector_shape_set_y(self, y);
     return mp_const_none;
 }
 MP_DEFINE_CONST_FUN_OBJ_2(vectorio_vector_shape_set_y_obj, vectorio_vector_shape_obj_set_y);
diff --git a/shared-bindings/vectorio/VectorShape.h b/shared-bindings/vectorio/VectorShape.h
@@ -21,13 +21,13 @@ void common_hal_vectorio_vector_shape_construct(vectorio_vector_shape_t *self,
 void common_hal_vectorio_vector_shape_set_dirty(void *self);
 
 mp_int_t common_hal_vectorio_vector_shape_get_x(vectorio_vector_shape_t *self);
-bool common_hal_vectorio_vector_shape_set_x(vectorio_vector_shape_t *self, mp_int_t x);
+void common_hal_vectorio_vector_shape_set_x(vectorio_vector_shape_t *self, mp_int_t x);
 
 mp_obj_tuple_t *common_hal_vectorio_vector_shape_get_location(vectorio_vector_shape_t *self);
 void common_hal_vectorio_vector_shape_set_location(vectorio_vector_shape_t *self, mp_obj_t xy);
 
 mp_int_t common_hal_vectorio_vector_shape_get_y(vectorio_vector_shape_t *self);
-bool common_hal_vectorio_vector_shape_set_y(vectorio_vector_shape_t *self, mp_int_t y);
+void common_hal_vectorio_vector_shape_set_y(vectorio_vector_shape_t *self, mp_int_t y);
 
 mp_obj_t common_hal_vectorio_vector_shape_get_pixel_shader(vectorio_vector_shape_t *self);
 void common_hal_vectorio_vector_shape_set_pixel_shader(vectorio_vector_shape_t *self, mp_obj_t pixel_shader);
diff --git a/shared-module/vectorio/VectorShape.c b/shared-module/vectorio/VectorShape.c
@@ -166,8 +166,10 @@ void common_hal_vectorio_vector_shape_construct(vectorio_vector_shape_t *self,
     vectorio_ishape_t ishape,
     mp_obj_t pixel_shader, int32_t x, int32_t y) {
     VECTORIO_SHAPE_DEBUG("%p vector_shape_construct x:%3d, y:%3d\n", self, x, y);
-    common_hal_vectorio_vector_shape_set_x(self, x);
-    common_hal_vectorio_vector_shape_set_y(self, y);
+    vectorio_vector_shape_validate_x_bounds(x);
+    self->x = x;
+    vectorio_vector_shape_validate_y_bounds(y);
+    self->y = y;
     self->pixel_shader = pixel_shader;
     self->ishape = ishape;
     self->absolute_transform = &null_transform; // Critical to have a valid transform before getting screen area.
@@ -184,16 +186,14 @@ mp_int_t common_hal_vectorio_vector_shape_get_x(vectorio_vector_shape_t *self) {
 }
 
 
-bool common_hal_vectorio_vector_shape_set_x(vectorio_vector_shape_t *self, mp_int_t x) {
+void common_hal_vectorio_vector_shape_set_x(vectorio_vector_shape_t *self, mp_int_t x) {
     VECTORIO_SHAPE_DEBUG("%p set_x %d\n", self, x);
     if (self->x == x) {
-        return false;  // it's not dirty
-    }
-    if (x < SHRT_MIN || x > SHRT_MAX) {
-        mp_raise_ValueError_varg(translate("unsupported %q type"), MP_QSTR_point);
+        return;
     }
+    vectorio_vector_shape_validate_x_bounds(x);
     self->x = x;
-    return true;  // it's dirty
+    common_hal_vectorio_vector_shape_set_dirty(self);
 }
 
 
@@ -203,16 +203,14 @@ mp_int_t common_hal_vectorio_vector_shape_get_y(vectorio_vector_shape_t *self) {
 }
 
 
-bool common_hal_vectorio_vector_shape_set_y(vectorio_vector_shape_t *self, mp_int_t y) {
+void common_hal_vectorio_vector_shape_set_y(vectorio_vector_shape_t *self, mp_int_t y) {
     VECTORIO_SHAPE_DEBUG("%p set_y %d\n", self, y);
     if (self->y == y) {
-        return false;  // it's not dirty
-    }
-    if (y < SHRT_MIN || y > SHRT_MAX) {
-        mp_raise_ValueError_varg(translate("unsupported %q type"), MP_QSTR_point);
+        return;
     }
+    vectorio_vector_shape_validate_y_bounds(y);
     self->y = y;
-    return true;  // it's dirty
+    common_hal_vectorio_vector_shape_set_dirty(self);
 }
 
 mp_obj_tuple_t *common_hal_vectorio_vector_shape_get_location(vectorio_vector_shape_t *self) {
@@ -239,14 +237,37 @@ void common_hal_vectorio_vector_shape_set_location(vectorio_vector_shape_t *self
         || !mp_obj_get_int_maybe(tuple_items[ 1 ], &y)) {
         mp_raise_ValueError_varg(translate("unsupported %q type"), MP_QSTR_point);
     }
-    bool dirty = common_hal_vectorio_vector_shape_set_x(self, x);
-    dirty |= common_hal_vectorio_vector_shape_set_y(self, y);
+    bool dirty = false;
+    if (self->x != x) {
+        vectorio_vector_shape_validate_x_bounds(x);
+        self->x = x;
+        dirty = true;
+    }
+    if (self->y != y) {
+        vectorio_vector_shape_validate_y_bounds(y);
+        self->y = y;
+        dirty = true;
+    }
     if (dirty) {
         common_hal_vectorio_vector_shape_set_dirty(self);
     }
 }
 
 
+void vectorio_vector_shape_validate_x_bounds(mp_int_t x) {
+    if (x < SHRT_MIN || x > SHRT_MAX) {
+        mp_raise_ValueError_varg(translate("%q must be between %d and %d"), MP_QSTR_x, SHRT_MIN, SHRT_MAX);
+    }
+}
+
+
+void vectorio_vector_shape_validate_y_bounds(mp_int_t y) {
+    if (y < SHRT_MIN || y > SHRT_MAX) {
+        mp_raise_ValueError_varg(translate("%q must be between %d and %d"), MP_QSTR_y, SHRT_MIN, SHRT_MAX);
+    }
+}
+
+
 mp_obj_t common_hal_vectorio_vector_shape_get_pixel_shader(vectorio_vector_shape_t *self) {
     VECTORIO_SHAPE_DEBUG("%p get_pixel_shader\n", self);
     return self->pixel_shader;
diff --git a/shared-module/vectorio/VectorShape.h b/shared-module/vectorio/VectorShape.h
@@ -51,4 +51,7 @@ bool vectorio_vector_shape_fill_area(vectorio_vector_shape_t *self, const _displ
 bool vectorio_vector_shape_get_previous_area(vectorio_vector_shape_t *self, displayio_area_t *out_area);
 void vectorio_vector_shape_finish_refresh(vectorio_vector_shape_t *self);
 
+void vectorio_vector_shape_validate_x_bounds(mp_int_t x);
+void vectorio_vector_shape_validate_y_bounds(mp_int_t y);
+
 #endif // MICROPY_INCLUDED_SHARED_MODULE_VECTORIO_SHAPE_H
