Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Harfbuzz #224

Closed
Frenzie opened this issue Aug 3, 2018 · 18 comments
Closed

Harfbuzz #224

Frenzie opened this issue Aug 3, 2018 · 18 comments

Comments

@Frenzie
Copy link
Member

Frenzie commented Aug 3, 2018

Do. Want.

(not spamming the other repo)

github.com/buggins/coolreader/pull/24

It looks amazingly uninvolved at a quick glance. (The PR is gigantic, but most of that is just harfbuzz itself.)

@robert00s
Copy link
Contributor

Is it possible to integrate with our crengine repository?

@Frenzie
Copy link
Member Author

Frenzie commented Aug 4, 2018

I suspect that we haven't really touched the affected parts of lvfntman.cpp/h but I haven't checked yet.

See #122 (comment) for how to do that in a couple of minutes.

However, in our case it would require more involved changes in the koreader-base repo as well for it to actually build. But with any luck those commits will just integrate with no or minimal effort.

@poire-z
Copy link
Contributor

poire-z commented Aug 4, 2018

I suspect that we haven't really touched the affected parts of lvfntman.cpp/h

I did a bit in https://github.com/koreader/crengine/pull/212/files#diff-39c6d98ff2d0122472cff214e1510f8b . So that handling of soft-hyphens would need to be removed and put back after that patch.
I'm also not so sure about the lvtinydom.cpp IsUnicodeSpace() added space chars, see #220 (comment).
(it's been merged really quickly upstream, but we must be sure it's safe to use in our context).

Is that stuff all about and only kerning? That kind of ffff or ligature stuff?
https://raw.githubusercontent.com/tangrams/harfbuzz-example/master/img/capture.png
(in that case, I'm not at all excited :)
But if yes, and I dunno how much this is a it must be rendered like that or a preference thing, but we may want to make it another toggable option.

Anyway, the most fun will be for you FOC (Friends of Cmake) (I can of course help with the crengine parts).

@Frenzie
Copy link
Member Author

Frenzie commented Aug 4, 2018

Much of the time that crengine looks a bit blah, as if you were looking at something on a computer screen even though E Ink can look as good as cheap newspaper, it's kerning and spacing related. So I'm inclined to think it could be potentially huge in the short term.

In the potential long term, Harfbuzz (or something like it) is what you need for RTL.

@poire-z
Copy link
Contributor

poire-z commented Aug 5, 2018

A few notes, things met while trying to understand what harfbuzz is.

https://harfbuzz.github.io/hello-harfbuzz.html simple usage and what it doesn't do

https://sourceforge.net/projects/freetype/files/freetype2/2.5.3/
FreeType can now use the HarfBuzz library to greatly improve the auto-hinting of fonts that use OpenType features
We don't build freetype with harfbuzz (and it looks complicated, needing 2 builds) - dunno how much this could just improve stuff.

https://github.com/ArtifexSoftware/mupdf/blob/master/source/fitz/harfbuzz.c
MuPDF uses its own patched harfbuzz, and the comments in this file state some caveats.
Also, I don't know much about the effect of having libmupdf including is own hardfbuzz and libcrengine using some external libharfbuzz, if there can be conflicts, etc...

@cramoisi
Copy link
Contributor

cramoisi commented Aug 7, 2018

Is that stuff all about and only kerning? That kind of ffff or ligature stuff?
(in that case, I'm not at all excited :)

I love this ligature thing so much :) It's good for "readability"

@Frenzie
Copy link
Member Author

Frenzie commented Aug 7, 2018

Also needs buggins/coolreader#29

@poire-z
Copy link
Contributor

poire-z commented Aug 12, 2018

I had a try with upstream 24+29, which applied almost ok, but we'll have to make it a toggable.
The render time for my big book that went with yesterday's PR from 11s to 5s goes up to 13s with harfbuzz.
Some screenshots (althought text looks always worse on the emulator than on eink):

Font Bitter, No Harfbuzz, with Font hinting auto:
image
Font Bitter, Harfbuzz, with Font hinting auto:
image

Font Bitter, No Harfbuzz, with Font hinting off (nearly the same with native):
image
Font Bitter, Harfbuzz, with Font hinting off (nearly the same with native):
image

Bigger font size, Font Bitter, No Harfbuzz, with Font hinting auto:
image
Bigger font size, Font Bitter, Harfbuzz, with Font hinting auto:
image

Bigger font size, Font Bitter, No Harfbuzz, with Font hinting off:
image
Bigger font size, Font Bitter, Harfbuzz, with Font hinting off:
image

I'm personally not really sensitive to font rendering details. But I just felt it got worse with harfbuzz than without when using font hinting auto. Had to switch to font hinting off (or native) to get something correct.

Anyway, that was some home made build.
I couldn't figure out how to make cmake build harfbuzz.
Took some inspiration from @NiLuJe http://trac.ak-team.com/trac/browser/niluje/Configs/trunk/Kindle/Misc/x-compile.sh, and came up with this patch to base base_harfbuzz1a.diff.txt (I mostly just copied the FREETYPE section and replaced freetype with harfbuzz...) and this patch to crengine (including upstream 24+29) crengine_harfbuzz1a.diff.txt
kodev build kept making a static libharfbuzz.a. I had to manually do this to make a .so and be able to test:

In /koreader/base/thirdparty/harfbuzz/build/x86_64-linux-gnu-debug/harfbuzz-prefix/src/harfbuzz $

FREETYPE_LIBS="-L/koreader/base/thirdparty/freetype2/build/x86_64-linux-gnu-debug/freetype2-prefix/src/freetype2-build -lfreetype" FREETYPE_CFLAGS=-I/koreader/base/thirdparty/freetype2/build/x86_64-linux-gnu-debug/freetype2-prefix/src/freetype2-build/include/freetype2 ./autogen.sh --enable-shared --without-coretext --without-uniscribe --without-cairo --without-glib --without-gobject --without-graphite2 --without-icu --with-freetype=yes

So, if there's some interest for this harfbuzz rendering (that I can't appreciate :), it would be nice if @NiLuJe or @Frenzie could take over the base build of harfbuzz, because I don't want to start hating CMake as much as you do :)
(Also, I don't really get the interaction between freetype and harfbuzz: it needed, and I had a hardtime making it happen, to be linked against freetype. But I see in @NiLuJe above script and in other projects that people then relink freetype against harfbuzz. Would we need that too?!)

@NiLuJe
Copy link
Member

NiLuJe commented Aug 12, 2018

FT itself can use HB to do... stuff... with it for the auto-hinter. I'm not quite sure to what extent, and if that even applies to Latin scripts at all.

  • FreeType uses the HarfBuzz library to improve auto-hinting of
  • OpenType fonts. If available, many glyphs not directly addressable
  • by a font's character map will be hinted also.

FreeType can now use the HarfBuzz library to greatly improve the
auto-hinting of fonts that use OpenType features: Many glyphs
that are part of such features but don't have cmap entries are
now handled properly, for example small caps or superscripts.

Which gets tricky because there's a circular dependency: HB unconditionally needs FT to build, and FT optionally needs HB ;).
So when you're bootstrapping, as we are, you need to do HB-less FT -> HB -> FT-with-HB (i.e., build FT twice :/).

Quick question about your tests: does Bitter actually have decent GPOS kerning tables?

@poire-z
Copy link
Contributor

poire-z commented Aug 12, 2018

No idea :| (but I was using the .otf versions). Tried a few others (Noto, FreeSerif, Caecilia) and saw nothing much more striking, so went back to a font I'm confortable with.

@cramoisi
Copy link
Contributor

@poire-z could you share your homemade built ?

@poire-z
Copy link
Contributor

poire-z commented Aug 13, 2018

Dunno if it will work if you just drop them in your koreader-emulator-x86_64-linux-gnu-debug/koreader/libs or if there are hardcoded full paths: (removed)

@cramoisi
Copy link
Contributor

tx !

@Frenzie
Copy link
Member Author

Frenzie commented Aug 14, 2018

The other repo has a ticket about the performance thing you noticed now:

https://github.com/buggins/coolreader/issues/37

@cramoisi
Copy link
Contributor

cramoisi commented Sep 9, 2018

Hi everyone. When enabling it I come across a lot of "concatenated" words (some words are displayed glued together); did some of you experienced this issue ?

@NiLuJe
Copy link
Member

NiLuJe commented Sep 9, 2018

Yes, side-effect of using unhinted metrics with non-unhinted rendering.

Unfortunately, doing it "right" (or something resembling right) is hard & expensive.

Currently, the only way you might get typographically sane results is when combining it with disabled font hinting, too.

Results vary heavily depending on the exact font, font size & DPI. Sometimes doing it wrong looks appealing ;).

@cramoisi
Copy link
Contributor

cramoisi commented Sep 9, 2018

Could I get it right by editing the font ?

@NiLuJe
Copy link
Member

NiLuJe commented Sep 9, 2018

Nope.

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

No branches or pull requests

5 participants