Skip to content

Conversation

khaledhosny
Copy link
Collaborator

@khaledhosny khaledhosny commented Oct 30, 2017

This is an attempt to run all shaping tests with both ft and ot font functions, individual tests can still specify which font functions to use as before. There were a few tests using CFF fonts, these now explicitely set ft functions.

There is still a couple of failures that need investigation:

AIL: tests/cluster
===================

Running tests in ../../../test/shaping/tests/cluster.tests
hb-shape fonts/sha1sum/6466d38c62e73a39202435a4f73bf5d6acbb73c0.ttf --cluster-level=2 --unicodes U+0078,U+030A,U+0058,U+030A
FT funcs: [gid2=0+1083|gid4=1@-555,-8+0|gid1=2+1200|gid4=3@-614,349+0]
OT funcs: [gid2=0+1083|gid4=1@-542,-8+0|gid1=2+1200|gid4=3@-601,349+0]
hb-shape fonts/sha1sum/43ef465752be9af900745f72fe29cb853a1401a5.ttf --cluster-level=1 --unicodes U+05D4,U+05B7,U+05E9,U+05BC,U+05C1,U+05B8,U+05DE,U+05B4,U+05DD
1 tests failed.
FAIL tests/cluster.tests (exit status: 1)

FAIL: tests/vertical
====================

Running tests in ../../../test/shaping/tests/vertical.tests
hb-shape fonts/sha1sum/191826b9643e3f124d865d617ae609db6a2ce203.ttf --direction=t --unicodes U+300C
FT funcs: [uni300C.vert=0@-512,-578+0,-1024]
OT funcs: [uni300C.vert=0@-512,-880+0,-1024]
hb-shape fonts/sha1sum/f9b1dd4dcb515e757789a22cb4241107746fd3d0.ttf --direction=t --font-funcs=ft --unicodes U+0041,U+0042
hb-shape fonts/sha1sum/f9b1dd4dcb515e757789a22cb4241107746fd3d0.ttf --direction=t --font-funcs=ot --unicodes U+0041,U+0042
1 tests failed.
FAIL tests/vertical.tests (exit status: 1)

The clusters failure is especially suspicious.

@behdad
Copy link
Member

behdad commented Oct 30, 2017

Thanks Khaled!

@behdad
Copy link
Member

behdad commented Oct 30, 2017

Reference mode is used to (re-)generate expected output, by passing --reference to it manually on the commandline.

@khaledhosny
Copy link
Collaborator Author

OK, I can restore reference mode.

@behdad
Copy link
Member

behdad commented Oct 31, 2017

What's the next step here?

@khaledhosny
Copy link
Collaborator Author

We need to figure out why these two tests are failing, or just force ft funcs for them for now.

@behdad
Copy link
Member

behdad commented Nov 1, 2017

@behdad
Copy link
Member

behdad commented Nov 1, 2017

Might be nice to be able to run tests without FT as well...

@behdad
Copy link
Member

behdad commented Nov 1, 2017

I know it sounds like over-engineering, but maybe I can add a --list-font-funcs to hb-view / hb-shape and we loop over them.

Same about Unicode funcs perhaps. But then, we don't want to fail test suite when glib or ICU is old.

@behdad
Copy link
Member

behdad commented Nov 1, 2017

For the other one, I wonder if get_contour_point is involved...

@khaledhosny
Copy link
Collaborator Author

get_contour_point does not seem to be called in the clusters failure, it does not seem to be related to clusters level either. May be it is fallback glyph positioning? The font does not have any layout tables…

@khaledhosny
Copy link
Collaborator Author

Looks like extents issue indeed:

$ hb-shape 6466d38c62e73a39202435a4f73bf5d6acbb73c0.ttf --unicodes U+030A --font-funcs=ft --show-extents
[gid4=0+0<-211,1673,450,-432>]
$ hb-shape 6466d38c62e73a39202435a4f73bf5d6acbb73c0.ttf --unicodes U+030A --font-funcs=ot --show-extents
[gid4=0+0<-224,1673,450,-432>]

@khaledhosny
Copy link
Collaborator Author

It looks like a Freetype/ft functions bug:

<mtx name="uni030A" width="0" lsb="-224"/>

@khaledhosny
Copy link
Collaborator Author

Having --list-font-funcs sounds good, though looping and comparing the output of each against each is going to stretch my rudimentary shell scripting skills 😄

These are CFF fonts and ot functions don’t support CFF glyph names yet.
The next commit will run all tests with ot functions.
Freetype and OT font functions give different positions for some glyphs
in this font (OT seems to be correct), but that is not what we are
interested in in this test.

See #590 (comment).
@behdad
Copy link
Member

behdad commented Nov 1, 2017

cc @lemzwerg

@behdad behdad merged commit f124501 into harfbuzz:master Nov 1, 2017
behdad pushed a commit that referenced this pull request Nov 1, 2017
behdad pushed a commit that referenced this pull request Nov 1, 2017
Freetype and OT font functions give different positions for some glyphs
in this font (OT seems to be correct), but that is not what we are
interested in in this test.

See #590 (comment).
@behdad
Copy link
Member

behdad commented Nov 1, 2017

Thank you Khaled!

@khaledhosny khaledhosny deleted the font-ot-tests branch November 1, 2017 16:38
@lemzwerg
Copy link
Contributor

lemzwerg commented Nov 1, 2017

I'm a bit clueless what to check or test on the FreeType side. Please advise.

@behdad
Copy link
Member

behdad commented Nov 1, 2017

I'm a bit clueless what to check or test on the FreeType side. Please advise.

Ok, actually not FreeType's fault. Not quite.

If you run:

hb-shape 6466d38c62e73a39202435a4f73bf5d6acbb73c0.ttf --unicodes U+030A --font-funcs=ft --show-extents
[gid4=0+0<-211,1673,450,-432>]

the lsb is reported -211, but it should be -224. I looked into the font. Nothing stands out. lsb and xmin of the glyph are both -224. It's a composite glyph, has one component, but the math on that one is also correct. However, the component does have 0x4 flag bit on, which is ROUND_XY_TO_GRID.

Is there any way to disable that rounding from FreeType API?

Proper fix is for hb-ft to really use unscaled FT_Face. Or scale to UPEM. Right now we scale to UPEM/64, and try hard to disable all rounding in FreeType, but apparently we failed in this case.

@behdad
Copy link
Member

behdad commented Nov 1, 2017

For the purposes of fixing the test, we can just disable that flag.

@lemzwerg
Copy link
Contributor

lemzwerg commented Nov 1, 2017

No, there is no API to disable this. What do you suggest? Maybe automatically disabling the flag if the glyph gets loaded unscaled?

@khaledhosny
Copy link
Collaborator Author

For the purposes of fixing the test, we can just disable that flag.

Done in #597.

@behdad
Copy link
Member

behdad commented Nov 1, 2017

No, there is no API to disable this. What do you suggest? Maybe automatically disabling the flag if the glyph gets loaded unscaled?

For unscaled glyph load, there is no subpixel precision so it will always be rounded. hb-ft is abusing FreeType API, loading with scaling, but treating each 1/64. of pixel as a unit. It's that rounding that we don't like, but I'm not suggesting that you rush to implement it.

Such disabling is useful in rendering situations that support subpixel positioning. So, some way to disable it is desirable IMO. I would say if vertical hinting is disabled, then vertical position shouldn't be rounded, and if horizontal hinting is disabled (whatever affects advance width hinting), then horizontal position not rounded.

behdad pushed a commit that referenced this pull request Nov 1, 2017
See #590 (comment).

I simply removed the composite glyph and use the referenced simple
glyph directly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants