Use OT rather than FT functions to query metrics from Harfbuzz #164
Comments
If only things were that simple: |
|
Do you have the same set of available weights I do? |
|
Ah, actually I just have regular installed. However, if SILE was selecting a different weight for you, then the "Set font" line in the test output would be different. I think. You could try manually forcing the weight to 200 and seeing what you get. |
|
The "Set font" line in the output doesn't show a different font face no matter what weight I set (the weight value shows up, but the actual face picked isn't noted there). 200 seems to be the default and no, setting it to 200 manually doesn't change my output. Setting it to something more or less does that that, but none of the values I tried netted me the same metrics as the test. |
|
OK, so I was using Noto Sans regular 1.003 whereas Travis was using Noto Sans Light 1.004. Now I am using the same version as Travis. This was discovered using our new handy font fingerprinting feature. I also want to add support for groking the font OpenType tables and pulling out metadata. (And, in the future, other things...) |
|
...but that hasn't helped. Now we have the same Harfbuzz version (1.0.1 on each), same font, but different metrics being returned. It's a Harfbuzz thing. Here's Ubuntu 12: Here's OS X: @behdad, help? |
|
It's caused by some FreeType weirdness on OS X that I never spent the time to track down. Switching away from hb-ft to hb-ot for font funcs implementation (eg, --font-funcs=ot passed to hb-shape) should address this. |
|
That works for hb-shape; excellent. But SILE can't move to the OT funcs until |
That's on my todo for today. |
An upstream issues in Harfbuzz causes different metrics to be returned on different systems. Per issue sile-typesetter#164 this sets the expected output of the test to be what the Travis VM ends up reporting. This well allow the test suite to pass (this test) until such a time as `bh_ot_get_glyph_extents` is implemented and the test starts returning consistent results across systems.
|
@simoncozens I added an update to the expected output of this test to another test output related PR to bring the Travis test run that much closer to passing ever though this issue is outstanding. |
alerque
changed the title
alerque
changed the title
|
@alerque: I'm not sure this is needed now; I spent some time blitzing test results last night and now we're down to two failing builds - neither are related to font metrics: one is for the non-ICU tokenizer, and one is some Lua 5.1 UTF8 weirdness. |
An upstream issues in Harfbuzz causes different metrics to be returned on different systems. Per issue sile-typesetter#164 this sets the expected output of the test to be what the Travis VM ends up reporting. This well allow the test suite to pass (this test) until such a time as `bh_ot_get_glyph_extents` is implemented and the test starts returning consistent results across systems.
In HarfBuzz master now. Please test. It's my intention to implement vertical positioning using glyf table as well as VORG table, and then make a release. At that point ot-font will have better vertical support than FreeType (which doesn't implement VORG at all). |
|
Thank you! This looks good. Most things work - it gives different metrics to FreeType so obviously most of the regression tests fail. Japanese, Javanese, Tibetan and Arabic work nicely. Something goes horribly wrong when I try to typeset the documentation file, let's see... lots of tofu when I use Helvetica Neue, but probably because it's a dfont, d'oh. Ah, now, something goes mad with Gentium Book Basic. x_advance is fine but metrics are out of whack: > SILE.shaper:shapeToken("SILE", SILE.font.loadDefaults({ font = "Gentium Book Basic" }))
{
{
codepoint = 54,
depth = 3.57421875,
glyphWidth = 91.2451171875,
height = 8.7939453125,
index = 0,
name = "",
width = 4.8828125,
},
{
codepoint = 44,
depth = -3.1982421875,
glyphWidth = 35.1123046875,
height = 103.1298828125,
index = 1,
name = "",
width = 3.076171875,
},
{
codepoint = 47,
depth = -11.7041015625,
glyphWidth = 97.5,
height = 22.5,
index = 2,
name = "",
width = 4.873046875,
},
{
codepoint = 40,
depth = -17.5,
glyphWidth = 119.4482421875,
height = 81.2744140625,
index = 3,
name = "",
width = 5.05859375,
},
}Compare: > SILE.shaper:shapeToken("SILE", SILE.font.loadDefaults({ font = "Gentium" }))
{
{
codepoint = 51,
depth = 0.146484375,
glyphWidth = 3.75,
height = 6.298828125,
index = 0,
name = "",
width = 4.7607421875,
},
{
codepoint = 41,
depth = 0,
glyphWidth = 2.197265625,
height = 6.15234375,
index = 1,
name = "",
width = 2.978515625,
},
{
codepoint = 44,
depth = 0,
glyphWidth = 4.365234375,
height = 6.15234375,
index = 2,
name = "",
width = 4.7607421875,
},
{
codepoint = 37,
depth = 0,
glyphWidth = 4.4140625,
height = 6.15234375,
index = 3,
name = "",
width = 4.9609375,
},
}Otherwise, nice job. Now we have to decide whether or not to make harfbuzz >1.0.3 a dependency. ;-) |
Yeah. It's easy to implement. Lets see. Filed harfbuzz/harfbuzz#128 cc @brawer.
That's weird. I don't have a simple tool to write out the extents. Let me implement something like that and test.. |
|
I'll do it! |
|
Boom: |
|
Awesome. Thanks. Pull-request please? :) |
|
Fixed now. |
simoncozens
changed the title
|
That works really nicely; thanks @behdad! Now - what do we do about SILE? The original bug isn't really a problem any more because Travis is now passing most tests (and those that are failing are to do with ICU emulation). So we don't need the OT branch. Do we want it? Pros:
Cons:
I think it's something that we do want eventually but depending on bleeding-edge scares me. Perhaps the right approach is to test if the OT features work and use them if available, but that makes the code complicated... Any other opinions? |
Cool.
I'm confused by the numbers... Note that I have not yet optimized hb-ot-font... But this is a great test case for our performance.
In the end I think you should drop FreeType. It's a rasterizer and has no place in a layout system.
Yeah, wait until hb-ot-font is complete. Needs VORG and glyph-name implementations. Other than that, it's also missing CFF support. Normally you don't need glyph extents with CFF as CFF fonts should have GPOS... However, if you rely on glyph extents in Sile (why do you?) then that's a problem. So yeah, for now, lets work towards that goal, but don't rush switching just yet. |
|
And yes, you can always decided which one to use at runtime. |
|
Ah, I was just trying to debug why (vertical) Japanese wasn't working. :-) SILE uses x_advances for width (and y_advance for height in vertical layouts), but it also uses glyph height and depth information for computing the leading between successive baselines. |
Yeah, other way around. :-) Using FreeType was 113s, OT-features did it in 81s. |
simoncozens
added
enhancement
|
Yeah, we'd need to implement CFF glyph extents then (which we should cache... how's the performance with FT with CFF fonts BTW? Should be awful...) |
|
We're doing this now. It seems to work (even vertically-typeset Japanese) without the CFF glyph extents. |
I don't know if this is a font version problem or a SILE problem, but
tests/vertical.silis being very inconsistent. Every system seems to have a different idea of how the metrics should go.@simoncozens's system has produced one thing, yet my system is consistently producing another and the Travis test environment is producing a third set of values for the same test.
Comparing for example just the first line of the diff when tested on my system:
While Travis has this diff:
For reference on my system there are a whole bunch of weights for this font:
And the default selection is actually one of the lighter weights:
It's possible that the issue here is font weigt selection. I played with setting
\font[weight=x]in Sile but none of the weights matched up with the metrics currently in the master branch.The text was updated successfully, but these errors were encountered: