Skip to content

perf: Remove hasPrintableChars pass#971

Merged
daveallie merged 1 commit intocrosspoint-reader:masterfrom
znelson:printable-chars
Feb 19, 2026
Merged

perf: Remove hasPrintableChars pass#971
daveallie merged 1 commit intocrosspoint-reader:masterfrom
znelson:printable-chars

Conversation

@znelson
Copy link
Contributor

@znelson znelson commented Feb 18, 2026 •
edited
Loading

Summary

What is the goal of this PR?

hasPrintableChars does a pass over text before rendering. It looks up glyphs in the font and measures dimensions, returning early if the text results in zero size.

This additional pass doesn't offer any benefit over moving straight to rendering the text, because the rendering loop already gracefully handles missing glyphs. This change saves an extra pass over all rendered text.

Note that both hasPrintableChars and renderChar replace missing glyphs with glyph = getGlyph(REPLACEMENT_GLYPH), so there's no difference for characters which are not present in the font.

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? NO

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 18, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6ec5fc5 and 27d0111.

📒 Files selected for processing (5)
  • lib/EpdFont/EpdFont.cpp
  • lib/EpdFont/EpdFont.h
  • lib/EpdFont/EpdFontFamily.cpp
  • lib/EpdFont/EpdFontFamily.h
  • lib/GfxRenderer/GfxRenderer.cpp
💤 Files with no reviewable changes (5)
  • lib/EpdFont/EpdFont.h
  • lib/GfxRenderer/GfxRenderer.cpp
  • lib/EpdFont/EpdFontFamily.h
  • lib/EpdFont/EpdFontFamily.cpp
  • lib/EpdFont/EpdFont.cpp
📝 Walkthrough

Walkthrough

Removed the hasPrintableChars method from the font API across EpdFont and EpdFontFamily classes. Deleted early-return guards in GfxRenderer's text rendering methods that previously skipped rendering when fonts reported no printable characters. Rendering now proceeds for all codepoints, relying on glyph fallback for non-renderable characters.

Changes

Cohort / File(s) Summary
Font API Removal
lib/EpdFont/EpdFont.h, lib/EpdFont/EpdFont.cpp, lib/EpdFont/EpdFontFamily.h, lib/EpdFont/EpdFontFamily.cpp		 Removed hasPrintableChars(const char* string) method declaration and implementation from EpdFont; removed corresponding delegating method from EpdFontFamily that called the removed EpdFont method.
Renderer Guard Removal
lib/GfxRenderer/GfxRenderer.cpp		 Deleted early-return guards in drawText() and drawTextRotated90CW() that checked font.hasPrintableChars(text, style) before rendering, allowing rendering to proceed for all codepoints and rely on glyph fallback for non-renderable characters.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • ngxson
  • osteotek
  • jdk2pq
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'perf: Remove hasPrintableChars pass' accurately describes the main change: removing a pre-rendering performance optimization pass that checks for printable characters.
Description check ✅ Passed The description clearly explains the purpose of removing hasPrintableChars, detailing the performance benefit and how the rendering loop already handles missing glyphs equivalently.

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

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

@znelson znelson marked this pull request as ready for review February 18, 2026 15:35
daveallie
daveallie approved these changes Feb 19, 2026
View reviewed changes
Copy link
Member

@daveallie daveallie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I had actually removed this in a local branch but never got it up, keen to drop it

@daveallie daveallie merged commit 448a77f into crosspoint-reader:master Feb 19, 2026
10 checks passed
@daveallie daveallie mentioned this pull request Feb 19, 2026
Merged
saslv pushed a commit to saslv/crosspoint-reader that referenced this pull request Feb 19, 2026
@znelson @saslv
perf: Remove hasPrintableChars pass (crosspoint-reader#971)
5773f29 
## Summary

**What is the goal of this PR?**

`hasPrintableChars` does a pass over text before rendering. It looks up
glyphs in the font and measures dimensions, returning early if the text
results in zero size.

This additional pass doesn't offer any benefit over moving straight to
rendering the text, because the rendering loop already gracefully
handles missing glyphs. This change saves an extra pass over all
rendered text.

Note that both `hasPrintableChars` and `renderChar` replace missing
glyphs with `glyph = getGlyph(REPLACEMENT_GLYPH)`, so there's no
difference for characters which are not present in the font.

---

### 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? _**NO**_
@coderabbitai coderabbitai bot mentioned this pull request Feb 19, 2026
Merged
el pushed a commit to el/crosspoint-reader that referenced this pull request Feb 19, 2026
@znelson @el
perf: Remove hasPrintableChars pass (crosspoint-reader#971)
2fd310d 
## Summary

**What is the goal of this PR?**

`hasPrintableChars` does a pass over text before rendering. It looks up
glyphs in the font and measures dimensions, returning early if the text
results in zero size.

This additional pass doesn't offer any benefit over moving straight to
rendering the text, because the rendering loop already gracefully
handles missing glyphs. This change saves an extra pass over all
rendered text.

Note that both `hasPrintableChars` and `renderChar` replace missing
glyphs with `glyph = getGlyph(REPLACEMENT_GLYPH)`, so there's no
difference for characters which are not present in the font.

---

### 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? _**NO**_
lukestein pushed a commit to lukestein/crosspoint-reader that referenced this pull request Feb 20, 2026
@znelson @lukestein
perf: Remove hasPrintableChars pass (crosspoint-reader#971)
22f4646 
## Summary

**What is the goal of this PR?**

`hasPrintableChars` does a pass over text before rendering. It looks up
glyphs in the font and measures dimensions, returning early if the text
results in zero size.

This additional pass doesn't offer any benefit over moving straight to
rendering the text, because the rendering loop already gracefully
handles missing glyphs. This change saves an extra pass over all
rendered text.

Note that both `hasPrintableChars` and `renderChar` replace missing
glyphs with `glyph = getGlyph(REPLACEMENT_GLYPH)`, so there's no
difference for characters which are not present in the font.

---

### 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? _**NO**_
@znelson znelson deleted the printable-chars branch February 20, 2026 14:29
This was referenced Feb 21, 2026
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@osteotek osteotek osteotek approved these changes

@daveallie daveallie daveallie approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@znelson @osteotek @daveallie