fix: Fix inter-word spacing rounding error in text layout (#1311)
## Summary
**What is the goal of this PR?**
### Problem
Inter-word gap widths were computed as two separately-snapped integers:
```cpp
gap = getSpaceWidth(fontId, style); // fp4::toPixel(spaceAdvance)
gap += getSpaceKernAdjust(fontId, leftCp, rightCp, style); // fp4::toPixel(kern1 + kern2)
```
Because `fp4::toPixel(a) + fp4::toPixel(b)` can differ from
`fp4::toPixel(a + b)` by +/-1 pixel when the fractional parts straddle a
rounding boundary, each inter-word space could be one pixel wider or
narrower than the correct value. This affected line-break width
decisions and word-position accumulation across the whole paragraph
layout pipeline.
### Fix
Replaces `getSpaceKernAdjust()` with `getSpaceAdvance(fontId, leftCp,
rightCp, style)`, which combines the space glyph advance and both
flanking kern values (`kern(leftCp, ' ')` + `kern(' ', rightCp)`) into a
single fixed-point sum before the snap:
```cpp
return fp4::toPixel(spaceAdvanceFP + kern(leftCp, ' ') + kern(' ', rightCp));
```
This is the same single-snap pattern already used by `getTextAdvanceX`
for word widths.
### Changes
- **`GfxRenderer`**: Replaces `getSpaceKernAdjust()` with
`getSpaceAdvance()`. `getSpaceWidth()` is retained for the
single-space-word case in `measureWordWidth` where no adjacent-word kern
context is available.
- **`ParsedText`**: All four call sites (`computeLineBreaks`,
`computeHyphenatedLineBreaks`, and both loops in `extractLine`) updated
to use `getSpaceAdvance()`. The now-redundant `spaceWidth`
pre-computation and parameter are removed from all three internal layout
functions.
---
### 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 to analyze for
correctness**_
This commit is contained in:
@@ -943,13 +943,18 @@ int GfxRenderer::getSpaceWidth(const int fontId, const EpdFontFamily::Style styl
|
||||
return spaceGlyph ? fp4::toPixel(spaceGlyph->advanceX) : 0; // snap 12.4 fixed-point to nearest pixel
|
||||
}
|
||||
|
||||
int GfxRenderer::getSpaceKernAdjust(const int fontId, const uint32_t leftCp, const uint32_t rightCp,
|
||||
const EpdFontFamily::Style style) const {
|
||||
int GfxRenderer::getSpaceAdvance(const int fontId, const uint32_t leftCp, const uint32_t rightCp,
|
||||
const EpdFontFamily::Style style) const {
|
||||
const auto fontIt = fontMap.find(fontId);
|
||||
if (fontIt == fontMap.end()) return 0;
|
||||
const auto& font = fontIt->second;
|
||||
const int kernFP = font.getKerning(leftCp, ' ', style) + font.getKerning(' ', rightCp, style); // 4.4 fixed-point
|
||||
return fp4::toPixel(kernFP); // snap 4.4 fixed-point to nearest pixel
|
||||
const EpdGlyph* spaceGlyph = font.getGlyph(' ', style);
|
||||
const int32_t spaceAdvanceFP = spaceGlyph ? static_cast<int32_t>(spaceGlyph->advanceX) : 0;
|
||||
// Combine space advance + flanking kern into one fixed-point sum before snapping.
|
||||
// Snapping the combined value avoids the +/-1 px error from snapping each component separately.
|
||||
const int32_t kernFP = static_cast<int32_t>(font.getKerning(leftCp, ' ', style)) +
|
||||
static_cast<int32_t>(font.getKerning(' ', rightCp, style));
|
||||
return fp4::toPixel(spaceAdvanceFP + kernFP);
|
||||
}
|
||||
|
||||
int GfxRenderer::getKerning(const int fontId, const uint32_t leftCp, const uint32_t rightCp,
|
||||
|
||||
@@ -110,9 +110,10 @@ class GfxRenderer {
|
||||
void drawText(int fontId, int x, int y, const char* text, bool black = true,
|
||||
EpdFontFamily::Style style = EpdFontFamily::REGULAR) const;
|
||||
int getSpaceWidth(int fontId, EpdFontFamily::Style style = EpdFontFamily::REGULAR) const;
|
||||
/// Returns the kerning adjustment for a space between two codepoints:
|
||||
/// kern(leftCp, ' ') + kern(' ', rightCp). Returns 0 if kerning is unavailable.
|
||||
int getSpaceKernAdjust(int fontId, uint32_t leftCp, uint32_t rightCp, EpdFontFamily::Style style) const;
|
||||
/// Returns the total inter-word advance: fp4::toPixel(spaceAdvance + kern(leftCp,' ') + kern(' ',rightCp)).
|
||||
/// Using a single snap avoids the +/-1 px rounding error that arises when space advance and kern are
|
||||
/// snapped separately and then added as integers.
|
||||
int getSpaceAdvance(int fontId, uint32_t leftCp, uint32_t rightCp, EpdFontFamily::Style style) const;
|
||||
/// Returns the kerning adjustment between two adjacent codepoints.
|
||||
int getKerning(int fontId, uint32_t leftCp, uint32_t rightCp, EpdFontFamily::Style style) const;
|
||||
int getTextAdvanceX(int fontId, const char* text, EpdFontFamily::Style style) const;
|
||||
|
||||
Reference in New Issue
Block a user