From 4ee406897bbde963b539dc2168e8ac536fefa072 Mon Sep 17 00:00:00 2001 From: Jeremy Klein Date: Mon, 25 May 2026 15:03:01 -0700 Subject: [PATCH] feat: tiled grayscale rendering to drop the storeBwBuffer peak (#2106) Tiled grayscale rendering to drop the storeBwBuffer peak largest contiguous free block (the value that actually drives OOM on the C3) from ~114 KB to ~82-90 KB. This renders each grayscale plane band-by-band into a small (~8 KB) scratch and streams each band straight to controller RAM (community-sdk writeGrayscalePlaneStrip), leaving the BW framebuffer intact. No save, no restore; controller RAM is re-synced for the next differential turn directly from the live framebuffer. Three writers honor the active band target so per-band re-rendering stays cheap and correct: - drawPixel (text) redirects writes to the band scratch and clips to it. - renderCharImpl skips glyphs whose physical y-extent is outside the band before the bitmap decode (glyphIntersectsStrip), so the per-band re-render doesn't pay N x glyph decode. - DirectPixelWriter (images) writes the band scratch via getWriteTarget instead of the framebuffer. Without this, image pixels wrote the live BW frame directly and cleanup re-synced that corruption, leaving thin outlines after navigating away from an image. Controller specifics live in the SDK (X4 setRamArea windowing, X3 PTL); the reader checks supportsStripGrayscale() and is otherwise controller-agnostic. Measured on hardware (X4 and X3, text and images, visually correct): - Grayscale scratch ~8 KB vs ~50 KB save; largest contiguous free block held at full size during grayscale instead of dropping ~25-32 KB. - X4 text page about +25 ms/page; X3 page time is dominated by its intrinsic grayscale refresh, not tiling. Depends on community-sdk #13 (the writeGrayscalePlaneStrip API). The submodule bump here points at that branch, so until #13 merges the submodule won't resolve from upstream and CI will fail there; keeping this a draft until then. Will rebase onto master and re-point the submodule to the merged SDK commit once #13 lands. Did you use AI tools to help write this code? partial --- lib/Epub/Epub/converters/DirectPixelWriter.h | 17 ++- lib/GfxRenderer/GfxRenderer.cpp | 81 +++++++++++- lib/GfxRenderer/GfxRenderer.h | 43 ++++++ lib/hal/HalDisplay.cpp | 7 + lib/hal/HalDisplay.h | 6 + open-x4-sdk | 2 +- src/activities/reader/EpubReaderActivity.cpp | 132 +++++++++++++------ 7 files changed, 244 insertions(+), 44 deletions(-) diff --git a/lib/Epub/Epub/converters/DirectPixelWriter.h b/lib/Epub/Epub/converters/DirectPixelWriter.h index bc66c2f7..25cb5755 100644 --- a/lib/Epub/Epub/converters/DirectPixelWriter.h +++ b/lib/Epub/Epub/converters/DirectPixelWriter.h @@ -16,6 +16,12 @@ struct DirectPixelWriter { uint8_t* fb; GfxRenderer::RenderMode mode; uint16_t displayWidthBytes; // Runtime framebuffer stride (X4: 100, X3: 99) + // Active write target: for tiled grayscale, fb is the band scratch, originY is + // the band's top physical row, and clipRows is the band height. Off-band + // pixels are dropped. With no strip active these collapse to the full frame + // (originY 0, clipRows panelHeight) so the clip doubles as a bounds guard. + int originY; + int clipRows; // Orientation is collapsed into a linear transform: // phyX = phyXBase + x * phyXStepX + y * phyXStepY @@ -28,7 +34,9 @@ struct DirectPixelWriter { int rowPhyXBase, rowPhyYBase; void init(GfxRenderer& renderer) { - fb = renderer.getFrameBuffer(); + fb = renderer.getWriteTarget(); + originY = renderer.getWriteOriginY(); + clipRows = renderer.getWriteRows(); mode = renderer.getRenderMode(); displayWidthBytes = renderer.getDisplayWidthBytes(); @@ -120,7 +128,12 @@ struct DirectPixelWriter { const int phyX = rowPhyXBase + logicalX * phyXStepX; const int phyY = rowPhyYBase + logicalX * phyYStepX; - const uint16_t byteIndex = phyY * displayWidthBytes + (phyX >> 3); + // Band-local row. The unsigned compare drops both off-band pixels (strip + // mode) and any out-of-frame row (full-frame mode) in one branch. + const int sy = phyY - originY; + if (static_cast(sy) >= static_cast(clipRows)) return; + + const uint16_t byteIndex = static_cast(sy * displayWidthBytes + (phyX >> 3)); const uint8_t bitMask = 1 << (7 - (phyX & 7)); if (state) { diff --git a/lib/GfxRenderer/GfxRenderer.cpp b/lib/GfxRenderer/GfxRenderer.cpp index a294531a..d6859cc3 100644 --- a/lib/GfxRenderer/GfxRenderer.cpp +++ b/lib/GfxRenderer/GfxRenderer.cpp @@ -148,6 +148,23 @@ static void renderCharImpl(const GfxRenderer& renderer, GfxRenderer::RenderMode const int left = glyph->left; const int top = glyph->top; + // Tiled-grayscale band culling: if this glyph's physical y-extent is entirely + // outside the active strip, skip it before the expensive bitmap decode. This + // is what makes per-band re-rendering cheap. No-op outside strip mode. + if constexpr (rotation == TextRotation::Rotated90CW) { + const int ob = cursorX + fontData->ascender - top; + const int ib = cursorY - left; + if (!renderer.glyphIntersectsStrip(ob, ib - (width - 1), ob + height - 1, ib)) { + return; + } + } else { + const int gx0 = cursorX + left; + const int gy0 = cursorY - top; + if (!renderer.glyphIntersectsStrip(gx0, gy0, gx0 + width - 1, gy0 + height - 1)) { + return; + } + } + const uint8_t* bitmap = renderer.getGlyphBitmap(fontData, glyph); if (bitmap != nullptr) { @@ -238,14 +255,26 @@ void GfxRenderer::drawPixel(const int x, const int y, const bool state) const { return; } + // Tiled grayscale: redirect writes to the strip scratch and clip to the + // current band. Single predictable branch on the hot per-pixel path. + uint8_t* target = frameBuffer; + uint32_t rowY = static_cast(phyY); + if (_stripActive) { + if (phyY < _stripY0 || phyY >= _stripY0 + _stripRows) { + return; // pixel outside the band currently being rendered + } + target = _stripBuf; + rowY = static_cast(phyY - _stripY0); + } + // Calculate byte position and bit position - const uint32_t byteIndex = static_cast(phyY) * panelWidthBytes + (phyX / 8); + const uint32_t byteIndex = rowY * panelWidthBytes + (phyX / 8); const uint8_t bitPosition = 7 - (phyX % 8); // MSB first if (state) { - frameBuffer[byteIndex] &= ~(1 << bitPosition); // Clear bit + target[byteIndex] &= ~(1 << bitPosition); // Clear bit } else { - frameBuffer[byteIndex] |= 1 << bitPosition; // Set bit + target[byteIndex] |= 1 << bitPosition; // Set bit } } @@ -964,9 +993,47 @@ static unsigned long start_ms = 0; void GfxRenderer::clearScreen(const uint8_t color) const { start_ms = millis(); + if (_stripActive) { + // Clear only the active band's scratch, not the shared framebuffer. + memset(_stripBuf, color, static_cast(panelWidthBytes) * _stripRows); + return; + } display.clearScreen(color); } +void GfxRenderer::beginStripTarget(uint8_t* scratch, int stripY0, int stripRows) const { + // Band is caller-guaranteed in-bounds (the reader's grayscale loop computes + // it); assert catches future misuse in debug before it mis-renders or wraps + // the downstream uint16_t cast in writeGrayscalePlaneStrip. + assert(scratch != nullptr && stripRows > 0 && stripY0 >= 0 && stripY0 <= static_cast(panelHeight) - stripRows); + _stripBuf = scratch; + _stripY0 = stripY0; + _stripRows = stripRows; + _stripActive = true; +} + +void GfxRenderer::endStripTarget() const { + _stripActive = false; + _stripBuf = nullptr; + _stripY0 = 0; + _stripRows = 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(orientation, x0, y0, &ax, &ay, panelWidth, panelHeight); + rotateCoordinates(orientation, x1, y1, &bx, &by, panelWidth, panelHeight); + const int minY = ay < by ? ay : by; + const int maxY = ay > by ? ay : by; + return !(maxY < _stripY0 || minY >= _stripY0 + _stripRows); +} + void GfxRenderer::invertScreen() const { for (uint32_t i = 0; i < frameBufferSize; i++) { frameBuffer[i] = ~frameBuffer[i]; @@ -1368,6 +1435,14 @@ void GfxRenderer::copyGrayscaleMsbBuffers() const { display.copyGrayscaleMsbBuff void GfxRenderer::displayGrayBuffer() const { display.displayGrayBuffer(fadingFix); } +void GfxRenderer::writeGrayscalePlaneStrip(bool lsbPlane, const uint8_t* scratch, int yStart, int numRows) const { + // Guard the uint16_t casts below: a negative would wrap to a huge length. + assert(yStart >= 0 && numRows > 0 && yStart <= static_cast(panelHeight) - numRows); + display.writeGrayscalePlaneStrip(lsbPlane, scratch, static_cast(yStart), static_cast(numRows)); +} + +bool GfxRenderer::supportsStripGrayscale() const { return display.supportsStripGrayscale(); } + void GfxRenderer::freeBwBufferChunks() { for (auto& bwBufferChunk : bwBufferChunks) { if (bwBufferChunk) { diff --git a/lib/GfxRenderer/GfxRenderer.h b/lib/GfxRenderer/GfxRenderer.h index a97b2b9f..78f10922 100644 --- a/lib/GfxRenderer/GfxRenderer.h +++ b/lib/GfxRenderer/GfxRenderer.h @@ -54,6 +54,18 @@ class GfxRenderer { // as before, concentrated in a single pointer instead of four fields. mutable FontCacheManager* fontCacheManager_ = nullptr; + // Tiled grayscale strip target. When active, drawPixel()/clearScreen() + // operate on a caller-owned scratch holding one horizontal band of physical + // rows [_stripY0, _stripY0 + _stripRows) (panelWidthBytes wide) instead of + // the shared framebuffer, clipping pixels outside the band. Lets grayscale + // planes render band-by-band straight to the controller without destroying + // the BW framebuffer (no storeBwBuffer). Mutable because the render path is + // const. See beginStripTarget()/endStripTarget(). + mutable uint8_t* _stripBuf = nullptr; + mutable int _stripY0 = 0; + mutable int _stripRows = 0; + mutable bool _stripActive = false; + void renderChar(const EpdFontFamily& fontFamily, uint32_t cp, int* x, int* y, bool pixelState, EpdFontFamily::Style style) const; void freeBwBufferChunks(); @@ -114,6 +126,31 @@ class GfxRenderer { void clearScreen(uint8_t color = 0xFF) const; void getOrientedViewableTRBL(int* outTop, int* outRight, int* outBottom, int* outLeft) const; + // Tiled grayscale strip target. While active, drawPixel() and clearScreen() + // operate on `scratch` (panelWidthBytes * stripRows bytes, holding physical + // rows [stripY0, stripY0 + stripRows)) instead of the framebuffer; pixels + // whose physical row falls outside the band are clipped. The clip is applied + // after the orientation rotate, so it is orientation-agnostic. Used to render + // grayscale planes band-by-band without a full second buffer. + void beginStripTarget(uint8_t* scratch, int stripY0, int stripRows) const; + void endStripTarget() const; + + // Band culling for tiled grayscale. Takes a glyph bounding box in logical + // screen coords and returns false only when a strip is active AND the box's + // physical y-extent lies entirely outside the active band, letting callers + // skip an expensive bitmap decode. Returns true when no strip is active. + // Corners are rotated to physical, so it is orientation-aware. + bool glyphIntersectsStrip(int x0, int y0, int x1, int y1) const; + + // Active pixel-write target for raw writers (DirectPixelWriter) 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, panelHeight)). Writers subtract the origin and clip to the + // extent, so they honor tiled-grayscale banding without per-pixel method calls. + uint8_t* getWriteTarget() const { return _stripActive ? _stripBuf : frameBuffer; } + int getWriteOriginY() const { return _stripActive ? _stripY0 : 0; } + int getWriteRows() const { return _stripActive ? _stripRows : panelHeight; } + // Drawing void drawPixel(int x, int y, bool state = true) const; void drawLine(int x1, int y1, int x2, int y2, bool state = true) const; @@ -172,6 +209,12 @@ class GfxRenderer { void copyGrayscaleLsbBuffers() const; void copyGrayscaleMsbBuffers() const; void displayGrayBuffer() const; + + // Tiled grayscale (X4): stream one band of a plane straight to controller RAM + // from `scratch` (panelWidthBytes * numRows, physical rows [yStart, yStart+ + // numRows)), bypassing the framebuffer. supportsStripGrayscale() gates use. + void writeGrayscalePlaneStrip(bool lsbPlane, const uint8_t* scratch, int yStart, int numRows) const; + bool supportsStripGrayscale() const; bool storeBwBuffer(); // Returns true if buffer was stored successfully void restoreBwBuffer(); // Restore and free the stored buffer void cleanupGrayscaleWithFrameBuffer() const; diff --git a/lib/hal/HalDisplay.cpp b/lib/hal/HalDisplay.cpp index a528db9f..11f08758 100644 --- a/lib/hal/HalDisplay.cpp +++ b/lib/hal/HalDisplay.cpp @@ -89,6 +89,13 @@ void HalDisplay::cleanupGrayscaleBuffers(const uint8_t* bwBuffer) { einkDisplay. void HalDisplay::displayGrayBuffer(bool turnOffScreen) { einkDisplay.displayGrayBuffer(turnOffScreen); } +void HalDisplay::writeGrayscalePlaneStrip(bool lsbPlane, const uint8_t* rows, uint16_t yStart, uint16_t numRows) { + einkDisplay.writeGrayscalePlaneStrip(lsbPlane ? EInkDisplay::GRAY_PLANE_LSB : EInkDisplay::GRAY_PLANE_MSB, rows, + yStart, numRows); +} + +bool HalDisplay::supportsStripGrayscale() const { return einkDisplay.supportsStripGrayscale(); } + uint16_t HalDisplay::getDisplayWidth() const { return einkDisplay.getDisplayWidth(); } uint16_t HalDisplay::getDisplayHeight() const { return einkDisplay.getDisplayHeight(); } diff --git a/lib/hal/HalDisplay.h b/lib/hal/HalDisplay.h index 3d43126a..628296cb 100644 --- a/lib/hal/HalDisplay.h +++ b/lib/hal/HalDisplay.h @@ -54,6 +54,12 @@ class HalDisplay { void displayGrayBuffer(bool turnOffScreen = false); + // Tiled grayscale: stream one band of a plane (lsbPlane selects LSB/MSB RAM) + // straight to the controller; supportsStripGrayscale() gates the path. See + // EInkDisplay::writeGrayscalePlaneStrip. + void writeGrayscalePlaneStrip(bool lsbPlane, const uint8_t* rows, uint16_t yStart, uint16_t numRows); + bool supportsStripGrayscale() const; + // Runtime geometry passthrough uint16_t getDisplayWidth() const; uint16_t getDisplayHeight() const; diff --git a/open-x4-sdk b/open-x4-sdk index ff444b0f..ee4c58dd 160000 --- a/open-x4-sdk +++ b/open-x4-sdk @@ -1 +1 @@ -Subproject commit ff444b0f2c9211341e8a4a6c9d8d9c6bac03f3e8 +Subproject commit ee4c58ddfe9fac7d0f8f657bf83768db79c8a96c diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index f1bde915..d36afc59 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -909,50 +910,105 @@ void EpubReaderActivity::renderContents(std::unique_ptr page, const int or } const auto tDisplay = millis(); - // Save bw buffer to reset buffer state after grayscale data sync - renderer.storeBwBuffer(); - const auto tBwStore = millis(); + // Tiled grayscale: render each plane band-by-band into a small scratch and + // stream straight to the controller, leaving the BW framebuffer intact so no + // full-frame storeBwBuffer is needed; controller RAM is re-synced from the + // live framebuffer afterward. The page is re-rendered ceil(H/STRIP_ROWS) times + // per plane, but renderCharImpl culls out-of-band glyphs before decode so the + // cost stays close to one render. Both text (drawPixel) and images + // (DirectPixelWriter) honor the active strip target. + if (SETTINGS.textAntiAliasing && renderer.supportsStripGrayscale()) { + constexpr int STRIP_ROWS = 80; + const int gh = renderer.getDisplayHeight(); + const int gwBytes = renderer.getDisplayWidthBytes(); - // grayscale rendering - // TODO: Only do this if font supports it - if (SETTINGS.textAntiAliasing) { - renderer.clearScreen(0x00); - renderer.setRenderMode(GfxRenderer::GRAYSCALE_LSB); - page->render(renderer, SETTINGS.getReaderFontId(), orientedMarginLeft, orientedMarginTop); - renderer.copyGrayscaleLsbBuffers(); - const auto tGrayLsb = millis(); + auto scratch = makeUniqueNoThrow(static_cast(gwBytes) * STRIP_ROWS); + if (!scratch) { + LOG_ERR("ERS", "OOM: grayscale strip scratch (%d bytes); skipping AA this page", gwBytes * STRIP_ROWS); + } else { + // Bands may be streamed in any order: X4 windows each via setRamArea, X3 + // via PTL. + renderer.setRenderMode(GfxRenderer::GRAYSCALE_LSB); + 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); + renderer.clearScreen(0x00); + page->render(renderer, SETTINGS.getReaderFontId(), orientedMarginLeft, orientedMarginTop); + renderer.endStripTarget(); + renderer.writeGrayscalePlaneStrip(true, scratch.get(), y, rows); + } + const auto tGrayLsb = millis(); - // Render and copy to MSB buffer - renderer.clearScreen(0x00); - renderer.setRenderMode(GfxRenderer::GRAYSCALE_MSB); - page->render(renderer, SETTINGS.getReaderFontId(), orientedMarginLeft, orientedMarginTop); - renderer.copyGrayscaleMsbBuffers(); - const auto tGrayMsb = millis(); + // MSB plane. + renderer.setRenderMode(GfxRenderer::GRAYSCALE_MSB); + 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); + renderer.clearScreen(0x00); + page->render(renderer, SETTINGS.getReaderFontId(), orientedMarginLeft, orientedMarginTop); + renderer.endStripTarget(); + renderer.writeGrayscalePlaneStrip(false, scratch.get(), y, rows); + } + const auto tGrayMsb = millis(); - // display grayscale part - renderer.displayGrayBuffer(); - const auto tGrayDisplay = millis(); - renderer.setRenderMode(GfxRenderer::BW); - // restore the bw data - renderer.restoreBwBuffer(); - const auto tBwRestore = millis(); + renderer.setRenderMode(GfxRenderer::BW); + renderer.displayGrayBuffer(); + const auto tGrayDisplay = millis(); - const auto tEnd = millis(); - LOG_DBG("ERS", - "Page render: prewarm=%lums bw_render=%lums display=%lums bw_store=%lums " - "gray_lsb=%lums gray_msb=%lums gray_display=%lums bw_restore=%lums total=%lums", - tPrewarm - t0, tBwRender - tPrewarm, tDisplay - tBwRender, tBwStore - tDisplay, tGrayLsb - tBwStore, - tGrayMsb - tGrayLsb, tGrayDisplay - tGrayMsb, tBwRestore - tGrayDisplay, tEnd - t0); + // BW framebuffer is intact; re-sync controller RAM for the next + // differential page turn directly from it. + renderer.cleanupGrayscaleWithFrameBuffer(); + const auto tCleanup = millis(); + + const auto tEnd = millis(); + LOG_DBG("ERS", + "Page render (tiled): prewarm=%lums bw_render=%lums display=%lums gray_lsb=%lums " + "gray_msb=%lums gray_display=%lums cleanup=%lums total=%lums", + tPrewarm - t0, tBwRender - tPrewarm, tDisplay - tBwRender, tGrayLsb - tDisplay, tGrayMsb - tGrayLsb, + tGrayDisplay - tGrayMsb, tCleanup - tGrayDisplay, tEnd - t0); + } } else { - // restore the bw data - renderer.restoreBwBuffer(); - const auto tBwRestore = millis(); + // Fallback path for a controller without strip support. grayscale rendering + // TODO: Only do this if font supports it + if (SETTINGS.textAntiAliasing) { + // Save the BW frame before the grayscale passes overwrite it, restore + // after. Only needed when grayscale actually renders. + renderer.storeBwBuffer(); + const auto tBwStore = millis(); - const auto tEnd = millis(); - LOG_DBG("ERS", - "Page render: prewarm=%lums bw_render=%lums display=%lums bw_store=%lums bw_restore=%lums total=%lums", - tPrewarm - t0, tBwRender - tPrewarm, tDisplay - tBwRender, tBwStore - tDisplay, tBwRestore - tBwStore, - tEnd - t0); + renderer.clearScreen(0x00); + renderer.setRenderMode(GfxRenderer::GRAYSCALE_LSB); + page->render(renderer, SETTINGS.getReaderFontId(), orientedMarginLeft, orientedMarginTop); + renderer.copyGrayscaleLsbBuffers(); + const auto tGrayLsb = millis(); + + // Render and copy to MSB buffer + renderer.clearScreen(0x00); + renderer.setRenderMode(GfxRenderer::GRAYSCALE_MSB); + page->render(renderer, SETTINGS.getReaderFontId(), orientedMarginLeft, orientedMarginTop); + renderer.copyGrayscaleMsbBuffers(); + const auto tGrayMsb = millis(); + + // display grayscale part + renderer.displayGrayBuffer(); + const auto tGrayDisplay = millis(); + renderer.setRenderMode(GfxRenderer::BW); + renderer.restoreBwBuffer(); + const auto tBwRestore = millis(); + + const auto tEnd = millis(); + LOG_DBG("ERS", + "Page render: prewarm=%lums bw_render=%lums display=%lums bw_store=%lums " + "gray_lsb=%lums gray_msb=%lums gray_display=%lums bw_restore=%lums total=%lums", + tPrewarm - t0, tBwRender - tPrewarm, tDisplay - tBwRender, tBwStore - tDisplay, tGrayLsb - tBwStore, + tGrayMsb - tGrayLsb, tGrayDisplay - tGrayMsb, tBwRestore - tGrayDisplay, tEnd - t0); + } else { + // No anti-aliasing: BW frame already displayed above, no grayscale to + // render, so no save/restore. + const auto tEnd = millis(); + LOG_DBG("ERS", "Page render: prewarm=%lums bw_render=%lums display=%lums total=%lums", tPrewarm - t0, + tBwRender - tPrewarm, tDisplay - tBwRender, tEnd - t0); + } } }