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

Issue 739381 link

Starred by 6 users

Issue metadata

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



Sign in to add a comment

Rendering regressions with latest FreeType roll

Project Member Reported by npm@chromium.org, Jul 5 2017

Issue description

I tried updating PDFium to match Chromium's current FreeType version, 7819aeb622a94be0d89caf8382f290d0266c4aed. Our tests had many failures, and some of them are actual rendering regressions.

The problem can be reproduced in Chrome Canary (I tried Mac). One such bad example is this PDF file: https://pdfium.googlesource.com/pdfium_tests/+/master/fx/FRC_8.2.2_part1/FRC_1_8.2.2__Title_edit.pdf

I attached screenshots of before and after.
FreeType should be rolled back if upstream fixes don't come quickly. Could you CC our FreeType contact? I forgot his email.
 
Before.png
101 KB View Download
After.png
100 KB View Download

Comment 1 by drott@chromium.org, Jul 5 2017

Cc: lemzw...@googlemail.com behdad@chromium.org

Comment 2 by drott@chromium.org, Jul 5 2017

Nicolás, would you have time to bisect this using binary search on the FreeType revisions between the previous roll and this one and placing those in the DEPS file + rebuilding?

Indeed, this would be very helpful, since I don't use pdfium – and I can't reproduce the issue with either evince or okular.

Comment 4 by npm@chromium.org, Jul 5 2017

Using the PDF above, I bisected to:

commit 75cb071b3fbfa2315c5d458fee2bb465a14568ae
Author: Alexei Podtelezhnikov <apodtele@gmail.com>
Date:   Wed Jun 21 22:52:37 2017 -0400

    [sfnt] Synthesize a Unicode charmap if one is missing.
    
    * src/sfnt/ttcmap.h (tt_cmap_unicode_class_rec): Declare it.
    * src/sfnt/ttcmap.c (tt_get_glyph_name, tt_cmap_unicode_init,
    tt_cmap_unicode_done, tt_cmap_unicode_char_index,
    tt_cmap_unicode_char_next, tt_cmap_unicode_class_rec): Implement
    synthetic Unicode charmap class.
    (tt_get_cmap_info): Make sure the callback is available.
    
    * src/sfnt/sfobjs.c (sfnt_load_face)
    [FT_CONFIG_OPTION_POSTSCRIPT_NAMES]: If Unicode charmap is missing,
    synthesize one.
    
    * include/freetype/config/ftoption.h: Document it.
    * devel/ftoption.h: Ditto.

Note that I've been investigating some other issues related to this change, see https://bugs.chromium.org/p/chromium/issues/detail?id=738325 . Though I think that one may be a PDFium issue, currently looking to see.
Are you using head of FreeType's git master?  There was a bug in this very commit, fixed with abeb28f161bc11be4fcdb8af50ab4b736d34a3e5...

Comment 7 by npm@chromium.org, Jul 5 2017

Even Chromium's FreeType version is before abeb28f161bc11be4fcdb8af50ab4b736d34a3e5, that's why I didn't try checking TOT. But if I try that hash, this PDF renders correctly again. So I suppose we should soon roll FreeType again in both Chromium and PDFium?
Chromium's FreeType is one commit before abeb28f161bc11be4fcdb8af50ab4b736d34a3e5, and I can confirm rolling past that also works around  issue 738325  (though I think that issue probably still wants to be fixed in PDFium). I'll start rolling Chromium's FreeType now.
Hmmm... unfortunately FreeType change http://git.savannah.gnu.org/cgit/freetype/freetype2.git/commit/?id=1c85479d2d1de54a4b592ffbef0ae24f498053d2 is causing the Windows compile bots https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.chromium.win%2Fwin_chromium_rel_ng%2F482337%2F%2B%2Frecipes%2Fsteps%2Fcompile__with_patch_%2F0%2Fstdout to complain about FT_Stream_SeekSet having "'<': signed/unsigned mismatch".

Fortunately we can still roll to just before this right now.
Cc: caryclark@chromium.org npm@chromium.org dsinclair@chromium.org ajha@chromium.org kavvaru@chromium.org brajkumar@chromium.org
 Issue 738325  has been merged into this issue.
Project Member

Comment 12 by bugdroid1@chromium.org, Jul 6 2017

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

commit 83b55ffcf303b35caaf048c76c9f9b89da2c3698
Author: Ben Wagner <bungeman@chromium.org>
Date: Thu Jul 06 15:15:41 2017

Roll src/third_party/freetype/src/ 7819aeb62..c56d8851e (3 commits)

https://chromium.googlesource.com/chromium/src/third_party/freetype2.git/+log/7819aeb622a9..c56d8851ea98

$ git log 7819aeb62..c56d8851e --date=short --no-merges --format='%ad %ae %s'
2017-07-03 apodtele * src/base/ftlcdfil.c (ft_lcd_filter_fir): Improve code.
2017-07-03 wl [truetype] Integer overflow.
2017-07-01 apodtele * src/sfnt/sfobjs.c (sfnt_load_face): Ignore No_Unicode_Glyph_Name.

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

BUG= chromium:739381 

Change-Id: I78bb7d4c160330ec1a76a9462077fdd2e8dc7719
Reviewed-on: https://chromium-review.googlesource.com/559896
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Commit-Queue: Ben Wagner <bungeman@google.com>
Cr-Commit-Position: refs/heads/master@{#484598}
[modify] https://crrev.com/83b55ffcf303b35caaf048c76c9f9b89da2c3698/DEPS
[modify] https://crrev.com/83b55ffcf303b35caaf048c76c9f9b89da2c3698/third_party/freetype/README.chromium
[modify] https://crrev.com/83b55ffcf303b35caaf048c76c9f9b89da2c3698/third_party/freetype/roll-freetype.sh

 Issue 739613  has been merged into this issue.
Cc: -bunge...@chromium.org
Owner: bunge...@chromium.org
Status: Fixed (was: Untriaged)
Labels: Merge-Request-60
Requesting merge of the FreeType roll in comment #12 ( https://chromium.googlesource.com/chromium/src.git/+/83b55ffcf303b35caaf048c76c9f9b89da2c3698 ) to M60. 
Project Member

Comment 16 by sheriffbot@chromium.org, Aug 1 2017

Labels: -Merge-Request-60 Hotlist-Merge-Review Merge-Review-60
This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 17 by bugdroid1@chromium.org, Aug 1 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/tools/buildspec/+/110624096f7dbe2e59a8fd877269f10a9c52eac0

commit 110624096f7dbe2e59a8fd877269f10a9c52eac0
Author: Ben Wagner <bungeman@chromium.org>
Date: Tue Aug 01 03:32:04 2017

Please add appropriate OSs.
Labels: -Merge-Review-60 merge-merged-3112 OS-All
This issue was re-introduced into M60 by https://bugs.chromium.org/p/chromium/issues/detail?id=726631#c71 , but is no longer an issue due to the change in Comment #17. Updating labels to reflect current reality and make sure this is on the correct lists.

Sign in to add a comment