[PATCH fbtest 00/17] Export feature and large ellipses support

[PATCH fbtest 00/17] Export feature and large ellipses support

[Geert Uytterhoeven]

	Hi all,

Recently, I ran into a segmentation fault when running test002 ("Draw a
grid and some ellipses") on a system with a display resolution of
2560x1440.  This turned out to be due to a 32-bit overflow in the
ellipse drawing routines.

This patch series:
  - Adds support for operating on a virtual buffer in RAM instead of on
    a real frame buffer device, and exporting the result as a PPM image,
    which is useful for e.g. testing drawing algorithms on screen sizes
    not supported by your hardware,
  - Fixes the drawing of ellipses on large displays,
  - Contains some small fixe and improvements.

I intend to apply these to
https://git.kernel.org/pub/scm/linux/kernel/git/geert/fbtest.git/.

Thanks for your comments!

Geert Uytterhoeven (17):
  Add support for exporting virtual test images
  tests: Print test description in debug mode
  Test002: Fix test description
  drawops: Extract do_circle()
  drawops: Use "y == 0" in draw_ellipse_points()
  drawops: Remove always-false check in generic_fill_ellipse()
  drawops: Refactor generic_draw_ellipse()
  drawops: Return early in generic_{draw,fill}_ellipse()
  drawops: Extract do_ellipse()
  drawops: Make de in do_circle() unsigned
  drawops: Make dT1 and dS1 in do_ellipse() unsigned
  drawops: Fix crash when drawing large ellipses
  tests: Clear frame buffer before each test
  Make variables that are never negative unsigned
  test013: Fix off-by-one error in maximum circle calculation
  visops: Mark fall-through switch case
  util: Use __attribute__

 console.c            |   2 +-
 drawops/generic.c    | 427 ++++++++++++++++++++++---------------------
 export.c             | 176 ++++++++++++++++++
 fb.c                 |  11 +-
 include/export.h     |  14 ++
 include/fb.h         |   1 +
 include/types.h      |   3 +
 include/util.h       |   3 +-
 main.c               |  33 +++-
 pixmap.c             |   6 +-
 tests.c              |   7 +-
 tests/test002.c      |   2 +-
 tests/test004.c      |   2 +-
 tests/test006.c      |   2 +-
 tests/test007.c      |   2 +-
 tests/test009.c      |   2 +-
 tests/test010.c      |   2 +-
 tests/test013.c      |   4 +-
 util.c               |   6 +
 visops/pseudocolor.c |   1 +
 visual.c             |   2 +-
 21 files changed, 476 insertions(+), 232 deletions(-)
 create mode 100644 export.c
 create mode 100644 include/export.h

-- 
2.34.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH fbtest 01/17] Add support for exporting virtual test images

[Geert Uytterhoeven]

Add support for operating on a virtual buffer in RAM instead of on a
real frame buffer device, and exporting the result as a PPM image.
The size of the virtual buffer is configurable, but for now the format
is fixed to Truecolor 8:8:8:0.

This is useful for e.g. testing drawing algorithms on screen sizes not
supported by your hardware.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 export.c         | 176 +++++++++++++++++++++++++++++++++++++++++++++++
 fb.c             |   9 +++
 include/export.h |  14 ++++
 include/util.h   |   1 +
 main.c           |  33 ++++++---
 tests.c          |   3 +
 util.c           |   6 ++
 7 files changed, 233 insertions(+), 9 deletions(-)
 create mode 100644 export.c
 create mode 100644 include/export.h

diff --git a/export.c b/export.c
new file mode 100644
index 0000000000000000..ddf24518cadceaf3
--- /dev/null
+++ b/export.c
@@ -0,0 +1,176 @@
+
+/*
+ *  Virtual Frame Buffer Export
+ *
+ *  (C) Copyright 2024 Glider bv
+ *
+ *  This file is subject to the terms and conditions of the GNU General Public
+ *  License. See the file COPYING in the main directory of this archive for
+ *  more details.
+ */
+
+#define _GNU_SOURCE
+
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "types.h"
+#include "export.h"
+#include "fb.h"
+#include "util.h"
+
+
+static const char *export_prefix;
+static unsigned int export_xres = DEFAULT_EXPORT_XRES;
+static unsigned int export_yres = DEFAULT_EXPORT_YRES;
+
+static void export_setup(void)
+{
+    const char *param = Opt_Export;
+    char *end;
+
+    /* Prefix */
+    end = strchr(param, ',');
+    if (end)
+	    *end++ = '\0';
+
+    export_prefix = param;
+    Debug("export_prefix = %s\n", export_prefix);
+
+    /* Optional horizontal resolution */
+    param = end;
+    if (!param)
+	    return;
+
+    end = strchr(param, ',');
+    if (end)
+	    *end++ = '\0';
+
+    export_xres = atoi(param);
+    Debug("export_xres = %u\n", export_xres);
+
+    /* Optional vertical resolution */
+    param = end;
+    if (!param)
+	    return;
+
+    end = strchr(param, ',');
+    if (end)
+	    *end++ = '\0';
+
+    export_yres = atoi(param);
+    Debug("export_yres = %u\n", export_yres);
+}
+
+void export_init(void)
+{
+    export_setup();
+
+    fb_var.xres = export_xres;
+    fb_var.yres = export_yres;
+    fb_var.xres_virtual = export_xres;
+    fb_var.yres_virtual = export_yres;
+    fb_var.xoffset = 0;
+    fb_var.yoffset = 0;
+    fb_var.bits_per_pixel = 32;
+    fb_var.grayscale = 0;
+    fb_var.red.offset = 16;
+    fb_var.red.length = 8;
+    fb_var.red.msb_right = 0;
+    fb_var.green.offset = 8;
+    fb_var.green.length = 8;
+    fb_var.green.msb_right = 0;
+    fb_var.blue.offset = 0;
+    fb_var.blue.length = 8;
+    fb_var.blue.msb_right = 0;
+    fb_var.transp.offset = 0;
+    fb_var.transp.length = 0;
+    fb_var.transp.msb_right = 0;
+    fb_var.nonstd = 0;
+    fb_var.activate = 0;
+    fb_var.height = fb_var.xres / 4;
+    fb_var.width = fb_var.yres / 4;
+    fb_var.accel_flags = 0;
+    fb_var.pixclock = 0;
+    fb_var.left_margin = 0;
+    fb_var.right_margin = 0;
+    fb_var.upper_margin = 0;
+    fb_var.lower_margin = 0;
+    fb_var.hsync_len = 0;
+    fb_var.vsync_len = 0;
+    fb_var.sync = 0;
+    fb_var.vmode = FB_VMODE_NONINTERLACED;
+    fb_var.rotate = 0;
+    fb_var.colorspace = 0;
+    fb_var.reserved[0] = 0;
+    fb_var.reserved[1] = 0;
+    fb_var.reserved[2] = 0;
+    fb_var.reserved[3] = 0;
+
+    strcpy(fb_fix.id, "fbtest");
+    fb_fix.smem_start = 0;
+    fb_fix.smem_len = fb_var.xres * fb_var.yres * 4;
+    fb_fix.type = FB_TYPE_PACKED_PIXELS;
+    fb_fix.type_aux = 0;
+    fb_fix.visual = FB_VISUAL_TRUECOLOR;
+    fb_fix.xpanstep = 0;
+    fb_fix.ypanstep = 0;
+    fb_fix.ywrapstep = 0;
+    fb_fix.line_length = fb_var.xres * 4;
+    fb_fix.mmio_start = 0;
+    fb_fix.mmio_len = 0;
+    fb_fix.accel = FB_ACCEL_NONE;
+    fb_fix.capabilities = 0;
+    fb_fix.reserved[0] = 0;
+    fb_fix.reserved[1] = 0;
+
+    fb = malloc(fb_fix.smem_len);
+    if (!fb)
+	Fatal("malloc %u: %s\n", fb_fix.smem_len, strerror(errno));
+}
+
+void export_fb(const char *name)
+{
+    unsigned int x, y;
+    u32 *src, pixel;
+    char *filename;
+    u8 *line, *dst;
+    FILE *stream;
+    int res;
+
+    res = asprintf(&filename, "%s%s.ppm", export_prefix, name);
+    if (res < 0)
+	Fatal("asprintf: %s\n", strerror(errno));
+
+    line = malloc(fb_var.xres * 3);
+    if (!line)
+	Fatal("malloc %u: %s\n", fb_var.xres * 3, strerror(errno));
+
+    Debug("Exporting to %s\n", filename);
+    stream = fopen(filename, "w");
+    if (!stream)
+	Fatal("fopen %s: %s\n", filename, strerror(errno));
+
+    fputs("P6\n", stream);
+    fprintf(stream, "%u %u 255\n", fb_var.xres, fb_var.yres);
+
+    src = (u32 *)fb;
+    for (y = 0; y < fb_var.yres; y++) {
+	dst = line;
+	for (x = 0; x < fb_var.xres; x++) {
+	    pixel = *src++;
+	    *dst++ = (pixel >> fb_var.red.offset) & 0xff;
+	    *dst++ = (pixel >> fb_var.green.offset) & 0xff;
+	    *dst++ = (pixel >> fb_var.blue.offset) & 0xff;
+	}
+	res = fwrite(line, 3, fb_var.xres, stream);
+	if (res < fb_var.xres)
+	    Fatal("fwrite: %s\n", strerror(errno));
+    }
+
+    fclose(stream);
+    free(line);
+    free(filename);
+}
diff --git a/fb.c b/fb.c
index c22bdece6dfeb654..ab351d15c82e09d9 100644
--- a/fb.c
+++ b/fb.c
@@ -20,6 +20,7 @@
 #include <unistd.h>
 
 #include "types.h"
+#include "export.h"
 #include "fb.h"
 #include "util.h"
 #include "colormap.h"
@@ -429,6 +430,11 @@ static void var_fix_validate(void)
 
 void fb_init(void)
 {
+    if (Opt_Export) {
+	export_init();
+	return;
+    }
+
     Debug("fb_init()\n");
     fb_open();
     fb_get_var();
@@ -490,6 +496,9 @@ void fb_init(void)
 
 void fb_cleanup(void)
 {
+    if (Opt_Export)
+	return;
+
     Debug("fb_cleanup()\n");
     if (saved_fb)
 	fb_restore();
diff --git a/include/export.h b/include/export.h
new file mode 100644
index 0000000000000000..e3ef18b7b8e3ec14
--- /dev/null
+++ b/include/export.h
@@ -0,0 +1,14 @@
+
+/*
+ *  (C) Copyright 2024 Glider bv
+ *
+ *  This file is subject to the terms and conditions of the GNU General Public
+ *  License. See the file COPYING in the main directory of this archive for
+ *  more details.
+ */
+
+#define DEFAULT_EXPORT_XRES	640
+#define DEFAULT_EXPORT_YRES	480
+
+extern void export_init(void);
+extern void export_fb(const char *name);
diff --git a/include/util.h b/include/util.h
index 64d810448d5779e4..5a6b5ab40d383d5a 100644
--- a/include/util.h
+++ b/include/util.h
@@ -103,6 +103,7 @@ extern double benchmark(void (*func)(unsigned long n, void *data), void *data);
      *   Command line options
      */
 
+extern const char *Opt_Export;
 extern const char *Opt_Fbdev;
 extern int Opt_Debug;
 extern int Opt_List;
diff --git a/main.c b/main.c
index a13c80078543c7d1..9887fbd6936ecc1c 100644
--- a/main.c
+++ b/main.c
@@ -16,6 +16,7 @@
 
 #include "types.h"
 #include "util.h"
+#include "export.h"
 #include "fb.h"
 #include "drawops.h"
 #include "visual.h"
@@ -28,6 +29,7 @@
 const char *ProgramName;
 
 const char *Opt_Fbdev = DEFAULT_FBDEV;
+const char *Opt_Export;
 int Opt_Debug = 0;
 int Opt_List = 0;
 int Opt_Quiet = 0;
@@ -42,16 +44,21 @@ static void Usage(void) __attribute__ ((noreturn));
 
 static void Usage(void)
 {
-    printf("%s: [options] [test ...]\n\n"
+    printf("%s: [options] [<test> ...]\n\n"
 	   "Valid options are:\n"
-	   "    -h, --help       Display this usage information\n"
-	   "    -f, --fbdev dev  Specify frame buffer device (default: %s)\n"
-	   "    -d, --debug      Enable debug mode\n"
-	   "    -l, --list       List tests only, don't run them\n"
-	   "    -q, --quiet      Suppress messages\n"
-	   "    -v, --verbose    Enable verbose mode\n"
+	   "    -h, --help         Display this usage information\n"
+	   "    -e, --export <prefix>[,<xres>[,<yres>]]\n"
+	   "                       Do not use a frame buffer device, but operate on a\n"
+	   "                       virtual buffer in RAM, and export the result as a PPM\n"
+	   "                       image in <prefix><test>.ppm (default size: %ux%u)\n"
+	   "    -f, --fbdev <dev>  Specify frame buffer device (default: %s)\n"
+	   "    -d, --debug        Enable debug mode\n"
+	   "    -l, --list         List tests only, don't run them\n"
+	   "    -q, --quiet        Suppress messages\n"
+	   "    -v, --verbose      Enable verbose mode\n"
 	   "\n",
-	   ProgramName, DEFAULT_FBDEV);
+	   ProgramName, DEFAULT_EXPORT_XRES, DEFAULT_EXPORT_YRES,
+	   DEFAULT_FBDEV);
     exit(1);
 }
 
@@ -78,7 +85,15 @@ int main(int argc, char *argv[])
     while (argc > 1 && argv[1][0] == '-') {
 	if (!strcmp(argv[1], "-h") || !strcmp(argv[1], "--help"))
 	    Usage();
-	else if (!strcmp(argv[1], "-f") || !strcmp(argv[1], "--fbdev")) {
+	else if (!strcmp(argv[1], "-e") || !strcmp(argv[1], "--export")) {
+	    if (argc <= 2)
+		Usage();
+	    else {
+		Opt_Export = argv[2];
+		argv += 2;
+		argc -= 2;
+	    }
+	} else if (!strcmp(argv[1], "-f") || !strcmp(argv[1], "--fbdev")) {
 	    if (argc <= 2)
 		Usage();
 	    else {
diff --git a/tests.c b/tests.c
index 6f6f818ac4ade8b3..c92236f869f41594 100644
--- a/tests.c
+++ b/tests.c
@@ -12,6 +12,7 @@
 #include <stdio.h>
 #include <string.h>
 
+#include "export.h"
 #include "types.h"
 #include "fb.h"
 #include "visual.h"
@@ -81,6 +82,8 @@ static void run_one_test(const struct test *test)
     switch (res) {
 	case TEST_OK:
 	    Message("%s: PASSED\n", test->name);
+	    if (Opt_Export)
+		export_fb(test->name);
 	    break;
 
 	case TEST_FAIL:
diff --git a/util.c b/util.c
index f199ace3501b0362..a946c12091f608f0 100644
--- a/util.c
+++ b/util.c
@@ -125,6 +125,9 @@ void Fatal(const char *fmt, ...)
 
 void wait_for_key(int timeout)
 {
+    if (Opt_Export)
+	    return;
+
     /* FIXME: no keypress handling yet */
     sleep(2);
 }
@@ -138,6 +141,9 @@ void wait_ms(int ms)
 {
     struct timespec req;
 
+    if (Opt_Export)
+	    return;
+
     req.tv_sec = ms/1000;
     req.tv_nsec = (ms % 1000)*1000000;
     nanosleep(&req, NULL);
-- 
2.34.1

[PATCH fbtest 02/17] tests: Print test description in debug mode

[Geert Uytterhoeven]

Print not only the test name, but also the test description.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 tests.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests.c b/tests.c
index c92236f869f41594..b1573032372e8090 100644
--- a/tests.c
+++ b/tests.c
@@ -55,7 +55,7 @@ static void run_one_test(const struct test *test)
 {
     enum test_res res;
 
-    Debug("Running test %s\n", test->name);
+    Debug("Running test %s (%s)\n", test->name, test->desc);
 
     if (test->visual != VISUAL_NONE && !visual_set(test->visual)) {
 	Debug("Visual %d not supported\n", test->visual);
-- 
2.34.1

[PATCH fbtest 03/17] Test002: Fix test description

[Geert Uytterhoeven]

The test does not draw circles, but ellipses.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 tests/test002.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test002.c b/tests/test002.c
index 8f7cab0537535fcb..e24f0b3c629cf369 100644
--- a/tests/test002.c
+++ b/tests/test002.c
@@ -49,7 +49,7 @@ static enum test_res test002_func(void)
 
 const struct test test002 = {
     .name =	"test002",
-    .desc =	"Draw a grid and some circles",
+    .desc =	"Draw a grid and some ellipses",
     .visual =	VISUAL_MONO,
     .func =	test002_func,
 };
-- 
2.34.1

[PATCH fbtest 04/17] drawops: Extract do_circle()

[Geert Uytterhoeven]

generic_draw_circle() and generic_fill_circle() are very similar.
Reimplement them as wrappers around a common helper function.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drawops/generic.c | 51 +++++++++++++++++------------------------------
 1 file changed, 18 insertions(+), 33 deletions(-)

diff --git a/drawops/generic.c b/drawops/generic.c
index b2543b877a3a6154..9fd8aafb69a868a2 100644
--- a/drawops/generic.c
+++ b/drawops/generic.c
@@ -209,35 +209,6 @@ static void draw_circle_points(u32 cx, u32 cy, u32 x, u32 y, pixel_t pixel)
     }
 }
 
-void generic_draw_circle(u32 x, u32 y, u32 r, pixel_t pixel)
-{
-    u32 x1 = 0;
-    u32 y1 = r;
-    int d = 1-r;
-    int de = 3;
-    int dse = -2*r+5;
-
-    do {
-	draw_circle_points(x, y, x1, y1, pixel);
-	if (d < 0) {	// Select E
-	    d += de;
-	    de += 2;
-	    dse += 2;
-	} else {	// Select SE
-	    d += dse;
-	    de += 2;
-	    dse += 4;
-	    y1--;
-	}
-	x1++;
-    } while (x1 <= y1);
-}
-
-
-    /*
-     *  Draw a filled circle
-     */
-
 static void fill_circle_points_x(u32 cx, u32 cy, u32 x, u32 y, pixel_t pixel)
 {
     if (x == 0) {
@@ -259,7 +230,10 @@ static void fill_circle_points_y(u32 cx, u32 cy, u32 x, u32 y, pixel_t pixel)
     }
 }
 
-void generic_fill_circle(u32 x, u32 y, u32 r, pixel_t pixel)
+typedef void (*draw_func_t)(u32 cx, u32 cy, u32 x, u32 y, pixel_t pixel);
+
+static void do_circle(u32 x, u32 y, u32 r, pixel_t pixel, draw_func_t draw_x,
+		      draw_func_t draw_y)
 {
     u32 x1 = 0;
     u32 y1 = r;
@@ -268,7 +242,7 @@ void generic_fill_circle(u32 x, u32 y, u32 r, pixel_t pixel)
     int dse = -2*r+5;
 
     do {
-	fill_circle_points_y(x, y, x1, y1, pixel);
+	draw_y(x, y, x1, y1, pixel);
 	if (d < 0) {	// Select E
 	    d += de;
 	    de += 2;
@@ -277,14 +251,25 @@ void generic_fill_circle(u32 x, u32 y, u32 r, pixel_t pixel)
 	    d += dse;
 	    de += 2;
 	    dse += 4;
-	    if (x1 != y1)
-		fill_circle_points_x(x, y, x1, y1, pixel);
+	    if (draw_x && x1 != y1)
+		draw_x(x, y, x1, y1, pixel);
 	    y1--;
 	}
 	x1++;
     } while (x1 <= y1);
 }
 
+void generic_draw_circle(u32 x, u32 y, u32 r, pixel_t pixel)
+{
+    do_circle(x, y, r, pixel, NULL, draw_circle_points);
+}
+
+
+void generic_fill_circle(u32 x, u32 y, u32 r, pixel_t pixel)
+{
+    do_circle(x, y, r, pixel, fill_circle_points_x, fill_circle_points_y);
+}
+
 
     /*
      *  Draw an ellipse using a differential version of the Bresenham algorithm
-- 
2.34.1

[PATCH fbtest 05/17] drawops: Use "y == 0" in draw_ellipse_points()

[Geert Uytterhoeven]

Replace the rare "0 == y" check by the more common "y == 0" check.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drawops/generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drawops/generic.c b/drawops/generic.c
index 9fd8aafb69a868a2..ba8154b7ee34ca18 100644
--- a/drawops/generic.c
+++ b/drawops/generic.c
@@ -281,7 +281,7 @@ static void draw_ellipse_points(u32 cx, u32 cy, u32 x, u32 y, pixel_t pixel)
     if (x == 0) {
 	set_pixel(cx, cy-y, pixel);
 	set_pixel(cx, cy+y, pixel);
-    } else if (0 == y) {
+    } else if (y == 0) {
 	set_pixel(cx-x, cy, pixel);
 	set_pixel(cx+x, cy, pixel);
     } else {
-- 
2.34.1

[PATCH fbtest 06/17] drawops: Remove always-false check in generic_fill_ellipse()

[Geert Uytterhoeven]

x1 is u32, so it can never be negative.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drawops/generic.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drawops/generic.c b/drawops/generic.c
index ba8154b7ee34ca18..e81dea1ba23d6595 100644
--- a/drawops/generic.c
+++ b/drawops/generic.c
@@ -467,8 +467,6 @@ void generic_fill_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel)
 		    dS2 += 4*b2;
 		    dT2 += 4*b2;
 		    x1--;
-		    if (x1 < 0)
-			break;
 		    y1++;
 		    fill_ellipse_points(x, y, x1, y1, pixel);
 		} else {
-- 
2.34.1

[PATCH fbtest 07/17] drawops: Refactor generic_draw_ellipse()

[Geert Uytterhoeven]

Make generic_draw_ellipse() more similar to generic_fill_ellipse().

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drawops/generic.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drawops/generic.c b/drawops/generic.c
index e81dea1ba23d6595..4b64c20cc0fe68bc 100644
--- a/drawops/generic.c
+++ b/drawops/generic.c
@@ -309,15 +309,18 @@ void generic_draw_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel)
 	    int dS2 = -4*a2*(b-1);
 	    int dT2 = dS2+2*a2;
 
-	    draw_ellipse_points(x, y, x1, y1, pixel);
-	    do {
+	    while (1) {
 		if (S < 0) {
+		    draw_ellipse_points(x, y, x1, y1, pixel);
 		    S += dS1;
 		    T += dT1;
 		    dS1 += 4*b2;
 		    dT1 += 4*b2;
 		    x1++;
 		} else if (T < 0) {
+		    draw_ellipse_points(x, y, x1, y1, pixel);
+		    if (y1 == 0)
+			break;
 		    S += dS1+dS2;
 		    T += dT1+dT2;
 		    dS1 += 4*b2;
@@ -327,14 +330,16 @@ void generic_draw_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel)
 		    x1++;
 		    y1--;
 		} else {
+		    draw_ellipse_points(x, y, x1, y1, pixel);
+		    if (y1 == 0)
+			break;
 		    S += dS2;
 		    T += dT2;
 		    dS2 += 4*a2;
 		    dT2 += 4*a2;
 		    y1--;
 		}
-		draw_ellipse_points(x, y, x1, y1, pixel);
-	    } while (y1 > 0);
+	    }
 	} else {
 	    u32 x1 = a;
 	    u32 y1 = 0;
@@ -353,6 +358,7 @@ void generic_draw_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel)
 		    dS1 += 4*a2;
 		    dT1 += 4*a2;
 		    y1++;
+		    draw_ellipse_points(x, y, x1, y1, pixel);
 		} else if (T < 0) {
 		    S += dS1+dS2;
 		    T += dT1+dT2;
@@ -362,14 +368,15 @@ void generic_draw_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel)
 		    dT2 += 4*b2;
 		    x1--;
 		    y1++;
+		    draw_ellipse_points(x, y, x1, y1, pixel);
 		} else {
 		    S += dS2;
 		    T += dT2;
 		    dS2 += 4*b2;
 		    dT2 += 4*b2;
 		    x1--;
+		    draw_ellipse_points(x, y, x1, y1, pixel);
 		}
-		draw_ellipse_points(x, y, x1, y1, pixel);
 	    } while (x1 > 0);
 	}
     }
-- 
2.34.1

[PATCH fbtest 08/17] drawops: Return early in generic_{draw,fill}_ellipse()

[Geert Uytterhoeven]

This reduces indentation in the largest branches.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drawops/generic.c | 324 +++++++++++++++++++++++-----------------------
 1 file changed, 162 insertions(+), 162 deletions(-)

diff --git a/drawops/generic.c b/drawops/generic.c
index 4b64c20cc0fe68bc..471aefe38d43aaa4 100644
--- a/drawops/generic.c
+++ b/drawops/generic.c
@@ -295,90 +295,90 @@ static void draw_ellipse_points(u32 cx, u32 cy, u32 x, u32 y, pixel_t pixel)
 void generic_draw_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel)
 {
     if (a == b)
-	draw_circle(x, y, a, pixel);
-    else {
-	u32 a2 = a*a;
-	u32 b2 = b*b;
-	if (a <= b) {
-	    u32 x1 = 0;
-	    u32 y1 = b;
-	    int S = a2*(1-2*b)+2*b2;
-	    int T = b2-2*a2*(2*b-1);
-	    int dT1 = 4*b2;
-	    int dS1 = dT1+2*b2;
-	    int dS2 = -4*a2*(b-1);
-	    int dT2 = dS2+2*a2;
-
-	    while (1) {
-		if (S < 0) {
-		    draw_ellipse_points(x, y, x1, y1, pixel);
-		    S += dS1;
-		    T += dT1;
-		    dS1 += 4*b2;
-		    dT1 += 4*b2;
-		    x1++;
-		} else if (T < 0) {
-		    draw_ellipse_points(x, y, x1, y1, pixel);
-		    if (y1 == 0)
-			break;
-		    S += dS1+dS2;
-		    T += dT1+dT2;
-		    dS1 += 4*b2;
-		    dT1 += 4*b2;
-		    dS2 += 4*a2;
-		    dT2 += 4*a2;
-		    x1++;
-		    y1--;
-		} else {
-		    draw_ellipse_points(x, y, x1, y1, pixel);
-		    if (y1 == 0)
-			break;
-		    S += dS2;
-		    T += dT2;
-		    dS2 += 4*a2;
-		    dT2 += 4*a2;
-		    y1--;
-		}
+	return draw_circle(x, y, a, pixel);
+
+    u32 a2 = a*a;
+    u32 b2 = b*b;
+
+    if (a <= b) {
+	u32 x1 = 0;
+	u32 y1 = b;
+	int S = a2*(1-2*b)+2*b2;
+	int T = b2-2*a2*(2*b-1);
+	int dT1 = 4*b2;
+	int dS1 = dT1+2*b2;
+	int dS2 = -4*a2*(b-1);
+	int dT2 = dS2+2*a2;
+
+	while (1) {
+	    if (S < 0) {
+		draw_ellipse_points(x, y, x1, y1, pixel);
+		S += dS1;
+		T += dT1;
+		dS1 += 4*b2;
+		dT1 += 4*b2;
+		x1++;
+	    } else if (T < 0) {
+		draw_ellipse_points(x, y, x1, y1, pixel);
+		if (y1 == 0)
+		    break;
+		S += dS1+dS2;
+		T += dT1+dT2;
+		dS1 += 4*b2;
+		dT1 += 4*b2;
+		dS2 += 4*a2;
+		dT2 += 4*a2;
+		x1++;
+		y1--;
+	    } else {
+		draw_ellipse_points(x, y, x1, y1, pixel);
+		if (y1 == 0)
+		    break;
+		S += dS2;
+		T += dT2;
+		dS2 += 4*a2;
+		dT2 += 4*a2;
+		y1--;
 	    }
-	} else {
-	    u32 x1 = a;
-	    u32 y1 = 0;
-            int S = b2*(1-2*a)+2*a2;
-            int T = a2-2*b2*(2*a-1);
-            int dT1 = 4*a2;
-            int dS1 = dT1+2*a2;
-            int dS2 = -4*b2*(a-1);
-            int dT2 = dS2+2*b2;
-
-	    draw_ellipse_points(x, y, x1, y1, pixel);
-	    do {
-		if (S < 0) {
-		    S += dS1;
-		    T += dT1;
-		    dS1 += 4*a2;
-		    dT1 += 4*a2;
-		    y1++;
-		    draw_ellipse_points(x, y, x1, y1, pixel);
-		} else if (T < 0) {
-		    S += dS1+dS2;
-		    T += dT1+dT2;
-		    dS1 += 4*a2;
-		    dT1 += 4*a2;
-		    dS2 += 4*b2;
-		    dT2 += 4*b2;
-		    x1--;
-		    y1++;
-		    draw_ellipse_points(x, y, x1, y1, pixel);
-		} else {
-		    S += dS2;
-		    T += dT2;
-		    dS2 += 4*b2;
-		    dT2 += 4*b2;
-		    x1--;
-		    draw_ellipse_points(x, y, x1, y1, pixel);
-		}
-	    } while (x1 > 0);
 	}
+    } else {
+	u32 x1 = a;
+	u32 y1 = 0;
+	int S = b2*(1-2*a)+2*a2;
+	int T = a2-2*b2*(2*a-1);
+	int dT1 = 4*a2;
+	int dS1 = dT1+2*a2;
+	int dS2 = -4*b2*(a-1);
+	int dT2 = dS2+2*b2;
+
+	draw_ellipse_points(x, y, x1, y1, pixel);
+	do {
+	    if (S < 0) {
+		S += dS1;
+		T += dT1;
+		dS1 += 4*a2;
+		dT1 += 4*a2;
+		y1++;
+		draw_ellipse_points(x, y, x1, y1, pixel);
+	    } else if (T < 0) {
+		S += dS1+dS2;
+		T += dT1+dT2;
+		dS1 += 4*a2;
+		dT1 += 4*a2;
+		dS2 += 4*b2;
+		dT2 += 4*b2;
+		x1--;
+		y1++;
+		draw_ellipse_points(x, y, x1, y1, pixel);
+	    } else {
+		S += dS2;
+		T += dT2;
+		dS2 += 4*b2;
+		dT2 += 4*b2;
+		x1--;
+		draw_ellipse_points(x, y, x1, y1, pixel);
+	    }
+	} while (x1 > 0);
     }
 }
 
@@ -403,88 +403,88 @@ static void fill_ellipse_points(u32 cx, u32 cy, u32 x, u32 y, pixel_t pixel)
 void generic_fill_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel)
 {
     if (a == b)
-	fill_circle(x, y, a, pixel);
-    else {
-	u32 a2 = a*a;
-	u32 b2 = b*b;
-	if (a <= b) {
-	    u32 x1 = 0;
-	    u32 y1 = b;
-	    int S = a2*(1-2*b)+2*b2;
-	    int T = b2-2*a2*(2*b-1);
-	    int dT1 = 4*b2;
-	    int dS1 = dT1+2*b2;
-	    int dS2 = -4*a2*(b-1);
-	    int dT2 = dS2+2*a2;
-
-	    while (1) {
-		if (S < 0) {
-		    S += dS1;
-		    T += dT1;
-		    dS1 += 4*b2;
-		    dT1 += 4*b2;
-		    x1++;
-		} else if (T < 0) {
-		    fill_ellipse_points(x, y, x1, y1, pixel);
-		    if (y1 == 0)
-			break;
-		    S += dS1+dS2;
-		    T += dT1+dT2;
-		    dS1 += 4*b2;
-		    dT1 += 4*b2;
-		    dS2 += 4*a2;
-		    dT2 += 4*a2;
-		    x1++;
-		    y1--;
-		} else {
-		    fill_ellipse_points(x, y, x1, y1, pixel);
-		    if (y1 == 0)
-			break;
-		    S += dS2;
-		    T += dT2;
-		    dS2 += 4*a2;
-		    dT2 += 4*a2;
-		    y1--;
-		}
+	return fill_circle(x, y, a, pixel);
+
+    u32 a2 = a*a;
+    u32 b2 = b*b;
+
+    if (a <= b) {
+	u32 x1 = 0;
+	u32 y1 = b;
+	int S = a2*(1-2*b)+2*b2;
+	int T = b2-2*a2*(2*b-1);
+	int dT1 = 4*b2;
+	int dS1 = dT1+2*b2;
+	int dS2 = -4*a2*(b-1);
+	int dT2 = dS2+2*a2;
+
+	while (1) {
+	    if (S < 0) {
+		S += dS1;
+		T += dT1;
+		dS1 += 4*b2;
+		dT1 += 4*b2;
+		x1++;
+	    } else if (T < 0) {
+		fill_ellipse_points(x, y, x1, y1, pixel);
+		if (y1 == 0)
+		    break;
+		S += dS1+dS2;
+		T += dT1+dT2;
+		dS1 += 4*b2;
+		dT1 += 4*b2;
+		dS2 += 4*a2;
+		dT2 += 4*a2;
+		x1++;
+		y1--;
+	    } else {
+		fill_ellipse_points(x, y, x1, y1, pixel);
+		if (y1 == 0)
+		    break;
+		S += dS2;
+		T += dT2;
+		dS2 += 4*a2;
+		dT2 += 4*a2;
+		y1--;
 	    }
-	} else {
-	    u32 x1 = a;
-	    u32 y1 = 0;
-            int S = b2*(1-2*a)+2*a2;
-            int T = a2-2*b2*(2*a-1);
-            int dT1 = 4*a2;
-            int dS1 = dT1+2*a2;
-            int dS2 = -4*b2*(a-1);
-            int dT2 = dS2+2*b2;
-
-	    fill_ellipse_points(x, y, x1, y1, pixel);
-	    do {
-		if (S < 0) {
-		    S += dS1;
-		    T += dT1;
-		    dS1 += 4*a2;
-		    dT1 += 4*a2;
-		    y1++;
-		    fill_ellipse_points(x, y, x1, y1, pixel);
-		} else if (T < 0) {
-		    S += dS1+dS2;
-		    T += dT1+dT2;
-		    dS1 += 4*a2;
-		    dT1 += 4*a2;
-		    dS2 += 4*b2;
-		    dT2 += 4*b2;
-		    x1--;
-		    y1++;
-		    fill_ellipse_points(x, y, x1, y1, pixel);
-		} else {
-		    S += dS2;
-		    T += dT2;
-		    dS2 += 4*b2;
-		    dT2 += 4*b2;
-		    x1--;
-		}
-	    } while (x1 > 0);
 	}
+    } else {
+	u32 x1 = a;
+	u32 y1 = 0;
+	int S = b2*(1-2*a)+2*a2;
+	int T = a2-2*b2*(2*a-1);
+	int dT1 = 4*a2;
+	int dS1 = dT1+2*a2;
+	int dS2 = -4*b2*(a-1);
+	int dT2 = dS2+2*b2;
+
+	fill_ellipse_points(x, y, x1, y1, pixel);
+	do {
+	    if (S < 0) {
+		S += dS1;
+		T += dT1;
+		dS1 += 4*a2;
+		dT1 += 4*a2;
+		y1++;
+		fill_ellipse_points(x, y, x1, y1, pixel);
+	    } else if (T < 0) {
+		S += dS1+dS2;
+		T += dT1+dT2;
+		dS1 += 4*a2;
+		dT1 += 4*a2;
+		dS2 += 4*b2;
+		dT2 += 4*b2;
+		x1--;
+		y1++;
+		fill_ellipse_points(x, y, x1, y1, pixel);
+	    } else {
+		S += dS2;
+		T += dT2;
+		dS2 += 4*b2;
+		dT2 += 4*b2;
+		x1--;
+	    }
+	} while (x1 > 0);
     }
 }
 
-- 
2.34.1

[PATCH fbtest 09/17] drawops: Extract do_ellipse()

[Geert Uytterhoeven]

generic_draw_ellipse() and generic_fill_ellipse() are very similar.
Reimplement them as wrappers around a common helper function.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drawops/generic.c | 132 ++++++++++------------------------------------
 1 file changed, 28 insertions(+), 104 deletions(-)

diff --git a/drawops/generic.c b/drawops/generic.c
index 471aefe38d43aaa4..b3218f50d86c6d4c 100644
--- a/drawops/generic.c
+++ b/drawops/generic.c
@@ -292,101 +292,6 @@ static void draw_ellipse_points(u32 cx, u32 cy, u32 x, u32 y, pixel_t pixel)
     }
 }
 
-void generic_draw_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel)
-{
-    if (a == b)
-	return draw_circle(x, y, a, pixel);
-
-    u32 a2 = a*a;
-    u32 b2 = b*b;
-
-    if (a <= b) {
-	u32 x1 = 0;
-	u32 y1 = b;
-	int S = a2*(1-2*b)+2*b2;
-	int T = b2-2*a2*(2*b-1);
-	int dT1 = 4*b2;
-	int dS1 = dT1+2*b2;
-	int dS2 = -4*a2*(b-1);
-	int dT2 = dS2+2*a2;
-
-	while (1) {
-	    if (S < 0) {
-		draw_ellipse_points(x, y, x1, y1, pixel);
-		S += dS1;
-		T += dT1;
-		dS1 += 4*b2;
-		dT1 += 4*b2;
-		x1++;
-	    } else if (T < 0) {
-		draw_ellipse_points(x, y, x1, y1, pixel);
-		if (y1 == 0)
-		    break;
-		S += dS1+dS2;
-		T += dT1+dT2;
-		dS1 += 4*b2;
-		dT1 += 4*b2;
-		dS2 += 4*a2;
-		dT2 += 4*a2;
-		x1++;
-		y1--;
-	    } else {
-		draw_ellipse_points(x, y, x1, y1, pixel);
-		if (y1 == 0)
-		    break;
-		S += dS2;
-		T += dT2;
-		dS2 += 4*a2;
-		dT2 += 4*a2;
-		y1--;
-	    }
-	}
-    } else {
-	u32 x1 = a;
-	u32 y1 = 0;
-	int S = b2*(1-2*a)+2*a2;
-	int T = a2-2*b2*(2*a-1);
-	int dT1 = 4*a2;
-	int dS1 = dT1+2*a2;
-	int dS2 = -4*b2*(a-1);
-	int dT2 = dS2+2*b2;
-
-	draw_ellipse_points(x, y, x1, y1, pixel);
-	do {
-	    if (S < 0) {
-		S += dS1;
-		T += dT1;
-		dS1 += 4*a2;
-		dT1 += 4*a2;
-		y1++;
-		draw_ellipse_points(x, y, x1, y1, pixel);
-	    } else if (T < 0) {
-		S += dS1+dS2;
-		T += dT1+dT2;
-		dS1 += 4*a2;
-		dT1 += 4*a2;
-		dS2 += 4*b2;
-		dT2 += 4*b2;
-		x1--;
-		y1++;
-		draw_ellipse_points(x, y, x1, y1, pixel);
-	    } else {
-		S += dS2;
-		T += dT2;
-		dS2 += 4*b2;
-		dT2 += 4*b2;
-		x1--;
-		draw_ellipse_points(x, y, x1, y1, pixel);
-	    }
-	} while (x1 > 0);
-    }
-}
-
-
-    /*
-     *  Draw a filled ellipse
-     */
-
 static void fill_ellipse_points(u32 cx, u32 cy, u32 x, u32 y, pixel_t pixel)
 {
     if (x == 0) {
@@ -400,11 +305,9 @@ static void fill_ellipse_points(u32 cx, u32 cy, u32 x, u32 y, pixel_t pixel)
     }
 }
 
-void generic_fill_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel)
+static void do_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel,
+		       draw_func_t draw_inner, draw_func_t draw_outer)
 {
-    if (a == b)
-	return fill_circle(x, y, a, pixel);
-
     u32 a2 = a*a;
     u32 b2 = b*b;
 
@@ -420,13 +323,15 @@ void generic_fill_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel)
 
 	while (1) {
 	    if (S < 0) {
+		if (draw_inner)
+		    draw_inner(x, y, x1, y1, pixel);
 		S += dS1;
 		T += dT1;
 		dS1 += 4*b2;
 		dT1 += 4*b2;
 		x1++;
 	    } else if (T < 0) {
-		fill_ellipse_points(x, y, x1, y1, pixel);
+		draw_outer(x, y, x1, y1, pixel);
 		if (y1 == 0)
 		    break;
 		S += dS1+dS2;
@@ -438,7 +343,7 @@ void generic_fill_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel)
 		x1++;
 		y1--;
 	    } else {
-		fill_ellipse_points(x, y, x1, y1, pixel);
+		draw_outer(x, y, x1, y1, pixel);
 		if (y1 == 0)
 		    break;
 		S += dS2;
@@ -458,7 +363,7 @@ void generic_fill_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel)
 	int dS2 = -4*b2*(a-1);
 	int dT2 = dS2+2*b2;
 
-	fill_ellipse_points(x, y, x1, y1, pixel);
+	draw_outer(x, y, x1, y1, pixel);
 	do {
 	    if (S < 0) {
 		S += dS1;
@@ -466,7 +371,7 @@ void generic_fill_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel)
 		dS1 += 4*a2;
 		dT1 += 4*a2;
 		y1++;
-		fill_ellipse_points(x, y, x1, y1, pixel);
+		draw_outer(x, y, x1, y1, pixel);
 	    } else if (T < 0) {
 		S += dS1+dS2;
 		T += dT1+dT2;
@@ -476,18 +381,37 @@ void generic_fill_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel)
 		dT2 += 4*b2;
 		x1--;
 		y1++;
-		fill_ellipse_points(x, y, x1, y1, pixel);
+		draw_outer(x, y, x1, y1, pixel);
 	    } else {
 		S += dS2;
 		T += dT2;
 		dS2 += 4*b2;
 		dT2 += 4*b2;
 		x1--;
+		if (draw_inner)
+		    draw_inner(x, y, x1, y1, pixel);
 	    }
 	} while (x1 > 0);
     }
 }
 
+void generic_draw_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel)
+{
+    if (a == b)
+	draw_circle(x, y, a, pixel);
+    else
+	do_ellipse(x, y, a, b, pixel, draw_ellipse_points,
+		   draw_ellipse_points);
+}
+
+void generic_fill_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel)
+{
+    if (a == b)
+	fill_circle(x, y, a, pixel);
+    else
+	do_ellipse(x, y, a, b, pixel, NULL, fill_ellipse_points);
+}
+
 
     /*
      *  Copy a rectangular area
-- 
2.34.1

[PATCH fbtest 10/17] drawops: Make de in do_circle() unsigned

[Geert Uytterhoeven]

"de" is never negative, so it should be unsigned.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drawops/generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drawops/generic.c b/drawops/generic.c
index b3218f50d86c6d4c..5c068e10d28fbdfe 100644
--- a/drawops/generic.c
+++ b/drawops/generic.c
@@ -238,7 +238,7 @@ static void do_circle(u32 x, u32 y, u32 r, pixel_t pixel, draw_func_t draw_x,
     u32 x1 = 0;
     u32 y1 = r;
     int d = 1-r;
-    int de = 3;
+    unsigned int de = 3;
     int dse = -2*r+5;
 
     do {
-- 
2.34.1

[PATCH fbtest 11/17] drawops: Make dT1 and dS1 in do_ellipse() unsigned

[Geert Uytterhoeven]

"dT1" and "dS1" are never negative, so they should be unsigned.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drawops/generic.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drawops/generic.c b/drawops/generic.c
index 5c068e10d28fbdfe..5fd971b59bc698fe 100644
--- a/drawops/generic.c
+++ b/drawops/generic.c
@@ -316,8 +316,8 @@ static void do_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel,
 	u32 y1 = b;
 	int S = a2*(1-2*b)+2*b2;
 	int T = b2-2*a2*(2*b-1);
-	int dT1 = 4*b2;
-	int dS1 = dT1+2*b2;
+	unsigned int dT1 = 4*b2;
+	unsigned int dS1 = dT1+2*b2;
 	int dS2 = -4*a2*(b-1);
 	int dT2 = dS2+2*a2;
 
@@ -358,8 +358,8 @@ static void do_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel,
 	u32 y1 = 0;
 	int S = b2*(1-2*a)+2*a2;
 	int T = a2-2*b2*(2*a-1);
-	int dT1 = 4*a2;
-	int dS1 = dT1+2*a2;
+	unsigned int dT1 = 4*a2;
+	unsigned int dS1 = dT1+2*a2;
 	int dS2 = -4*b2*(a-1);
 	int dT2 = dS2+2*b2;
 
-- 
2.34.1

[PATCH fbtest 12/17] drawops: Fix crash when drawing large ellipses

[Geert Uytterhoeven]

"test002" crashes when run with a display resolution of e.g. 2560x1440
pixels, due to 32-bit overflow in the ellipse drawing routine.

Fix this by creating a copy that uses 64-bit arithmetic.  Use a
heuristic to pick either the 32-bit or the 64-bit version, to avoid the
overhead of the 64-bit version on small systems with small displays.
Replace (unsigned) int by u32/s32 in the 32-bit version for clarity.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drawops/generic.c | 127 +++++++++++++++++++++++++++++++++++++++++-----
 include/types.h   |   3 ++
 2 files changed, 116 insertions(+), 14 deletions(-)

diff --git a/drawops/generic.c b/drawops/generic.c
index 5fd971b59bc698fe..c4cfad3223773a23 100644
--- a/drawops/generic.c
+++ b/drawops/generic.c
@@ -305,8 +305,98 @@ static void fill_ellipse_points(u32 cx, u32 cy, u32 x, u32 y, pixel_t pixel)
     }
 }
 
-static void do_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel,
-		       draw_func_t draw_inner, draw_func_t draw_outer)
+static void do_ellipse_32(u32 x, u32 y, u32 a, u32 b, pixel_t pixel,
+			  draw_func_t draw_inner, draw_func_t draw_outer)
+{
+    u32 a2 = a*a;
+    u32 b2 = b*b;
+
+    if (a <= b) {
+	u32 x1 = 0;
+	u32 y1 = b;
+	s32 S = a2*(1-2*b)+2*b2;
+	s32 T = b2-2*a2*(2*b-1);
+	u32 dT1 = 4*b2;
+	u32 dS1 = dT1+2*b2;
+	s32 dS2 = -4*a2*(b-1);
+	s32 dT2 = dS2+2*a2;
+
+	while (1) {
+	    if (S < 0) {
+		if (draw_inner)
+		    draw_inner(x, y, x1, y1, pixel);
+		S += dS1;
+		T += dT1;
+		dS1 += 4*b2;
+		dT1 += 4*b2;
+		x1++;
+	    } else if (T < 0) {
+		draw_outer(x, y, x1, y1, pixel);
+		if (y1 == 0)
+		    break;
+		S += dS1+dS2;
+		T += dT1+dT2;
+		dS1 += 4*b2;
+		dT1 += 4*b2;
+		dS2 += 4*a2;
+		dT2 += 4*a2;
+		x1++;
+		y1--;
+	    } else {
+		draw_outer(x, y, x1, y1, pixel);
+		if (y1 == 0)
+		    break;
+		S += dS2;
+		T += dT2;
+		dS2 += 4*a2;
+		dT2 += 4*a2;
+		y1--;
+	    }
+	}
+    } else {
+	u32 x1 = a;
+	u32 y1 = 0;
+	s32 S = b2*(1-2*a)+2*a2;
+	s32 T = a2-2*b2*(2*a-1);
+	u32 dT1 = 4*a2;
+	u32 dS1 = dT1+2*a2;
+	s32 dS2 = -4*b2*(a-1);
+	s32 dT2 = dS2+2*b2;
+
+	draw_outer(x, y, x1, y1, pixel);
+	do {
+	    if (S < 0) {
+		S += dS1;
+		T += dT1;
+		dS1 += 4*a2;
+		dT1 += 4*a2;
+		y1++;
+		draw_outer(x, y, x1, y1, pixel);
+	    } else if (T < 0) {
+		S += dS1+dS2;
+		T += dT1+dT2;
+		dS1 += 4*a2;
+		dT1 += 4*a2;
+		dS2 += 4*b2;
+		dT2 += 4*b2;
+		x1--;
+		y1++;
+		draw_outer(x, y, x1, y1, pixel);
+	    } else {
+		S += dS2;
+		T += dT2;
+		dS2 += 4*b2;
+		dT2 += 4*b2;
+		x1--;
+		if (draw_inner)
+		    draw_inner(x, y, x1, y1, pixel);
+	    }
+	} while (x1 > 0);
+    }
+}
+
+static void do_ellipse_64(u32 x, u32 y, u32 a, u32 b, pixel_t pixel,
+			  draw_func_t draw_inner, draw_func_t draw_outer)
 {
     u32 a2 = a*a;
     u32 b2 = b*b;
@@ -314,12 +404,12 @@ static void do_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel,
     if (a <= b) {
 	u32 x1 = 0;
 	u32 y1 = b;
-	int S = a2*(1-2*b)+2*b2;
-	int T = b2-2*a2*(2*b-1);
-	unsigned int dT1 = 4*b2;
-	unsigned int dS1 = dT1+2*b2;
-	int dS2 = -4*a2*(b-1);
-	int dT2 = dS2+2*a2;
+	s64 S = a2*(1-2LL*b)+2LL*b2;
+	s64 T = b2-2LL*a2*(2LL*b-1);
+	u64 dT1 = 4*b2;
+	u64 dS1 = dT1+2*b2;
+	s64 dS2 = -4LL*a2*(b-1);
+	s64 dT2 = dS2+2*a2;
 
 	while (1) {
 	    if (S < 0) {
@@ -356,12 +446,12 @@ static void do_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel,
     } else {
 	u32 x1 = a;
 	u32 y1 = 0;
-	int S = b2*(1-2*a)+2*a2;
-	int T = a2-2*b2*(2*a-1);
-	unsigned int dT1 = 4*a2;
-	unsigned int dS1 = dT1+2*a2;
-	int dS2 = -4*b2*(a-1);
-	int dT2 = dS2+2*b2;
+	s64 S = b2*(1-2LL*a)+2LL*a2;
+	s64 T = a2-2LL*b2*(2LL*a-1);
+	u64 dT1 = 4*a2;
+	u64 dS1 = dT1+2*a2;
+	s64 dS2 = -4LL*b2*(a-1);
+	s64 dT2 = dS2+2*b2;
 
 	draw_outer(x, y, x1, y1, pixel);
 	do {
@@ -395,6 +485,15 @@ static void do_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel,
     }
 }
 
+static void do_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel,
+		       draw_func_t draw_inner, draw_func_t draw_outer)
+{
+    if ((a + 576) * (b + 576) < 1440000)
+	do_ellipse_32(x, y, a, b, pixel, draw_inner, draw_outer);
+    else
+	do_ellipse_64(x, y, a, b, pixel, draw_inner, draw_outer);
+}
+
 void generic_draw_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel)
 {
     if (a == b)
diff --git a/include/types.h b/include/types.h
index 9112ba6855b61eaa..0e3c76521469912f 100644
--- a/include/types.h
+++ b/include/types.h
@@ -21,6 +21,9 @@ typedef unsigned short u16;
 typedef unsigned int u32;
 typedef unsigned long long u64;
 
+typedef int s32;
+typedef long long s64;
+
 #if defined(__LP64__) || defined(__alpha__) || defined(__ia64__) || \
     defined(__mips64__) || defined(__powerpc64__) || defined(__s390x__) || \
     defined(__sparc64__) || defined(__x86_64__) || defined(__hppa64__)
-- 
2.34.1

[PATCH fbtest 13/17] tests: Clear frame buffer before each test

[Geert Uytterhoeven]

Make sure the frame buffer is cleared before each test, to avoid
leftover artifacts from a previous test.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 include/fb.h | 1 +
 tests.c      | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/include/fb.h b/include/fb.h
index 1df4e48311b6c1e9..6506ec120aa962ae 100644
--- a/include/fb.h
+++ b/include/fb.h
@@ -28,6 +28,7 @@ extern int fb_set_cmap(void);
 extern int fb_pan(u32 xoffset, u32 yoffset);
 extern void fb_map(void);
 extern void fb_unmap(void);
+extern void fb_clear(void);
 
 
     /*
diff --git a/tests.c b/tests.c
index b1573032372e8090..22a8a0f489c2bc06 100644
--- a/tests.c
+++ b/tests.c
@@ -78,6 +78,8 @@ static void run_one_test(const struct test *test)
 	}
     }
 
+    fb_clear();
+
     res = test->func();
     switch (res) {
 	case TEST_OK:
-- 
2.34.1

[PATCH fbtest 14/17] Make variables that are never negative unsigned

[Geert Uytterhoeven]

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 console.c       | 2 +-
 fb.c            | 2 +-
 pixmap.c        | 6 +++---
 tests/test004.c | 2 +-
 tests/test006.c | 2 +-
 tests/test007.c | 2 +-
 tests/test009.c | 2 +-
 tests/test010.c | 2 +-
 visual.c        | 2 +-
 9 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/console.c b/console.c
index 0ed858e25f52c9d5..adcc78cd1d725f37 100644
--- a/console.c
+++ b/console.c
@@ -118,7 +118,7 @@ static void con_store_char(unsigned char c)
 {
     const unsigned char *src;
     unsigned char *dst;
-    int y;
+    unsigned int y;
 
     if (bitmap_width+con_font->width > bitmap_max_width)
 	con_flush();
diff --git a/fb.c b/fb.c
index ab351d15c82e09d9..4bc6d785e3e9070a 100644
--- a/fb.c
+++ b/fb.c
@@ -533,7 +533,7 @@ void fb_cleanup(void)
 
 static void fb_dump_cmap(void)
 {
-    int i;
+    unsigned int i;
 
     Debug("Colormap start = %d len = %d\n", fb_cmap.start, fb_cmap.len);
     for (i = 0; i < fb_cmap.len; i++)
diff --git a/pixmap.c b/pixmap.c
index 8cd21a62ff5235a8..a7c11909f2d5255d 100644
--- a/pixmap.c
+++ b/pixmap.c
@@ -71,7 +71,7 @@ static void image_bw_to_pixmap(const struct image *image, pixel_t *pixmap)
     pixel_t black, white;
     const unsigned char *src;
     pixel_t *dst;
-    int i, j, k;
+    unsigned int i, j, k;
 
     black = match_color(&clut_mono[0]);
     white = match_color(&clut_mono[1]);
@@ -101,7 +101,7 @@ static void image_lut256_to_pixmap(const struct image *image, pixel_t *pixmap)
     rgba_t color;
     const unsigned char *src;
     pixel_t *dst;
-    int i;
+    unsigned int i;
 
     color.a = 65535;
     if (image->type == IMAGE_GREY256) {
@@ -135,7 +135,7 @@ static void image_rgb888_to_pixmap(const struct image *image, pixel_t *pixmap)
     const unsigned char *src;
     pixel_t *dst;
     rgba_t color;
-    int i;
+    unsigned int i;
 
     src = image->data;
     dst = pixmap;
diff --git a/tests/test004.c b/tests/test004.c
index 9b1e79ef9077219d..308c1ee60899e5a9 100644
--- a/tests/test004.c
+++ b/tests/test004.c
@@ -27,7 +27,7 @@ static enum test_res test004_func(void)
 {
     const struct image *image;
     pixel_t *pixmap;
-    int x, y, width, height, i;
+    unsigned int x, y, width, height, i;
 
     image = &penguin;
     pixmap = create_pixmap(image);
diff --git a/tests/test006.c b/tests/test006.c
index 20ba90963ea10797..c3b6722fafbeaf3c 100644
--- a/tests/test006.c
+++ b/tests/test006.c
@@ -23,7 +23,7 @@
 
 static enum test_res test006_func(void)
 {
-    int i, j;
+    unsigned int i, j;
     pixel_t pixels[2];
     u32 x0, x1, y0, y1;
 
diff --git a/tests/test007.c b/tests/test007.c
index d6e7864c5b36d2bb..8fd1613c8b23cfb2 100644
--- a/tests/test007.c
+++ b/tests/test007.c
@@ -51,7 +51,7 @@ static void increase_level(int *component)
 
 static enum test_res test007_func(void)
 {
-    int i;
+    unsigned int i;
 
     fill_rect(0, 0, fb_var.xres, fb_var.yres, black_pixel);
     for (i = 0; i < red_len; i++)
diff --git a/tests/test009.c b/tests/test009.c
index 4f36950737c811df..a62724ff547c04ea 100644
--- a/tests/test009.c
+++ b/tests/test009.c
@@ -27,7 +27,7 @@ static enum test_res test009_func(void)
 {
     const struct image *image;
     pixel_t *pixmap;
-    int x, y, width, height;
+    unsigned int x, y, width, height;
 
     image = &penguin;
     pixmap = create_pixmap(image);
diff --git a/tests/test010.c b/tests/test010.c
index 6ed97e8e51bfb77e..5b26b773e873eeec 100644
--- a/tests/test010.c
+++ b/tests/test010.c
@@ -24,7 +24,7 @@
 
 static enum test_res test010_func(void)
 {
-    int i, j;
+    unsigned int i, j;
 
     con_init(&FONT);
     con_puts("Hello, world!\n");
diff --git a/visual.c b/visual.c
index e8782950df62178a..c5dbed3095d85e54 100644
--- a/visual.c
+++ b/visual.c
@@ -91,7 +91,7 @@ static u32 reverse32(u32 x)
 pixel_t *create_component_table(u32 size, u32 offset, int msb_right, u32 bpp)
 {
     pixel_t *table, pixel;
-    int i;
+    unsigned int i;
 
     if (!size)
 	return NULL;
-- 
2.34.1

[PATCH fbtest 15/17] test013: Fix off-by-one error in maximum circle calculation

[Geert Uytterhoeven]

A circle with radius R has an actual diameter of 2*R+1 instead of 2*R.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 tests/test013.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/test013.c b/tests/test013.c
index 96fc1cd216b81b0a..25904270901eaded 100644
--- a/tests/test013.c
+++ b/tests/test013.c
@@ -63,10 +63,10 @@ static enum test_res test013_func(void)
 
     while (1)
 	for (i = 0; i < sizeof(sizes)/sizeof(*sizes); i++) {
-	    size = sizes[i];
+	    size = sizes[i] + 1;
 	    if (size > fb_var.xres_virtual || size > fb_var.yres_virtual)
 		goto out;
-	    benchmark_circles(size/2);
+	    benchmark_circles(sizes[i]/2);
 	    sizes[i] *= 10;
 	}
 
-- 
2.34.1

[PATCH fbtest 16/17] visops: Mark fall-through switch case

[Geert Uytterhoeven]

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 visops/pseudocolor.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/visops/pseudocolor.c b/visops/pseudocolor.c
index 2c3f4d9ac29b7b5e..112e2144979afbf3 100644
--- a/visops/pseudocolor.c
+++ b/visops/pseudocolor.c
@@ -50,6 +50,7 @@ void pseudocolor_create_tables(u32 bpp)
 	switch (bpp % 3) {
 	    case 2:
 		red_bits++;
+		__attribute__ ((fallthrough));
 	    case 1:
 		green_bits++;
 	}
-- 
2.34.1

[PATCH fbtest 17/17] util: Use __attribute__

[Geert Uytterhoeven]

Replace the rare __attribute by the more common __attribute__.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 include/util.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/util.h b/include/util.h
index 5a6b5ab40d383d5a..1433b87b8dbefcfc 100644
--- a/include/util.h
+++ b/include/util.h
@@ -40,7 +40,7 @@ extern void Warning(const char *fmt, ...)
 extern void Error(const char *fmt, ...)
     __attribute__ ((format (printf, 1, 2)));
 extern void Fatal(const char *fmt, ...)
-    __attribute__ ((noreturn)) __attribute ((format (printf, 1, 2)));
+    __attribute__ ((noreturn)) __attribute__ ((format (printf, 1, 2)));
 extern void Debug(const char *fmt, ...)
     __attribute__ ((format (printf, 1, 2)));
 
-- 
2.34.1

Re: [PATCH fbtest 12/17] drawops: Fix crash when drawing large ellipses

[Helge Deller]

On 12/15/24 11:45, Geert Uytterhoeven wrote:
> "test002" crashes when run with a display resolution of e.g. 2560x1440
> pixels, due to 32-bit overflow in the ellipse drawing routine.
>
> Fix this by creating a copy that uses 64-bit arithmetic.  Use a
> heuristic to pick either the 32-bit or the 64-bit version, to avoid the
> overhead of the 64-bit version on small systems with small displays.

I see you always build the 32- and 64-bit versions, so when you mean
overhead you mean runtime overhead, not compiled binary size overhead.
So, just wondering:
Did you maybe measured how much slower the 64-bit version is on slow 32-bit systems?
I'm fine with your decision to build both, but I'm wondering if it's really necessary
to keep two versions for a "test tool"?

Helge

> Replace (unsigned) int by u32/s32 in the 32-bit version for clarity.
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
>   drawops/generic.c | 127 +++++++++++++++++++++++++++++++++++++++++-----
>   include/types.h   |   3 ++
>   2 files changed, 116 insertions(+), 14 deletions(-)
>
> diff --git a/drawops/generic.c b/drawops/generic.c
> index 5fd971b59bc698fe..c4cfad3223773a23 100644
> --- a/drawops/generic.c
> +++ b/drawops/generic.c
> @@ -305,8 +305,98 @@ static void fill_ellipse_points(u32 cx, u32 cy, u32 x, u32 y, pixel_t pixel)
>       }
>   }
>
> -static void do_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel,
> -		       draw_func_t draw_inner, draw_func_t draw_outer)
> +static void do_ellipse_32(u32 x, u32 y, u32 a, u32 b, pixel_t pixel,
> +			  draw_func_t draw_inner, draw_func_t draw_outer)
> +{
> +    u32 a2 = a*a;
> +    u32 b2 = b*b;
> +
> +    if (a <= b) {
> +	u32 x1 = 0;
> +	u32 y1 = b;
> +	s32 S = a2*(1-2*b)+2*b2;
> +	s32 T = b2-2*a2*(2*b-1);
> +	u32 dT1 = 4*b2;
> +	u32 dS1 = dT1+2*b2;
> +	s32 dS2 = -4*a2*(b-1);
> +	s32 dT2 = dS2+2*a2;
> +
> +	while (1) {
> +	    if (S < 0) {
> +		if (draw_inner)
> +		    draw_inner(x, y, x1, y1, pixel);
> +		S += dS1;
> +		T += dT1;
> +		dS1 += 4*b2;
> +		dT1 += 4*b2;
> +		x1++;
> +	    } else if (T < 0) {
> +		draw_outer(x, y, x1, y1, pixel);
> +		if (y1 == 0)
> +		    break;
> +		S += dS1+dS2;
> +		T += dT1+dT2;
> +		dS1 += 4*b2;
> +		dT1 += 4*b2;
> +		dS2 += 4*a2;
> +		dT2 += 4*a2;
> +		x1++;
> +		y1--;
> +	    } else {
> +		draw_outer(x, y, x1, y1, pixel);
> +		if (y1 == 0)
> +		    break;
> +		S += dS2;
> +		T += dT2;
> +		dS2 += 4*a2;
> +		dT2 += 4*a2;
> +		y1--;
> +	    }
> +	}
> +    } else {
> +	u32 x1 = a;
> +	u32 y1 = 0;
> +	s32 S = b2*(1-2*a)+2*a2;
> +	s32 T = a2-2*b2*(2*a-1);
> +	u32 dT1 = 4*a2;
> +	u32 dS1 = dT1+2*a2;
> +	s32 dS2 = -4*b2*(a-1);
> +	s32 dT2 = dS2+2*b2;
> +
> +	draw_outer(x, y, x1, y1, pixel);
> +	do {
> +	    if (S < 0) {
> +		S += dS1;
> +		T += dT1;
> +		dS1 += 4*a2;
> +		dT1 += 4*a2;
> +		y1++;
> +		draw_outer(x, y, x1, y1, pixel);
> +	    } else if (T < 0) {
> +		S += dS1+dS2;
> +		T += dT1+dT2;
> +		dS1 += 4*a2;
> +		dT1 += 4*a2;
> +		dS2 += 4*b2;
> +		dT2 += 4*b2;
> +		x1--;
> +		y1++;
> +		draw_outer(x, y, x1, y1, pixel);
> +	    } else {
> +		S += dS2;
> +		T += dT2;
> +		dS2 += 4*b2;
> +		dT2 += 4*b2;
> +		x1--;
> +		if (draw_inner)
> +		    draw_inner(x, y, x1, y1, pixel);
> +	    }
> +	} while (x1 > 0);
> +    }
> +}
> +
> +static void do_ellipse_64(u32 x, u32 y, u32 a, u32 b, pixel_t pixel,
> +			  draw_func_t draw_inner, draw_func_t draw_outer)
>   {
>       u32 a2 = a*a;
>       u32 b2 = b*b;
> @@ -314,12 +404,12 @@ static void do_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel,
>       if (a <= b) {
>   	u32 x1 = 0;
>   	u32 y1 = b;
> -	int S = a2*(1-2*b)+2*b2;
> -	int T = b2-2*a2*(2*b-1);
> -	unsigned int dT1 = 4*b2;
> -	unsigned int dS1 = dT1+2*b2;
> -	int dS2 = -4*a2*(b-1);
> -	int dT2 = dS2+2*a2;
> +	s64 S = a2*(1-2LL*b)+2LL*b2;
> +	s64 T = b2-2LL*a2*(2LL*b-1);
> +	u64 dT1 = 4*b2;
> +	u64 dS1 = dT1+2*b2;
> +	s64 dS2 = -4LL*a2*(b-1);
> +	s64 dT2 = dS2+2*a2;
>
>   	while (1) {
>   	    if (S < 0) {
> @@ -356,12 +446,12 @@ static void do_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel,
>       } else {
>   	u32 x1 = a;
>   	u32 y1 = 0;
> -	int S = b2*(1-2*a)+2*a2;
> -	int T = a2-2*b2*(2*a-1);
> -	unsigned int dT1 = 4*a2;
> -	unsigned int dS1 = dT1+2*a2;
> -	int dS2 = -4*b2*(a-1);
> -	int dT2 = dS2+2*b2;
> +	s64 S = b2*(1-2LL*a)+2LL*a2;
> +	s64 T = a2-2LL*b2*(2LL*a-1);
> +	u64 dT1 = 4*a2;
> +	u64 dS1 = dT1+2*a2;
> +	s64 dS2 = -4LL*b2*(a-1);
> +	s64 dT2 = dS2+2*b2;
>
>   	draw_outer(x, y, x1, y1, pixel);
>   	do {
> @@ -395,6 +485,15 @@ static void do_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel,
>       }
>   }
>
> +static void do_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel,
> +		       draw_func_t draw_inner, draw_func_t draw_outer)
> +{
> +    if ((a + 576) * (b + 576) < 1440000)
> +	do_ellipse_32(x, y, a, b, pixel, draw_inner, draw_outer);
> +    else
> +	do_ellipse_64(x, y, a, b, pixel, draw_inner, draw_outer);
> +}
> +
>   void generic_draw_ellipse(u32 x, u32 y, u32 a, u32 b, pixel_t pixel)
>   {
>       if (a == b)
> diff --git a/include/types.h b/include/types.h
> index 9112ba6855b61eaa..0e3c76521469912f 100644
> --- a/include/types.h
> +++ b/include/types.h
> @@ -21,6 +21,9 @@ typedef unsigned short u16;
>   typedef unsigned int u32;
>   typedef unsigned long long u64;
>
> +typedef int s32;
> +typedef long long s64;
> +
>   #if defined(__LP64__) || defined(__alpha__) || defined(__ia64__) || \
>       defined(__mips64__) || defined(__powerpc64__) || defined(__s390x__) || \
>       defined(__sparc64__) || defined(__x86_64__) || defined(__hppa64__)

Re: [PATCH fbtest 12/17] drawops: Fix crash when drawing large ellipses

[Geert Uytterhoeven]

Hi Helge,

On Sun, Dec 15, 2024 at 4:08 PM Helge Deller <deller@gmx.de> wrote:
> On 12/15/24 11:45, Geert Uytterhoeven wrote:
> > "test002" crashes when run with a display resolution of e.g. 2560x1440
> > pixels, due to 32-bit overflow in the ellipse drawing routine.
> >
> > Fix this by creating a copy that uses 64-bit arithmetic.  Use a
> > heuristic to pick either the 32-bit or the 64-bit version, to avoid the
> > overhead of the 64-bit version on small systems with small displays.
>
> I see you always build the 32- and 64-bit versions, so when you mean
> overhead you mean runtime overhead, not compiled binary size overhead.

Exactly.

> So, just wondering:
> Did you maybe measured how much slower the 64-bit version is on slow 32-bit systems?
> I'm fine with your decision to build both, but I'm wondering if it's really necessary
> to keep two versions for a "test tool"?

On ARM Cortex-A9, draw_ellipse(400, 240, 300, 239, ...) with a
dummy (empty) set_pixel() method using the 64-bit version takes 44%
longer than the 32-bit version, so I think it is worthwhile to have
both versions.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH fbtest 12/17] drawops: Fix crash when drawing large ellipses

[Helge Deller]

On 12/18/24 17:29, Geert Uytterhoeven wrote:
> Hi Helge,
>
> On Sun, Dec 15, 2024 at 4:08 PM Helge Deller <deller@gmx.de> wrote:
>> On 12/15/24 11:45, Geert Uytterhoeven wrote:
>>> "test002" crashes when run with a display resolution of e.g. 2560x1440
>>> pixels, due to 32-bit overflow in the ellipse drawing routine.
>>>
>>> Fix this by creating a copy that uses 64-bit arithmetic.  Use a
>>> heuristic to pick either the 32-bit or the 64-bit version, to avoid the
>>> overhead of the 64-bit version on small systems with small displays.
>>
>> I see you always build the 32- and 64-bit versions, so when you mean
>> overhead you mean runtime overhead, not compiled binary size overhead.
>
> Exactly.
>
>> So, just wondering:
>> Did you maybe measured how much slower the 64-bit version is on slow 32-bit systems?
>> I'm fine with your decision to build both, but I'm wondering if it's really necessary
>> to keep two versions for a "test tool"?
>
> On ARM Cortex-A9, draw_ellipse(400, 240, 300, 239, ...) with a
> dummy (empty) set_pixel() method using the 64-bit version takes 44%
> longer than the 32-bit version, so I think it is worthwhile to have
> both versions.

Oh, I didn't expect that much.
I agree it's worthwhile to have both versions.

Helge

Re: [PATCH fbtest 00/17] Export feature and large ellipses support

[Geert Uytterhoeven]

On Sun, Dec 15, 2024 at 11:45 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Recently, I ran into a segmentation fault when running test002 ("Draw a
> grid and some ellipses") on a system with a display resolution of
> 2560x1440.  This turned out to be due to a 32-bit overflow in the
> ellipse drawing routines.
>
> This patch series:
>   - Adds support for operating on a virtual buffer in RAM instead of on
>     a real frame buffer device, and exporting the result as a PPM image,
>     which is useful for e.g. testing drawing algorithms on screen sizes
>     not supported by your hardware,
>   - Fixes the drawing of ellipses on large displays,
>   - Contains some small fixe and improvements.
>
> I intend to apply these to
> https://git.kernel.org/pub/scm/linux/kernel/git/geert/fbtest.git/.

Pushed, with the Cortex-A9 performance figure added to the
description for [PATCH fbtest 12/17].

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds