From 16ffc731f3c64b36a486a27d87b10742cf5bb4ba Mon Sep 17 00:00:00 2001
From: George Waters <gwatersdev@gmail.com>
Date: Thu, 21 May 2020 21:57:45 -0400
Subject: [PATCH] Implement negative step for pixelbuf slices

---
 shared-bindings/_pixelbuf/PixelBuf.c | 27 +++++++++++++--------------
 shared-bindings/_pixelbuf/PixelBuf.h |  2 +-
 shared-module/_pixelbuf/PixelBuf.c   | 15 +++++++++++----
 3 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/shared-bindings/_pixelbuf/PixelBuf.c b/shared-bindings/_pixelbuf/PixelBuf.c
index d8f1810db8..88f3fd684a 100644
--- a/shared-bindings/_pixelbuf/PixelBuf.c
+++ b/shared-bindings/_pixelbuf/PixelBuf.c
@@ -300,17 +300,20 @@ STATIC mp_obj_t pixelbuf_pixelbuf_subscr(mp_obj_t self_in, mp_obj_t index_in, mp
         size_t length = common_hal__pixelbuf_pixelbuf_get_len(self_in);
         mp_seq_get_fast_slice_indexes(length, index_in, &slice);
 
-        if (slice.step < 0) {
-            mp_raise_IndexError(translate("Negative step not supported"));
+        size_t slice_len;
+        if (slice.step > 0) {
+            slice_len = slice.stop - slice.start;
+        }else{
+            slice_len = 1 + slice.start - slice.stop;
+        }
+        if (slice.step > 1 || slice.step < -1) {
+            size_t step = slice.step > 0 ? slice.step : slice.step * -1;
+            slice_len = (slice_len / step) + (slice_len % step ? 1 : 0);
         }
 
         if (value == MP_OBJ_SENTINEL) { // Get
-            size_t len = slice.stop - slice.start;
-            if (slice.step > 1) {
-                len = (len / slice.step) + (len % slice.step ? 1 : 0);
-            }
-            mp_obj_tuple_t* t = MP_OBJ_TO_PTR(mp_obj_new_tuple(len, NULL));
-            for (uint i = 0; i < len; i++) {
+            mp_obj_tuple_t* t = MP_OBJ_TO_PTR(mp_obj_new_tuple(slice_len, NULL));
+            for (uint i = 0; i < slice_len; i++) {
                 t->items[i] = common_hal__pixelbuf_pixelbuf_get_pixel(self_in, i * slice.step + slice.start);
             }
             return MP_OBJ_FROM_PTR(t);
@@ -321,10 +324,6 @@ STATIC mp_obj_t pixelbuf_pixelbuf_subscr(mp_obj_t self_in, mp_obj_t index_in, mp
                 mp_raise_ValueError(translate("tuple/list required on RHS"));
             }
 
-            size_t dst_len = (slice.stop - slice.start);
-            if (slice.step > 1) {
-                dst_len = (dst_len / slice.step) + (dst_len % slice.step ? 1 : 0);
-            }
             mp_obj_t *src_objs;
             size_t num_items;
             if (MP_OBJ_IS_TYPE(value, &mp_type_list)) {
@@ -336,9 +335,9 @@ STATIC mp_obj_t pixelbuf_pixelbuf_subscr(mp_obj_t self_in, mp_obj_t index_in, mp
                 num_items = l->len;
                 src_objs = l->items;
             }
-            if (num_items != dst_len) {
+            if (num_items != slice_len) {
                 mp_raise_ValueError_varg(translate("Unmatched number of items on RHS (expected %d, got %d)."),
-                                                   dst_len, num_items);
+                                                   slice_len, num_items);
             }
 
             common_hal__pixelbuf_pixelbuf_set_pixels(self_in, slice.start, slice.stop, slice.step, src_objs);
diff --git a/shared-bindings/_pixelbuf/PixelBuf.h b/shared-bindings/_pixelbuf/PixelBuf.h
index 68d6d4eefc..d058f43768 100644
--- a/shared-bindings/_pixelbuf/PixelBuf.h
+++ b/shared-bindings/_pixelbuf/PixelBuf.h
@@ -47,6 +47,6 @@ void common_hal__pixelbuf_pixelbuf_fill(mp_obj_t self, mp_obj_t item);
 void common_hal__pixelbuf_pixelbuf_show(mp_obj_t self);
 mp_obj_t common_hal__pixelbuf_pixelbuf_get_pixel(mp_obj_t self, size_t index);
 void common_hal__pixelbuf_pixelbuf_set_pixel(mp_obj_t self, size_t index, mp_obj_t item);
-void common_hal__pixelbuf_pixelbuf_set_pixels(mp_obj_t self_in, size_t start, size_t stop, size_t step, mp_obj_t* values);
+void common_hal__pixelbuf_pixelbuf_set_pixels(mp_obj_t self_in, size_t start, size_t stop, mp_int_t step, mp_obj_t* values);
 
 #endif  // CP_SHARED_BINDINGS_PIXELBUF_PIXELBUF_H
diff --git a/shared-module/_pixelbuf/PixelBuf.c b/shared-module/_pixelbuf/PixelBuf.c
index 31350b875c..c32544483c 100644
--- a/shared-module/_pixelbuf/PixelBuf.c
+++ b/shared-module/_pixelbuf/PixelBuf.c
@@ -216,12 +216,19 @@ void _pixelbuf_set_pixel(pixelbuf_pixelbuf_obj_t* self, size_t index, mp_obj_t v
     _pixelbuf_set_pixel_color(self, index, r, g, b, w);
 }
 
-void common_hal__pixelbuf_pixelbuf_set_pixels(mp_obj_t self_in, size_t start, size_t stop, size_t step, mp_obj_t* values) {
+void common_hal__pixelbuf_pixelbuf_set_pixels(mp_obj_t self_in, size_t start, size_t stop, mp_int_t step, mp_obj_t* values) {
     pixelbuf_pixelbuf_obj_t* self = native_pixelbuf(self_in);
     size_t source_i = 0;
-    for (size_t target_i = start; target_i < stop; target_i += step) {
-        _pixelbuf_set_pixel(self, target_i, values[source_i]);
-        source_i++;
+    if (step > 0) {
+        for (size_t target_i = start; target_i < stop; target_i += step) {
+            _pixelbuf_set_pixel(self, target_i, values[source_i]);
+            source_i++;
+        }
+    }else{
+        for (size_t target_i = start; target_i >= stop; target_i += step) {
+            _pixelbuf_set_pixel(self, target_i, values[source_i]);
+            source_i++;
+        }
     }
     if (self->auto_write) {
         common_hal__pixelbuf_pixelbuf_show(self_in);