New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 769635 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Regression in FreeType 6f2b6f8f72ffb5017ab00fca83185b21f1a9f56d

Project Member Reported by npm@chromium.org, Sep 28 2017

Issue description

I was going to upgrade FreeType to match Chromium. The upgrade from 7e50824288fac5a36c2938fdb3e1c949ea53f982 to 6f2b6f8f72ffb5017ab00fca83185b21f1a9f56d is causing some problems in PDFium. For example, see the diff in the Algerian font text here https://pdfium-review.googlesource.com/c/pdfium_tests/+/15050/1/fx/font/font_1_embedded_font_en_feature_expected.pdf.1.png

Since Chrome is currently in that version, issues in PDF rendering can also be seen by opening PDFs in Canary. Is this a known problem, should I bisect?
 

Comment 1 by npm@chromium.org, Sep 28 2017

Cc: lemzw...@googlemail.com
Bisected. The Algerian font badness seems to be caused by this commit:

commit 6d04bd991bf4ab1c77f0cffe2f317920d00b6c46
Author: Werner Lemberg <wl@gnu.org>
Date:   Thu Sep 21 21:22:51 2017 +0200

    [truetype] Integer overflow (#52082).
    
    * src/truetype/ttinterp.c (Ins_MDRP): Avoid FT_ABS.

This is already fixed in git.  Please update.
Note that 63be40bccf02bd280da14c0249658e5531b3ea20 was landed because of regressions caused by 6d04bd991bf4ab1c77f0cffe2f317920d00b6c46 , but the roll from 7e50824288fac5a36c2938fdb3e1c949ea53f982 to 6f2b6f8f72ffb5017ab00fca83185b21f1a9f56d should contain both. It there another change that should be considered?

I'll see about making a test case for this with just FreeType.
Attached is the subset font in question extracted from the pdf from the test. I've opened this with "ftview 16 algerian.ttf" at current tip of tree FreeType ( ec7d2e5f683dab0d1471cbc1f25d0e65aae63b5d ) and noticed that it looks bad. I then reverted both 63be40bccf02bd280da14c0249658e5531b3ea20 and 6d04bd991bf4ab1c77f0cffe2f317920d00b6c46  and re-ran, and then it looked fine.
algerian.ttf
8.3 KB Download
I added some printfs around the above testcase, and one of the differences is with the values

exc->GS.single_width_cutin:  48
exc->GS.single_width_value:  13
                  org_dist: 153

Old code: false
New code: true

After re-working all the logic from the beginning, it appears the test here should be all && and no ||, in other words the test should be

    if ( exc->GS.single_width_cutin > 0              &&
         ( org_dist < exc->GS.single_width_value +
                        exc->GS.single_width_cutin &&
           org_dist > exc->GS.single_width_value -
                        exc->GS.single_width_cutin ) )

Status: ExternalDependency (was: Untriaged)
I've commented on https://savannah.nongnu.org/bugs/index.php?52082 so that all the discussion around this issue ends up in one place for future reference.

Note that Chromium only very recently rolled FreeType to 6f2b6f8, and I'm currently looking at an issue rolling one commit past that. If we can't roll in a FreeType fix for this soon I'll revert the FreeType roll to 6f2b6f8 in Chromium (unfortunately any fix will land at tip of tree FreeType and there are a lot of changes which landed in FreeType recently, I may use my magic branch powers to check in just this fix on a local branch to avoid an enormous roll later).
Cc: -bunge...@chromium.org npm@chromium.org
Owner: bunge...@chromium.org
Status: Assigned (was: ExternalDependency)
Upstream has committed the fix, we just need to roll it in now.

@npm: if you want to be ahead of the curve, try rolling pdfium's FreeType to c06b9cf56d0421275f3dd39488613026f23b79f2 (current tip of tree). I'd be rolling Chromium to that, but I need to fix some build file shenanigans for  https://bugs.chromium.org/p/chromium/issues/detail?id=768938 first. Note that the Type1/CFF hinting changes from the gsoc landed in there, so if you have test PDFs with Type1 fonts they may change rendering (hopefully for the better).

Comment 8 by npm@chromium.org, Oct 5 2017

FYI I rolled to ae7dc1f62d826083d418e86cce3f66a76dff038a (that commit fixes compile errors on Win).
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9eecb296392236dc45916ac448c78103db17e2c8

commit 9eecb296392236dc45916ac448c78103db17e2c8
Author: Ben Wagner <bungeman@chromium.org>
Date: Thu Oct 12 03:50:29 2017

Roll freetype to m63 branch to pick up bug fix.

https://chromium.googlesource.com/chromium/src/third_party/freetype2/+log/6f2b6f8..c3a51e4

This cherry-picks one bugfix needed to properly display several
TrueType fonts into the new chromium/milestone/63 branch. This
fix landed upstream after a very large set of changes which
still have issues being worked out.

Change-Id: I97d9ed47211087e060ccbd70d811e055182a4e12

BUG= chromium:769635 
TBR=drott@chromium.org

Change-Id: I97d9ed47211087e060ccbd70d811e055182a4e12
Reviewed-on: https://chromium-review.googlesource.com/714683
Commit-Queue: Ben Wagner <bungeman@chromium.org>
Reviewed-by: Ben Wagner <bungeman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508251}
[modify] https://crrev.com/9eecb296392236dc45916ac448c78103db17e2c8/DEPS
[modify] https://crrev.com/9eecb296392236dc45916ac448c78103db17e2c8/third_party/freetype/README.chromium

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/94e34d5bd10db429c38ca8c209c6ee5f129751df

commit 94e34d5bd10db429c38ca8c209c6ee5f129751df
Author: Ben Wagner <bungeman@chromium.org>
Date: Mon Oct 16 15:31:07 2017

Roll src/third_party/freetype/src/c3a51e430ca5c..ae7dc1f62 (52 commits)

https://chromium.googlesource.com/chromium/src/third_party/freetype2.git/+log/c3a51e430ca5c..ae7dc1f62d82

$ git log 6f2b6f8f7..ae7dc1f62 --date=short --no-merges --format='%ad %ae %s'
2017-09-28 apodtele [smooth, raster] Miscellaneous cleanups.
2017-09-28 bungeman [truetype] Really, really fix #52082.
2017-09-28 wl * src/psaux/psintrp.c (cf2_doStems): Fix integer overflow.
2017-09-28 ewaldhew * src/cid/cidgload.c (cid_slot_load_glyph): Fix memory leak.
2017-09-28 apodtele Bitmap metrics presetting [1/2].
2017-09-28 ewaldhew Fix compiler warning.
2017-09-27 wl * src/sfnt/ttload.c (tt_face_load_font_dir): Fix compiler warning.
2017-09-26 wl Copyright notices, formatting, whitespace, minor doc fixes.
2017-09-25 wl Fix compiler warnings.
2017-09-25 ewaldhew Minor fixes.
2017-09-25 ewaldhew Documentation fixes.
2017-09-25 ewaldhew Move `psdecode' into `psobjs'.
2017-09-25 ewaldhew Fix Type 1 hinting.
2017-09-25 ewaldhew Add tracing for hints.
2017-09-25 ewaldhew Minor fixes.
2017-09-25 ewaldhew Use the new engine.
2017-09-25 ewaldhew Add Adobe engine configuration.
2017-09-25 ewaldhew Change subfont synthesis for CID fonts.
2017-09-25 ewaldhew Switch to Adobe engine.
2017-09-25 ewaldhew Extend Adobe interpreter (seac).
2017-09-25 ewaldhew Extend Adobe interpreter (flex in callothersubr).
2017-09-25 ewaldhew Extend Adobe interpreter (callothersubr).
2017-09-25 ewaldhew Extend Adobe interpreter (pop).
2017-09-25 ewaldhew Extend Adobe interpreter (callsubr).
2017-09-25 ewaldhew Extend Adobe interpreter (div, four-byte numbers).
2017-09-25 ewaldhew Extend Adobe interpreter (hstem, vstem, hstem3, vstem3).
2017-09-25 ewaldhew Extend Adobe interpreter (hsbw, sbw).
2017-09-25 ewaldhew Extend Adobe interpreter (setcurrentpoint).
2017-09-25 ewaldhew Extend Adobe interpreter (closepath).
2017-09-25 ewaldhew Add Type 1 operations to Adobe CFF interpreter.
2017-09-25 ewaldhew Fixes for rendering.
2017-09-25 ewaldhew Add missing objects (2/2).
2017-09-25 ewaldhew Add missing objects for Type 1 (1/2).
2017-09-25 ewaldhew Allow `type1' module to use the Adobe engine.
2017-09-25 ewaldhew Add Adobe engine configuration.
2017-09-25 ewaldhew Move and rename `CFF_Driver'.
2017-09-25 ewaldhew Reorganize object fields.
2017-09-25 ewaldhew Prepare for Type 1 mode.
2017-09-25 ewaldhew Use the new objects.
2017-09-24 ewaldhew Objects for new interpreter (part 2).
2017-09-24 ewaldhew Add objects for new interpreter.
2017-09-24 ewaldhew Rename files.
2017-09-24 ewaldhew Minor fix.
2017-09-24 ewaldhew Move `cff_random' into `psaux' service.
2017-09-24 ewaldhew Move struct declarations to `freetype/internal'.
2017-09-24 ewaldhew Add new service for inter-module calls.
2017-09-24 ewaldhew Add callbacks for inter-module calls.
2017-09-24 ewaldhew Create new `PSAux' service interface entries.
2017-09-24 ewaldhew Move CFF builder components into `psaux' module.
2017-09-24 ewaldhew Move CFF decoder components into `psaux' module.
2017-09-25 ewaldhew [psaux, cff] Move Adobe's engine components into `psaux' module.
2017-09-24 apodtele Tweak per-face LCD filtering controls.

Created with:
  roll-dep src/third_party/freetype/src
R=bungeman@chromium.org,drott@chromium.org

Change-Id: Ia6768660a35c6db2f844324078abfbd6524798d9

BUG= chromium:769635 

Change-Id: Ia6768660a35c6db2f844324078abfbd6524798d9
Reviewed-on: https://chromium-review.googlesource.com/707218
Commit-Queue: Ben Wagner <bungeman@chromium.org>
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509051}
[modify] https://crrev.com/94e34d5bd10db429c38ca8c209c6ee5f129751df/DEPS
[modify] https://crrev.com/94e34d5bd10db429c38ca8c209c6ee5f129751df/third_party/freetype/BUILD.gn
[modify] https://crrev.com/94e34d5bd10db429c38ca8c209c6ee5f129751df/third_party/freetype/README.chromium
[modify] https://crrev.com/94e34d5bd10db429c38ca8c209c6ee5f129751df/third_party/freetype/include/freetype-custom-config/ftmodule.h

The fix has been cherry-picked into m63, and tip of tree has now rolled past the fix as well, so this should now be all fixed up.
Status: Fixed (was: Assigned)

Sign in to add a comment