sile-typesetter / sile

The SILE Typesetter — Simon’s Improved Layout Engine
https://sile-typesetter.org
MIT License
1.64k stars 96 forks source link

Harfbuzz with Graphite is messing up Arabic #124

Closed alerque closed 9 years ago

alerque commented 9 years ago

I'm still trying to track down issues with the regression tests across platforms. For some reason Arabic doesn't want to behave itself. Specifically the tests/arabic.sil file (as current in my #122 PR) doesn't compile the same way on any of OSX, my Linux system or Travis's environment. All three systems now have exactly the same font versions, so that's not the issue. Perhaps you can point me in the direction of what could be the issue.

As you can see here, the leading migrates with the OSX version being larger (in black here):

arabic-offset

Additionally, there are individual character differences in various places. Some of them are subtle, others less so (OSX version on the right):

diffpdf_028

Is this an issue with font-config? Harfbuzz? Where could these differences be introduced that would vary three ways between systems? Note I'm not seeing this in any of the English language tests.

simoncozens commented 9 years ago

I would guess Harfbuzz or freetype?

alerque commented 9 years ago

That looks like a promising lead but:

What else could it be? Could Lua 5.3 vs. 5.2 be throwing this off somehow?

khaledhosny commented 9 years ago

The image on the left is certainly wrong. It does not look like a HarfBuzz issue to me (unless Scheherazade is decomposing the Alef and positioning the Hamza as a combining mark).

simoncozens commented 9 years ago

@alerque, what do you mean that images are wrong? If images are not being sized properly, then we can ignore fonts/Harfbuzz/Freetype and just concentrate on Lua.

alerque commented 9 years ago

@simoncozens Okay my bad. I didn't examine that one closely enough and now that I have it's clearly a different sort of problem. That's a float vs. int output problem throwing off the debugger. The generated PDF is identical. (edit: Reported in #125)

@khaledhosny Thanks for the insight. My Turkish doesn't get me very far into Arabic! BTW, as of a few minutes ago your fork of Libertine is available as a Homebrew Cask (brew tap caskroom/fonts; brew cask install font-libertine) and an Archlinux package.

alerque commented 9 years ago

Alright I've confirmed that:

alerque commented 9 years ago

So this does seem to be SILE specific. The same content rendered in the same font on the same system in Gedit, Firefox, Chromium, LibreOffice and XeLaTeX all work fine. All of those have slightly different font rendering pipelines, but none of them have a problem with positioning the Arabic diacritic marks.

Here is a minimal working SILE example:

\begin[papersize=a5]{document}
\script[src=packages/bidi]
\font[size=40pt,language=ar,script=Arab]
\thisframeRTL
\font[family=Noto Naskh Arabic]{ألمّ} — \font[family=Scheherazade]{ألمّ}
\end{document}

bug-124 pdf_032

And here is a LaTeX equivalent:

\documentclass[a5paper,40pt]{scrartcl}
\usepackage{polyglossia}
\setdefaultlanguage{arabic}
\newfontfamily\scheherazade[Script=Arabic]{Scheherazade}
\newfontfamily\notonaskh[Script=Arabic]{Noto Naskh Arabic}
\begin{document}
\RTL
{\notonaskh ألمّ} — {\scheherazade ألمّ}
\end{document}

bug-124 sil pdf_031

alerque commented 9 years ago

As I surmised would be the case based on newer and older versions working on other systems, updating my system to Freetype 2.6 and Harfbuzz 0.9.41 and recompiling SILE made no difference.

simoncozens commented 9 years ago

@khaledhosny: That is exactly what is happening. Noto Sans is returning a U+0623 Alef+Hamza above for the rightmost glyph; Scheherezade is returning this:

  {
    codepoint = 272,
    depth = 0.029296875,
    height = 4.8291015625,
    name = "uni0627",
    width = 1.4501953125,
  },
  {
    codepoint = 1052,
    depth = -4.931640625,
    height = 6.4794921875,
    name = "uni0654",
    width = 0,
    x_offset = -0.046875,
    y_offset = 0.375,
  },
}
alerque commented 9 years ago

That's interesting, but what could be cause some systems to typeset Scheherezade correctly while others trip up over the two points?

simoncozens commented 9 years ago

Graphite installed versus not installed?

alerque commented 9 years ago

Maybe. In that event ① it is the with variant that is failing and ② I'm not sure how I can even test it without. My system's harfbuzz package depends on it, as do a bunch of other things.

simoncozens commented 9 years ago

Bingo - it's a graphite problem. (Paging @khaledhosny)

And it's easy to test with Homebrew now. :-)

brew uninstall harfbuzz; brew install harfbuzz; ./sile examples/bug-124.sil

produces this:

bug-124-without-graphite

brew uninstall harfbuzz; brew install harfbuzz --with-graphite2; ./sile examples/bug-124.sil

produces this:

bug-124-with-graphite

khaledhosny commented 9 years ago

I can reproduce it with hb-view, with `graphite2:

hb-view scheherazade.ttf --shapers=graphite2 ألمّ

scheherazade-gr2

hb-view scheherazade.ttf --shapers=ot ألمّ

scheherazade-ot

So it looks like a Graphite bug or (more likely) a bug in Graphite integration in HarfBuzz. I wonder if the patch in http://lists.freedesktop.org/archives/harfbuzz/2015-June/004902.html handles this. CC @mhosken.

mhosken commented 9 years ago

Dear Khaled,

I can reproduce it with hb-view, with `graphite2:

hb-view /usr/share/fonts/TTF/scheherazade.ttf --shapers=graphite2 ألمّ

scheherazade-gr2

hb-view /usr/share/fonts/TTF/scheherazade.ttf --shapers=ot ألمّ

scheherazade-ot

So it looks like a Graphite bug or (more likely) a bug in Graphite integration in HarfBuzz. I wonder if the patch in http://lists.freedesktop.org/archives/harfbuzz/2015-June/004902.html handles this. CC @mhosken.

Yes. This is fixed by that patch. Just waiting for it to be applied.

Yours, Martin

simoncozens commented 9 years ago

Well, this isn't a SILE bug, so I'm closing it. Will be fixed next time @behdad releases Harfbuzz.

behdad commented 9 years ago

New HarfBuzz should have the fix.

alerque commented 9 years ago

Spiffy. Thanks for the release and the heads up @behdad!