View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0038354 | FPC | Packages | public | 2021-01-14 01:41 | 2021-02-17 14:26 |
Reporter | joellinn | Assigned To | Michael Van Canneyt | ||
Priority | normal | Severity | minor | Reproducibility | always |
Status | resolved | Resolution | fixed | ||
Platform | All | ||||
Fixed in Version | 3.3.1 | ||||
Summary | 0038354: TFPFontCacheItem.TextWidth() reports wrong value for (some?) monospace fonts | ||||
Description | Size calculation is broken for monospace fonts like 'UbuntuMono-Regular' or 'CourierNewPSMT'. For example ('UbuntuMono-Regular', fontsize=10): 'w' = 437.35 'e' = 0.00 | ||||
Steps To Reproduce | ``` const pt = 10.0; var s: string; w1, w2: single; begin s := THETEXT; with gTTFontCache.Find('Ubuntu') do begin w1 := TextWidth(s, pt); end; with gTTFontCache.Find('UbuntuMono-Regular') do begin w2 := TextWidth(s, pt); end; WriteLn(Format('%f %f', [w1, w2])); ``` | ||||
Tags | No tags attached. | ||||
Fixed in Revision | 48694 | ||||
FPCOldBugId | |||||
FPCTarget | 3.2.2 | ||||
Attached Files |
|
related to | 0035251 | resolved | Michael Van Canneyt | fcl-pdf: Writing text in "Courier New" TTF font generates wrong PDF document |
|
Proposed PATCH is attached. There is an out of bounds array read in the character width table (htmx), which especially effects monospace fonts because they usually have just one entry in that table. 0001-fcl-pdf-fix-for-monospace-fonts.-fixes-38354.patch (2,454 bytes)
From be62c011961bc3d2dfb981cbf7cebdd4a7e43e01 Mon Sep 17 00:00:00 2001 From: Joel Linn <jl@conductive.de> Date: Thu, 14 Jan 2021 11:57:08 +0100 Subject: [PATCH] fcl-pdf: fix for monospace fonts. fixes #38354 --- packages/fcl-pdf/src/fpparsettf.pp | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/packages/fcl-pdf/src/fpparsettf.pp b/packages/fcl-pdf/src/fpparsettf.pp index fc7c243b5c..9dc055fca4 100644 --- a/packages/fcl-pdf/src/fpparsettf.pp +++ b/packages/fcl-pdf/src/fpparsettf.pp @@ -822,13 +822,13 @@ begin if embed and not Embeddable then raise ETTF.Create(rsFontEmbeddingNotAllowed); PrepareEncoding(Encoding); -// MissingWidth:=ToNatural(Widths[Chars[CharCodes^[32]]].AdvanceWidth); // Char(32) - Space character - FMissingWidth := Widths[Chars[CharCodes^[32]]].AdvanceWidth; // Char(32) - Space character +// MissingWidth:=ToNatural(GetAdvanceWidth(Chars[CharCodes^[32]])); // Char(32) - Space character + FMissingWidth := GetAdvanceWidth(Chars[CharCodes^[32]]); // Char(32) - Space character for I:=0 to 255 do begin if (CharCodes^[i]>=0) and (CharCodes^[i]<=High(Chars)) - and (Widths[Chars[CharCodes^[i]]].AdvanceWidth> 0) and (CharNames^[i]<> '.notdef') then - CharWidth[I]:= ToNatural(Widths[Chars[CharCodes^[I]]].AdvanceWidth) + and (GetAdvanceWidth(Chars[CharCodes^[i]])> 0) and (CharNames^[i]<> '.notdef') then + CharWidth[I]:= ToNatural(GetAdvanceWidth(Chars[CharCodes^[I]])) else CharWidth[I]:= FMissingWidth; end; @@ -930,8 +930,19 @@ begin end; function TTFFileInfo.GetAdvanceWidth(AIndex: word): word; -begin - Result := Widths[AIndex].AdvanceWidth; +var + i: SizeInt; +begin + // There may be more glyphs than elements in the array, in which + // case the last entry is to be used. + // https://docs.microsoft.com/en-us/typography/opentype/spec/hmtx + i := Length(Widths); + if AIndex >= i then + Dec(i) + else + i := AIndex; + + Result := Widths[i].AdvanceWidth; end; function TTFFileInfo.ItalicAngle: single; @@ -972,7 +983,7 @@ function TTFFileInfo.GetMissingWidth: integer; begin if FMissingWidth = 0 then begin - FMissingWidth := Widths[Chars[CharCodes^[32]]].AdvanceWidth; // 32 is in reference to the Space character + FMissingWidth := GetAdvanceWidth(Chars[CharCodes^[32]]); // 32 is in reference to the Space character end; Result := FMissingWidth; end; -- 2.27.0 |
|
There is a reference to the width array in the ttfsubsetter that needs to be fixed as well. It also reads the LSB values which should be handled differently according to // https://docs.microsoft.com/en-us/typography/opentype/spec/hmtx . They are not truncated. Maybe it's a better idea to always do SetLength(FWidths, GlyphCount) inside ParseHmtx, fill it like now until numberOfHMetrics. After that, read the packed LSB array into FWidths by adding the last advanceWidth to every entry. It wastes a bit of RAM but removes the complexity from accessing the values (and automatically fixes all software that uses Widths[i] instead of GetAdcanceWidth()). That however is a design decision to be made by the maintainers ("how closely should TTFFileInfo mirror the actual structure of the File") |
|
Applied, thank you very much ! |
Date Modified | Username | Field | Change |
---|---|---|---|
2021-01-14 01:41 | joellinn | New Issue | |
2021-01-14 12:02 | joellinn | Note Added: 0128312 | |
2021-01-14 12:02 | joellinn | File Added: 0001-fcl-pdf-fix-for-monospace-fonts.-fixes-38354.patch | |
2021-01-14 12:35 | joellinn | Note Added: 0128314 | |
2021-01-14 16:54 | Michael Van Canneyt | Assigned To | => Michael Van Canneyt |
2021-01-14 16:54 | Michael Van Canneyt | Status | new => assigned |
2021-01-20 17:13 | Michael Van Canneyt | Relationship added | related to 0035251 |
2021-02-17 14:26 | Michael Van Canneyt | Status | assigned => resolved |
2021-02-17 14:26 | Michael Van Canneyt | Resolution | open => fixed |
2021-02-17 14:26 | Michael Van Canneyt | Fixed in Version | => 3.3.1 |
2021-02-17 14:26 | Michael Van Canneyt | Fixed in Revision | => 48694 |
2021-02-17 14:26 | Michael Van Canneyt | FPCTarget | => 3.2.2 |
2021-02-17 14:26 | Michael Van Canneyt | Note Added: 0128971 |