Open nigeltao opened 8 years ago
Since this is a font rasterizer not a text layout engine, I suggest support for kerning be dropped (that is a job for HarfBuzz and other text layout engine) and since many obsolete parts of FreeType have been dropped, the API probably should make it hard for people to misuse the rasterizer as a text layout engine.
That's probably a fair point. In https://github.com/golang/go/issues/16904 "proposal: x/image packages to render TrueType fonts" it may be worth having a low-level sfnt package, which is a font rasterizer (and exposes e.g. kerning and other text layout tables) and a high-level truetype package, which uses those tables, as well as a separate Go package, equivalent to HarfBuzz) to lay out text well.
Without detracting from HarfBuzz (definitely text layout is a business in itself), for simple needs I like the go approach of having straightforward things usable out of one Go packages (e.g., the http package) without having to combine tediously many packages together (e.g., th Apache HttpClient library).
For today, if its only a matter of patching the lib with a boundsCache similar to how glyphCache operates, Nigel I can propose you a patch, the code looks very neat. We just have to decide if we should not merge both caches together: I mean, if someone draws a glyph, there is a very high probability he will also measure its bounds and also measures its advance before or after. So, we could save even more a.glyphBuf.Load(...) operations by just filling the cache once with drawing, bounds and advance information. The savings for first time calls to a glyph would be not far away from 66%, and for later calls not far away from 100% (the difference being cache collisions, of course)
I suspect that we'd want separate caches, because the glyph image cache is also keyed by sub-pixel position, but glyph bounds and advances don't care about sub-pixel positions.
Ah right, these are different key structures. But then the caching action could be shared? That would have the same impact in terms of reducing the number of calls to glyphBuf.Load.
More specifically, when one of the 2 caches incurs a miss, we check the other cache and, if it also has a miss, we fill it too. That only requires factorizing out dedicated methods for the cache miss detection (one method for each cache), and the cache fill action (one for each).
FWIW, I just submitted a PR to add a simple map[rune] cache of advances. Maybe I missed something but it seemed to be trivial. For most layout cases (everything I've encountered so far at least) you just need the global font metrics and the glyph advance, so this at least covers a lot of ground. would be equally simple to add another cache of bounds if that was useful. Also, Kern is computed directly and does not require an expensive load operation, so not worth caching that.
Only face.Glyph results (glyph images) are cached.
This makes font.MeasureString calls more expensive than they ought to be.
See https://groups.google.com/forum/#!topic/golang-nuts/oqRV5P-HQIo for the original report.