golang / freetype

The Freetype font rasterizer in the Go programming language.
Other
778 stars 183 forks source link

Corruption when using WorkSans-Black #90

Open andydotxyz opened 2 years ago

andydotxyz commented 2 years ago

Describe the bug: When using the WorkSans-Black font certain cross-elements are corripted. The font shows gaps instead of a solid character, as per images below

To Reproduce: Steps to reproduce the behaviour:

Notice the holes in the output:

175044562-277cbace-4466-45d3-9b86-3b658bab9d97

The same does not happen when you download the font from https://www.1001fonts.com/work-sans-font.html...

zagiduller commented 1 year ago

I have the same issue with Nunito font https://fonts.google.com/specimen/Nunito изображение

Zyl9393 commented 1 year ago

Same problem with SplineSansMono-Regular.ttf.

image

Looks like we need to do a deep dive into TTF specification. As far as I know, usually shapes should only cut negative space out of themselves, not other shapes. Maybe this is some obscure data structure problem, where multiple closed loops are defined in a single shape, or something like that?

Zyl9393 commented 1 year ago

Did some debugging using the letter d. Here it is in its broken form:

image

Here it is after rendering both contours of the letter separately and awkwardly pasting them on top of each other in MS Paint:

image

This demonstrates that the contours are loaded correctly. The bug seems to be with how contours are blended. Specifically, the bulge of the letter d is drawn first, and then the vertical bar, suggesting the bug is somewhere in the method Add1(). (Add2() was not invoked for the vertical bar, and Add3() was not invoked at all; as far as I could figure out it is not used for true type fonts)

With a bit of luck, this leaves us with 100 lines of code to sift through. I'll come back to this another day.

egonelbre commented 1 year ago

My guess was that it's some winding thing... so, if you set Rasterizer.UseNonZeroWinding then it renders correctly for WorkSans-Black.

e.g. in https://github.com/golang/freetype/blob/master/truetype/face.go#L188 add a.r.UseNonZeroWinding = true.

Note: I'm not sure whether there's some font or glyph property that should determine whether to use it; or whether it'll correctly work with alpha values.

Zyl9393 commented 1 year ago

I tried your solution, and you're correct. My take on Add1() was a red herring. I found this documentation by Microsoft which explicitly states:

A point is considered to be an interior point of a glyph if it has a non-zero winding number.

I do not understand why one of these methods would work while the other one fails, but using the wrong one here is the cause of the problem. EDIT: I just realized, of course this does not work if multiple contours are considered at the same time. Basically, UseNonZeroWinding = false allows contours to cut into each other. This also explains how out of two different TTF files which describe the same font one can work and one can fail: constructing the contours such that both methods succeed is totally possible, and some font sites seem to do exactly that to the files they serve, by whatever means.

On another note, I actually failed to to get your solution to work on my first attempt. freetype.Context completely makes do without calling NewFace(). With your suggested fix, one needs to call truetype.NewFace() by oneself, then construct a *font.Drawer the Go way and use that to draw.

neurozero commented 1 year ago

The fix in the Goki fork works for me. It would be great to get this fix in the main implementation.

To be clear, this is a bug. Filled fonts should not be rendered with UseNonZeroWinding = false (which is the default in the Rasterizer)

The Montserrat Black font on Google fonts demonstrates the issue.