[gpu] Assign curves to bands using quantized bounds (#6132)
The fragment shader picks one horizontal and one vertical band from
the fragment position and walks only that band's curve list, so a
curve must be listed in every band its bounding box reaches into.
The encoder computed those bounding boxes from the unquantized
control points, but the shader only ever sees the coordinates after
they are rounded to quarter font units. When rounding pushed a
curve's extreme across a band boundary, the curve was left out of the
band it now reached into. Fragments landing in that sliver never saw
the curve, so their winding number was off by one and the pixel came
out empty in the middle of a filled area -- a thin black line through
the glyph, appearing and disappearing as the font size moves the
pixel grid.
The rounding error is at most an eighth of a font unit, so this is
invisible for an integer-coordinate glyph rendered at its own upem.
It gets easy to hit when the coordinates are fractional: CFF glyphs
after cu2qu, or any font whose hb_font_t scale is not its upem. With
hb_font_set_scale(font, 163, 163), as in issue #6131, more than half
of the glyphs of util/gpu/default-font.ttf had at least one band the
shader could reach but that did not list the curve.
Compute the bounds (and the is_horizontal/is_vertical tests) from the
quantized control points instead, and widen the band range by 1/1024
of a band so that the double arithmetic here cannot disagree with the
shader's float32 band index right at a boundary.
Verified with a CPU port of _hb_gpu_slug_single: comparing the banded
lookup against a walk over every curve of the glyph, at sizes 39, 47
and 107 with scale 163, default-font.ttf had 27 wrong pixels before
and none after. Encoded atlas size grows by 0.12%.
Adds test_encode_band_membership, which checks the invariant directly
on the encoded blob and fails before this change.
Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
diff --git a/src/hb-gpu-draw.cc b/src/hb-gpu-draw.cc
index 710b145..4165264 100644
--- a/src/hb-gpu-draw.cc
+++ b/src/hb-gpu-draw.cc
@@ -298,7 +298,7 @@
}
static inline double
-dequantize (int16_t v)
+dequantize (int v)
{
return (double) v / HB_GPU_UNITS_PER_EM;
}
@@ -325,17 +325,26 @@
return (int16_t) (offset - 32768u);
}
+/* Note: the bounds are computed from the *quantized* control points,
+ * because that is what the shader sees. Band membership derived from
+ * the unquantized bounds can leave a curve out of the band that its
+ * quantized extent reaches into; fragments in that sliver then never
+ * see the curve, get the wrong winding number, and render as a thin
+ * black line through the glyph. */
static hb_gpu_encode_curve_info_t
encode_curve_info (const hb_gpu_curve_t *c)
{
hb_gpu_encode_curve_info_t info;
- info.min_x = hb_min (hb_min (c->p1x, c->p2x), c->p3x);
- info.max_x = hb_max (hb_max (c->p1x, c->p2x), c->p3x);
- info.min_y = hb_min (hb_min (c->p1y, c->p2y), c->p3y);
- info.max_y = hb_max (hb_max (c->p1y, c->p2y), c->p3y);
- info.is_horizontal = c->p1y == c->p2y && c->p2y == c->p3y;
- info.is_vertical = c->p1x == c->p2x && c->p2x == c->p3x;
+ int p1x = quantize (c->p1x), p2x = quantize (c->p2x), p3x = quantize (c->p3x);
+ int p1y = quantize (c->p1y), p2y = quantize (c->p2y), p3y = quantize (c->p3y);
+
+ info.min_x = dequantize (hb_min (hb_min (p1x, p2x), p3x));
+ info.max_x = dequantize (hb_max (hb_max (p1x, p2x), p3x));
+ info.min_y = dequantize (hb_min (hb_min (p1y, p2y), p3y));
+ info.max_y = dequantize (hb_max (hb_max (p1y, p2y), p3y));
+ info.is_horizontal = p1y == p2y && p2y == p3y;
+ info.is_vertical = p1x == p2x && p2x == p3x;
info.hband_lo = 0;
info.hband_hi = -1;
info.vband_lo = 0;
@@ -484,6 +493,11 @@
double hband_size = height / num_hbands;
double vband_size = width / num_vbands;
+ /* The shader recomputes the band index from the fragment position in
+ * float32, this code does it in double. Widen the band range of each
+ * curve by a hair so the two cannot disagree at a band boundary. */
+ static const double BAND_EPSILON = 1.0 / 1024;
+
if (unlikely (!s.hband_curve_counts.resize (num_hbands) ||
!s.vband_curve_counts.resize (num_vbands)))
return nullptr;
@@ -498,8 +512,8 @@
if (!info.is_horizontal)
{
if (height > 0) {
- info.hband_lo = (int) floor ((info.min_y - min_y) / hband_size);
- info.hband_hi = (int) floor ((info.max_y - min_y) / hband_size);
+ info.hband_lo = (int) floor ((info.min_y - min_y) / hband_size - BAND_EPSILON);
+ info.hband_hi = (int) floor ((info.max_y - min_y) / hband_size + BAND_EPSILON);
info.hband_lo = hb_max (info.hband_lo, 0);
info.hband_hi = hb_min (info.hband_hi, (int) num_hbands - 1);
for (int b = info.hband_lo; b <= info.hband_hi; b++)
@@ -514,8 +528,8 @@
if (!info.is_vertical)
{
if (width > 0) {
- info.vband_lo = (int) floor ((info.min_x - min_x) / vband_size);
- info.vband_hi = (int) floor ((info.max_x - min_x) / vband_size);
+ info.vband_lo = (int) floor ((info.min_x - min_x) / vband_size - BAND_EPSILON);
+ info.vband_hi = (int) floor ((info.max_x - min_x) / vband_size + BAND_EPSILON);
info.vband_lo = hb_max (info.vband_lo, 0);
info.vband_hi = hb_min (info.vband_hi, (int) num_vbands - 1);
for (int b = info.vband_lo; b <= info.vband_hi; b++)
diff --git a/test/api/test-gpu.cc b/test/api/test-gpu.cc
index 42ec372..c18ed48 100644
--- a/test/api/test-gpu.cc
+++ b/test/api/test-gpu.cc
@@ -291,6 +291,115 @@
hb_gpu_draw_destroy (draw);
}
+/* The fragment shader picks a single horizontal and a single vertical
+ * band from the fragment position and only walks that band's curve
+ * list. So every curve must be listed in every band that its
+ * *quantized* bounding box reaches into; a curve missing from a band
+ * gives fragments in that band the wrong winding number, which shows
+ * up as a thin black line through the glyph.
+ *
+ * https://github.com/harfbuzz/harfbuzz/issues/6131 */
+static void
+assert_band_membership (hb_blob_t *blob)
+{
+ unsigned texel_count;
+ const hb_gpu_test_texel_t *t = blob_as_texels (blob, &texel_count);
+ if (texel_count < 2)
+ return;
+
+ /* Coordinates are stored as quarters of a font unit. */
+ double min_x = t[0].r / 4., min_y = t[0].g / 4.;
+ double max_x = t[0].b / 4., max_y = t[0].a / 4.;
+ unsigned num_hbands = (unsigned) t[1].r;
+ unsigned num_vbands = (unsigned) t[1].g;
+ g_assert_cmpuint (num_hbands, >, 0);
+ g_assert_cmpuint (num_vbands, >, 0);
+ g_assert_cmpuint (2 + num_hbands + num_vbands, <=, texel_count);
+
+ double hband_size = (max_y - min_y) / num_hbands;
+ double vband_size = (max_x - min_x) / num_vbands;
+
+ for (unsigned b = 0; b < num_hbands + num_vbands; b++)
+ {
+ hb_bool_t horizontal = b < num_hbands;
+ unsigned count = (unsigned) (uint16_t) t[2 + b].r;
+ unsigned list = (unsigned) ((int) t[2 + b].g + 32768);
+ g_assert_cmpuint (list + count, <=, texel_count);
+
+ for (unsigned ci = 0; ci < count; ci++)
+ {
+ unsigned c = (unsigned) ((int) t[list + ci].r + 32768);
+ g_assert_cmpuint (c + 1, <, texel_count);
+
+ /* Every band the curve reaches into, in the axis of this band. */
+ int lo, hi;
+ if (horizontal)
+ {
+ int q1 = t[c].g, q2 = t[c].a, q3 = t[c + 1].g;
+ if (q1 == q2 && q2 == q3) continue; /* horizontal: skipped by design */
+ if (!(hband_size > 0)) continue;
+ lo = (int) floor ((MIN (MIN (q1, q2), q3) / 4. - min_y) / hband_size);
+ hi = (int) floor ((MAX (MAX (q1, q2), q3) / 4. - min_y) / hband_size);
+ lo = MAX (lo, 0);
+ hi = MIN (hi, (int) num_hbands - 1);
+ }
+ else
+ {
+ int q1 = t[c].r, q2 = t[c].b, q3 = t[c + 1].r;
+ if (q1 == q2 && q2 == q3) continue; /* vertical: skipped by design */
+ if (!(vband_size > 0)) continue;
+ lo = (int) floor ((MIN (MIN (q1, q2), q3) / 4. - min_x) / vband_size);
+ hi = (int) floor ((MAX (MAX (q1, q2), q3) / 4. - min_x) / vband_size);
+ lo = MAX (lo, 0);
+ hi = MIN (hi, (int) num_vbands - 1);
+ }
+
+ for (int ob = lo; ob <= hi; ob++)
+ {
+ unsigned obb = horizontal ? (unsigned) ob : num_hbands + (unsigned) ob;
+ unsigned ocount = (unsigned) (uint16_t) t[2 + obb].r;
+ unsigned olist = (unsigned) ((int) t[2 + obb].g + 32768);
+ g_assert_cmpuint (olist + ocount, <=, texel_count);
+
+ hb_bool_t found = false;
+ for (unsigned oi = 0; oi < ocount && !found; oi++)
+ found = (unsigned) ((int) t[olist + oi].r + 32768) == c;
+ g_assert_true (found);
+ }
+ }
+ }
+}
+
+static void
+test_encode_band_membership (void)
+{
+ hb_face_t *face = hb_test_open_font_file (FONT_FILE);
+ g_assert_nonnull (face);
+ hb_font_t *font = hb_font_create (face);
+ /* A scale other than the font's upem makes the glyph coordinates
+ * fractional, so quantizing them rounds. Band membership derived
+ * from the unquantized coordinates then misses bands. */
+ hb_font_set_scale (font, 128, 128);
+
+ hb_gpu_draw_t *draw = hb_gpu_draw_create_or_fail ();
+ g_assert_nonnull (draw);
+
+ unsigned glyph_count = hb_face_get_glyph_count (face);
+ for (hb_codepoint_t gid = 0; gid < glyph_count; gid++)
+ {
+ hb_gpu_draw_clear (draw);
+ hb_gpu_draw_glyph (draw, font, gid);
+ hb_blob_t *blob = hb_gpu_draw_encode (draw, nullptr);
+ g_assert_nonnull (blob);
+ assert_band_membership (blob);
+ hb_blob_destroy (blob);
+ }
+
+ hb_gpu_draw_destroy (draw);
+ hb_font_destroy (font);
+ hb_face_destroy (face);
+}
+
static void
test_extents_saturate_overflow (void)
{
@@ -661,6 +770,7 @@
hb_test_add (test_encode_quantizes_extents_outward);
hb_test_add (test_encode_preserves_touching_contours);
hb_test_add (test_recycle_blob);
+ hb_test_add (test_encode_band_membership);
hb_test_add (test_extents_saturate_overflow);
hb_test_add (test_shapes);
