From 3a1d208ee7669f5438462e039d2bfaa833188bd1 Mon Sep 17 00:00:00 2001 From: Brad Barnett <127794626+bdbarnett@users.noreply.github.com> Date: Thu, 8 Oct 2026 00:39:48 -0500 Subject: [PATCH] Build as a user C module in CircuitPython too CircuitPython's make-based ports read USER_C_MODULES exactly as MicroPython 1.29 does, so micropython.mk now builds pygraphics into CircuitPython-compatible firmware with no apply script: - micropython.mk passes -DPYGRAPHICS_USER_C_MODULE=1, so a build with CIRCUITPY=1 still registers the module (the shared-bindings path through apply_cp_patches.sh is unchanged). - The warnings the drawing code trips on purpose are repeated per object, where they land after the -Werror=sign-compare/float-equal CircuitPython's MCU ports append late. - Polygon coordinates are read with memcpy instead of a cast: a memoryview can start at any byte, and a Cortex-M0+ faults on an unaligned load (atmel-samd builds with -Werror=cast-align). - The MONO_HLSB copy a PNG save makes comes from the GC heap in a user C module build: atmel-samd has no C heap, so malloc() didn't link. --- micropython.mk | 16 +++++++++++++++- src/gfx_bindings_mp.c | 5 ++++- src/gfx_module_cpy.c | 4 ++-- src/gfx_module_mp.c | 4 ++-- src/gfx_png.c | 19 ++++++++++++++++++- src/gfx_png.h | 7 +++++-- src/gfx_shapes.c | 16 ++++++++++------ 7 files changed, 56 insertions(+), 15 deletions(-) diff --git a/micropython.mk b/micropython.mk index 6d63cb7..62deb23 100644 --- a/micropython.mk +++ b/micropython.mk @@ -5,7 +5,10 @@ PYGRAPHICS_MOD_DIR := $(USERMOD_DIR) -CFLAGS_USERMOD += -I$(PYGRAPHICS_MOD_DIR)/src -Wno-unused-function -Wno-sign-compare -Wno-unused-const-variable +# PYGRAPHICS_USER_C_MODULE tells the C that it was built through USER_C_MODULES, +# which CircuitPython builds also honour. Its unix port sets CIRCUITPY=1, and +# without this the module would skip MP_REGISTER_MODULE and never be importable. +CFLAGS_USERMOD += -DPYGRAPHICS_USER_C_MODULE=1 -I$(PYGRAPHICS_MOD_DIR)/src -Wno-unused-function -Wno-sign-compare -Wno-unused-const-variable # Arc/polygon use Q15 LUT in gfx_trig.h — no libm required. # --- which pygraphics this firmware was built from -------------------------- @@ -39,3 +42,14 @@ SRC_USERMOD_C += \ $(PYGRAPHICS_MOD_DIR)/src/gfx_png.c \ $(PYGRAPHICS_MOD_DIR)/src/gfx_capabilities.c \ $(PYGRAPHICS_MOD_DIR)/src/gfx_area_mp.c + +# Warnings the drawing code trips on purpose: comparing an angle against an +# exact 0.0f to skip rotation, mixed-sign loop bounds, tables only some builds +# use. CFLAGS_USERMOD above carries them for MicroPython. They are given again +# per object because CircuitPython's MCU ports append -Wsign-compare and +# -Wfloat-equal (under -Werror) after the user module flags, and the later +# flag wins; a target-specific flag lands last. The object path is the one py/py.mk gives a user module's source since +# MicroPython 1.29: $(BUILD)//.o. +PYGRAPHICS_OBJ_CFLAGS := -Wno-unused-function -Wno-sign-compare -Wno-unused-const-variable -Wno-float-equal +$(foreach _src,$(filter $(PYGRAPHICS_MOD_DIR)/%,$(SRC_USERMOD_C)),\ + $(eval $(BUILD)/$(notdir $(PYGRAPHICS_MOD_DIR))/$(patsubst $(PYGRAPHICS_MOD_DIR)/%.c,%.o,$(_src)): CFLAGS += $(PYGRAPHICS_OBJ_CFLAGS))) diff --git a/src/gfx_bindings_mp.c b/src/gfx_bindings_mp.c index a8ce6e9..b48643a 100644 --- a/src/gfx_bindings_mp.c +++ b/src/gfx_bindings_mp.c @@ -2027,7 +2027,10 @@ const mp_obj_module_t mp_module_pygraphics = { .globals = (mp_obj_dict_t *)&graphics_module_globals, }; -#if CIRCUITPY +/* CIRCUITPY alone means the shared-bindings build (apply_cp_patches.sh), which + * registers the module itself. A CircuitPython build through USER_C_MODULES + * registers it here, as MicroPython does. */ +#if CIRCUITPY && !defined(PYGRAPHICS_USER_C_MODULE) /* Alias for any leftover shared-bindings that still reference mp_module_graphics. */ const mp_obj_module_t mp_module_graphics = { .base = { &mp_type_module }, diff --git a/src/gfx_module_cpy.c b/src/gfx_module_cpy.c index 6706d19..038ecf3 100644 --- a/src/gfx_module_cpy.c +++ b/src/gfx_module_cpy.c @@ -1199,7 +1199,7 @@ static PyObject *cpy_save(const gfx_fb_t *fb, const char *path) { } PyObject *mod = PyImport_ImportModule("pngio"); if (mod == NULL) { - free(owned); + gfx_png_release(owned); if (PyErr_ExceptionMatches(PyExc_ImportError)) { PyErr_SetString(PyExc_ImportError, "saving PNG needs pngio: install pydevices-desktop and Pillow (pip install pillow)"); @@ -1209,7 +1209,7 @@ static PyObject *cpy_save(const gfx_fb_t *fb, const char *path) { PyObject *png = NULL, *enc = NULL, *src = NULL, *pos = NULL, *kw = NULL, *meth = NULL; enc = PyObject_CallMethod(mod, "PngEncoder", NULL); src = PyByteArray_FromStringAndSize((const char *)buf, (Py_ssize_t)len); - free(owned); + gfx_png_release(owned); if (enc && src) { meth = PyObject_GetAttrString(enc, "encode"); pos = Py_BuildValue("(Oii)", src, (int)fb->width, (int)fb->height); diff --git a/src/gfx_module_mp.c b/src/gfx_module_mp.c index de81ab8..9dfcecb 100644 --- a/src/gfx_module_mp.c +++ b/src/gfx_module_mp.c @@ -173,13 +173,13 @@ static mp_obj_t gfxmp_png_bytes(const gfx_fb_t *fb) { png = mp_call_method_n_kw(3, 1, args); nlr_pop(); } else { - free(owned); + gfx_png_release(owned); if (mp_obj_exception_match(MP_OBJ_FROM_PTR(nlr.ret_val), MP_OBJ_FROM_PTR(&mp_type_ImportError))) { mp_raise_msg(&mp_type_ImportError, MP_ERROR_TEXT("saving PNG needs pngio (micropython-pydevices modules/pngio)")); } nlr_jump(nlr.ret_val); } - free(owned); + gfx_png_release(owned); return png; } diff --git a/src/gfx_png.c b/src/gfx_png.c index fbc93f9..e29edec 100644 --- a/src/gfx_png.c +++ b/src/gfx_png.c @@ -5,6 +5,23 @@ #include #include +/* Built as a user C module (micropython.mk), the copy comes from the GC heap + * and is left to the collector: some CircuitPython ports, atmel-samd among + * them, have no C heap, so malloc() doesn't link there. Everywhere else + * (CPython, MicroPython's CMake ports) it is malloc() and free(). */ +#if defined(PYGRAPHICS_USER_C_MODULE) +#include "py/misc.h" +#define GFX_PNG_ALLOC(n) m_malloc(n) +#define GFX_PNG_FREE(p) ((void)(p)) +#else +#define GFX_PNG_ALLOC(n) malloc(n) +#define GFX_PNG_FREE(p) free(p) +#endif + +void gfx_png_release(uint8_t *owned) { + GFX_PNG_FREE(owned); +} + int gfx_png_named(const char *path) { size_t n = strlen(path); return n >= 4 && path[n - 4] == '.' && (path[n - 3] | 0x20) == 'p' @@ -30,7 +47,7 @@ int gfx_png_source(const gfx_fb_t *fb, int *fmt, const uint8_t **buf, size_t *le return 0; case GFX_MHLSB: { /* one bit a pixel, widened to grey */ - uint8_t *g = (uint8_t *)malloc(px ? px : 1); + uint8_t *g = (uint8_t *)GFX_PNG_ALLOC(px ? px : 1); if (g == NULL) { return -1; } diff --git a/src/gfx_png.h b/src/gfx_png.h index caf4bb7..0688488 100644 --- a/src/gfx_png.h +++ b/src/gfx_png.h @@ -24,8 +24,11 @@ int gfx_png_wanted(const gfx_fb_t *fb, const char *path); int gfx_png_named(const char *path); /* The pixels for pngio: its format code, and the bytes -- the framebuffer's - * own, or for MONO_HLSB a GS8 copy returned in *owned (malloc'd; the caller - * frees it). -1 for a format PNG can't take. */ + * own, or for MONO_HLSB a GS8 copy returned in *owned (the caller hands it to + * gfx_png_release). -1 for a format PNG can't take. */ int gfx_png_source(const gfx_fb_t *fb, int *fmt, const uint8_t **buf, size_t *len, uint8_t **owned); +/* Release what gfx_png_source returned in *owned; NULL is fine. */ +void gfx_png_release(uint8_t *owned); + #endif /* GFX_PNG_H */ diff --git a/src/gfx_shapes.c b/src/gfx_shapes.c index 1864007..2a03428 100644 --- a/src/gfx_shapes.c +++ b/src/gfx_shapes.c @@ -253,6 +253,9 @@ int gfx_shapes_poly_int_from_buffer(const void *buf, size_t len, size_t itemsize return -1; } const char *ptr = (const char *)buf + offset; + /* memcpy, not a cast: a memoryview slice can start at any byte, and a + * Cortex-M0+ (RP2040) faults on an unaligned halfword or word load. */ + #define GFX_READ(type) do { type v_; memcpy(&v_, ptr, sizeof(v_)); *out = (int)v_; } while (0) switch (fmt[0]) { case 'b': *out = *(const signed char *)ptr; @@ -261,26 +264,27 @@ int gfx_shapes_poly_int_from_buffer(const void *buf, size_t len, size_t itemsize *out = *(const unsigned char *)ptr; return 0; case 'h': - *out = *(const short *)ptr; + GFX_READ(short); return 0; case 'H': - *out = *(const unsigned short *)ptr; + GFX_READ(unsigned short); return 0; case 'i': - *out = *(const int *)ptr; + GFX_READ(int); return 0; case 'I': - *out = (int)*(const unsigned int *)ptr; + GFX_READ(unsigned int); return 0; case 'l': - *out = (int)*(const long *)ptr; + GFX_READ(long); return 0; case 'L': - *out = (int)*(const unsigned long *)ptr; + GFX_READ(unsigned long); return 0; default: return -1; } + #undef GFX_READ } gfx_area_t gfx_shapes_poly(const gfx_canvas_t *canvas, int x, int y, const void *coords, size_t coords_len, size_t itemsize, const char *fmt, int col, int fill) {