From 14d5abcc2e4345154e8ffe73936b9fd0c9816fd0 Mon Sep 17 00:00:00 2001
From: Philip Stewart <philip.d.stewart@gmail.com>
Date: Sat, 5 Oct 2019 14:34:01 +0000
Subject: [PATCH] fix(gfx/display): Draw partially clipped primitives

Fix two bugs in the display/gfx module:

1. The animation of the simple_menu used in the main menu had the issue
   that there is a black line visible at the top.  This is due the
   gfx_puts method ignoring lines, where the top pixel of the string is
   above the top of the screen.  As gfx_puts uses gfx_setpixel which in
   turn ignores pixels outside of the screen, remove the check in
   gfx_puts.
2. X and Y coordinates were cast to unsigned-ints before being given to
   the gfx-library which means calls like circ(0, -10, 30) would be draw
   at coordinates like [0,65526].  Fix this by changing the data-type of
   all coordinates to signed-integers.

Also remove the x and y ranges from the documentation of the individual
python functions and instead add a general documentation about the
screen and it's size/coordinate system.
---
 Documentation/pycardium/display.rst | 16 ++++++++
 epicardium/epicardium.h             | 58 ++++++++++++++---------------
 epicardium/modules/display.c        | 30 +++++++--------
 lib/gfx/gfx.c                       |  3 --
 pycardium/modules/py/display.py     | 28 +++++++-------
 pycardium/modules/sys_display.c     | 38 +++++++++----------
 6 files changed, 93 insertions(+), 80 deletions(-)

diff --git a/Documentation/pycardium/display.rst b/Documentation/pycardium/display.rst
index 08bd0274c..ab2ee8aed 100644
--- a/Documentation/pycardium/display.rst
+++ b/Documentation/pycardium/display.rst
@@ -1,5 +1,21 @@
 ``display`` - Display
 =====================
 
+The display module let's you draw on the card10's display.
+Pixels are addressed from top left to bottom right with a range of x: 0 to 159 and y: 0 to 79.
+
+.. code-block:: text
+
+   0,0
+      +---------------------+
+      |                     |
+      |                     |
+      |                     |
+      |                     |
+      +---------------------+
+                           159,79
+
+Drawing operations are clipped, pixels outside of the screen will be ignored.
+
 .. automodule:: display
    :members:
diff --git a/epicardium/epicardium.h b/epicardium/epicardium.h
index 63dca8682..13358966d 100644
--- a/epicardium/epicardium.h
+++ b/epicardium/epicardium.h
@@ -1327,8 +1327,8 @@ API(API_DISP_UPDATE, int epic_disp_update());
 /**
  * Prints a string into the display framebuffer
  *
- * :param posx: x position to print to. 0 <= x <= 160
- * :param posy: y position to print to. 0 <= y <= 80
+ * :param posx: x position to print to.
+ * :param posy: y position to print to.
  * :param pString: string to print
  * :param fg: foreground color in rgb565
  * :param bg: background color in rgb565
@@ -1338,8 +1338,8 @@ API(API_DISP_UPDATE, int epic_disp_update());
  */
 API(API_DISP_PRINT,
     int epic_disp_print(
-	    uint16_t posx,
-	    uint16_t posy,
+	    int16_t posx,
+	    int16_t posy,
 	    const char *pString,
 	    uint16_t fg,
 	    uint16_t bg)
@@ -1360,8 +1360,8 @@ enum disp_font_name {
  * Prints a string into the display framebuffer with font type selectable
  *
  * :param fontName: number of font, use FontName enum
- * :param posx: x position to print to. 0 <= x <= 160
- * :param posy: y position to print to. 0 <= y <= 80
+ * :param posx: x position to print to.
+ * :param posy: y position to print to.
  * :param pString: string to print
  * :param fg: foreground color in rgb565
  * :param bg: background color in rgb565, no background is drawn if bg==fg
@@ -1371,8 +1371,8 @@ enum disp_font_name {
  */
 API(API_DISP_PRINT_ADV, int epic_disp_print_adv(
 	uint8_t font,
-	uint16_t posx,
-	uint16_t posy,
+	int16_t posx,
+	int16_t posy,
 	const char *pString,
 	uint16_t fg,
 	uint16_t bg
@@ -1391,24 +1391,24 @@ API(API_DISP_CLEAR, int epic_disp_clear(uint16_t color));
 /**
  * Draws a pixel on the display
  *
- * :param x: x position; 0 <= x <= 160
- * :param y: y position; 0 <= y <= 80
+ * :param x: x position;
+ * :param y: y position;
  * :param color: pixel color in rgb565
  * :return: ``0`` on success or a negative value in case of an error:
  *
  *    - ``-EBUSY``: Display was already locked from another task.
  */
 API(API_DISP_PIXEL, int epic_disp_pixel(
-	uint16_t x, uint16_t y, uint16_t color
+	int16_t x, int16_t y, uint16_t color
 ));
 
 /**
  * Draws a line on the display
  *
- * :param xstart: x starting position; 0 <= x <= 160
- * :param ystart: y starting position; 0 <= y <= 80
- * :param xend: x ending position; 0 <= x <= 160
- * :param yend: y ending position; 0 <= y <= 80
+ * :param xstart: x starting position
+ * :param ystart: y starting position
+ * :param xend: x ending position
+ * :param yend: y ending position
  * :param color: line color in rgb565
  * :param linestyle: 0 for solid, 1 for dottet (almost no visual difference)
  * :param pixelsize: thickness of the line; 1 <= pixelsize <= 8
@@ -1417,10 +1417,10 @@ API(API_DISP_PIXEL, int epic_disp_pixel(
  *    - ``-EBUSY``: Display was already locked from another task.
  */
 API(API_DISP_LINE, int epic_disp_line(
-	uint16_t xstart,
-	uint16_t ystart,
-	uint16_t xend,
-	uint16_t yend,
+	int16_t xstart,
+	int16_t ystart,
+	int16_t xend,
+	int16_t yend,
 	uint16_t color,
 	enum disp_linestyle linestyle,
 	uint16_t pixelsize
@@ -1429,10 +1429,10 @@ API(API_DISP_LINE, int epic_disp_line(
 /**
  * Draws a rectangle on the display
  *
- * :param xstart: x coordinate of top left corner; 0 <= x <= 160
- * :param ystart: y coordinate of top left corner; 0 <= y <= 80
- * :param xend: x coordinate of bottom right corner; 0 <= x <= 160
- * :param yend: y coordinate of bottom right corner; 0 <= y <= 80
+ * :param xstart: x coordinate of top left corner
+ * :param ystart: y coordinate of top left corner
+ * :param xend: x coordinate of bottom right corner
+ * :param yend: y coordinate of bottom right corner
  * :param color: line color in rgb565
  * :param fillstyle: 0 for empty, 1 for filled
  * :param pixelsize: thickness of the rectangle outline; 1 <= pixelsize <= 8
@@ -1441,10 +1441,10 @@ API(API_DISP_LINE, int epic_disp_line(
  *    - ``-EBUSY``: Display was already locked from another task.
  */
 API(API_DISP_RECT, int epic_disp_rect(
-	uint16_t xstart,
-	uint16_t ystart,
-	uint16_t xend,
-	uint16_t yend,
+	int16_t xstart,
+	int16_t ystart,
+	int16_t xend,
+	int16_t yend,
 	uint16_t color,
 	enum disp_fillstyle fillstyle,
 	uint16_t pixelsize
@@ -1464,8 +1464,8 @@ API(API_DISP_RECT, int epic_disp_rect(
  *    - ``-EBUSY``: Display was already locked from another task.
  */
 API(API_DISP_CIRC, int epic_disp_circ(
-	uint16_t x,
-	uint16_t y,
+	int16_t x,
+	int16_t y,
 	uint16_t rad,
 	uint16_t color,
 	enum disp_fillstyle fillstyle,
diff --git a/epicardium/modules/display.c b/epicardium/modules/display.c
index fe89fc324..c3a84812f 100644
--- a/epicardium/modules/display.c
+++ b/epicardium/modules/display.c
@@ -22,8 +22,8 @@ static int check_lock()
 }
 
 int epic_disp_print(
-	uint16_t posx,
-	uint16_t posy,
+	int16_t posx,
+	int16_t posy,
 	const char *pString,
 	uint16_t fg,
 	uint16_t bg
@@ -39,8 +39,8 @@ static const sFONT *font_map[] = {
 
 int epic_disp_print_adv(
 	uint8_t font,
-	uint16_t posx,
-	uint16_t posy,
+	int16_t posx,
+	int16_t posy,
 	const char *pString,
 	uint16_t fg,
 	uint16_t bg
@@ -76,7 +76,7 @@ int epic_disp_clear(uint16_t color)
 	}
 }
 
-int epic_disp_pixel(uint16_t x, uint16_t y, uint16_t color)
+int epic_disp_pixel(int16_t x, int16_t y, uint16_t color)
 {
 	int cl = check_lock();
 	if (cl < 0) {
@@ -88,10 +88,10 @@ int epic_disp_pixel(uint16_t x, uint16_t y, uint16_t color)
 }
 
 int epic_disp_line(
-	uint16_t xstart,
-	uint16_t ystart,
-	uint16_t xend,
-	uint16_t yend,
+	int16_t xstart,
+	int16_t ystart,
+	int16_t xend,
+	int16_t yend,
 	uint16_t color,
 	enum disp_linestyle linestyle,
 	uint16_t pixelsize
@@ -115,10 +115,10 @@ int epic_disp_line(
 }
 
 int epic_disp_rect(
-	uint16_t xstart,
-	uint16_t ystart,
-	uint16_t xend,
-	uint16_t yend,
+	int16_t xstart,
+	int16_t ystart,
+	int16_t xend,
+	int16_t yend,
 	uint16_t color,
 	enum disp_fillstyle fillstyle,
 	uint16_t pixelsize
@@ -154,8 +154,8 @@ int epic_disp_rect(
 }
 
 int epic_disp_circ(
-	uint16_t x,
-	uint16_t y,
+	int16_t x,
+	int16_t y,
 	uint16_t rad,
 	uint16_t color,
 	enum disp_fillstyle fillstyle,
diff --git a/lib/gfx/gfx.c b/lib/gfx/gfx.c
index 8e6c6115c..c076a8343 100644
--- a/lib/gfx/gfx.c
+++ b/lib/gfx/gfx.c
@@ -95,9 +95,6 @@ void gfx_puts(
 			x = 0;
 			y += font->Height;
 		}
-		// if the line is outside the display we return
-		if (y >= r->height)
-			return;
 
 		// now print the character
 		gfx_putchar(font, r, x, y, *str, fg, bg);
diff --git a/pycardium/modules/py/display.py b/pycardium/modules/py/display.py
index d7854aa3a..da5169908 100644
--- a/pycardium/modules/py/display.py
+++ b/pycardium/modules/py/display.py
@@ -75,8 +75,8 @@ class Display:
         :param text: Text to print
         :param fg: Foreground color (expects RGB triple)
         :param bg: Background color (expects RGB triple) or None for transparent background
-        :param posx: X-Position of the first character, 0 <= posx <= 159
-        :param posy: Y-Position of the first character, 0 <= posy <= 79
+        :param posx: X-Position of the first character
+        :param posy: Y-Position of the first character
         :param font: 0 <= font <= 4 (currently) selects a font
 
         Avaiable Fonts:
@@ -110,8 +110,8 @@ class Display:
         """
         Draws a pixel on the display
 
-        :param x: X coordinate, 0<= x <= 159
-        :param y: Y coordinate, 0<= y <= 79
+        :param x: X coordinate
+        :param y: Y coordinate
         :param col: color of the pixel (expects RGB tripple)
         """
 
@@ -134,10 +134,10 @@ class Display:
         """
         Draws a line on the display.
 
-        :param xs: X start coordinate, 0 <= xs <= 159
-        :param ys: Y start coordinate, 0 <= ys <= 79
-        :param xe: X end coordinate, 0 <= xe <= 159
-        :param ye: Y end coordinate, 0 <= ye <= 79
+        :param xs: X start coordinate
+        :param ys: Y start coordinate
+        :param xe: X end coordinate
+        :param ye: Y end coordinate
         :param col: color of the line (expects RGB triple)
         :param dotted: whether the line should be dotted or not
            (questionable implementation: draws every other pixel white, draws
@@ -154,10 +154,10 @@ class Display:
         """
         Draws a rectangle on the display.
 
-        :param xs: X start coordinate, 0 <= xs <= 159
-        :param ys: Y start coordinate, 0 <= ys <= 79
-        :param xe: X end coordinate, 0 <= xe <= 159
-        :param ye: Y end coordinate, 0 <= ye <= 79
+        :param xs: X start coordinate
+        :param ys: Y start coordinate
+        :param xe: X end coordinate
+        :param ye: Y end coordinate
         :param col: color of the outline and fill (expects RGB triple)
         :param filled: whether the rectangle should be filled or not
         :param size: size of the individual pixels, ranges from 1 to 8
@@ -172,8 +172,8 @@ class Display:
         """
         Draws a circle on the display.
 
-        :param x: center x coordinate, 0 <= x <= 159
-        :param y: center y coordinate, 0 <= y <= 79
+        :param x: center x coordinate
+        :param y: center y coordinate
         :param rad: radius
         :param col: color of the outline and fill (expects RGB triple)
         :param filled: whether the rectangle should be filled or not
diff --git a/pycardium/modules/sys_display.c b/pycardium/modules/sys_display.c
index 4f92ab92d..d03849544 100644
--- a/pycardium/modules/sys_display.c
+++ b/pycardium/modules/sys_display.c
@@ -39,11 +39,11 @@ static mp_obj_t mp_display_print(size_t n_args, const mp_obj_t *args)
 		mp_raise_TypeError("input text must be a string");
 	}
 	GET_STR_DATA_LEN(args[0], print, print_len);
-	uint32_t posx = mp_obj_get_int(args[1]);
-	uint32_t posy = mp_obj_get_int(args[2]);
-	uint32_t fg   = get_color(args[3]);
-	uint32_t bg   = get_color(args[4]);
-	int res = epic_disp_print(posx, posy, (const char *)print, fg, bg);
+	int32_t posx = mp_obj_get_int(args[1]);
+	int32_t posy = mp_obj_get_int(args[2]);
+	uint32_t fg  = get_color(args[3]);
+	uint32_t bg  = get_color(args[4]);
+	int res      = epic_disp_print(posx, posy, (const char *)print, fg, bg);
 	if (res < 0) {
 		mp_raise_OSError(-res);
 	}
@@ -60,8 +60,8 @@ static mp_obj_t mp_display_print_adv(size_t n_args, const mp_obj_t *args)
 		mp_raise_TypeError("input text must be a string");
 	}
 	GET_STR_DATA_LEN(args[0], print, print_len);
-	uint32_t posx    = mp_obj_get_int(args[1]);
-	uint32_t posy    = mp_obj_get_int(args[2]);
+	int32_t posx     = mp_obj_get_int(args[1]);
+	int32_t posy     = mp_obj_get_int(args[2]);
 	uint32_t fg      = get_color(args[3]);
 	uint32_t bg      = get_color(args[4]);
 	uint8_t fontName = mp_obj_get_int(args[5]);
@@ -80,8 +80,8 @@ static MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(
 /* draw pixel on the display */
 static mp_obj_t mp_display_pixel(size_t n_args, const mp_obj_t *args)
 {
-	uint16_t x   = mp_obj_get_int(args[0]);
-	uint16_t y   = mp_obj_get_int(args[1]);
+	int16_t x    = mp_obj_get_int(args[0]);
+	int16_t y    = mp_obj_get_int(args[1]);
 	uint16_t col = get_color(args[2]);
 
 	//TODO: Move sanity checks to epicardium
@@ -121,10 +121,10 @@ static MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(
 /* draw line on the display */
 static mp_obj_t mp_display_line(size_t n_args, const mp_obj_t *args)
 {
-	uint16_t xs  = mp_obj_get_int(args[0]);
-	uint16_t ys  = mp_obj_get_int(args[1]);
-	uint16_t xe  = mp_obj_get_int(args[2]);
-	uint16_t ye  = mp_obj_get_int(args[3]);
+	int16_t xs   = mp_obj_get_int(args[0]);
+	int16_t ys   = mp_obj_get_int(args[1]);
+	int16_t xe   = mp_obj_get_int(args[2]);
+	int16_t ye   = mp_obj_get_int(args[3]);
 	uint16_t col = get_color(args[4]);
 	uint16_t ls  = mp_obj_get_int(args[5]);
 	uint16_t ps  = mp_obj_get_int(args[6]);
@@ -155,10 +155,10 @@ static MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(
 /* draw rectangle on the display */
 static mp_obj_t mp_display_rect(size_t n_args, const mp_obj_t *args)
 {
-	uint16_t xs  = mp_obj_get_int(args[0]);
-	uint16_t ys  = mp_obj_get_int(args[1]);
-	uint16_t xe  = mp_obj_get_int(args[2]);
-	uint16_t ye  = mp_obj_get_int(args[3]);
+	int16_t xs   = mp_obj_get_int(args[0]);
+	int16_t ys   = mp_obj_get_int(args[1]);
+	int16_t xe   = mp_obj_get_int(args[2]);
+	int16_t ye   = mp_obj_get_int(args[3]);
 	uint16_t col = get_color(args[4]);
 	uint16_t fs  = mp_obj_get_int(args[5]);
 	uint16_t ps  = mp_obj_get_int(args[6]);
@@ -189,8 +189,8 @@ static MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(
 /* draw rectangle on the display */
 static mp_obj_t mp_display_circ(size_t n_args, const mp_obj_t *args)
 {
-	uint16_t x   = mp_obj_get_int(args[0]);
-	uint16_t y   = mp_obj_get_int(args[1]);
+	int16_t x    = mp_obj_get_int(args[0]);
+	int16_t y    = mp_obj_get_int(args[1]);
 	uint16_t rad = mp_obj_get_int(args[2]);
 	uint16_t col = get_color(args[3]);
 	uint16_t fs  = mp_obj_get_int(args[4]);
-- 
GitLab