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

Add SFNT Table support #199

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Add SFNT Table support #199

wants to merge 8 commits into from

Conversation

NCBM
Copy link
Contributor

@NCBM NCBM commented Sep 16, 2024

This pull request introduces SFNT Table support, in convenience of accessing font data like usWeightClass and usWidthClass.

@HinTak
Copy link
Collaborator

HinTak commented Sep 16, 2024

Thanks for the work. While the pull looks fine mostly, I am a little concerned about adding code just for completeness. So some examples would be nice - and given that you get at both horizontal vertical layout tables, you might need to use a CJK font for the example.

Also, equivalent higher functionalities for getting at some of the values for other kinds of fonts are provided by other API; particularly values from the maxp table. Reading the tables raw may not be the best approach.

Perhaps the most problematic aspect of this pull is lack of consideration for varying table sizes. As you put in the comments, os/2 table have gone through 5 versions and each larger than the other. Fonts with v3 and v4 OS/2 tables are still fairly common. You can't just read everything as v5.

The other tables have got through fewer version changes, but OS/2 is going to be problematic.

@HinTak
Copy link
Collaborator

HinTak commented Sep 16, 2024

Also, merely dumping font table content isn't a good use of freetype / freetype-py - ttx already does it well, and is python-based too.

@NCBM
Copy link
Contributor Author

NCBM commented Sep 17, 2024

Perhaps the most problematic aspect of this pull is lack of consideration for varying table sizes. As you put in the comments, os/2 table have gone through 5 versions and each larger than the other. Fonts with v3 and v4 OS/2 tables are still fairly common. You can't just read everything as v5.

The other tables have got through fewer version changes, but OS/2 is going to be problematic.

I have tested all my version 3/4 fonts on my computer and found that the usLowerOpticalPointSize and usUpperOpticalPointSize seems to be set 0 and 65535 without any error or segfault. Since it's originally defined as a C structure, I think freetype lib has given a default value, but I need to inspect the code of freetype.

Also, merely dumping font table content isn't a good use of freetype / freetype-py - ttx already does it well, and is python-based too.

Sorry that actually I have code for indexing local fonts with this library, which seems not to be a good use. It's my preference that I rarely use any XML-based data and interface unless it's essential.

@NCBM
Copy link
Contributor Author

NCBM commented Sep 17, 2024

I've checked the code and found the default values when loading OS/2 table in freetype@83af801b : src/sfnt/ttload.c : in function tt_face_load_os2 : L1244-1273.

    os2->ulCodePageRange1        = 0;
    os2->ulCodePageRange2        = 0;
    os2->sxHeight                = 0;
    os2->sCapHeight              = 0;
    os2->usDefaultChar           = 0;
    os2->usBreakChar             = 0;
    os2->usMaxContext            = 0;
    os2->usLowerOpticalPointSize = 0;
    os2->usUpperOpticalPointSize = 0xFFFF;

    if ( os2->version >= 0x0001 )
    {
      /* only version 1 tables */
      if ( FT_STREAM_READ_FIELDS( os2_fields_extra1, os2 ) )
        goto Exit;

      if ( os2->version >= 0x0002 )
      {
        /* only version 2 tables */
        if ( FT_STREAM_READ_FIELDS( os2_fields_extra2, os2 ) )
          goto Exit;

        if ( os2->version >= 0x0005 )
        {
          /* only version 5 tables */
          if ( FT_STREAM_READ_FIELDS( os2_fields_extra5, os2 ) )
            goto Exit;
        }
      }
    }

So it should be OK to use a full structure.

@HinTak
Copy link
Collaborator

HinTak commented Sep 17, 2024

ttx is a wrapper around the corresponding library - xml format output is just an option, I believe.

Some values from freetype are made-up, as you found. Freetype provides an abstraction over multiple font formats (and versions of font formats), so doing data dump on specific tables isn't "accurate" in that sense, nor a good use of it, since you cannot tell if some of the values are genuine from the font or not. I think freetype will auto-correct obviously incorrect / I.e. inconsistent values too, so what you are dumping isn't always necessarily 100% the same as inside the font tables. (Quite unlike ttx, which are accurate dumps)

@NCBM
Copy link
Contributor Author

NCBM commented Sep 17, 2024

Thanks for introducing me with it.

Honestly, I simply need usWeightClass and usWidthClass for sorting font weight and font width, and personally I just need an available value. Since my other works are with the help of freetype, I just simply keep using freetype and freetype-py.

@HinTak
Copy link
Collaborator

HinTak commented Sep 17, 2024

Adding the bits you need/want is fine, especially if you include your usage as an example.

Wholesale data-dumping of font tables with freetype / freetype-py isn't quite ideal, for the two / three reasons I gave: (1) freetype gives an abstraction across four formats / for format versions, so accessing data too specific to some font formats isn't ideal , (2) some of the values are made-up , emulated, or auto-corrected, and not "accurate" e.g. using freetype to detect "incorrect" values in some scenarios is basically stupid, as freetype hides the "wrong" values from you, (3) there are other and better tools for doing accurate table dumps.

@HinTak
Copy link
Collaborator

HinTak commented Sep 17, 2024

In particular, very few fonts actually have a PCLT table these days, so any return data structure is almost entirely fictional and made-up. Like-wise, post table is only found in fonts that claim some traditional postscript font usage(?) and somewhat rare to see them, unless they are from Adobe (the home of postscript).

So I'd be interested to know if you actually have any fonts with a PCLT table at all. You are basically asking to add untested / untestable code which returns meaning-less data, in the PCLT case at least.

@HinTak
Copy link
Collaborator

HinTak commented Sep 17, 2024

The PCLT table was used for some PCL embedding usage and probably only found in rather old fonts distributed by HP, I think.

@HinTak
Copy link
Collaborator

HinTak commented Sep 17, 2024

And vertical layout tables against fonts in non-vertical languages are going to be entirely made-up and fictional, too.

I think you'll find that you "can" retrieve a v* table from Times successful, for example, but all of its content is synthesised by freetype, and not present within the font file itself.

@NCBM
Copy link
Contributor Author

NCBM commented Sep 17, 2024

I tested all my fonts and found https://github.com/ArtifexSoftware/urw-base35-fonts and some old microsoft fonts have PCLT data.

@HinTak
Copy link
Collaborator

HinTak commented Sep 17, 2024

Well, that isn't saying much - Artifex have a few piece of PCL related software (ghostpcl / ghostpdl in particular), so fonts distributed by them have the corresponding table... and also, those fonts are decades old :-).

@NCBM
Copy link
Contributor Author

NCBM commented Sep 17, 2024

And vertical layout tables against fonts in non-vertical languages are going to be entirely made-up and fictional, too.

I think you'll find that you "can" retrieve a v* table from Times successful, for example, but all of its content is synthesised by freetype, and not present within the font file itself.

Do you mean "Times" instead of "Times New Roman"? I tested the later one and got no VHEA table.

@HinTak
Copy link
Collaborator

HinTak commented Sep 18, 2024

I came across this 7-year old comment regarding your usage of usWeightClass and usWidthClass:
https://www.gamedev.net/forums/topic/690570-font-weights-and-thickness-classification-in-freetype/

Basically they are often just the default values and no better description of a font's characteristics than just guess from part of its name (e.g. "Regular", "Narrow", if the font vendor cares enough to put a non-default value in the OS/2 table, he would have also named his font with a postfix, too).

While looking at what PCLT does in the opentype spec, I took a look at the post part (it is alphabetical so is just next section), and post has an interesting version change - 3.0 is smaller than 1.0, by allowing the not-very-useful-later-half missing. This is reflected in the comment "Glyph names follow in the 'post' table, but we don't load them by default."

So we have the 3rd case of why doing table dumps wholesale is not a good idea: freetype's view of the tables can be (1) missing fields partly made-up and fictional, (2) some wrong/inconsistent values auto-corrected, (3) not so useful but large data block not loaded by default.

@NCBM
Copy link
Contributor Author

NCBM commented Sep 18, 2024

Oh, I see. In my opinion guessing font style is not quite nice so these two fields are needed. Are there many implementations guessing the style from the style strings, even though this would be not quite elegant?

@NCBM
Copy link
Contributor Author

NCBM commented Sep 21, 2024

Since there are some arguments and this PR is unwilling to be accepted, I'm going to close it. If there are any parts acceptable, there will be new separate PRs.

@NCBM NCBM closed this Sep 21, 2024
@HinTak
Copy link
Collaborator

HinTak commented Sep 21, 2024

Sorry there isn't a strong reason to merge this in its current form. To be fair, two other pulls solve real problems impacting a fair number of users (non-English locale usage, and Mac os emoji's) have been open for years - one of them two years old.

I am inclined to leave this open for you or others to improve on when it becomes a more convincing addition, or for reference to later work.

@HinTak HinTak reopened this Sep 21, 2024
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.

2 participants