feat: X3 grayscale antialiasing improvements

[juicecultus]

Summary

Improves text antialiasing quality on the Xteink X3 (SSD1677) display to bring it closer to X4 rendering quality. Addresses white lines through letter strokes and ghosting artifacts during page turns and screen transitions.

Changes

Display Driver (open-x4-sdk)

• Dedicated X3 grayscale LUTs with tuned VDL drive strengths for dark gray (2 time units) and light gray (3 time units), with active GND hold on non-gray transitions to prevent floating source crosstalk
• Tight scan timing: TP2/TP3 reduced to 1 (total gate-on 7 units vs 17), minimizing parasitic charge leakage that caused white lines through letter strokes
• Fast diff BB reinforcement: Added mild VDH reinforcing pulse to lut_x3_bb_full so black pixels are actively driven during differential refreshes, clearing gray residue/ghosting
• displayGrayBuffer() updated to use the dedicated gray LUT bank instead of full refresh LUTs for X3

Rendering Pipeline

• Re-enabled light gray rendering for X3 text and images, now safe with dedicated gray LUTs providing proper 4-level gray
• Removed isLightGrayRestricted() gating that was limiting X3 to 3-level gray
• Runtime display dimensions in DirectPixelWriter and ScreenshotUtil, replaced hardcoded constants with runtime getters to support X3 792x528 resolution

Note

The open-x4-sdk submodule references a commit on juicecultus/community-sdk. A corresponding PR to open-x4-epaper/community-sdk should be merged first so the submodule ref resolves on upstream.

Testing

Tested on physical X3 hardware. White lines through letters significantly reduced, in-book ghosting improved via BB reinforcement, antialiasing visually closer to X4 quality.

AI Disclosure

Yes, AI was used to assist with the development of these changes.

[znelson]

There's some overlap with #1572 that needs to be resolved quick

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6cc25162-7118-4701-80ba-c8f83a215904

📥 Commits

Reviewing files that changed from the base of the PR and between fe6ef40 and 46e0843.

📒 Files selected for processing (1)

• lib/GfxRenderer/GfxRenderer.cpp

🚧 Files skipped from review as they are similar to previous changes (1)

• lib/GfxRenderer/GfxRenderer.cpp

📜 Recent review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: cppcheck
• GitHub Check: build

---

📝 Walkthrough

Walkthrough

DirectPixelWriter and ScreenshotUtil now use renderer-provided runtime display dimensions and stride via new GfxRenderer getters. GfxRenderer exposes three new accessors. Grayscale handling was unified (value 2 treated as light gray on all devices). Submodule pointer updated.

Changes

Cohort / File(s)	Summary
Renderer accessors lib/GfxRenderer/GfxRenderer.h	Added inline getters getDisplayWidth(), getDisplayHeight(), getDisplayWidthBytes() returning renderer panel dimensions/stride.
Framebuffer writer lib/Epub/Epub/converters/DirectPixelWriter.h	init() uses renderer.getDisplayWidthBytes() for cached stride; orientation math now derives phyW/phyH from renderer.getDisplayWidth()/getDisplayHeight() and computes phyXBase/phyYBase accordingly; writePixel() uses cached displayWidthBytes.
Rendering logic lib/GfxRenderer/GfxRenderer.cpp	Removed gpio.deviceIsX4() gating for GRAYSCALE_MSB; conditions in renderCharImpl and drawBitmap now treat 1 or 2 as light gray unconditionally; comments updated to reflect unified LUT behavior.
Screenshot utility src/util/ScreenshotUtil.cpp	takeScreenshot() and border calculations now use renderer.getDisplayWidth()/getDisplayHeight(); minor comment adjustment in saveFramebufferAsBmp() about kMaxRowSize wording only.
Submodule open-x4-sdk	Updated submodule pointer to commit a64a3c29bebc59b2ccdfe15492cfc4b5e4c26360.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• perf: Eliminate per-pixel overheads in image rendering #1293: Direct overlap—updated DirectPixelWriter to use GfxRenderer accessors and adjusted orientation math.
• fix: epub images not rendering correctly on x3 #1572: Related changes migrating framebuffer/screenshot code to use renderer runtime dimensions.

Suggested reviewers

• znelson

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: improving X3 grayscale antialiasing quality, which aligns with the overall objective of the PR.
Description check	✅ Passed	The description is well-related to the changeset, providing detailed context on display driver improvements, rendering pipeline changes, and testing verification.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/util/ScreenshotUtil.cpp (1)

28-29: Consider using getScreenWidth()/getScreenHeight() for clarity.

The current code swaps getDisplayHeight()/getDisplayWidth() because Portrait orientation maps physical height to logical width. While this works correctly, using the logical accessors would be clearer:

-    renderer.drawRect(6, 6, renderer.getDisplayHeight() - 12, renderer.getDisplayWidth() - 12, 2, true);
+    renderer.drawRect(6, 6, renderer.getScreenWidth() - 12, renderer.getScreenHeight() - 12, 2, true);

This makes the intent explicit and would remain correct if orientation handling ever changes.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/util/ScreenshotUtil.cpp` around lines 28 - 29, Replace the swapped
display dimension calls with the logical screen accessors to make intent
explicit: in the renderer.storeBwBuffer() block, change the renderer.drawRect
call that currently uses renderer.getDisplayHeight() and
renderer.getDisplayWidth() to use renderer.getScreenWidth() and
renderer.getScreenHeight() (or the appropriate logical-width/height accessors)
so the rectangle parameters reflect logical screen dimensions; update the call
referencing renderer.drawRect and leave renderer.storeBwBuffer() unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@open-x4-sdk`:
- Line 1: The PR currently pins the submodule to commit
121824b5089d3f584fc8b345a94d5e07821003aa which is not reachable on the
configured remote (https://github.com/open-x4-epaper/community-sdk.git); block
this merge and either update the submodule reference to a commit that exists on
the remote or wait until the upstream community-sdk PR is merged so that commit
121824b5089d3f584fc8b345a94d5e07821003aa becomes reachable, then update the
submodule pointer accordingly (ensure the submodule URL and commit hash in the
repo match the upstream remote and are fetchable in CI).

---

Nitpick comments:
In `@src/util/ScreenshotUtil.cpp`:
- Around line 28-29: Replace the swapped display dimension calls with the
logical screen accessors to make intent explicit: in the
renderer.storeBwBuffer() block, change the renderer.drawRect call that currently
uses renderer.getDisplayHeight() and renderer.getDisplayWidth() to use
renderer.getScreenWidth() and renderer.getScreenHeight() (or the appropriate
logical-width/height accessors) so the rectangle parameters reflect logical
screen dimensions; update the call referencing renderer.drawRect and leave
renderer.storeBwBuffer() unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 150fcd39-7cdf-4b11-be86-d8c8caf8a687

📥 Commits

Reviewing files that changed from the base of the PR and between cff3e12 and 581e601.

📒 Files selected for processing (5)

• lib/Epub/Epub/converters/DirectPixelWriter.h
• lib/GfxRenderer/GfxRenderer.cpp
• lib/GfxRenderer/GfxRenderer.h
• open-x4-sdk
• src/util/ScreenshotUtil.cpp

📜 Review details

🧰 Additional context used

🧠 Learnings (8)

📓 Common learnings

Learnt from: pablohc
Repo: crosspoint-reader/crosspoint-reader PR: 1554
File: src/activities/boot_sleep/SleepActivity.cpp:40-63
Timestamp: 2026-04-02T15:35:02.228Z
Learning: In crosspoint-reader/crosspoint-reader (src/activities/boot_sleep/SleepActivity.cpp, SleepActivity::renderBitmapSleepScreen), CrossPointSettings::SLEEP_SCREEN_FILTER::FILTER_CONTRAST is intentionally handled implicitly: `hasGreyscale` is computed as `bitmap.hasGreyscale() && SETTINGS.sleepScreenFilter == FILTER_NONE`. When FILTER_CONTRAST is selected, `hasGreyscale` evaluates to false, which skips the 4-level greyscale rendering path and falls back to 1-bit B&W dithered rendering, producing a visibly higher-contrast result on bitmaps with greyscale data. LOGO and BLANK screens are unaffected by FILTER_CONTRAST by design — they contain no bitmap content where a contrast effect would be meaningful. Do not flag FILTER_CONTRAST as unimplemented or a no-op in future reviews.

Learnt from: Tritlo
Repo: crosspoint-reader/crosspoint-reader PR: 1003
File: src/activities/reader/EpubReaderActivity.cpp:657-674
Timestamp: 2026-02-19T17:46:36.345Z
Learning: In src/activities/reader/EpubReaderActivity.cpp's renderContents() method, when uncached images exist, Phase 1 intentionally calls displayBuffer(forceFullRefresh) to perform a HALF_REFRESH (if needed), while Phase 2 intentionally calls renderer.displayBuffer() directly without forceFullRefresh. This is by design: Phase 1's refresh clears the screen properly to prevent ghosting, so Phase 2 can use a faster refresh mode for better performance. The grayscale anti-aliasing is handled separately after renderContents() via displayGrayBuffer().

📚 Learning: 2026-04-02T15:35:02.228Z

Learnt from: pablohc
Repo: crosspoint-reader/crosspoint-reader PR: 1554
File: src/activities/boot_sleep/SleepActivity.cpp:40-63
Timestamp: 2026-04-02T15:35:02.228Z
Learning: In crosspoint-reader/crosspoint-reader (src/activities/boot_sleep/SleepActivity.cpp, SleepActivity::renderBitmapSleepScreen), CrossPointSettings::SLEEP_SCREEN_FILTER::FILTER_CONTRAST is intentionally handled implicitly: `hasGreyscale` is computed as `bitmap.hasGreyscale() && SETTINGS.sleepScreenFilter == FILTER_NONE`. When FILTER_CONTRAST is selected, `hasGreyscale` evaluates to false, which skips the 4-level greyscale rendering path and falls back to 1-bit B&W dithered rendering, producing a visibly higher-contrast result on bitmaps with greyscale data. LOGO and BLANK screens are unaffected by FILTER_CONTRAST by design — they contain no bitmap content where a contrast effect would be meaningful. Do not flag FILTER_CONTRAST as unimplemented or a no-op in future reviews.

Applied to files:

• src/util/ScreenshotUtil.cpp
• lib/GfxRenderer/GfxRenderer.cpp

📚 Learning: 2026-03-09T11:53:24.166Z

Learnt from: jpirnay
Repo: crosspoint-reader/crosspoint-reader PR: 1360
File: src/activities/boot_sleep/SleepActivity.cpp:83-86
Timestamp: 2026-03-09T11:53:24.166Z
Learning: In crosspoint-reader/crosspoint-reader (ESP32-C3 e-ink reader), `CrossPointState::lastSleepImage` is a `uint8_t` with `UINT8_MAX` used as an "unset" sentinel. A sentinel collision at index 255 would require 256+ full-screen BMP sleep images (~240 KB each minimum); at that count the heap is exhausted building the `std::vector<std::string> files` long before the sentinel can collide, so the collision is unreachable in practice. Widening the type would also require a binary state file version bump (`serialization::readPod` reads exactly `sizeof(uint8_t)`), adding unjustified churn. Do not flag this as a bug in future reviews.

Applied to files:

• src/util/ScreenshotUtil.cpp

📚 Learning: 2026-02-27T22:49:59.600Z

Learnt from: ngxson
Repo: crosspoint-reader/crosspoint-reader PR: 1218
File: src/activities/ActivityManager.cpp:254-265
Timestamp: 2026-02-27T22:49:59.600Z
Learning: In this codebase, assertions are always enabled (no NDEBUG). Use assert() to crash on programmer errors and surface logic bugs during development and in production builds. Do not rely on asserts for runtime error handling; they should enforce invariants that must always hold. Keep asserts side-effect free and inexpensive, and avoid relying on them for user-visible failures. Include <cassert> where appropriate and document the invariant being tested.

Applied to files:

• src/util/ScreenshotUtil.cpp
• lib/GfxRenderer/GfxRenderer.cpp

📚 Learning: 2026-03-02T10:14:16.036Z

Learnt from: Uri-Tauber
Repo: crosspoint-reader/crosspoint-reader PR: 1245
File: lib/Epub/Epub/Section.cpp:277-308
Timestamp: 2026-03-02T10:14:16.036Z
Learning: Guideline: Strengthen serialization::readString to defend against unbounded growth when reading from disk data. Implement and enforce a maximum allowed length (e.g., a configured or reasonable constant) and validate the incoming length before resizing or allocating. Audit all call sites (e.g., BookMetadataCache, TextBlock, KOReaderCredentialStore, Section cache readers) to ensure they do not rely on unbounded len-based resizing. If the readString API must remain, add internal safeguards (bounds checks, length validation, and error handling) so per-call-site validations are not required. Ensure Section cache files remain versioned (SECTION_FILE_VERSION) and parameter mismatches invalidate caches, but do not rely on unsafe allocations; prefer safe, bounded reads with explicit errors on invalid data.

Applied to files:

• src/util/ScreenshotUtil.cpp
• lib/GfxRenderer/GfxRenderer.cpp

📚 Learning: 2026-03-28T11:06:29.611Z

Learnt from: pablohc
Repo: crosspoint-reader/crosspoint-reader PR: 1488
File: src/activities/home/HomeActivity.cpp:92-95
Timestamp: 2026-03-28T11:06:29.611Z
Learning: When reviewing crosspoint-reader code, avoid flagging a missing `renderer.displayBuffer()` call immediately after `GUI.drawPopup()` / `BaseTheme::drawPopup()`: `BaseTheme::drawPopup()` already calls `renderer.displayBuffer()` before returning, so the popup is guaranteed to be flushed to the e-ink panel before subsequent blocking work begins. Conversely, do not require a `renderer.displayBuffer()` call after `fillPopupProgress()`; it intentionally does not flush, so intermediate progress-bar updates may not appear unless the update granularity warrants an explicit flush elsewhere.

Applied to files:

• src/util/ScreenshotUtil.cpp
• lib/GfxRenderer/GfxRenderer.cpp

📚 Learning: 2026-02-19T17:46:36.345Z

Learnt from: Tritlo
Repo: crosspoint-reader/crosspoint-reader PR: 1003
File: src/activities/reader/EpubReaderActivity.cpp:657-674
Timestamp: 2026-02-19T17:46:36.345Z
Learning: In src/activities/reader/EpubReaderActivity.cpp's renderContents() method, when uncached images exist, Phase 1 intentionally calls displayBuffer(forceFullRefresh) to perform a HALF_REFRESH (if needed), while Phase 2 intentionally calls renderer.displayBuffer() directly without forceFullRefresh. This is by design: Phase 1's refresh clears the screen properly to prevent ghosting, so Phase 2 can use a faster refresh mode for better performance. The grayscale anti-aliasing is handled separately after renderContents() via displayGrayBuffer().

Applied to files:

• lib/Epub/Epub/converters/DirectPixelWriter.h

📚 Learning: 2026-03-17T15:27:17.468Z

Learnt from: znelson
Repo: crosspoint-reader/crosspoint-reader PR: 1413
File: lib/EpdFont/EpdFont.cpp:34-36
Timestamp: 2026-03-17T15:27:17.468Z
Learning: In crosspoint-reader/crosspoint-reader, `EpdFont::getGlyph()` falls back to `getGlyph(REPLACEMENT_GLYPH)` (U+FFFD) before returning `nullptr`. All fonts in this project are guaranteed to include a U+FFFD replacement glyph, so the `!glyph` null branch in `EpdFont::getTextBounds()` (and similar rendering paths) is unreachable in practice. Do not flag stale-state issues in that branch (e.g., leftover `lastBaseAdvanceFP`/`lastBaseTop` after a null glyph) as bugs in future reviews.

Applied to files:

• lib/GfxRenderer/GfxRenderer.cpp

🔇 Additional comments (8)

lib/GfxRenderer/GfxRenderer.cpp (2)

134-142: LGTM - Light gray rendering now enabled for both X3 and X4 devices.

The removal of the device-type gating and updated comment correctly reflect the PR's objective: with dedicated X3 gray LUTs in the display driver, both bmpVal == 1 (dark gray) and bmpVal == 2 (light gray) are now rendered in GRAYSCALE_MSB mode on all devices. The logic remains consistent with the GRAYSCALE_LSB check below (dark gray only at bmpVal == 1).

---

686-690: LGTM - Consistent grayscale handling for bitmap rendering.

The GRAYSCALE_MSB condition now matches the text rendering path in renderCharImpl, ensuring images and text receive identical 4-level gray treatment on both X3 and X4 devices.

lib/GfxRenderer/GfxRenderer.h (1)

160-162: LGTM - Clean runtime dimension accessors.

These inline getters correctly expose the runtime-initialized panel dimensions, enabling DirectPixelWriter and ScreenshotUtil to support both X4 (800×480) and X3 (792×528) displays without compile-time constants.

lib/Epub/Epub/converters/DirectPixelWriter.h (3)

18-36: LGTM - Runtime dimension initialization is correct.

The new displayWidthBytes member and initialization from the renderer's accessors properly support both X4 (800×480) and X3 (792×528) displays. The phyW/phyH local variables correctly feed into the orientation transform calculations below.

---

38-74: LGTM - Orientation transforms correctly use runtime dimensions.

Each orientation case properly substitutes the runtime phyW/phyH for the compile-time constants in the base coordinate calculations. The step coefficients remain unchanged as they only depend on direction, not panel size.

---

123-124: LGTM - Framebuffer indexing uses runtime dimension.

The byteIndex calculation correctly uses the cached displayWidthBytes member, enabling proper pixel addressing for both X4 and X3 display widths.

src/util/ScreenshotUtil.cpp (2)

17-18: LGTM - Screenshot dimensions now runtime-based.

Using renderer.getDisplayWidth()/getDisplayHeight() correctly passes the actual panel dimensions to saveFramebufferAsBmp, supporting both X4 (800×480) and X3 (792×528) displays.

---

79-80: LGTM - Buffer size correctly accounts for X3 resolution.

The increased kMaxRowSize from 64 to 68 bytes correctly accommodates X3's 528px dimension after 90° rotation: (528 + 31) / 32 * 4 = 68 bytes per padded BMP row. The comment clearly documents the rationale.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read submodule URL from .gitmodules
url="$(git config -f .gitmodules --get submodule.open-x4-sdk.url)"
sha="121824b5089d3f584fc8b345a94d5e07821003aa"

echo "Submodule URL: $url"
echo "Target SHA:    $sha"

# Verify commit object is reachable on remote
if git ls-remote "$url" | grep -qi "$sha"; then
  echo "OK: SHA is reachable on remote."
else
  echo "ERROR: SHA not found on remote refs. Submodule checkout may fail."
  exit 1
fi

Repository: crosspoint-reader/crosspoint-reader

Length of output: 267

---

Block merge until upstream community-sdk PR is merged and commit becomes reachable on remote.

Line 1 pins the submodule to commit 121824b5089d3f584fc8b345a94d5e07821003aa. This commit is currently not reachable on the configured remote (https://github.com/open-x4-epaper/community-sdk.git), which will cause submodule checkout failures during CI and fresh clones. Merge only after the upstream PR is merged and the commit is available on the remote.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@open-x4-sdk` at line 1, The PR currently pins the submodule to commit
121824b5089d3f584fc8b345a94d5e07821003aa which is not reachable on the
configured remote (https://github.com/open-x4-epaper/community-sdk.git); block
this merge and either update the submodule reference to a commit that exists on
the remote or wait until the upstream community-sdk PR is merged so that commit
121824b5089d3f584fc8b345a94d5e07821003aa becomes reachable, then update the
submodule pointer accordingly (ensure the submodule URL and commit hash in the
repo match the upstream remote and are fetchable in CI).

[znelson]

Ah, and the SDK submodule needs to be updated to point to the now merged commit open-x4-epaper/community-sdk@a64a3c2.

[juicecultus]

Thanks. Both issues are resolved, the CodeRabbit nitpick is on existing code behaviour from #1572, not something I introduced. Accepted renderer. method calls (more correct than #1572's display. global)

[itsthisjustin]

I will test this shortly and review

[carsonwick]

Tested as working for 4-bit sleep screen images (#944) The two mid tones seem slightly darker than the X4's. My understanding is that those may be changing too (#1614)? Thank you.