ENH: Add all font metrics for base 14 Type 1 PDF fonts.

[PJBrs]

This patch includes font metrics for the standard 14 fonts. This is intended to be useful for generating a text appearance stream, especially if you want to take into account right-aligned or centred text. (I have some other patches that include this as well as text wrapping.)

Note that some of this information was already included in _font_widths.py, but that information is incomplete. I thought it better to copy this information from pdfminer.six and be able to potentially benefit from their work later on, than to improve on what already was included here.

The first three patches introduce the new functionality. The last three patches are for moving the Font class to the new font metrics information and removing the old _font_widths.py file.

This is what the spec has to say about it:

9.6.2.2 Standard Type 1 fonts (standard 14 fonts) (PDF 1.0-1.7)

The PostScript language names of 14 Type 1 fonts, known as the standard 14 fonts, are as follows:
Times-Roman, Helvetica, Courier, Symbol, Times-Bold, Helvetica-Bold, Courier-Bold, ZapfDingbats,
Times-Italic, Helvetica-Oblique, Courier-Oblique, Times-BoldItalic, Helvetica-BoldOblique, CourierBoldOblique.

In PDF 1.0 to PDF 1.7, the FirstChar, LastChar, Widths and FontDescriptor (see Table 109: Entries in
a Type 1 font dictionary") were optional in Type 1 font dictionaries for the standard 14 fonts. PDF
processors supporting PDF 1.0 to PDF 1.7 files shall have these fonts, or their font metrics and suitable
substitution fonts, available.

These fonts, or their font metrics and suitable substitution fonts, shall be available to the PDF processor.

[PJBrs]

[about failing tests]
EDIT
I tested on the wrong branch and will investigate.

EDIT 2
Switching over the Font class to the new font metrics causes space changes in the extractor output. Otherwise the output is the same. I don't know this code, but I still assume that this is for the better. I can also drop the last three patches. But I'd like to hear from a reviewer to see what a good way forward would be.

The essentials in my view: having more complete font metrics available so that we can properly generate right-aligned and centered output.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.99%. Comparing base (ef81d5a) to head (1b9ea3e).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3363      +/-   ##
==========================================
+ Coverage   96.98%   96.99%   +0.01%     
==========================================
  Files          54       56       +2     
  Lines        9347     9384      +37     
  Branches     1713     1714       +1     
==========================================
+ Hits         9065     9102      +37     
  Misses        168      168              
  Partials      114      114              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[stefan6419846]

Thanks for the further changes. There still are some further changes I would like to see:

• Move the generator script code into pypdf itself instead of the resources. Otherwise, linting does not trigger for example.
• To which extent does the generator script re-use code from pdfminer.six?
• Why is there such a "strange" copyright detection logic instead of just reading the corresponding fields?
• Although there might be changes in a later PR, I would still prefer to have a proper object-oriented approach for the new data. Past has shown that nothing is more permanent than a temporary solution.

[PJBrs]

Thanks for the further changes. There still are some further changes I would like to see:

• Move the generator script code into pypdf itself instead of the resources. Otherwise, linting does not trigger for example.

No problem!

• To which extent does the generator script re-use code from pdfminer.six?

I think about 20%? The resulting formatting is still very like pdfminer (although their original script did not produce unicode codes but ints). EDIT:
grep -F -x -f /usr/lib64/python3.9/site-packages/pdfminer/fontmetrics.py resources/get_core_fontmetrics.py (excluding lines with only hashes or quotation marks):

            f = line.strip().split(" ")
            if not f:
                continue
            k = f[0]
            if k == "FontName":
                fontname = f[1]
                props = {"FontName": fontname, "Flags": 0}
            elif k == "C":
            elif k in ("CapHeight", "XHeight", "ItalicAngle", "Ascender", "Descender"):
                k = {"Ascender": "Ascent", "Descender": "Descent"}.get(k, k)
                props[k] = float(f[1])
            elif k in ("FontName", "FamilyName", "Weight"):
                k = {"FamilyName": "FontFamily", "Weight": "FontWeight"}.get(k, k)
                props[k] = f[1]
            elif k == "IsFixedPitch":
                if f[1].lower() == "true":
                    props["Flags"] = 64
            elif k == "FontBBox":
                props[k] = tuple(map(float, f[1:5]))

• Why is there such a "strange" copyright detection logic instead of just reading the corresponding fields?

This is how it is in the original AFM files:

Comment Copyright (c) 1985, 1987, 1989, 1990, 1997 Adobe Systems Incorporated. All Rights Reserved.

and

Notice Copyright (c) 1985, 1987, 1989, 1990, 1997 Adobe Systems Incorporated. All Rights Reserved.Helvetica is a trademark of Linotype-Hell AG and/or its subsidiaries.

So, the same copyright is in two locations, one after Comment and one after Notice. Only the second, however, includes mention of the trademark. And do notice the lack of a space between "Reserved." and "Helvetica". So, with all that, you indeed get a rather strange copyright detection logic!

• Although there might be changes in a later PR, I would still prefer to have a proper object-oriented approach for the new data. Past has shown that nothing is more permanent than a temporary solution.

OK. Then I think I need more information about what that would entail. What I think it does:

• Keep all information as it is now
• For the widths information, this can be used as I propose using it now, in _cmap.build_font_width_map
• Add a @dataclass font_descriptor to _cmap.py that can initialise itself using the information in core_fontmetrics.py
• Ideally, such a dataclass could actually use either the Type 1 font information from core_fontmetrics, or an existing /Descriptors dict within an embedded /Font resource. But I would need an easy test solution for that case (ideally, a form that doesn't use one of the core fonts).

Is that correct?

If not, perhaps it might be easier to reach out via discord, where I use the same user name.

[stefan6419846]

In the first step, it would be sufficient for me to just create a new dataclass with the properties outlined previously. Merging it with the existing class can always happen later, but I would like to see real structured data for now. We basically need to define the new dataclass and adapt the generator script to call the dataclass constructor accordingly.

Regarding the pdfminer.six code and the copyrights, I will need to have another look at it in the next days.

[PJBrs]

I'm afraid that means I'm officially out of my depth, I think I just lack the prerequisite knowledge to understand what you intend the generator script to do. If possible, please contact me on Discord one of these days so that we can discuss.

And as always, thanks for your comments!

[PJBrs]

P.S., this is what Google Gemini thinks:

import re
from dataclasses import dataclass
from typing import Dict, Tuple

@dataclass(frozen=True)
class Font:
    """
    Dataclass to store parsed information from a Type 1 font AFM file.
    """
    name: str
    family: str
    weight: str

    ascent: float
    descent: float
    cap_height: float
    x_height: float
    italic_angle: float
    flags: int
    bbox: Tuple[float, float, float, float]

    character_widths: Dict[str, int]

def parse_afm_file(afm_content: str) -> Font:
    """
    Parses the content of a Type 1 font AFM file and returns a Font dataclass instance.

    Args:
        afm_content: A string containing the full content of the AFM file.

    Returns:
        A Font dataclass instance populated with the parsed data.

    Raises:
        ValueError: If essential font properties are missing or malformed in the AFM content.
    """
    font_properties = {}
    character_widths = {}
    in_char_metrics_section = False

    lines = afm_content.splitlines()

    for line in lines:
        line = line.strip()
        if not line:
            continue

        # Check for start/end of character metrics section
        if line == "StartCharMetrics":
            in_char_metrics_section = True
            continue
        elif line == "EndCharMetrics":
            in_char_metrics_section = False
            continue

        if in_char_metrics_section:
            # Parse character metrics: C <char_code>; WX <width_x>; N <char_name>; ...
            match = re.match(r"C\s+\d+;\s+WX\s+([\d.]+);\s+N\s+([a-zA-Z0-9_.]+);", line)
            if match:
                width = int(float(match.group(1))) # Widths are typically integers in AFM
                char_name = match.group(2)
                character_widths[char_name] = width
        else:
            # Parse general font properties
            parts = line.split(' ', 1) # Split only on the first space
            if len(parts) == 2:
                key, value = parts[0], parts[1]
                font_properties[key] = value

    # Extract and convert properties, handling potential missing values
    try:
        name = font_properties.get("FontName", "Unknown")
        family = font_properties.get("FamilyName", "Unknown")
        weight = font_properties.get("Weight", "Unknown")

        ascent = float(font_properties.get("Ascender", 0.0))
        descent = float(font_properties.get("Descender", 0.0))
        cap_height = float(font_properties.get("CapHeight", 0.0))
        x_height = float(font_properties.get("XHeight", 0.0))
        italic_angle = float(font_properties.get("ItalicAngle", 0.0))

        # Calculate flags: bit 0 is set if IsFixedPitch is true
        is_fixed_pitch = font_properties.get("IsFixedPitch", "false").lower() == "true"
        flags = 1 if is_fixed_pitch else 0

        # Parse FontBBox
        bbox_str = font_properties.get("FontBBox", "0 0 0 0")
        bbox_values = tuple(map(float, bbox_str.split()))
        if len(bbox_values) != 4:
            raise ValueError(f"Malformed FontBBox: {bbox_str}")
        bbox = bbox_values

    except (KeyError, ValueError) as e:
        raise ValueError(f"Error parsing AFM file: Missing or malformed property - {e}")

    return Font(
        name=name,
        family=family,
        weight=weight,
        ascent=ascent,
        descent=descent,
        cap_height=cap_height,
        x_height=x_height,
        italic_angle=italic_angle,
        flags=flags,
        bbox=bbox,
        character_widths=character_widths
    )

[stefan6419846]

I have just used your script and adapted it to show what I mean: https://gist.github.com/stefan6419846/3d368b26ee5260a7886657909f26ca15 The adobe_glyphs module imported there is a standalone copy of https://github.com/py-pdf/pypdf/blob/main/pypdf/_codecs/adobe_glyphs.py

This will generate the code to create instances of the dataclass shown in #3363 (comment) together with the copyrights. The only things are are currently missing are the dataclass definition itself and the necessary imports as well as the mapping in the footer, but this should be easy enough to add to the script.

By the way: We should probably keep the script outside of the original code, but run some linting and testing on it nevertheless to ensure it matches our standards and does not break for some reason.

[PJBrs]

@stefan6419846 OK, now I understand better! Basically, you want the script to immediately produce the specific Font instances, instead of ending up in an intermediate form. With regard to linting / typing, as far as I can tell, I can just run ruff and mypy locally where it can find the script...? At least, that seems to work here.

I understand also, that using this, I can do something like:

if font_name in FONT_METRICS:
    font = FONT_METRICS[font_name]

and then do stuff like:

total_width = sum(font.character_widths[char] for char in "This is a long sentence")
print (total_width)

One question; Why do you only add 255 widths? The AFMs contain about 314 widths.

Second; so, this really collects a lot of information in the Font class. In my local patches, I need the widths (and I expect also some other metrics) for text wrapping a text stream when flattening an annotation. With my original patches, I changed build_font_width_map to include the character widths that are now in the Font instances in FONT_METRICS. However, the new Font class would not include character widths that are available for embedded fonts. So, how would you proceed with this? I see two ways forward:

1. Change build_font_width_map and use the new Font instances, something like this:

+    else:
+        font_name = str(ft["/BaseFont"])[1:]
+        if font_name in FONT_METRICS:
+            font_width_map = cast(Dict[str, float], FONT_METRICS[font_name].character_widths)

2. Change the Font dataclass so that you can initialise it with a /Font resource dictionary, set attributes bases on FONT_METRICS instances when available, and otherwise set character_widths using the existing build_font_width_map. This makes it possible, in the future, to also set the other metrics based on embedded font information, if available.

EDIT

3. Where would you add the Font class?
4. Where would you place the font generation script?

[stefan6419846]

With regard to linting / typing, as far as I can tell, I can just run ruff and mypy locally where it can find the script...?

Yes, although for the repository and its CI/CD, we might need to extend the current configuration. I am open to help with this if desired/required.

One question; Why do you only add 255 widths? The AFMs contain about 314 widths.

I used a mix of the original pdfminer.six code and your code for writing my script. The limitation is from pdfminer.six. This seems to mostly eliminate the characters with the code -1. I have no hard opinion on this/do not know what is correct.

However, the new Font class would not include character widths that are available for embedded fonts. So, how would you proceed with this?

I would split this into two separate topics. This PR should focus on the new container and retrieving the data from the AFM files to use them where appropriate. Possibly unifying this with handling embedded fonts could/should be a separate step in a PR afterwards. This way, we avoid too large PRs which simplifies reviewing the changes from my side.

[PJBrs]

@stefan6419846

I used your script and tried to improve it, and I added your font class.

To be fair, this PR more and more looks like you answering my bug report instead of me trying to contribute new code; please adjust copyright and attribution accordingly if and when you pull this.

I added the font class to pypdf/_font.py. I noticed that other files in this directory have a copyright notice and attribution, please advise, and add your name if you needed!

I just tested locally, and the following works:

            font_name = font_res["/BaseFont"]  # [/"Name"] often also exists, but is deprecated
            if font_name[1:] in FONT_METRICS:
                my_font = FONT_METRICS[font_name[1:]]
                print (sum( my_font.character_widths.get(char, 200) for char in "Such a long sentence, how long is€€€𒈙 it " ))

I noticed that codecov would like a test. I may be able to cobble up something, and otherwise it will be in a month or so.

[stefan6419846]

To be fair, this PR more and more looks like you answering my bug report instead of me trying to contribute new code; please adjust copyright and attribution accordingly if and when you pull this.

I am helping with getting the changes integrated. Without you doing the initial work, I would not have looked into this myself. Doing the changes to the parser has been a little side project, allowing me some insights into AFM files.

I noticed that codecov would like a test. I may be able to cobble up something, and otherwise it will be in a month or so.

Your initial PR integrated the new functionality into the existing code and adapted some tests. Wouldn't this be sufficient or does this have any side effects? Codecov is correctly complaining because the new code is never executed.

[PJBrs]

To be fair, this PR more and more looks like you answering my bug report instead of me trying to contribute new code; please adjust copyright and attribution accordingly if and when you pull this.

I am helping with getting the changes integrated. Without you doing the initial work, I would not have looked into this myself. Doing the changes to the parser has been a little side project, allowing me some insights into AFM files.

I noticed that codecov would like a test. I may be able to cobble up something, and otherwise it will be in a month or so.

Your initial PR integrated the new functionality into the existing code and adapted some tests. Wouldn't this be sufficient or does this have any side effects? Codecov is correctly complaining because the new code is never executed.

I wrote a very small test file now - tests/test_font.py:

"""Test font-related functionality."""


from pypdf._codecs.core_fontmetrics import FONT_METRICS


def test_font_metrics():
    font_name = "Helvetica"
    my_font = FONT_METRICS[font_name]
    assert my_font.family == "Helvetica"
    assert my_font.weight == "Medium"
    assert my_font.ascent == 718
    assert my_font.descent == -207

    test_string = "This is a long sentence. !@%%^€€€. çûįö¶´"
    charwidth = sum(my_font.character_widths[char] for char in test_string)
    assert charwidth == 19251

    font_name = "Courier-Bold"
    my_font = FONT_METRICS[font_name]
    assert my_font.italic_angle == 0
    assert my_font.flags == 64
    assert my_font.bbox == (-113.0, -250.0, 749.0, 801.0)

If you prefer, I'll add something to the writer test.

[stefan6419846]

I am okay with a simple test covering one of the basic entries and possibly one of the explicitly mapped ones as well.

[PJBrs]

I am okay with a simple test covering one of the basic entries and possibly one of the explicitly mapped ones as well.

Sounds like what I just pasted, right?

[PJBrs]

I redid the PR. I changed the dataclass name to FontDescriptor. I also added some code to actually use the fore font metrics in practice, by adding a class method to initialise FontDescriptor from a font resource in a PDF document.

The current PR is fully object oriented. I hope you like this version! It would offer me a good basis to implement text wrapping, text centering and right alignment in _writer.gen_appearance_stream().

For the future, it might be possible to:

1. Use the new font metrics in _cmap and within the Font class in the text extractor
2. Move the fontwidth calculation from cmap and the Font class in the text extractor
   to the FontDescriptor.from_font_res() method.
3. Add code to parse all font descriptor info from a font resource to the FontDescriptor.from_font_res() method.
4. Integrate the Font class from the text extractor and the new FontDescriptor class.

If I understand correctly, this would remove some code duplication and be a bit more object oriented.

[PJBrs]

@stefan6419846 Thanks for your previous review and especially for asking about the defaults. It's clear to me now that most defaults offered by Google Gemini were quite alright, but the whole flags thing was very probably wrong. I should have been more critical myself.

Interestingly, the original code that set the font flags to 64 for Courier (as copied from pdfminer.six) appears wrong as well; it tests for the IsFixed value, but that corresponds to the first bit of the flags (in other words, 1, not 64).

I don't think the flags are really important, otherwise the people at pdfminer.six would have noticed. I do think that I now have the correct values, having looked at Section 9.8.2 Font descriptor flags of the pdf specification.

[stefan6419846]

Thanks for looking into this again. I will have to delay the next proper review of this larger set of changes until September as I am on the move in the next weeks.

[PJBrs]

Enjoy your travels!

[PJBrs]

@stefan6419846 I implemented the change you requested!

[stefan6419846]

Thanks for your patience.

[PJBrs]

Thanks for your patience.

Same to you!