Rendering regressions with latest FreeType roll |
||||||
Issue descriptionI 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.
,
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?
,
Jul 5 2017
Indeed, this would be very helpful, since I don't use pdfium – and I can't reproduce the issue with either evince or okular.
,
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.
,
Jul 5 2017
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.
,
Jul 5 2017
Are you using head of FreeType's git master? There was a bug in this very commit, fixed with abeb28f161bc11be4fcdb8af50ab4b736d34a3e5...
,
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?
,
Jul 5 2017
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.
,
Jul 5 2017
,
Jul 5 2017
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.
,
Jul 5 2017
Issue 738325 has been merged into this issue.
,
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
,
Jul 6 2017
Issue 739613 has been merged into this issue.
,
Jul 6 2017
,
Aug 1 2017
Requesting merge of the FreeType roll in comment #12 ( https://chromium.googlesource.com/chromium/src.git/+/83b55ffcf303b35caaf048c76c9f9b89da2c3698 ) to M60.
,
Aug 1 2017
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
,
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
,
Aug 2 2017
Please add appropriate OSs.
,
Aug 2 2017
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 |
||||||
Comment 1 by drott@chromium.org
, Jul 5 2017