Bug 80093 - does not render ligatures for TeX Gyre Termes font anymore
Summary: does not render ligatures for TeX Gyre Termes font anymore
Status: RESOLVED FIXED
Alias: None
Product: poppler
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: poppler-bugs
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-16 11:33 UTC by Fabian Greffrath
Modified: 2014-09-30 18:29 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
bring back ligature rendering with TeX Gyre Hermes (798 bytes, patch)
2014-06-16 13:43 UTC, Fabian Greffrath
Details | Splinter Review
if glyph name lookup failed, try alternate naming scheme for ligatures (2.20 KB, patch)
2014-08-26 04:35 UTC, Fabian Greffrath
Details | Splinter Review
if glyph name lookup failed, try alternate naming scheme for ligatures (2.11 KB, patch)
2014-09-16 07:32 UTC, Fabian Greffrath
Details | Splinter Review
map standard and expert ligatures to aglfn names (3.59 KB, patch)
2014-09-21 00:57 UTC, Adrian Johnson
Details | Splinter Review

Description Fabian Greffrath 2014-06-16 11:33:31 UTC
Dear poppler devs,

I'd like to bring an issue between poppler and the TeX Gyre Hermes fonts to your attention. The TeX Gyre fonts are high-quality replacements for the Base35 Ghostscripts fonts and are thus prefered by fontconfig for text rendering. TeX Gyre Hermes is the replacement for Times in this set.

However, there is one issue with the OTF font files of that fonf: They have their "fi", "fl" etc. ligature glyphs named "f_i", "f_l", etc. This leads to the situation that these glyphs are not recognized by poppler anymore and texts set in Times and rendered with TeX Gyre Hermes contain white spaces where the ligatures should be.

This was reported to work well with previous versions of poppler (around 0.18) but does not with recent versions, so it should be considered a regression. The issue has been reported to the Debian bug tracker here:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=742767

and has been discussed on the fontconfig bug tracker here:
https://bugs.freedesktop.org/show_bug.cgi?id=73291

until the fontconfig devs found that issue would be solved by kicking the TeX Gyre Hermes font out of the list of valid replacements for Times. :/

I'd like to ask you to change poppler to be more tolerant and accept the glyphs called "f_i", "f_l" etc. as aliases for "fi" and "fl" etc. They are valid ligatures glyphs.

Thank you very much!

Fabian
Comment 1 Fabian Greffrath 2014-06-16 13:43:39 UTC
Created attachment 101172 [details] [review]
bring back ligature rendering with TeX Gyre Hermes

This little hack brings back ligature rendering entirely. It's only a proof of concept, but maybe it shows where the problem lies and hopefully helps solve the issue in a cleaner way.
Comment 2 James Cloos 2014-06-16 13:57:03 UTC
Comment on attachment 101172 [details] [review]
bring back ligature rendering with TeX Gyre Hermes

Review of attachment 101172 [details] [review]:
-----------------------------------------------------------------

The concept is good.

It probably should work both ways (ie, look for /fi if /f_i is not found, et cetera).
Comment 3 Fabian Greffrath 2014-06-30 07:10:06 UTC
Quoting Karl Berry on https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=742767

    The problem is that the ligatures are named
            f_i

Adobe wants it that way these days, as I understand it.  (Personally I
think they were terribly wrong to try to change something so fundamental
but, surprisingly, they didn't ask me. :)

See section 6 (or search for ligature) in
  http://sourceforge.net/adobe/aglfn/wiki/AGL%20Specification
(and probably other adobe documents too, that's just what I found first).

In practice, since rendering software is so random about it, perhaps
fonts should include ligatures under both names.  Sigh.
Comment 4 Christopher Yeleighton 2014-06-30 09:20:16 UTC
Comment on attachment 101172 [details] [review]
bring back ligature rendering with TeX Gyre Hermes

The following code would be better IMHO:

size_t const len = strlen (name);
if ((len == 02 || len == 03) && * name == 'f') 
{ /* interleave underscores */
char new_name [06] = { * name, '_', name [01], '_', name [02] };
new_name [ (len << 01) - 01 ] = 0;
codeToGID[i] = FT_Get_Name_Index(face, new_name);
}
Comment 5 Fabian Greffrath 2014-06-30 10:23:31 UTC
Thank you very much, I was hoping for some more general code like this. However, please note that there are even more complex ligatures possible than just the f[f]? ones, like e.g. the o_f_f_i one mentioned in the Adobe document.
Comment 6 Fabian Greffrath 2014-07-21 08:36:52 UTC
This code does both directions.

	const size_t namelen = strlen(name);

	if (!strchr(name, '_'))
	{
		// try with interleaved underscores
		int i;
		char *tempname = malloc ((2 * namelen + 1) * sizeof (*tempname));

		tempname[0] = name[0];
		for (i = 1; i < namelen; i++)
		{
			tempname[2*i]   = name[i];
			tempname[2*i-1] = '_';
		}
	}
	else
	{
		// try without interleaved underscores
		int i, j;
		char *tempname = malloc ((namelen + 1) * sizeof (*tempname));

		for (i = j = 0; i < namelen; i++)
		{
			if (name[i] != '_')
				tempname[j++] = name[i];
		}
	}
Comment 7 Fabian Greffrath 2014-07-21 08:41:40 UTC
NB: This is just a concept. It should check for (namelen > 0) before processing the strings, it should explicitely add trailing '\0' to the tempname buffers and free() the tempname buffer afterwards.
Comment 8 Fabian Greffrath 2014-08-26 04:35:12 UTC
Created attachment 105262 [details] [review]
if glyph name lookup failed, try alternate naming scheme for ligatures

Hi there,

I have finally found time to refine the approach and address the issues I outlined in my previous posts. This new patch works in both ways, i.e. 'fi' -> 'f_i' and 'f_i' -> 'fi' and takes the glyph name length into account. I have restricted it to 2 >= length >= 7, because we expect something in the range between 'fi' and 'o_f_f_i'.

 - Fabian
Comment 9 Fabian Greffrath 2014-09-15 10:13:48 UTC
Maybe the code would be faster if I replaced *newname with newname[14].

Any comments?
Comment 10 Fabian Greffrath 2014-09-16 07:32:02 UTC
Created attachment 106357 [details] [review]
if glyph name lookup failed, try alternate naming scheme for ligatures

> Maybe the code would be faster if I replaced *newname with newname[14].

Well, here it is. Also, I have simplified "glyph name contains no underscores" loop a bit.
Comment 11 Adrian Johnson 2014-09-21 00:57:46 UTC
Created attachment 106593 [details] [review]
map standard and expert ligatures to aglfn names

The problem is with the TeX Gyre Hermes fonts. It is claiming to be a drop in replacement for the Standard 14 fonts but it is using names that are not compatible with Standard 14.

It does not matter that Adobe is recommending different names for new fonts. If TeX Gyre Hermes wants to be a replacement for the Standard 14 is has to provide the same encoding as defined in ISO32000 Annex D and PS Lang Ref Appendix E. There is no reason why the TeX fonts can't include both encodings. There are only five affected glyphs so it would only be a few extra bytes added to the fonts.

We can patch poppler to add the mapping for the ligatures but there is likely to be other software that depends on the PDF/PS standard names. So the best fix would be for TeX Gyre Hermes to support both names.

I don't agree with the patches that implement a generic two way mapping. The only affected glyphs are "fi" and "fl" in the Standard encoding and "ff", "ffi", and "ffl" in the Expert encoding. PDFs that are using other ligatures will have the fonts embedded since the Standard 14 fonts won't have these other ligatures. The patches are also missing support for the splash backend.

I am attaching a new patch that maps the five ligatures in both the cairo and splash backends.
Comment 12 Albert Astals Cid 2014-09-25 23:12:22 UTC
Errr, which ppdf do i use to test this? I can't find any attached or linked here.
Comment 13 Adrian Johnson 2014-09-25 23:30:58 UTC
The link is in the fontconfig bug:

http://www.cs.dartmouth.edu/~doug/mdmspe.pdf
Comment 14 Albert Astals Cid 2014-09-28 22:22:52 UTC
Sorry to be the stupid guy around, but this bug description kind of leaves a bit to be desired in the "how to reproduce" and "what is wrong".

So can someone please tell me "what is wrong" with that pdf, i see rendering fine, text search works etc.

Do i need some extra step? I see "TeX Gyre fonts" being mentioned but the file doesn't seem to use them. Is this for people that somehow are using those fonts as replacements in their fontconfig?
Comment 15 Fabian Greffrath 2014-09-29 05:20:11 UTC
> Is this for people that somehow are using those fonts as replacements in their fontconfig?

Exact. You need to tell fontconfig to prefer TeX Gyre Termes for rendering text that was set in Times.

Up to the latest commit to fontconfig GIT, this was the default, which is why the bug was initially assigned to fontconfig itself.

http://cgit.freedesktop.org/fontconfig/tree/conf.d/30-metric-aliases.conf
Comment 16 Albert Astals Cid 2014-09-30 16:49:40 UTC
Just pushed Adrian's patch.
Comment 17 Fabian Greffrath 2014-09-30 18:29:53 UTC
Finally, thank you so much!


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.