perf: Optimize fillRectDither with Byte-Aligned fillRectImpl (#2270)
## Summary
* **What is the goal of this PR?**
Replace the pixel-by-pixel `fillRectDither` implementation with a new
byte-aligned `fillRectImpl` that eliminates per-pixel
`rotateCoordinates` calls and Read-Modify-Write bitwise loops, yielding
a significant rendering speedup on ESP32 E-ink framebuffers.
Ported from @rhythmerc's crosspoint-reader fork commit 27ea625.
* **What changes are included?**
- **`GfxRenderer.cpp` — `fillRectDither` refactor:** The existing
`if/else if` chain is replaced with a `switch` statement that delegates
each `Color` case to the new `fillRectImpl<Color>()` template,
eliminating runtime branching.
- **`GfxRenderer.cpp` — new `fillRectImpl<C>()` template:** Core of the
optimization. Key behaviors:
- Clips the rectangle in logical space upfront.
- Rotates only **2 opposing corner points** (top-left and bottom-right)
into physical framebuffer space instead of rotating every pixel
individually.
- Derives physical-space `byteStart`/`byteEnd` and precomputes
`headMask` / `tailMask` for MSB-first partial-byte boundaries,
performing RMW only on the edge bytes.
- **Solid fills (`Black` / `White`):** Uses `memset` for all interior
full-byte runs per row — no per-pixel writes.
- **Dithered fills (`LightGray` / `DarkGray`):** Precomputes both parity
variants of `blackMask` (even/odd `py`) **outside** the row loop,
eliminating the previously re-evaluated 8-bit construction loop on every
physical row. Interior full bytes are then written with a single
`memset(whiteMask)`.
- Uses `if constexpr` throughout to dispatch on `Color` at compile time,
generating zero runtime branches per template instantiation.
- **`GfxRenderer.h`:** Declares the new private `fillRectImpl<Color>()`
template method with an explanatory doc-comment.
- **Explicit template instantiations** added for all four active `Color`
variants (`Black`, `White`, `LightGray`, `DarkGray`).
## Additional Context
* **Performance:** The primary motivation is ESP32 E-ink framebuffer
performance. The old path called `rotateCoordinates` and did a full RMW
for every single pixel in the rectangle. The new path calls
`rotateCoordinates` exactly **twice** per fill regardless of rectangle
size, then operates at byte granularity — a complexity reduction from
O(W×H) coordinate transforms to O(1).
* **Dither correctness:** The `blackMask` precomputation relies on the
dither pattern having period 2 in both logical X and Y, which makes the
per-row byte pattern repeat with period 2 in `py`. Reviewers should
verify the `lxBase`/`lyBase` derivations for all four orientations
(`Portrait`, `PortraitInverted`, `LandscapeClockwise`,
`LandscapeCounterClockwise`) match the inverse of `rotateCoordinates`.
* **Edge case — single-byte rows:** When `byteStart == byteEnd`, the
head and tail masks are ANDed together into a single `rectMask` to avoid
double-masking the same byte. This path should be tested with narrow
rectangles (width < 8px).
* **No behavioral change for `Color::Clear`:** The `Clear` case exits
early via `if constexpr` and is a no-op, matching the original behavior.
---
### AI Usage
While CrossPoint doesn't have restrictions on AI tools in contributing,
please be transparent about their usage as it
helps set the right context for reviewers.
Did you use AI tools to help write this code? _**< YES >**_
---------
Co-authored-by: Ryan Mercado <rmercado@firstdollar.com>
This commit is contained in:
Uri Tauber
co-authored by
Ryan Mercado
Ryan Mercado
parent
86a9b9c4a2
commit
a2f2eea79e
+185
-15
@@ -661,8 +661,10 @@ void GfxRenderer::drawRoundedRect(const int x, const int y, const int width, con
|
||||
}
|
||||
|
||||
void GfxRenderer::fillRect(const int x, const int y, const int width, const int height, const bool state) const {
|
||||
for (int fillY = y; fillY < y + height; fillY++) {
|
||||
drawLine(x, fillY, x + width - 1, fillY, state);
|
||||
if (state) {
|
||||
fillRectImpl<Color::Black>(x, y, width, height);
|
||||
} else {
|
||||
fillRectImpl<Color::White>(x, y, width, height);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -694,26 +696,194 @@ void GfxRenderer::drawPixelDither<Color::DarkGray>(const int x, const int y) con
|
||||
}
|
||||
|
||||
void GfxRenderer::fillRectDither(const int x, const int y, const int width, const int height, Color color) const {
|
||||
if (color == Color::Clear) {
|
||||
} else if (color == Color::Black) {
|
||||
fillRect(x, y, width, height, true);
|
||||
} else if (color == Color::White) {
|
||||
fillRect(x, y, width, height, false);
|
||||
} else if (color == Color::LightGray) {
|
||||
for (int fillY = y; fillY < y + height; fillY++) {
|
||||
for (int fillX = x; fillX < x + width; fillX++) {
|
||||
drawPixelDither<Color::LightGray>(fillX, fillY);
|
||||
switch (color) {
|
||||
case Color::Clear:
|
||||
break;
|
||||
case Color::Black:
|
||||
fillRectImpl<Color::Black>(x, y, width, height);
|
||||
break;
|
||||
case Color::White:
|
||||
fillRectImpl<Color::White>(x, y, width, height);
|
||||
break;
|
||||
case Color::LightGray:
|
||||
fillRectImpl<Color::LightGray>(x, y, width, height);
|
||||
break;
|
||||
case Color::DarkGray:
|
||||
fillRectImpl<Color::DarkGray>(x, y, width, height);
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
template <Color C>
|
||||
void GfxRenderer::fillRectImpl(const int x, const int y, const int width, const int height) const {
|
||||
if constexpr (C == Color::Clear) return;
|
||||
if (width <= 0 || height <= 0) return;
|
||||
if (fontCacheManager_ && fontCacheManager_->isScanning()) return;
|
||||
|
||||
// Clip in logical space.
|
||||
const int screenW = getScreenWidth();
|
||||
const int screenH = getScreenHeight();
|
||||
const int lx0 = std::max(0, x);
|
||||
const int ly0 = std::max(0, y);
|
||||
const int lx1 = std::min(screenW, x + width);
|
||||
const int ly1 = std::min(screenH, y + height);
|
||||
if (lx0 >= lx1 || ly0 >= ly1) return;
|
||||
|
||||
// Rotate the two opposing logical corners into physical-framebuffer space.
|
||||
// The bounding rect in physical space is the rect we need to fill — rotation
|
||||
// is rigid (no shear/stretch) so the bbox of the two corners IS the rect.
|
||||
int paX, paY, pbX, pbY;
|
||||
rotateCoordinates(orientation, lx0, ly0, &paX, &paY, panelWidth, panelHeight);
|
||||
rotateCoordinates(orientation, lx1 - 1, ly1 - 1, &pbX, &pbY, panelWidth, panelHeight);
|
||||
|
||||
const int phyX0 = std::min(paX, pbX);
|
||||
const int phyX1 = std::max(paX, pbX); // inclusive
|
||||
int phyY0 = std::min(paY, pbY);
|
||||
int phyY1 = std::max(paY, pbY);
|
||||
|
||||
// Strip mode: clip Y range to the active band and redirect writes.
|
||||
uint8_t* target = getWriteTarget();
|
||||
const int originY = getWriteOriginY();
|
||||
const int writeRows = getWriteRows();
|
||||
phyY0 = std::max(phyY0, originY);
|
||||
phyY1 = std::min(phyY1, originY + writeRows - 1);
|
||||
if (phyY0 > phyY1) return;
|
||||
|
||||
// Bit/byte layout: MSB-first within a byte, so phyX → bit (7 - (phyX & 7)).
|
||||
// Head and tail masks cover only the in-rect bits of the first/last byte.
|
||||
const int byteStart = phyX0 >> 3;
|
||||
const int byteEnd = phyX1 >> 3; // inclusive
|
||||
const uint8_t headMask = static_cast<uint8_t>(0xFFu >> (phyX0 & 7));
|
||||
const uint8_t tailMask = static_cast<uint8_t>(0xFFu << (7 - (phyX1 & 7)));
|
||||
const int32_t panelStride = static_cast<int32_t>(panelWidthBytes);
|
||||
|
||||
if constexpr (C == Color::Black || C == Color::White) {
|
||||
// Solid fill. Framebuffer: 0 = black, 1 = white.
|
||||
const uint8_t fillByte = (C == Color::Black) ? 0x00u : 0xFFu;
|
||||
for (int py = phyY0; py <= phyY1; ++py) {
|
||||
uint8_t* row = target + static_cast<int32_t>(py - originY) * panelStride;
|
||||
if (byteStart == byteEnd) {
|
||||
const uint8_t mask = headMask & tailMask;
|
||||
if constexpr (C == Color::Black) {
|
||||
row[byteStart] &= static_cast<uint8_t>(~mask);
|
||||
} else {
|
||||
row[byteStart] |= mask;
|
||||
}
|
||||
} else {
|
||||
if constexpr (C == Color::Black) {
|
||||
row[byteStart] &= static_cast<uint8_t>(~headMask);
|
||||
if (byteEnd > byteStart + 1) {
|
||||
memset(row + byteStart + 1, fillByte, byteEnd - byteStart - 1);
|
||||
}
|
||||
row[byteEnd] &= static_cast<uint8_t>(~tailMask);
|
||||
} else {
|
||||
row[byteStart] |= headMask;
|
||||
if (byteEnd > byteStart + 1) {
|
||||
memset(row + byteStart + 1, fillByte, byteEnd - byteStart - 1);
|
||||
}
|
||||
row[byteEnd] |= tailMask;
|
||||
}
|
||||
}
|
||||
}
|
||||
} else if (color == Color::DarkGray) {
|
||||
for (int fillY = y; fillY < y + height; fillY++) {
|
||||
for (int fillX = x; fillX < x + width; fillX++) {
|
||||
drawPixelDither<Color::DarkGray>(fillX, fillY);
|
||||
} else {
|
||||
// Dither (LightGray / DarkGray). Both patterns have period 2 in logical
|
||||
// (x, y), so per physical row we precompute one byte that represents the
|
||||
// pattern across an 8-pixel stretch — every full byte in the row uses
|
||||
// that same value.
|
||||
//
|
||||
// dlxPerPhyX / dlyPerPhyX: how logical (x, y) change as phyX increments
|
||||
// along a physical row. Derived from inverting rotateCoordinates.
|
||||
int dlxPerPhyX = 0, dlyPerPhyX = 0;
|
||||
switch (orientation) {
|
||||
case Portrait:
|
||||
dlxPerPhyX = 0;
|
||||
dlyPerPhyX = 1;
|
||||
break;
|
||||
case PortraitInverted:
|
||||
dlxPerPhyX = 0;
|
||||
dlyPerPhyX = -1;
|
||||
break;
|
||||
case LandscapeClockwise:
|
||||
dlxPerPhyX = -1;
|
||||
dlyPerPhyX = 0;
|
||||
break;
|
||||
case LandscapeCounterClockwise:
|
||||
dlxPerPhyX = 1;
|
||||
dlyPerPhyX = 0;
|
||||
break;
|
||||
}
|
||||
|
||||
// The dither pattern has period 2 in logical space, and each orientation
|
||||
// maps py to logical coords with a fixed parity relationship. The
|
||||
// blackMask byte therefore repeats with period 2 in py. Precompute both
|
||||
// variants outside the row loop to eliminate the per-row switch + 8-bit
|
||||
// construction loop.
|
||||
uint8_t blackMasks[2];
|
||||
for (int parityIdx = 0; parityIdx < 2; ++parityIdx) {
|
||||
const int samplePy = phyY0 + parityIdx;
|
||||
int lxBase = 0, lyBase = 0;
|
||||
switch (orientation) {
|
||||
case Portrait:
|
||||
lxBase = panelHeight - 1 - samplePy;
|
||||
lyBase = byteStart * 8;
|
||||
break;
|
||||
case PortraitInverted:
|
||||
lxBase = samplePy;
|
||||
lyBase = panelWidth - 1 - byteStart * 8;
|
||||
break;
|
||||
case LandscapeClockwise:
|
||||
lxBase = panelWidth - 1 - byteStart * 8;
|
||||
lyBase = panelHeight - 1 - samplePy;
|
||||
break;
|
||||
case LandscapeCounterClockwise:
|
||||
lxBase = byteStart * 8;
|
||||
lyBase = samplePy;
|
||||
break;
|
||||
}
|
||||
uint8_t mask = 0;
|
||||
for (int b = 0; b < 8; ++b) {
|
||||
const int lx = lxBase + b * dlxPerPhyX;
|
||||
const int ly = lyBase + b * dlyPerPhyX;
|
||||
bool isBlack;
|
||||
if constexpr (C == Color::LightGray) {
|
||||
isBlack = ((lx & 1) == 0) && ((ly & 1) == 0);
|
||||
} else { // DarkGray
|
||||
isBlack = (((lx + ly) & 1) == 0);
|
||||
}
|
||||
if (isBlack) mask |= static_cast<uint8_t>(1u << (7 - b));
|
||||
}
|
||||
blackMasks[samplePy & 1] = mask;
|
||||
}
|
||||
|
||||
for (int py = phyY0; py <= phyY1; ++py) {
|
||||
const uint8_t blackMask = blackMasks[py & 1];
|
||||
const uint8_t whiteMask = static_cast<uint8_t>(~blackMask);
|
||||
|
||||
// Dither writes BOTH inks (the slow path called drawPixel for every
|
||||
// pixel — setting or clearing — so we must do the same). Inside the
|
||||
// rect mask: write whiteMask (1s where white, 0s where black). Outside
|
||||
// the rect mask: leave the framebuffer untouched.
|
||||
uint8_t* row = target + static_cast<int32_t>(py - originY) * panelStride;
|
||||
if (byteStart == byteEnd) {
|
||||
const uint8_t rectMask = headMask & tailMask;
|
||||
row[byteStart] = static_cast<uint8_t>((row[byteStart] & ~rectMask) | (rectMask & whiteMask));
|
||||
} else {
|
||||
row[byteStart] = static_cast<uint8_t>((row[byteStart] & ~headMask) | (headMask & whiteMask));
|
||||
if (byteEnd > byteStart + 1) {
|
||||
// Period 2, so every full byte in this row is exactly whiteMask.
|
||||
memset(row + byteStart + 1, whiteMask, byteEnd - byteStart - 1);
|
||||
}
|
||||
row[byteEnd] = static_cast<uint8_t>((row[byteEnd] & ~tailMask) | (tailMask & whiteMask));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
template void GfxRenderer::fillRectImpl<Color::Black>(int, int, int, int) const;
|
||||
template void GfxRenderer::fillRectImpl<Color::White>(int, int, int, int) const;
|
||||
template void GfxRenderer::fillRectImpl<Color::LightGray>(int, int, int, int) const;
|
||||
template void GfxRenderer::fillRectImpl<Color::DarkGray>(int, int, int, int) const;
|
||||
|
||||
void GfxRenderer::maskRoundedRectOutsideCorners(const int x, const int y, const int width, const int height,
|
||||
const int radius, const Color color) const {
|
||||
if (radius <= 0 || color == Color::Clear) {
|
||||
|
||||
@@ -81,6 +81,12 @@ class GfxRenderer {
|
||||
void drawPixelDither(int x, int y) const;
|
||||
template <Color color>
|
||||
void fillArc(int maxRadius, int cx, int cy, int xDir, int yDir) const;
|
||||
// Byte-aligned, orientation-specialized rectangle fill. Rotates the rect's
|
||||
// two opposing corners into physical-framebuffer space once, then walks each
|
||||
// physical row with head-mask / middle memset / tail-mask byte writes — no
|
||||
// per-pixel rotation, no per-pixel RMW.
|
||||
template <Color color>
|
||||
void fillRectImpl(int x, int y, int width, int height) const;
|
||||
|
||||
public:
|
||||
explicit GfxRenderer(HalDisplay& halDisplay)
|
||||
|
||||
Reference in New Issue
Block a user