Add the picogame game engine module - #11199
Conversation
Retained-mode 2D engine for writing games in CircuitPython: a Scene with dirty-rect rendering over Sprite/Tilemap/Canvas/StripDraw/ Particles/Triangles layers, plus collision, noise and text helpers - and enough pseudo-3D primitives (project, raycast, mode-7, triangle batches) for simple 3D games. Renders portably through any BusDisplay; optional port backends add an async-DMA SPI path (raspberrypi, espressif) and a RAM-framebuffer target for DVI/HSTX scanout boards. Gated by CIRCUITPY_PICOGAME (off by default); enabled on pajenicko_picopad and adafruit_fruit_jam.
tannewt
left a comment
tannewt
left a comment
There was a problem hiding this comment.
Thanks for the PR! I've got a few organizational comments but most of the self-contained stuff is totally fine as-is.
| //| """Get the tile at (tx, ty) -> int; with ``value``, set it (and mark dirty) -> None. | ||
| //| The optional keyword ``flip_x``/``flip_y``/``transpose`` flags orient the tile - together |
There was a problem hiding this comment.
I'd split this into two different functions. This is a weird get/set pattern that I'm not a fan of.
There was a problem hiding this comment.
This is a complex change that alter the API. I need to think about it further. It's one of the first features in the engine and is used extensively in many places.
There was a problem hiding this comment.
I'm in favor of splitting it into tile() and set_tile() - it adds a verb when the state changes and results in slightly fewer changes overall (it's used in more than 200 places in the engine, games, docs and tutorials). It might even be a bit faster, since the read won't have to parse extra arguments.
There was a problem hiding this comment.
I'd have it get_tile() to match set_tile() but that sounds fine to me.
There was a problem hiding this comment.
In the end, it involved changes in more than 500 places, but it's done 🙂. There were far fewer getters than setters, so I implemented it as get_tile/set_tile, just as you wanted.
Review feedback. The types that were consolidated in __init__.c (Bitmap, Sprite, StripDraw, Triangles, Framebuffer) each get their own shared-bindings file, matching Canvas/Scene/Tilemap/Particles/Display, so __init__.c is the module level only: its docstring, the module functions and the globals table. The docstring now also says how picogame relates to displayio - same display object, different way to drive it, no retained pixels - which is the first thing a reader of that file should learn. The eight error messages the module added are replaced with ones CircuitPython already ships (mp_arg_validate_type / mp_arg_error_invalid / mp_arg_validate_length_min, and m_malloc_fail for the unreachable scene-cap guard): the module now contributes no new strings to the translations.
tannewt
left a comment
tannewt
left a comment
There was a problem hiding this comment.
Thanks for splitting these apart. Many of the primitives are giving me "displayio" vibes so I think we could work to integrate more of this in the long term. Totally fine to have it separate now.
| #if CIRCUITPY_PICOGAME_FAST_DISPLAY | ||
| { MP_ROM_QSTR(MP_QSTR_Display), MP_ROM_PTR(&picogame_display_type) }, | ||
| #endif |
There was a problem hiding this comment.
Generally I don't like attributes only added in some cases. The way we do this is by raising NotImplementedError from the shared-module/common-hal implementation to give a clearer error message. I think it'd be good to do here too.
There was a problem hiding this comment.
I replaced it with two new constants FAST_DISPLAY_SUPPORTED and FRAMEBUFFER_SUPPORTED for features detection (it is used in picogame libs).
The file header still said the types were consolidated here, which stopped being true when they moved into their own files, and the displayio paragraph now names the shared object instead of calling it "the seam". Splitting Sprite out also left `bitmap` documented twice - once in the attribute summary and again at its property - which mypy rejects, so `make check-stubs` (the docs job) failed. The summary entry is gone; the fuller description at the property stays.
Replace the combined getter/setter with two methods, per review. get_tile(tx, ty) is a plain 3-arg binding (no kwarg parsing on the read path); set_tile(tx, ty, value) keeps the flip_x/flip_y/transpose orientation keywords. Out-of-range behavior is unchanged: reads return 0, writes are ignored.
Builds without the corresponding backend get a stub type whose constructor raises NotImplementedError (the same core message the unsupported rgb444= path uses), so the module's attributes no longer vary with build flags. Adds FAST_DISPLAY_SUPPORTED and FRAMEBUFFER_SUPPORTED alongside RGB444_SUPPORTED for feature detection. +216 B on RP2040.
Two measured optimizations from device profiling: - fill_triangles: route the edge-slope divides through a 32-bit path when the deltas fit in 16 bits (they almost always do); the 64-bit soft-divide dominated the rasterizer on M33. - blit_bitmap_scaled: opaque PAL8 sprites at power-of-two scales (2x/4x/8x) skip the generic DDA scaler - one palette lookup per source pixel, repeated rows copied. Measured 1.5x on a full scaled blit (RP2350), byte-identical to the generic path across 200k randomized windows. Powers of two only: at other scales the generic step rounding samples differently, so those keep the existing path.
2 participants
First step of #11198: the picogame core C module.
The module itself is self-contained (
shared-bindings/picogame,shared-module/picogame, optional port backends incommon-hal/picogame). Outside of it this PR only touches:pajenicko_picopad- enables the engine. To fit it next to the Wi-Fi stack the board drops peripherals it physically lacks (_EVE,qrio,picodvi) and switches from -O3 to -O2 plus a few measured loop flags (engine kernels within ±1 % of -O3, ~150 KB smaller). 87.9 % of the firmware region used.adafruit_fruit_jam- enables the engine with the RAM-framebuffer target (DVI/HSTX scanout). 92.2 % used.locale/circuitpython.pot- regenerated.Left out for follow-up PRs (per the issue discussion): core1 rendering and the ROMFS asset region.
Both board builds, sphinx docs and translations pass locally.