Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an M5 Tab5 (ESP32-P4) LCD output backend to NightDriverStrip and updates core rendering/effects paths to better support larger logical matrix resolutions (e.g., 160×90 scaling to 1280×720) while keeping performance acceptable.
Changes:
- Introduces a new
M5LCDoutput driver/backend (M5TabGFX) and integrates it into setup, device config, and settings UI/options. - Widens various “pixel count”/index plumbing (
XY,xy(),PostProcessFrame, WiFi/local draw counts) tosize_tand adds row-major fast paths for blur/noise/transforms. - Updates several matrix effects (GIF/JPEG scaling, geometry, stack usage, motion scaling) to render correctly beyond the historical 64×32/63×32 constraints.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/ws281xgfx.cpp | Updates PostProcessFrame signature to size_t pixel counts. |
| include/ws281xgfx.h | Updates PostProcessFrame/xy() return types to size_t. |
| src/webserver_settings.cpp | Accepts m5lcd/apa102 as outputs.driver runtime settings values. |
| src/taskmgr.cpp | Adjusts watchdog registration/reconfiguration logic, with Tab5-specific handling. |
| src/network.cpp | Avoids Tab5-hosted WiFi accessor calls in event callback; refines reconnect logic. |
| src/main.cpp | Adds Tab5 init (M5Unified, rotation, SDIO WiFi pins) and selects M5 LCD hardware init. |
| src/m5tabgfx.cpp | New Tab5 LCD backend: PSRAM framebuffer + scaling into the DSI scanout buffer, caption compositing. |
| include/m5tabgfx.h | Declares M5TabGFX backend and row-major xy() implementation. |
| src/hub75gfx.cpp | Updates PostProcessFrame signature; improves inward/outward movement scaling; avoids caption copy. |
| include/hub75gfx.h | Updates xy() return type and PostProcessFrame signature; makes SetCaption override explicit. |
| src/gfxbase.cpp | Adds row-major optimized blur implementations; changes global XY() to return size_t; fixes polar angle encoding. |
| include/gfxbase.h | Changes XY/xy() to size_t; adds FillGetNoiseEdges() and SetCaption() hook; updates PostProcessFrame signature. |
| src/gfxbase_transforms.cpp | Adds row-major fast paths and scalable step sizes for MoveInwardX/MoveOutwardsX. |
| src/gfxbase_noise.cpp | Adds FillGetNoiseEdges() and row-major fast paths for general fractional noise movers. |
| src/effects.cpp | Adds JPEG output scaling transform support for matrix JPEG decoding. |
| include/effects.h | Extends ConfigureMatrixJpegDecoder to accept optional source dimensions. |
| src/effectmanager.cpp | Enables splash effect manager init for M5 LCD builds too. |
| src/effectmanager_runtime.cpp | Uses generic GFXBase::SetCaption() instead of HUB75-only captioning. |
| src/drawing.cpp | Converts pixel-drawn counters to size_t and adapts delay logic API signatures. |
| src/deviceconfig.cpp | Adds driver name for M5LCD; introduces IsFixedMatrixBuild(); adjusts defaults (serpentine) accordingly. |
| include/deviceconfig.h | Adds M5LCD enum value, compiled-driver selection, and IsFixedMatrixBuild(). |
| src/deviceconfig_validation.cpp | Treats fixed-matrix builds (HUB75/M5LCD) as requiring compiled topology settings. |
| src/deviceconfig_unified.cpp | Adds m5lcd parsing; uses fixed-matrix logic for compiled max width/height. |
| src/deviceconfig_settings_specs.cpp | Adds m5lcd as a selectable output driver in schema-backed options. |
| include/globals.h | Adds USE_M5LCD and M5TAB feature flags and updates “exactly one transport” checks. |
| include/effects/strip/misceffects.h | Extends splash logo support to M5LCD and configures JPEG scaling for the logo draw. |
| include/effects/matrix/spectrumeffects.h | Fixes radial indexing math to scale correctly with wider matrices. |
| include/effects/matrix/PatternWave.h | Reworks wave drawing to scale and draw continuous lines at higher resolutions. |
| include/effects/matrix/PatternSwirl.h | Uses beatsin16 to prevent coordinate truncation on larger dimensions. |
| include/effects/matrix/PatternStocks.h | Scales text/graph layout for wider matrices; adjusts quote request points and graph spacing. |
| include/effects/matrix/PatternSMSmoke.h | Uses edge-noise refresh + alternates warp/blur axes to keep performance on large matrices. |
| include/effects/matrix/PatternRadar.h | Updates pixel index type to size_t. |
| include/effects/matrix/PatternPongClock.h | Scales ball/bat physics with matrix dimensions and adjusts bounds accordingly. |
| include/effects/matrix/PatternMisc.h | Scales Aurora-derived orbit effects to arbitrary aspect ratios/dimensions. |
| include/effects/matrix/PatternLife.h | Fixes stack overflow risk by CRC-ing alive state row-by-row instead of allocating a full temp matrix. |
| include/effects/matrix/PatternCircuit.h | Scales movement step with matrix width; reduces per-frame random load; fixes Start() override and init. |
| include/effects/matrix/PatternAnimatedGIF.h | Improves GIF scaling to fill arbitrary matrix sizes and avoids gaps when scaling up. |
| platformio.ini | Adds Tab5 build environment and Mesmerizer Tab configuration; adjusts PSRAM flags for certain envs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| void M5TabGFX::fillRectangle(int x0, int y0, int x1, int y1, CRGB color) | ||
| { | ||
| x0 = std::max(0, x0); | ||
| y0 = std::max(0, y0); | ||
| x1 = std::min(static_cast<int>(_width), x1); | ||
| y1 = std::min(static_cast<int>(_height), y1); | ||
|
|
||
| for (int y = y0; y < y1; ++y) | ||
| std::fill(leds + y * _width + x0, leds + y * _width + x1, color); | ||
| } |
rbergen
left a comment
rbergen
left a comment
There was a problem hiding this comment.
Overall this looks good - solid work getting the Tab5 backend up and running. Two things below I think are blockers and should be fixed before merge. The rest are questions worth answering, plus a few nits. Your call on those.
| if (!isStaAssociated() && | ||
| (explicitCredentials || millisAtLastAttempt == 0 || millis() - millisAtLastAttempt >= retryDelay)) |
There was a problem hiding this comment.
Blocker: !isStaAssociated() gates the whole reconnect block, including the explicit-credentials path. That contradicts the comment right above it: if we're already associated and Improv submits new credentials, we now skip reconnecting entirely, so we keep serving the old network. Not scoped to the Tab5, it's a global change. Needs a fix, or a narrower guard around just the async-race case it's meant to address.
| CRGB *scratch = static_cast<CRGB *>( | ||
| heap_caps_calloc(width * 2U, sizeof(CRGB), MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT)); | ||
| if (!scratch) | ||
| throw std::runtime_error("Unable to allocate native blur scratch rows"); |
There was a problem hiding this comment.
Blocker: The row-major fast path in blurColumns mallocs and frees scratch memory on every call (same pattern repeats around line 654). HUB75GFX::xy() is row-major too, so this hits every existing HUB75 build, not just the Tab5, and BlurFrame runs once a frame in about ten built-in effects. That's heap churn in a per-frame hot path, on MALLOC_CAP_INTERNAL, on a device that runs 24/7. Make the buffer persistent and allocate it once.
| constexpr uint32_t kFadeInTime = 500; | ||
| constexpr uint32_t kFadeOutTime = 1000; |
There was a problem hiding this comment.
Nit: kFadeInTime/kFadeOutTime are defined twice (again at 132-133). Worth hoisting to class-level constants so they can't drift apart.
|
|
||
| [dev_m5_tab5] | ||
| extends = base | ||
| platform = https://github.com/pioarduino/platform-espressif32/releases/download/55.03.31-1/platform-espressif32.zip |
There was a problem hiding this comment.
Nit: dev_m5_tab5 pins to a direct release-zip URL on a third-party platform-espressif32 fork. Probably unavoidable until P4 support lands upstream, worth a comment saying so, so it's not mistaken for an oversight later.
Standardized every ESP32 environment on pinned pioarduino 55.03.37.This is the first release supporting Python 3.14 on macOS/Linux. Removed the mixture of official platformio/espressif32 and pioarduino platforms. Updated legacy partition tables to meet the newer toolchain’s minimum writable-NVS size. Verified from an empty PLATFORMIO_CORE_DIR, simulating a fresh enlistment under Python 3.14: mesmerizer: success mesmerizer_tab: success Coding-standard audits: success git diff --check: success The build now downloads the compatible pioarduino platform and its tools directly from [platformio.ini](/Volumes/SSD-RAID/source/repos/NightDriverStrip/platformio.ini), without requiring developers to downgrade Python or modify VS Code settings.
This adds M5 TAB support, including its 1280x720 display. Renders all effects, some better than others. FIxed many effects to "draw up" in resolution, as many never had to draw outside the 63x32 box before!
mainas the target branch.