Page MenuHomePhabricator

Terminology font rendering issue (textgrid issue)
Closed, ResolvedPublic

Description

The vertical positioning of fonts is broken. Tested on both stable and git.

Edit by @tasn:
Reduced the priority to normal, as a workaround is in place and it's going to be fixed upstream.
We need to remember to revert @jpeg's hack once a fix is applied upstream. We shouldn't release with this hack.

ApB created this task.Nov 20 2015, 10:29 AM
ApB updated the task description. (Show Details)
ApB raised the priority of this task from to Incoming Queue.
ApB added a project: Terminology.
ApB added a subscriber: ApB.

It's only with the textgrid. Occurs with harfbuzz>= 1.1.

billiob changed the visibility from "All Users" to "Public (No Login Required)".Nov 20 2015, 1:33 PM
billiob triaged this task as Showstopper Issues priority.Nov 20 2015, 3:07 PM
0keh added a subscriber: 0keh.Nov 22 2015, 7:34 PM

On other webpages it is announced to reinstall freetype2, but on arch it did not help.

0keh merged a task: Restricted Maniphest Task.Nov 22 2015, 7:54 PM
0keh added a subscriber: jeyzu.

@billiob, can you come up with a simple textgrid example file to replicate the problem?

I think a dumb textgrid object with a ttf font on it will show you the issue.

tasn added a comment.Nov 23 2015, 7:37 AM

I wonder if it also happens with harfbuzz from git, I saw some commits about vertical scaling, not sure if relevant (might be). I haven't looked yet.

TwoD added a subscriber: TwoD.Nov 23 2015, 1:30 PM

fyi - happens instantly on arch atm since arch just went to hb 1.1 - no effort needed to reproduce except upgrade to hb 1.1 and run terminology. all chars drawn are shifted down by "1".

at this point - not sure if it's a bug in efl or in hb. could be either.

I updated my arch linux and make check is also broken now on text test.

fanf42 added a subscriber: fanf42.Nov 24 2015, 1:41 AM

I can confirm that:

  • on current arch, terminology font rendering is broken
  • this happened when upgrading to harfbuzz-1.1.0-1
  • reverting to harfbuzz-1.0.6-2 correct the font rendering issue.

(and thanks for the nice people on #e who helped me find the problem :)

Just to add that rebuilding freetype2 does not fix the issue (extending on the comment from 0keh).
I suspected 2 upstream harfbuzz commits:

  1. https://github.com/behdad/harfbuzz/commit/5bc28b5f688ee90d103d052e98bc15d6e0e7e0b1
  2. https://github.com/behdad/harfbuzz/commit/49ef630936325b2e56a870fcef9aa8473a8f8526

Reverting commit #1 did not solve the issue, but I was unable to revert commit #2 to test. This will probably require a "real" regression testing, that I just don't have the time to perform right now.

tasn added a comment.Nov 24 2015, 3:10 AM

It's extra interesting, because it also happens with bitmap fonts, which harfbuzz shouldn't really care much about.

Unless, @raster, does textgrid also (like textblock) calculate the ascent of all of the fonts in the font list? Or just the current one?

gregkh added a subscriber: gregkh.Nov 24 2015, 11:16 AM

@tasn: the ascent is calculated once per font using 'O' (capitalized o).

It seems it's only for the current font.

@tasn - awesome timing for this - i cant help atm as i've had half of yesterday be a seminar and i'm moving house today... internet going down soon! :)

On gregkh's screenshot at https://plus.google.com/u/0/+gregkroahhartman/posts/EJBC3JmGTxS, like Alan Cox said, the font looks top aligned instead of being bottom aligned.

Tomorrow's evening, I'll try to find time to git bisect harfbuzz.
Harfbuzz 1.1.1 still produces the issue.

Yes, the font looks top-aligned for some very odd reason.

rethab added a subscriber: rethab.Nov 24 2015, 10:19 PM
jpeg added a comment.Nov 24 2015, 11:12 PM

The offending line in EFL is here:

EAPI Eina_Bool
evas_common_font_ot_populate_text_props(const Eina_Unicode *text,
      Evas_Text_Props *props, int len, Evas_Text_Props_Mode mode)
{
   //...
   ot_itr->y_offset = positions->y_offset;
   //...
}

With Harfbuzz 1.0.6 we get 0, always. With 1.1.0+ we get varying values.

I'm not able to properly bisect harbuzz though (I've had 1.1.1 work fine when installed in a different prefix).

jpeg added a comment.Nov 24 2015, 11:41 PM

I managed to bisect. The offending commit in harfbuzz is:

44f82750807475aa5b16099ccccd917d488df703 is the first bad commit
commit 44f82750807475aa5b16099ccccd917d488df703
Author: Behdad Esfahbod <behdad@behdad.org>
Date:   Wed Nov 4 20:40:05 2015 -0800

    [ft] Remove font funcs that do nothing

:040000 040000 68d6f48906a747e5809c2ab8a693e994a60fd730 ab09b34288dfd746741902d6218b7d82148126e9 M	src

Kinda strange. But my guess is that hb_ft_get_glyph_v_kerning was called and always returned 0.
And now another function is called.

See http://cgit.freedesktop.org/harfbuzz/commit/?id=44f82750807475aa5b16099ccccd917d488df703

jpeg added a comment.Nov 24 2015, 11:50 PM

See 8ccea8233c144f723470da09a01487484c269440.
I'm not happy about this commit, as I suspect that the behaviour change happened in harfbuzz or freetype2.

tasn added a comment.Nov 25 2015, 3:24 AM

@jpeg, I haven't gotten around to actually debugging it yet (will do so now), but what do you mean by "varying values"? Arbitrary values for the same glyph? Or consistent non-zero?
As mentioned on the ML, I wonder why it works for textblock and not for textgrid, possibly because of the use of opentype fonts vs bitmaps.
We get the "positions" buffer straight from harfbuzz (allocation included), so it is possible it's not zeroed out before it's set.

Anyhow, I'll investigate further now. I'll also open a ticket for the harfbuzz people (seems like we haven't yet).

tasn added a comment.Nov 25 2015, 3:36 AM

More info: It has nothing to do with bitmap fonts. Same font works with textblock and opentype fonts are kinda (less) broken with terminology. I suspect the problem is with terminology, will keep you posted as I go.

fanf42 added a comment.EditedNov 25 2015, 3:51 AM

If it helps, here is a screenshot with dejavu sans mono: https://www.enlightenment.org/ss/e-565429ca4e0b37.80422233.jpg

Also, I tried with bitmap fonts: they were correctly aligned, but the cursor was vertically misplaced.

tasn added a comment.Nov 25 2015, 4:29 AM

Reported to the harfbuzz people: https://github.com/behdad/harfbuzz/issues/187

I wonder why this doesn't affect textblock and only affects textgrid, though the values returned by harfbuzz do seem to be broken.

Still not sure, but textgrid differs from textblock in shaping. It passes EVAS_TEXT_PROPS_MODE_NONE, while textblock passes EVAS_TEXT_PROPS_MODE_SHAPE. One uses the "fallback" shaper, while the other uses the "ot" shaper.
Again, not really sure how and why the fallback shaper acts like this, but already mentioned my findings down the path of this shaper in @tasn's github ticket.

@tasn, @raster, any good reason why not to use EVAS_TEXT_PROPS_MODE_SHAPE? If not, I'll patch it as it feels like a valid fix.

tasn added a comment.Nov 25 2015, 7:17 AM

You probably don't want to shape, because you just want the individual glyphs, and because of how textgrid works that it does them one at a time, shaping will maybe break some glyphs that will get the standalone shaping that they may have, which is wrong.

Good info for the github ticket though.

tasn updated the task description. (Show Details)Nov 25 2015, 8:41 AM
tasn lowered the priority of this task from Showstopper Issues to Normal.

Issue should be fixed with harfbuzz 1.1.2.

tasn closed this task as Resolved.Nov 27 2015, 1:27 AM
tasn claimed this task.

As @billiob said, it's fixed upstream, so I'm closing this one.

Ah so that's what it was! Thank all of you who worked on the issue, here's ALT Linux bug report just in case: https://bugzilla.altlinux.org/show_bug.cgi?id=31575 (in Russian) -- seen that with http://nightly.altlinux.org/sisyphus/flavours/enlightenment/