Files
Zach Nelson 075ad7d021 fix: Use font metrics for combining mark positioning (#1310)
## Summary

**What is the goal of this PR?** (e.g., Implements the new feature for
file uploading.)

Combining diacritical marks (U+0300–U+036F) were positioned using a
heuristic that centered them at the midpoint of the base glyph's
**advance width**. This worked acceptably for Bookerly but produced
visibly off-center marks for Noto Sans due to a fundamental difference
in how the two fonts design their combining mark metrics.

Builds on the work of #1037.

## The problem

The two built-in body fonts encode combining mark `left` offsets with
very different conventions:

| Mark | Bookerly `left` | Noto Sans `left` |
|---|---|---|
| U+0301 (acute) | -2 | -10 |
| U+0300 (grave) | -5 | -15 |
| U+0302 (circumflex) | -5 | -5 |
| U+0323 (dot below) | -2 | -11 |

Noto Sans uses large negative `left` values because its marks are
designed for placement at the post-advance cursor position, with `left`
pulling the bitmap back over the base glyph. Bookerly uses small offsets
because its marks sit closer to the glyph origin. The old `advance/2`
centering split the difference poorly — it happened to land close to
correct for Bookerly but placed Noto Sans marks roughly 6px left of
center on a typical lowercase letter.

There was also a bug in the vertical gap heuristic. It unconditionally
computed a `raiseBy` value to prevent above-baseline marks from
colliding with tall base glyphs, but it applied the same logic to
**below-baseline** marks like cedilla (U+0327), dot below (U+0323), and
ogonek (U+0328). For those marks, the math produced a large positive
raise (e.g., 24px for dot-below on 'a'), launching them above the
x-height instead of keeping them below the baseline.

## The fix

**Horizontal positioning**: Instead of centering at `advance/2`, align
the mark bitmap's visual midpoint directly over the base glyph bitmap's
visual midpoint. This uses the base glyph's actual `left` and `width`
rather than its advance width, producing correct results regardless of
how the font encodes its mark offsets.

**Vertical positioning**: The raise heuristic now checks `markTop -
markHeight > 0` and skips below-baseline marks entirely, leaving them at
their font-designed position.

**Consolidation**: The shared math is extracted into two `constexpr`
helpers (`combiningMark::centerOver` and
`combiningMark::raiseAboveBase`) in `EpdFontData.h`, eliminating the
previously triplicated inline calculations across `drawText`,
`drawTextRotated90CW`, and `getTextBounds`. The `MIN_COMBINING_GAP_PX`
constant is also centralized as `combiningMark::MIN_GAP_PX`.

| Before | After |
| -- | -- |
| <img
src="https://github.com/user-attachments/files/25752257/before-noto.bmp"
width="250" /> | <img
src="https://github.com/user-attachments/files/25752258/after-noto.bmp"
width="250" /> |
| <img
src="https://github.com/user-attachments/files/25752259/before-bookerly.bmp"
width="250" /> | <img
src="https://github.com/user-attachments/files/25752260/after-bookerly.bmp"
width="250" /> |

---

### 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
differences between Noto Sans and Bookerly font metrics**_

---------

Co-authored-by: Uri Tauber <142022451+Uri-Tauber@users.noreply.github.com>
2026-04-14 14:35:50 -05:00

133 lines
6.7 KiB
C++

// From
// https://github.com/vroland/epdiy/blob/c61e9e923ce2418150d54f88cea5d196cdc40c54/src/epd_internals.h
#pragma once
#include <cstdint>
/// Font metrics use "fixed-point 4" (4 fractional bits, i.e. 1/16-pixel
/// resolution). Both the 12.4 glyph advances (uint16_t) and the 4.4 kern
/// values (int8_t) share the same 4 fractional bits, so they can be freely
/// added before snapping to whole pixels.
///
/// Rendering and measurement use "differential rounding": each glyph step
/// (previous advance + current kern) is combined in fixed-point and snapped
/// to a pixel as one unit. This guarantees identical character pairs always
/// produce the same pixel spacing, regardless of position on the line.
///
/// The helpers below eliminate the raw bit-shifts that would otherwise be
/// scattered across every layout / measurement call site.
namespace fp4 {
constexpr int FRAC_BITS = 4;
constexpr int32_t HALF = 1 << (FRAC_BITS - 1); // 8, added before shift for round-to-nearest
/// Convert an integer pixel value to 12.4 fixed-point.
constexpr int32_t fromPixel(int px) { return static_cast<int32_t>(px) << FRAC_BITS; }
/// Snap a fixed-point value to the nearest integer pixel.
constexpr int toPixel(int32_t fp) { return static_cast<int>((fp + HALF) >> FRAC_BITS); }
/// Convert a fixed-point value to float (mainly useful for debug logging).
constexpr float toFloat(int32_t fp) { return fp / static_cast<float>(1 << FRAC_BITS); }
} // namespace fp4
/// Helpers for positioning Unicode combining marks (U+0300 ff.) over a
/// preceding base glyph without GPOS anchor tables.
namespace combiningMark {
constexpr int MIN_GAP_PX = 1;
/// Compute the cursor-X at which to render a combining mark so its bitmap
/// is visually centered over the base glyph's bitmap.
constexpr int centerOver(int baseCursorPos, int baseLeft, int baseWidth, int markLeft, int markWidth) {
return baseCursorPos + baseLeft + baseWidth / 2 - markWidth / 2 - markLeft;
}
/// Rotated-90CW variant of centerOver. In the rotated coordinate system
/// renderCharImpl uses (cursorY - left) instead of (cursorX + left), so
/// every left/width term inverts sign.
constexpr int centerOverRotated90CW(int baseCursorPos, int baseLeft, int baseWidth, int markLeft, int markWidth) {
return baseCursorPos - baseLeft - baseWidth / 2 + markWidth / 2 + markLeft;
}
/// For combining marks that sit entirely above the baseline, compute how many
/// pixels to raise the mark so there is at least MIN_GAP_PX between its bottom
/// edge and the top of the base glyph. Returns 0 for marks that extend to or
/// below the baseline (e.g. cedilla, dot-below, ogonek).
constexpr int raiseAboveBase(int markTop, int markHeight, int baseTop) {
if (markTop - markHeight <= 0) return 0;
const int gap = markTop - markHeight - baseTop;
return (gap < MIN_GAP_PX) ? (MIN_GAP_PX - gap) : 0;
}
} // namespace combiningMark
/// Fixed-point conventions used by EpdGlyph and EpdFontData:
/// advanceX: 12.4 unsigned fixed-point in uint16_t (use fp4::toPixel)
/// kernMatrix: 4.4 signed fixed-point in int8_t (use fp4::toPixel)
/// Both share 4 fractional bits so they combine directly in an accumulator.
/// Font data stored PER GLYPH
typedef struct {
uint8_t width; ///< Bitmap dimensions in pixels
uint8_t height; ///< Bitmap dimensions in pixels
uint16_t advanceX; ///< Distance to advance cursor (x axis), 12.4 fixed-point in pixels
int16_t left; ///< X dist from cursor pos to UL corner
int16_t top; ///< Y dist from cursor pos to UL corner
uint16_t dataLength; ///< Size of the font data.
uint32_t dataOffset; ///< Pointer into EpdFont->bitmap (or within-group offset for compressed fonts)
} EpdGlyph;
/// Compressed font group: a DEFLATE-compressed block of glyph bitmaps
typedef struct {
uint32_t compressedOffset; ///< Byte offset into compressed data array
uint32_t compressedSize; ///< Compressed DEFLATE stream size
uint32_t uncompressedSize; ///< Decompressed size
uint16_t glyphCount; ///< Number of glyphs in this group
uint32_t firstGlyphIndex; ///< First glyph index in the global glyph array
} EpdFontGroup;
/// Glyph interval structure
typedef struct {
uint32_t first; ///< The first unicode code point of the interval
uint32_t last; ///< The last unicode code point of the interval
uint32_t offset; ///< Index of the first code point into the glyph array
} EpdUnicodeInterval;
/// Maps a codepoint to a kerning class ID, sorted by codepoint for binary search.
/// Class IDs are 1-based; codepoints not in the table have implicit class 0 (no kerning).
typedef struct {
uint16_t codepoint; ///< Unicode codepoint
uint8_t classId; ///< 1-based kerning class ID
} __attribute__((packed)) EpdKernClassEntry;
/// Ligature substitution for a specific glyph pair, sorted by `pair` for binary search.
/// `pair` encodes (leftCodepoint << 16 | rightCodepoint) for single-key lookup.
typedef struct {
uint32_t pair; ///< Packed codepoint pair (left << 16 | right)
uint32_t ligatureCp; ///< Codepoint of the replacement ligature glyph
} __attribute__((packed)) EpdLigaturePair;
/// Data stored for FONT AS A WHOLE
typedef struct {
const uint8_t* bitmap; ///< Glyph bitmaps, concatenated
const EpdGlyph* glyph; ///< Glyph array
const EpdUnicodeInterval* intervals; ///< Valid unicode intervals for this font
uint32_t intervalCount; ///< Number of unicode intervals.
uint8_t advanceY; ///< Newline distance (y axis)
int ascender; ///< Maximal height of a glyph above the base line
int descender; ///< Maximal height of a glyph below the base line
bool is2Bit;
const EpdFontGroup* groups; ///< NULL for uncompressed fonts
uint16_t groupCount; ///< 0 for uncompressed fonts
const uint16_t* glyphToGroup; ///< Per-glyph group ID (nullptr for contiguous-group fonts)
const EpdKernClassEntry* kernLeftClasses; ///< Sorted left-side class map (nullptr if none)
const EpdKernClassEntry* kernRightClasses; ///< Sorted right-side class map (nullptr if none)
const int8_t* kernMatrix; ///< Flat leftClassCount x rightClassCount matrix, 4.4 fixed-point in pixels
uint16_t kernLeftEntryCount; ///< Entries in kernLeftClasses
uint16_t kernRightEntryCount; ///< Entries in kernRightClasses
uint8_t kernLeftClassCount; ///< Number of distinct left classes (matrix rows)
uint8_t kernRightClassCount; ///< Number of distinct right classes (matrix cols)
const EpdLigaturePair* ligaturePairs; ///< Sorted ligature pair table (nullptr if none)
uint32_t ligaturePairCount; ///< Number of entries in ligaturePairs
} EpdFontData;