fix: Correct word width and space calculations by znelson · Pull Request #963 · crosspoint-reader/crosspoint-reader

znelson · 2026-02-18T05:23:10Z

Summary

What is the goal of this PR?

This change fixes an issue I noticed while reading where occasionally, especially in italics, some words would have too much space between them. The problem was that word width calculations were including any negative X overhang, and combined with a space before the word, that can lead to an inconsistently large space.

Additional Context

Screenshots of some problematic text:

In CrossPoint 1.0	With this change

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 · 2026-02-18T05:23:20Z

📝 Walkthrough

Walkthrough

This PR updates the graphics renderer's text measurement methods to respect font styles by adding a style parameter to getSpaceWidth and getTextAdvanceX methods. All callers are updated to pass the appropriate style when measuring text, replacing the previous hardcoded REGULAR style lookups.

Changes

Cohort / File(s)	Summary
GfxRenderer Core `lib/GfxRenderer/GfxRenderer.h`, `lib/GfxRenderer/GfxRenderer.cpp`	Method signatures updated for `getSpaceWidth` and `getTextAdvanceX` to accept `EpdFontFamily::Style` parameter with default value for `getSpaceWidth`; implementations now use the provided style instead of hardcoding REGULAR style for glyph lookups.
Callers `lib/Epub/Epub/ParsedText.cpp`, `lib/Epub/Epub/blocks/TextBlock.cpp`	Method calls updated to pass the text style parameter to `getSpaceWidth` and `getTextAdvanceX`, ensuring width measurements respect the applied font style.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: correcting word width and space calculations, which directly addresses the core issue described in the PR of fixing spacing problems in text rendering.
Description check	✅ Passed	The description is clearly related to the changeset, explaining the motivation behind the width calculation fixes and providing visual evidence of the improvement through before/after screenshots.

_{✏️ 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.}

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

lib/GfxRenderer/GfxRenderer.cpp (1)
711-730: ⚠️ Potential issue | 🔴 Critical

Add null checks before dereferencing glyph pointers.

Lines 717 and 729 dereference getGlyph() without null checks. While EpdFontFamily::getFont() correctly masks the UNDERLINE bit and falls back to a valid font variant, the underlying EpdFont::getGlyph() can still return nullptr for codepoints not in the font data (see EpdFont.cpp line 83). This creates a crash path when a codepoint is missing from all available font variants.

Add null checks or fallback to a replacement glyph (e.g., space width or a default value) before dereferencing, similar to the pattern in EpdFont::getTextBounds() which handles missing glyphs gracefully.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/GfxRenderer/GfxRenderer.cpp` around lines 711 - 730,
GfxRenderer::getSpaceWidth and GfxRenderer::getTextAdvanceX currently
dereference the result of fontMap.at(fontId).getGlyph(...) without checking for
nullptr; change both to check the returned glyph pointer and, if null, use a
safe fallback (e.g., use the space glyph advanceX if available or a default 0
width) before accessing ->advanceX; ensure getTextAdvanceX also handles missing
glyphs when iterating codepoints (using utf8NextCodepoint) and sums the fallback
width to avoid crashes for missing codepoints, mirroring the graceful handling
used in EpdFont::getTextBounds().

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 530d439 and 904fa34.

📒 Files selected for processing (4)

lib/Epub/Epub/ParsedText.cpp
lib/Epub/Epub/blocks/TextBlock.cpp
lib/GfxRenderer/GfxRenderer.cpp
lib/GfxRenderer/GfxRenderer.h

🧰 Additional context used

🧬 Code graph analysis (1)

lib/GfxRenderer/GfxRenderer.h (2)

lib/GfxRenderer/GfxRenderer.cpp (4)

getSpaceWidth (711-718)

getSpaceWidth (711-711)

getTextAdvanceX (720-732)

getTextAdvanceX (720-720)

lib/EpdFont/EpdFontFamily.h (1)

EpdFontFamily (4-24)

🔇 Additional comments (3)

lib/GfxRenderer/GfxRenderer.h (1)

106-107: Style-aware spacing API looks good.

Exposing style on the width/advance APIs aligns with the rest of the text methods and avoids hardcoded REGULAR lookups.

lib/Epub/Epub/blocks/TextBlock.cpp (1)

35-37: Advance-width prefix measurement is a good fix.

Using getTextAdvanceX for the em-space prefix should keep underline alignment consistent across styles.

lib/Epub/Epub/ParsedText.cpp (1)

32-53: Advance-width measurement for word layout looks correct.

This should prevent italic overhangs from inflating inter-word spacing, and the soft-hyphen stripping keeps measurements aligned with rendering.

🤖 Prompt for all review comments with AI agents

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

Outside diff comments:
In `@lib/GfxRenderer/GfxRenderer.cpp`:
- Around line 711-730: GfxRenderer::getSpaceWidth and
GfxRenderer::getTextAdvanceX currently dereference the result of
fontMap.at(fontId).getGlyph(...) without checking for nullptr; change both to
check the returned glyph pointer and, if null, use a safe fallback (e.g., use
the space glyph advanceX if available or a default 0 width) before accessing
->advanceX; ensure getTextAdvanceX also handles missing glyphs when iterating
codepoints (using utf8NextCodepoint) and sums the fallback width to avoid
crashes for missing codepoints, mirroring the graceful handling used in
EpdFont::getTextBounds().

# Conflicts: # lib/GfxRenderer/GfxRenderer.cpp

osteotek · 2026-02-19T06:41:40Z

@znelson I think that's a valid fix, what was the reason behind closing it?

znelson · 2026-02-19T07:03:58Z

@osteotek I think it is correct, too. I had another bug in my working branch and for a moment there I thought it was caused by this change.

## Summary **What is the goal of this PR?** This change fixes an issue I noticed while reading where occasionally, especially in italics, some words would have too much space between them. The problem was that word width calculations were including any negative X overhang, and combined with a space before the word, that can lead to an inconsistently large space. ## Additional Context Screenshots of some problematic text: | In CrossPoint 1.0 | With this change | | -- | -- | | <img src="https://github.com/user-attachments/assets/87bf0e4b-341f-4ba9-b3ea-38c13bd26363" width="400" /> | <img src="https://github.com/user-attachments/assets/bf11ba20-c297-4ce1-aa07-43477ef86fc2" width="400" /> | --- ### 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**_

fix: Correct word width and space calculations

904fa34

znelson marked this pull request as ready for review February 18, 2026 05:29

coderabbitai bot reviewed Feb 18, 2026

View reviewed changes

Merge branch 'master' into italic-word-spacing

7a03cb2

# Conflicts: # lib/GfxRenderer/GfxRenderer.cpp

znelson closed this Feb 19, 2026

znelson reopened this Feb 19, 2026

daveallie approved these changes Feb 19, 2026

View reviewed changes

daveallie merged commit f1740db into crosspoint-reader:master Feb 19, 2026
11 checks passed

daveallie mentioned this pull request Feb 19, 2026

chore(release): 1.1.0 Release Candidate #992

Merged

znelson deleted the italic-word-spacing branch February 19, 2026 14:14

This was referenced Feb 19, 2026

fix: Fixed book title in home screen #1013

Merged

perf: Reduce ParsedText layout time 7–9% via word-width cache and hyphenation early exit #1027

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: Correct word width and space calculations#963

fix: Correct word width and space calculations#963
daveallie merged 2 commits intocrosspoint-reader:masterfrom
znelson:italic-word-spacing

znelson commented Feb 18, 2026

Uh oh!

coderabbitai bot commented Feb 18, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Uh oh!

coderabbitai bot left a comment

Uh oh!

osteotek commented Feb 19, 2026

Uh oh!

znelson commented Feb 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

znelson commented Feb 18, 2026

Summary

Additional Context

AI Usage

Uh oh!

coderabbitai bot commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

osteotek commented Feb 19, 2026

Uh oh!

znelson commented Feb 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai bot commented Feb 18, 2026 •

edited

Loading