Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/image/font/opentype: Face.Kern always returns the same value #46727

Open
polyscone opened this issue Jun 13, 2021 · 2 comments
Open

x/image/font/opentype: Face.Kern always returns the same value #46727

polyscone opened this issue Jun 13, 2021 · 2 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@polyscone
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.16.4 windows/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
set GO111MODULE=
set GOARCH=amd64
set GOBIN=D:\projects\go\bin
set GOCACHE=D:\devenv\go\1.16.4\..\cache
set GOENV=C:\Users\Polyscone\AppData\Roaming\go\env
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=D:\projects\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=D:\projects\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=D:\devenv\go\1.16.4
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=D:\devenv\go\1.16.4\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.16.4
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=NUL
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\POLYSC~1\AppData\Local\Temp\go-build1223181173=/tmp/go-build -gno-record-gcc-switches

What did you do?

I parsed the arial.ttf font on my Windows 10 machine and attempted to use the Face.Kern method to adjust the horizontal position of some glyphs as I rendered them.

(Face.Kern: https://pkg.go.dev/golang.org/x/image@v0.0.0-20210607152325-775e3b0c77b9/font/opentype#Face.Kern)

What did you expect to see?

I expected to see Face.Kern return a different value based on the size of the font face being used.

What did you see instead?

I always see the same number for a given pair of characters being returned regardless of the size of the font face being used.

I'm not sure if this is intended or not, but given that other methods on Face, for example, GlyphBounds, return different values based on the size of the face being used, I assumed that Face.Kern should do the same.

I brought this up as an issue here: hajimehoshi/ebiten#1671, where @hajimehoshi pointed out that it's being used without any additional scaling here: https://cs.opensource.google/go/x/image/+/775e3b0c:font/font.go;l=228;drc=3f4726a040e860b3702c9dab9e3b94cad413328f, which also leads me to believe that Face.Kern always returning the same number for a given pair of glyphs isn't the expected behaviour.

As can be seen in my linked issue in the Ebiten repo, my current workaround is to scale it myself:

kernScale := fixed.Int26_6(facePtSize * (faceDPI / fontUnitsPerEm) * 64.0)
fx += face.Kern(prevR, r).Mul(kernScale)

I'm not sure if this is actually an accurate way to do it since I don't know much about fonts myself, but it seems to work for me for now.

If this is actually the intended behaviour then a bit more documentation explaining that it needs to be scaled by the user would be helpful, but based on what I've seen so far I'm guessing this is actually a bug.

@gopherbot gopherbot added this to the Unreleased milestone Jun 13, 2021
@hajimehoshi
Copy link
Member

CC @nigeltao

@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 13, 2021
@tinne26
Copy link

tinne26 commented Feb 14, 2022

As far as I can tell, the issue/fix is as obvious and simple as replacing https://cs.opensource.google/go/x/image/+/6944b10b:font/opentype/opentype.go;l=138 from:

k, err := f.f.Kern(&f.buf, x0, x1, fixed.Int26_6(f.f.UnitsPerEm()), f.hinting)

To:

k, err := f.f.Kern(&f.buf, x0, x1, f.scale, f.hinting)

I think there are enough parallels with f.f.GlyphAdvance and f.f.LoadGlyph to see why this is the case. A font.Face has a specific size per spec, and here it's not being applied.

I'm too inexperienced with github and too weak-willed to go through all the necessary steps to contribute a pull request, but at least I can point out that there's really not much to investigate here with regards to the issue itself. The related tests are another story https://cs.opensource.google/go/x/image/+/6944b10b:font/opentype/opentype_test.go;l=127.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants