Skip to content

Incorrect text kerning logic in layout_glyphs #761

@eventualbuddha

Description

@eventualbuddha

Note: I found these bugs using an AI agent as part of my work on a task to streamline my cargo workspace's dependency tree. I've been happy with imageproc, but decided to remove it because of some unavoidable heavyweight dependencies like nalgebra. I only use a small amount of imageproc's overall API, so I decided to inline the few functions I did use. GitHub Copilot reviewed the inlined functions and pointed out some bugs, which I initially dismissed but decided I should try to prove it wrong. Instead, I found that it was correct.

imageproc does not have an AI policy as far as I've been able to find, so I'm including this preface because I want to be clear about my use of AI agents in finding and evaluating these bugs. I used Copilot as mentioned above, and then used Gemini to generate a sample program to validate the fix. I am not using it as a substitute for understanding, as I believe I understand the bugs and why the fix is correct. I've been writing Rust as a part of my work's tech stack since 2021, so I believe I can reasonably claim I've taken steps to ensure this is not AI slop.

I looked at other users of ab_glyph to see how they perform text layout and found that the main issue below (Reversed Kerning Arguments) is done correctly in other crates:

Below is the analysis of the bugs and a proposed fix as written by Gemini and edited by me. If it's helpful, I can create a pull request to fix this issue.


The imageproc::drawing::text::layout_glyphs function currently has three related bugs in how it calculates and applies kerning offsets between adjacent text characters, resulting in incorrect visual spacing for fonts.

Issues

  1. Reversed Kerning Arguments: The ab_glyph::ScaleFont::kern function expects (first, second) (left glyph, right glyph). The current implementation calls it with (glyph_id, prev) which evaluates to (right, left).
  2. Incorrect Offset Application Timing: The kerning offset is added to w after the current glyph has already been positioned using w. This means the kerning does not apply to the current glyph, but incorrectly shifts all subsequent glyphs.
  3. Skipped Whitespace: The font.outline_glyph() method returns None for whitespace characters like spaces. Because the kerning check and prev = Some(...) assignment are nested inside this block, spaces are skipped. This breaks kerning relationships across whitespace and loses advances.

Visual Impact

Below is an example rendering the text "AV TA T. WA" with a TrueType font (DejaVuSans.ttf). The before image shows awkward, overly wide gaps between natural kerning pairs (like T and A) due to the bugs above.

Before Fix:
Image

After Fix:
Image

(Notice how the letters A & V, T & A, and W & A tuck under each other nicely, and the period sits closely to the T.)

Suggested Fix

The fix is to move the kerning evaluation before positioning the glyph, correct the argument order, and ensure whitespace characters update the prev tracker and advance width:

--- a/src/drawing/text.rs
+++ b/src/drawing/text.rs
@@ -23,16 +23,20 @@ fn layout_glyphs(
 
     for c in text.chars() {
         let glyph_id = font.glyph_id(c);
+
+        if let Some(prev) = prev {
+            w += font.kern(prev, glyph_id);
+        }
+
         let glyph = glyph_id.with_scale_and_position(scale, point(w, font.ascent()));
-        w += font.h_advance(glyph_id);
+
         if let Some(g) = font.outline_glyph(glyph) {
-            if let Some(prev) = prev {
-                w += font.kern(glyph_id, prev);
-            }
-            prev = Some(glyph_id);
             let bb = g.px_bounds();
             f(g, bb);
         }
+
+        w += font.h_advance(glyph_id);
+        prev = Some(glyph_id);
     }
 
     let w = w.ceil();

Here is the sample program used to generate the above images:

//! Demonstrates text kerning using a TrueType font.
//!
//! Run with: `cargo run --example text_kerning`

use ab_glyph::{FontRef, PxScale};
use image::{Rgb, RgbImage};
use imageproc::drawing::draw_text_mut;

fn main() {
    let font_data = include_bytes!("../tests/data/fonts/DejaVuSans.ttf");
    let font = FontRef::try_from_slice(font_data).unwrap();
    let scale = PxScale { x: 60.0, y: 60.0 };

    let mut img = RgbImage::new(450, 100);
    let text = "AV TA T. WA";
    
    // Draw text with kerning pairs (A and V, T and A, etc.)
    draw_text_mut(&mut img, Rgb([255, 255, 255]), 10, 20, scale, &font, text);
    
    img.save("text_kerning.png").unwrap();
    println!("Saved text kerning example to text_kerning.png");
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions