font: have Glyph return !ok for U+FFFD substitute
The other return values may still be non-zero, but this lets callers
identify when substitution happens.
"TODO: is falling back on the U+FFFD glyph the responsibility of the
Drawer or the Face?" was resolved. The answer is "the Face". For
kerning, the previous rune is unchanged (and not set to U+FFFD).
This also fixes an inconsistency in the basicfont.Face implementation,
where GlyphAdvance and GlyphBounds would unconditionally return a
non-zero advance, but Glyph could return a zero advance when the Face
doesn't have a U+FFFD entry.
Fixes golang/go#58252
Change-Id: Ie97e68e1d5e2efd13c9e84ad12db4495d83a5ca3
Reviewed-on: https://go-review.googlesource.com/c/image/+/474376
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Nigel Tao (INACTIVE; USE @golang.org INSTEAD) <nigeltao@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Nigel Tao <nigeltao@golang.org>
diff --git a/font/basicfont/basicfont.go b/font/basicfont/basicfont.go
index 1550381..173c061 100644
--- a/font/basicfont/basicfont.go
+++ b/font/basicfont/basicfont.go
@@ -89,41 +89,50 @@
func (f *Face) Glyph(dot fixed.Point26_6, r rune) (
dr image.Rectangle, mask image.Image, maskp image.Point, advance fixed.Int26_6, ok bool) {
-loop:
- for _, rr := range [2]rune{r, '\ufffd'} {
- for _, rng := range f.Ranges {
- if rr < rng.Low || rng.High <= rr {
- continue
- }
- maskp.Y = (int(rr-rng.Low) + rng.Offset) * (f.Ascent + f.Descent)
- ok = true
- break loop
+ if found, rng := f.find(r); rng != nil {
+ maskp.Y = (int(found-rng.Low) + rng.Offset) * (f.Ascent + f.Descent)
+ x := int(dot.X+32)>>6 + f.Left
+ y := int(dot.Y+32) >> 6
+ dr = image.Rectangle{
+ Min: image.Point{
+ X: x,
+ Y: y - f.Ascent,
+ },
+ Max: image.Point{
+ X: x + f.Width,
+ Y: y + f.Descent,
+ },
}
- }
- if !ok {
- return image.Rectangle{}, nil, image.Point{}, 0, false
- }
- x := int(dot.X+32)>>6 + f.Left
- y := int(dot.Y+32) >> 6
- dr = image.Rectangle{
- Min: image.Point{
- X: x,
- Y: y - f.Ascent,
- },
- Max: image.Point{
- X: x + f.Width,
- Y: y + f.Descent,
- },
+ return dr, f.Mask, maskp, fixed.I(f.Advance), r == found
}
-
- return dr, f.Mask, maskp, fixed.I(f.Advance), true
+ return image.Rectangle{}, nil, image.Point{}, 0, false
}
func (f *Face) GlyphBounds(r rune) (bounds fixed.Rectangle26_6, advance fixed.Int26_6, ok bool) {
- return fixed.R(0, -f.Ascent, f.Width, +f.Descent), fixed.I(f.Advance), true
+ if found, rng := f.find(r); rng != nil {
+ return fixed.R(0, -f.Ascent, f.Width, +f.Descent), fixed.I(f.Advance), r == found
+ }
+ return fixed.Rectangle26_6{}, 0, false
}
func (f *Face) GlyphAdvance(r rune) (advance fixed.Int26_6, ok bool) {
- return fixed.I(f.Advance), true
+ if found, rng := f.find(r); rng != nil {
+ return fixed.I(f.Advance), r == found
+ }
+ return 0, false
+}
+
+func (f *Face) find(r rune) (rune, *Range) {
+ for {
+ for i, rng := range f.Ranges {
+ if (rng.Low <= r) && (r < rng.High) {
+ return r, &f.Ranges[i]
+ }
+ }
+ if r == '\ufffd' {
+ return 0, nil
+ }
+ r = '\ufffd'
+ }
}
diff --git a/font/font.go b/font/font.go
index d1a7535..6b9b9bc 100644
--- a/font/font.go
+++ b/font/font.go
@@ -38,7 +38,10 @@
// glyph at the sub-pixel destination location dot, and that glyph's
// advance width.
//
- // It returns !ok if the face does not contain a glyph for r.
+ // It returns !ok if the face does not contain a glyph for r. This includes
+ // returning !ok for a fallback glyph (such as substituting a U+FFFD glyph
+ // or OpenType's .notdef glyph), in which case the other return values may
+ // still be non-zero.
//
// The contents of the mask image returned by one Glyph call may change
// after the next Glyph call. Callers that want to cache the mask must make
@@ -49,7 +52,10 @@
// GlyphBounds returns the bounding box of r's glyph, drawn at a dot equal
// to the origin, and that glyph's advance width.
//
- // It returns !ok if the face does not contain a glyph for r.
+ // It returns !ok if the face does not contain a glyph for r. This includes
+ // returning !ok for a fallback glyph (such as substituting a U+FFFD glyph
+ // or OpenType's .notdef glyph), in which case the other return values may
+ // still be non-zero.
//
// The glyph's ascent and descent are equal to -bounds.Min.Y and
// +bounds.Max.Y. The glyph's left-side and right-side bearings are equal
@@ -60,7 +66,10 @@
// GlyphAdvance returns the advance width of r's glyph.
//
- // It returns !ok if the face does not contain a glyph for r.
+ // It returns !ok if the face does not contain a glyph for r. This includes
+ // returning !ok for a fallback glyph (such as substituting a U+FFFD glyph
+ // or OpenType's .notdef glyph), in which case the other return values may
+ // still be non-zero.
GlyphAdvance(r rune) (advance fixed.Int26_6, ok bool)
// Kern returns the horizontal adjustment for the kerning pair (r0, r1). A
@@ -150,14 +159,10 @@
if prevC >= 0 {
d.Dot.X += d.Face.Kern(prevC, c)
}
- dr, mask, maskp, advance, ok := d.Face.Glyph(d.Dot, c)
- if !ok {
- // TODO: is falling back on the U+FFFD glyph the responsibility of
- // the Drawer or the Face?
- // TODO: set prevC = '\ufffd'?
- continue
+ dr, mask, maskp, advance, _ := d.Face.Glyph(d.Dot, c)
+ if !dr.Empty() {
+ draw.DrawMask(d.Dst, dr, d.Src, image.Point{}, mask, maskp, draw.Over)
}
- draw.DrawMask(d.Dst, dr, d.Src, image.Point{}, mask, maskp, draw.Over)
d.Dot.X += advance
prevC = c
}
@@ -170,14 +175,10 @@
if prevC >= 0 {
d.Dot.X += d.Face.Kern(prevC, c)
}
- dr, mask, maskp, advance, ok := d.Face.Glyph(d.Dot, c)
- if !ok {
- // TODO: is falling back on the U+FFFD glyph the responsibility of
- // the Drawer or the Face?
- // TODO: set prevC = '\ufffd'?
- continue
+ dr, mask, maskp, advance, _ := d.Face.Glyph(d.Dot, c)
+ if !dr.Empty() {
+ draw.DrawMask(d.Dst, dr, d.Src, image.Point{}, mask, maskp, draw.Over)
}
- draw.DrawMask(d.Dst, dr, d.Src, image.Point{}, mask, maskp, draw.Over)
d.Dot.X += advance
prevC = c
}
@@ -227,16 +228,12 @@
if prevC >= 0 {
advance += f.Kern(prevC, c)
}
- b, a, ok := f.GlyphBounds(c)
- if !ok {
- // TODO: is falling back on the U+FFFD glyph the responsibility of
- // the Drawer or the Face?
- // TODO: set prevC = '\ufffd'?
- continue
+ b, a, _ := f.GlyphBounds(c)
+ if !b.Empty() {
+ b.Min.X += advance
+ b.Max.X += advance
+ bounds = bounds.Union(b)
}
- b.Min.X += advance
- b.Max.X += advance
- bounds = bounds.Union(b)
advance += a
prevC = c
}
@@ -251,16 +248,12 @@
if prevC >= 0 {
advance += f.Kern(prevC, c)
}
- b, a, ok := f.GlyphBounds(c)
- if !ok {
- // TODO: is falling back on the U+FFFD glyph the responsibility of
- // the Drawer or the Face?
- // TODO: set prevC = '\ufffd'?
- continue
+ b, a, _ := f.GlyphBounds(c)
+ if !b.Empty() {
+ b.Min.X += advance
+ b.Max.X += advance
+ bounds = bounds.Union(b)
}
- b.Min.X += advance
- b.Max.X += advance
- bounds = bounds.Union(b)
advance += a
prevC = c
}
@@ -278,13 +271,7 @@
if prevC >= 0 {
advance += f.Kern(prevC, c)
}
- a, ok := f.GlyphAdvance(c)
- if !ok {
- // TODO: is falling back on the U+FFFD glyph the responsibility of
- // the Drawer or the Face?
- // TODO: set prevC = '\ufffd'?
- continue
- }
+ a, _ := f.GlyphAdvance(c)
advance += a
prevC = c
}
@@ -298,13 +285,7 @@
if prevC >= 0 {
advance += f.Kern(prevC, c)
}
- a, ok := f.GlyphAdvance(c)
- if !ok {
- // TODO: is falling back on the U+FFFD glyph the responsibility of
- // the Drawer or the Face?
- // TODO: set prevC = '\ufffd'?
- continue
- }
+ a, _ := f.GlyphAdvance(c)
advance += a
prevC = c
}
diff --git a/font/opentype/opentype.go b/font/opentype/opentype.go
index 231fdbe..694ac47 100644
--- a/font/opentype/opentype.go
+++ b/font/opentype/opentype.go
@@ -133,8 +133,8 @@
// Kern satisfies the font.Face interface.
func (f *Face) Kern(r0, r1 rune) fixed.Int26_6 {
- x0 := f.index(r0)
- x1 := f.index(r1)
+ x0, _ := f.f.GlyphIndex(&f.buf, r0)
+ x1, _ := f.f.GlyphIndex(&f.buf, r1)
k, err := f.f.Kern(&f.buf, x0, x1, fixed.Int26_6(f.f.UnitsPerEm()), f.hinting)
if err != nil {
return 0
@@ -251,22 +251,19 @@
}
f.rast.Draw(&f.mask, f.mask.Bounds(), image.Opaque, image.Point{})
- return dr, &f.mask, f.mask.Rect.Min, advance, true
+ return dr, &f.mask, f.mask.Rect.Min, advance, x != 0
}
// GlyphBounds satisfies the font.Face interface.
func (f *Face) GlyphBounds(r rune) (bounds fixed.Rectangle26_6, advance fixed.Int26_6, ok bool) {
- bounds, advance, err := f.f.GlyphBounds(&f.buf, f.index(r), f.scale, f.hinting)
- return bounds, advance, err == nil
+ x, _ := f.f.GlyphIndex(&f.buf, r)
+ bounds, advance, err := f.f.GlyphBounds(&f.buf, x, f.scale, f.hinting)
+ return bounds, advance, (err == nil) && (x != 0)
}
// GlyphAdvance satisfies the font.Face interface.
func (f *Face) GlyphAdvance(r rune) (advance fixed.Int26_6, ok bool) {
- advance, err := f.f.GlyphAdvance(&f.buf, f.index(r), f.scale, f.hinting)
- return advance, err == nil
-}
-
-func (f *Face) index(r rune) sfnt.GlyphIndex {
x, _ := f.f.GlyphIndex(&f.buf, r)
- return x
+ advance, err := f.f.GlyphAdvance(&f.buf, x, f.scale, f.hinting)
+ return advance, (err == nil) && (x != 0)
}
