From 09ec0908cc5d8789f6bb9e3758f67be72a77e374 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Sun, 24 May 2026 17:47:03 +0200 Subject: [PATCH] Some X3 optimisations for band striping --- lib/GfxRenderer/GfxRenderer.cpp | 66 ++++++++++++++++++-- lib/GfxRenderer/GfxRenderer.h | 41 +++++++++++- open-x4-sdk | 2 +- src/activities/reader/EpubReaderActivity.cpp | 46 ++++++++------ 4 files changed, 127 insertions(+), 28 deletions(-) diff --git a/lib/GfxRenderer/GfxRenderer.cpp b/lib/GfxRenderer/GfxRenderer.cpp index 984f9b30..3d16d584 100644 --- a/lib/GfxRenderer/GfxRenderer.cpp +++ b/lib/GfxRenderer/GfxRenderer.cpp @@ -1945,6 +1945,32 @@ void GfxRenderer::beginStripTarget(uint8_t* scratch, int stripY0, int stripRows) stripY0_ = stripY0; stripRows_ = stripRows; stripActive_ = true; + + // Latch the orientation→phyY linear coefficients used by glyphIntersectsStrip() + // so the cull is one multiply-add per bbox corner instead of a switch. + // Derived from rotateCoordinates() with only the y-output retained. + switch (getOrientation()) { + case Portrait: + stripPhyYStepX_ = -1; + stripPhyYStepY_ = 0; + stripPhyYBase_ = panelHeight - 1; + break; + case LandscapeClockwise: + stripPhyYStepX_ = 0; + stripPhyYStepY_ = -1; + stripPhyYBase_ = panelHeight - 1; + break; + case PortraitInverted: + stripPhyYStepX_ = 1; + stripPhyYStepY_ = 0; + stripPhyYBase_ = 0; + break; + case LandscapeCounterClockwise: + stripPhyYStepX_ = 0; + stripPhyYStepY_ = 1; + stripPhyYBase_ = 0; + break; + } } void GfxRenderer::endStripTarget() const { @@ -1954,16 +1980,44 @@ void GfxRenderer::endStripTarget() const { stripRows_ = 0; } +bool GfxRenderer::acquireStripScratch() { + if (stripScratch_) return true; + if (panelWidthBytes == 0 || panelHeight == 0) { + LOG_ERR("GFX", "acquireStripScratch called before begin()"); + return false; + } + int rows = STRIP_SCRATCH_TARGET_BYTES / panelWidthBytes; + if (rows < 1) rows = 1; + if (rows > static_cast(panelHeight)) rows = panelHeight; + const size_t bytes = static_cast(panelWidthBytes) * rows; + stripScratch_ = static_cast(heap_caps_malloc(bytes, MALLOC_CAP_8BIT | MALLOC_CAP_DEFAULT)); + if (!stripScratch_) { + LOG_INF("GFX", "Strip scratch alloc failed (%zu bytes)", bytes); + return false; + } + stripScratchRows_ = rows; + return true; +} + +void GfxRenderer::releaseStripScratch() { + if (!stripScratch_) return; + heap_caps_free(stripScratch_); + stripScratch_ = nullptr; + stripScratchRows_ = 0; +} + bool GfxRenderer::glyphIntersectsStrip(int x0, int y0, int x1, int y1) const { if (!stripActive_) { return true; } - // Rotate the two opposite bbox corners to physical coords. For 90-degree - // orientations the physical bbox stays axis-aligned, so min/max of the two - // rotated corners' Y bounds the glyph's physical y-extent. - int ax, ay, bx, by; - rotateCoordinates(getOrientation(), x0, y0, &ax, &ay, panelWidth, panelHeight); - rotateCoordinates(getOrientation(), x1, y1, &bx, &by, panelWidth, panelHeight); + // Use the precomputed (stepX, stepY, base) latched in beginStripTarget() so + // each call is two multiply-adds + a range check, no rotateCoordinates + // switch. The four 90-degree orientations all reduce to "phyY depends on + // exactly one of (x, y)" — exactly one of stepX/stepY is non-zero — so phyY + // is monotonic across the bbox and the two opposite-corner phyY values + // bracket the full physical y-extent. + const int ay = stripPhyYStepX_ * x0 + stripPhyYStepY_ * y0 + stripPhyYBase_; + const int by = stripPhyYStepX_ * x1 + stripPhyYStepY_ * y1 + stripPhyYBase_; const int minY = ay < by ? ay : by; const int maxY = ay > by ? ay : by; return !(maxY < stripY0_ || minY >= stripY0_ + stripRows_); diff --git a/lib/GfxRenderer/GfxRenderer.h b/lib/GfxRenderer/GfxRenderer.h index dad90cdd..4b0567b1 100644 --- a/lib/GfxRenderer/GfxRenderer.h +++ b/lib/GfxRenderer/GfxRenderer.h @@ -85,6 +85,26 @@ class GfxRenderer { mutable int stripRows_ = 0; mutable bool stripActive_ = false; + // Precomputed orientation→physicalY linear coefficients for the band-cull + // fast path. Latched once in beginStripTarget() and used by every + // glyphIntersectsStrip() call to avoid the 4-case rotateCoordinates switch + // per glyph. phyY = stripPhyYStepX_ * x + stripPhyYStepY_ * y + stripPhyYBase_, + // with steps in {-1, 0, 1}. Orientation can't change mid-pass; the strip + // session is the natural latch point. + mutable int8_t stripPhyYStepX_ = 0; + mutable int8_t stripPhyYStepY_ = 0; + mutable int stripPhyYBase_ = 0; + + // Session-owned strip scratch. acquireStripScratch() allocates once (sized + // STRIP_SCRATCH_TARGET_BYTES, rounded to a whole number of rows of + // panelWidthBytes) and the buffer persists until releaseStripScratch(). + // Allocating per page turn fragments the tight ESP32-C3 heap badly enough + // to cause AA to suspend after a few pages; hold one buffer for the reader + // session instead. The strip height we pick at acquire time is exposed via + // getStripScratchRows() so the caller plans its band loop around it. + uint8_t* stripScratch_ = nullptr; + int stripScratchRows_ = 0; + void renderChar(const EpdFontFamily& fontFamily, uint32_t cp, int* x, int* y, bool pixelState, EpdFontFamily::Style style) const; void freeBwBufferChunks(); @@ -108,7 +128,10 @@ class GfxRenderer { orientation(static_cast(Portrait)), fadingFix(false), textDarkness(1) {} - ~GfxRenderer() { freeBwBufferChunks(); } + ~GfxRenderer() { + freeBwBufferChunks(); + releaseStripScratch(); + } static constexpr int VIEWABLE_MARGIN_TOP = 9; static constexpr int VIEWABLE_MARGIN_RIGHT = 3; @@ -245,6 +268,22 @@ class GfxRenderer { void beginStripTarget(uint8_t* scratch, int stripY0, int stripRows) const; void endStripTarget() const; + // Session-owned strip scratch lifecycle. Reader activities call acquire on + // onEnter() and release on onExit(); the buffer is then reused across all + // page-turn AA passes for that session. Acquire is idempotent and returns + // true on success or when the buffer is already held. Sizing uses the + // current panel geometry, so begin() must have run first. + bool acquireStripScratch(); + void releaseStripScratch(); + uint8_t* getStripScratch() const { return stripScratch_; } + int getStripScratchRows() const { return stripScratchRows_; } + + // Target byte budget for the session-owned strip scratch. ~24 KB lands at + // 240 rows on both X4 (100 B/row, panel 480) → 2 bands/plane and X3 + // (99 B/row, panel 528) → 3 bands/plane. acquire clamps to panelHeight so + // a smaller panel never over-allocates. + static constexpr int STRIP_SCRATCH_TARGET_BYTES = 24000; + // Active pixel-write target for raw writers that bypass drawPixel for speed. // When a strip target is active these return the band scratch plus its // physical-row origin and extent; otherwise the full framebuffer ([0, diff --git a/open-x4-sdk b/open-x4-sdk index 66e013fb..213e2065 160000 --- a/open-x4-sdk +++ b/open-x4-sdk @@ -1 +1 @@ -Subproject commit 66e013fb7f2d6a4102ce9a22991f5399e7c97ee7 +Subproject commit 213e206597dd21ae903619cd78edbf1c004ef902 diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 31b4b062..c4638e37 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -129,10 +129,11 @@ inline void logReaderMemSnapshot(const char*) {} // // Returns true when the strip path ran end-to-end (controller now holds the AA // planes and the live BW frame is clean). Returns false when the controller -// doesn't support strip grayscale OR the scratch allocation fails — caller -// should fall back to the legacy storeBwBufferRect path. +// doesn't support strip grayscale OR the session strip scratch isn't held +// (acquireStripScratch in onEnter failed, e.g. heap was already too tight at +// reader open). Caller should fall back to the legacy storeBwBufferRect path. // -// The page is re-rendered ceil(panelHeight/STRIP_ROWS) times per plane, but +// The page is re-rendered ceil(panelHeight/stripRows) times per plane, but // renderCharImpl culls out-of-band glyphs before bitmap decode so the cost // stays close to one render. Only renderTextOnly() is called here, matching the // legacy AA pass — images and HRs do not participate in grayscale. @@ -146,31 +147,27 @@ bool runTiledGrayscalePass(GfxRenderer& renderer, const Page& page, int fontId, // effect on the next page flip without rebooting. renderer.setFastGrayscaleLut(fastAA); - // Strip height trades scratch size for the number of re-renders. Each render - // pays layout + glyph-cull overhead even when bitmap decode is skipped, so - // fewer/bigger bands win as long as the scratch fits. 240 rows × ~100 bytes - // ≈ ~24 KB — still well below the legacy partial-snapshot footprint while - // cutting X3 (480 px) to 2 bands/plane and X4 (800 px) to 4 bands/plane. - constexpr int STRIP_ROWS = 240; - const int gh = renderer.getDisplayHeight(); - const int gwBytes = renderer.getDisplayWidthBytes(); - - auto scratch = std::unique_ptr(new (std::nothrow) uint8_t[static_cast(gwBytes) * STRIP_ROWS]); - if (!scratch) { - LOG_INF("ERS", "Tiled grayscale: scratch alloc failed (%d bytes); falling back to legacy path", - gwBytes * STRIP_ROWS); + // Strip scratch is owned by GfxRenderer for the reader session + // (acquireStripScratch in onEnter, releaseStripScratch in onExit). Allocating + // per page turn fragmented the ESP32-C3 heap badly enough to flip AA into the + // "suspended low memory" state after a few pages, so this path now refuses + // and falls back to the legacy snapshot if no session scratch is held. + uint8_t* const scratch = renderer.getStripScratch(); + const int stripRows = renderer.getStripScratchRows(); + if (!scratch || stripRows <= 0) { return false; } + const int gh = renderer.getDisplayHeight(); auto renderPlane = [&](GfxRenderer::RenderMode mode, bool lsbPlane) { renderer.setRenderMode(mode); - for (int y = 0; y < gh; y += STRIP_ROWS) { - const int rows = (gh - y < STRIP_ROWS) ? (gh - y) : STRIP_ROWS; - renderer.beginStripTarget(scratch.get(), y, rows); + for (int y = 0; y < gh; y += stripRows) { + const int rows = (gh - y < stripRows) ? (gh - y) : stripRows; + renderer.beginStripTarget(scratch, y, rows); renderer.clearScreen(0x00); page.renderTextOnly(renderer, fontId, marginLeft, contentTop); renderer.endStripTarget(); - renderer.writeGrayscalePlaneStrip(lsbPlane, scratch.get(), y, rows); + renderer.writeGrayscalePlaneStrip(lsbPlane, scratch, y, rows); } }; @@ -288,6 +285,14 @@ void EpubReaderActivity::onEnter() { } logReaderMemSnapshot("onEnter_after_orientation"); + // Allocate the strip scratch once per reader session so tiled grayscale + // (runTiledGrayscalePass) doesn't have to malloc ~24 KB on every page turn. + // Failure is non-fatal: the AA pass falls back to the legacy snapshot path. + if (!renderer.acquireStripScratch()) { + LOG_INF("ERS", "Strip scratch unavailable; tiled grayscale will fall back to legacy snapshot"); + } + logReaderMemSnapshot("onEnter_after_strip_scratch"); + epub->setupCacheDir(); logReaderMemSnapshot("onEnter_after_setupCacheDir"); @@ -402,6 +407,7 @@ void EpubReaderActivity::onExit() { epub.reset(); currentPageFootnotes.clear(); currentPageFootnotes.shrink_to_fit(); + renderer.releaseStripScratch(); logReaderMemSnapshot("onExit_after_release"); }