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

Issue 700926 link

Starred by 1 user

Issue metadata

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

Blocked on:
issue pdfium:601

Blocking:
issue 669453



Sign in to add a comment

Link FreeType on Windows and use as fallback for DirectWrite, shared with PDFium

Project Member Reported by drott@chromium.org, Mar 13 2017

Issue description

OpenType Font Variations are the next big OpenType 1.8 typographic feature across OSes and browsers, coming to various platforms in 2017. In order to stay ahead and support this feature on the majority of Windows installations (only preview / Win10 Anniversary edition fast ring has some variation font support) we need to link FreeType on Windows and use it as an alternative font paint code path for variable fonts.

PDFium already links FreeType, so if we can adapt the build system accordingly and PDFium is happy with a very recent FreeType version, and continues to be happy with it when we upgrade FreeType for Chromium and Skia, then we can bring variation font support to Windows without a duplicate copy of FreeType and without adding much to Chrome's binary size. We're also keeping PDFium safe as new FreeType releases usually also address new security issues.

My current measurements look as follows, building development (non-official) release build, non-component build:

+--------------+------------+-------+-----------------+--------+--------+-------------+
|              |  chrome.dll|      Δ| chrome_child.dll|       Δ|      ΣΔ| freetype.dll|
+--------------+------------+-------+-----------------+--------+--------+-------------+
|Original:     | 229,069,312|       |      333,937,152|        |        |             |
|PDFium with   |            |       |                 |        |        |             |
|own FreeType  |            |       |                 |        |        |             |
+--------------+------------+-------+-----------------+--------+--------+-------------+
|PDFium using  | 229,090,304| 20,992|      334,061,568| 124,416| 145,408|             |
|freetype.dll  |            |       |                 |        |        |             |
+--------------+------------+-------+-----------------+--------+--------+-------------+
|PDFium and    | 229,094,912| 25,600|      334,191,104| 253,952| 279,552|             |
|Skia using    |            |       |                 |        |        |             |
|freetype.dll  |            |       |                 |        |        |             |
+--------------+------------+-------+-----------------+--------+--------+-------------+
|Component     |            |       |                 |        |        |      814,080|
|build         |            |       |                 |        |        |             |
|freetype.dll  |            |       |                 |        |        |             |
+--------------+------------+-------+-----------------+--------+--------+-------------+

So we can get variation support on Windows for a mere 280kb, 0.05% binary size increase (given this build configuration).

 

Comment 1 by drott@chromium.org, Mar 15 2017

Status: Started (was: Assigned)

Comment 2 by thakis@chromium.org, Mar 24 2017

Cc: scottmg@chromium.org
I don't have any particular concerns about adding FreeType as I think it's an important feature (and someday, I'd love to drop DirectWrite entirely too), but those binary size numbers are way out of whack and don't make any sense to me.

It's generally not useful to compare plain release build as the sizes are completely different than what we ship. That said, I don't even know how you managed to create 229M/333M binaries.

You can extract a binary out of an installer from e.g. https://pantheon.corp.google.com/storage/browser/chrome-signed/desktop-5c0tCh/59.0.3050.0/ to see current sizes.

Is PDFium really linked into the browser? :( Sigh.

Comment 4 by drott@chromium.org, Mar 24 2017

To clarify, I won't be "adding FreeType", but instead sharing FreeType between PDFium and Blink. Yes, PDFium which contains a relatively slim FreeType with fewer compiled files is already linked into our binaries. I would merely compile a few more files in FreeType since since Skia and Blink need more FreeType features, and then share this instance.

For the above table, I used a gn configuration as follows:
"use_goma=true
is_component_build=false"
and ran ninja, in the various configurations with my work in progress CLs applied to enable sharing FreeType.

I briefly tried the extraction using 7Zip or msiexec but the result of the 7Zip extraction did not make too much sense a list of files without extensions, the largest one starting with "Binary..." - how do I do it?

But yes, in any event, if the whole standalone installer is about 50MB, then we're far off. Is this triggered by the use_goma line? Are there still too many symbols in it? Or what is this order of magnitude difference?

I only tested and measured with the non-component, non-official release build so far to get a rough idea of the relative binary size changes. Would it be too coarse to assume that the results with a real official build would at least be roughly proportional?

Or in other words, I would tend to conclude that a full round of official builds for all configurations mentioned above might be taking a lot of time, and since already in this configuration the difference seems small, it might be okay to proceed with linking FreeType shared between PDFium and Blink and keep a close eye on the "sizes" tests on Windows for chrome.dll and chrome_child.dll on the performance dashboard. What do you think?




PDFium should only be in chrome_child.dll. I will open a new bug and investigate.

I've been testing chrome branded + official builds (with Goma) on Windows for a different issue. Those take a long time to link but the output sizes are comparible to the real Chrome builds.
I have src-internal in my Windows Chromium checkout. My build config is below, and the chrome.dll built with this config is 47 MB.

use_goma = true
goma_dir = "c:\\src\\goma"
target_cpu = "x64"
is_official_build = true
is_chrome_branded = true
is_debug = false
I modified third_party\pdfium\core\fxcrt\fx_basic_bstring.cpp and rebuilt the chrome target. Ninja showed only 3 steps: build fx_basic_bstring.obj, link fxcrt.lib, link chrome_child.dll.
Thanks for checking. I just wasn't clear on why chrome.dll changed size in #c0 then.
Project Member

Comment 10 by bugdroid1@chromium.org, Mar 28 2017

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

commit e15f5107bfba9080481fd78ed35c69c81fc41ac3
Author: drott <drott@chromium.org>
Date: Tue Mar 28 20:42:41 2017

Allow FreeType compilation and linkage from third_party on Windows

Create an OWNERS file for this directory and add myself as well. This is
preparation for using a shared FreeType between Blink and PDFium, but
not actually used as a depedency to any Windows targets yet.

BUG= 700926 

Review-Url: https://codereview.chromium.org/2781773003
Cr-Commit-Position: refs/heads/master@{#460211}

[modify] https://crrev.com/e15f5107bfba9080481fd78ed35c69c81fc41ac3/build/config/freetype/BUILD.gn
[add] https://crrev.com/e15f5107bfba9080481fd78ed35c69c81fc41ac3/build/config/freetype/OWNERS

Comment 11 Deleted

Comment 12 by drott@chromium.org, Mar 29 2017

Labels: OS-Windows
Comparing master vs. shared-freetype using

1) https://pdfium-review.googlesource.com/c/2971/ (PDFium against shared FreeType)
2) https://codereview.chromium.org/2780133002 (using FreeType in Blink) 

for gn args:
use_goma=true
dcheck_always_on=false
is_official_build=true
is_chrome_branded=true

I now get:
Master	
51,111,936	chrome.dll
70,439,424	chrome_child.dll
	
Shared-FreeType	
51,112,960	chrome.dll
70,516,224	chrome_child.dll
	
Size Differences	
1,024	𝝙 chrome.dll
76,800	𝝙 chrome_child.dll

chrome.exe being identical in both build configurations.

So, I would conclude this is a rather marginal binary size increase given the functionality gain.

Great, that's closer to what I would have expected, and I agree seems reasonable. Thanks for checking.

(FTR, I believe setting full_wpo_on_official=true would get binary sizes that are closer to Canary, which are ~48M and ~60M, rather than ~51/~70.)
Project Member

Comment 14 by bugdroid1@chromium.org, Mar 30 2017

The following revision refers to this bug:
  https://pdfium.googlesource.com/pdfium/+/4b0671ab3e795bbb5e6aaf6305cae3171d73d241

commit 4b0671ab3e795bbb5e6aaf6305cae3171d73d241
Author: Dominik Röttsches <drott@chromium.org>
Date: Thu Mar 30 08:45:37 2017

Allow configuration of external embedder FreeType

Add a public config to fxfreetype to make includes visible and provide a
freetype_common group as a public dependency on fxcrt. freetype_common
switches between fxfreetype and //build/config/freetype, which gives
embedders the flexibility to configure the source of FreeType.

BUG= chromium:700926 

Change-Id: I73ae26979dcf69a419485def23c7a13dffa2a15d
Reviewed-on: https://pdfium-review.googlesource.com/2971
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>

[modify] https://crrev.com/4b0671ab3e795bbb5e6aaf6305cae3171d73d241/build_overrides/pdfium.gni
[modify] https://crrev.com/4b0671ab3e795bbb5e6aaf6305cae3171d73d241/BUILD.gn
[modify] https://crrev.com/4b0671ab3e795bbb5e6aaf6305cae3171d73d241/third_party/BUILD.gn
[modify] https://crrev.com/4b0671ab3e795bbb5e6aaf6305cae3171d73d241/pdfium.gni

Project Member

Comment 15 by bugdroid1@chromium.org, Mar 30 2017

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

commit 2dc62d75bfe14b3d5cd2a6b7cdd1d76515f29498
Author: pdfium-deps-roller <pdfium-deps-roller@chromium.org>
Date: Thu Mar 30 10:24:24 2017

Roll src/third_party/pdfium/ 1831ca943..4b0671ab3 (1 commit)

https://pdfium.googlesource.com/pdfium.git/+log/1831ca9439ec..4b0671ab3e79

$ git log 1831ca943..4b0671ab3 --date=short --no-merges --format='%ad %ae %s'
2017-03-30 drott Allow configuration of external embedder FreeType

Created with:
  roll-dep src/third_party/pdfium
BUG= 700926 

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls

TBR=dsinclair@chromium.org

Review-Url: https://codereview.chromium.org/2790433004
Cr-Commit-Position: refs/heads/master@{#460717}

[modify] https://crrev.com/2dc62d75bfe14b3d5cd2a6b7cdd1d76515f29498/DEPS

Project Member

Comment 16 by bugdroid1@chromium.org, Mar 30 2017

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

commit 5265e65e5ea1ae3987c27f76914c9dd9a41506cd
Author: drott <drott@chromium.org>
Date: Thu Mar 30 21:48:07 2017

Support for OpenType Font Variations on Windows

Enable support for variable fonts on Windows through using
SkFontMgr_Custom_Empty, a FreeType backed font-blob only SkTypeface
factory (no access to system fonts). We will use a hybrid DirectWrite
and FreeType font stack on Windows for at least as long as most of
Windows versions we support with Chrome still do not have native support
for font variations.

Thanks to Ben Wagner for the help with enabling and prototyping this.

BUG= 700926 

Review-Url: https://codereview.chromium.org/2780133002
Cr-Commit-Position: refs/heads/master@{#460895}

[modify] https://crrev.com/5265e65e5ea1ae3987c27f76914c9dd9a41506cd/DEPS
[modify] https://crrev.com/5265e65e5ea1ae3987c27f76914c9dd9a41506cd/build_overrides/pdfium.gni
[modify] https://crrev.com/5265e65e5ea1ae3987c27f76914c9dd9a41506cd/skia/BUILD.gn
[modify] https://crrev.com/5265e65e5ea1ae3987c27f76914c9dd9a41506cd/third_party/WebKit/LayoutTests/NeverFixTests
[modify] https://crrev.com/5265e65e5ea1ae3987c27f76914c9dd9a41506cd/third_party/WebKit/LayoutTests/fast/text/variable-fonts/variable-box-font-expected.html
[modify] https://crrev.com/5265e65e5ea1ae3987c27f76914c9dd9a41506cd/third_party/WebKit/LayoutTests/platform/linux/fast/text/chromium-linux-fontconfig-renderstyle-expected.png
[modify] https://crrev.com/5265e65e5ea1ae3987c27f76914c9dd9a41506cd/third_party/WebKit/Source/platform/fonts/FontCustomPlatformData.cpp
[modify] https://crrev.com/5265e65e5ea1ae3987c27f76914c9dd9a41506cd/third_party/freetype/BUILD.gn
[modify] https://crrev.com/5265e65e5ea1ae3987c27f76914c9dd9a41506cd/third_party/freetype/include/freetype-custom-config/ftmodule.h
[modify] https://crrev.com/5265e65e5ea1ae3987c27f76914c9dd9a41506cd/third_party/freetype/include/freetype-custom-config/ftoption.h

Project Member

Comment 17 by bugdroid1@chromium.org, Mar 30 2017

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

commit b0e1e4e71a556604c95d25562f9447c175e541be
Author: ortuno <ortuno@chromium.org>
Date: Thu Mar 30 22:38:20 2017

Revert of Support for OpenType Font Variations on Windows (patchset #8 id:140001 of https://codereview.chromium.org/2780133002/ )

Reason for revert:
Compile failure on linux bot: https://uberchromegw.corp.google.com/i/chromium.linux/builders/Linux%20Builder%20%28dbg%29%2832%29/builds/65396

[6410/39636] CXX obj/third_party/pdfium/formfiller/cffl_interactiveformfiller.o
FAILED: obj/third_party/pdfium/formfiller/cffl_interactiveformfiller.o
/b/c/goma_client/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF obj/third_party/pdfium/formfiller/cffl_interactiveformfiller.o.d -DV8_DEPRECATION_WARNINGS -DUSE_UDEV -DUSE_AURA=1 -DUSE_PANGO=1 -DUSE_CAIRO=1 -DUSE_GLIB=1 -DUSE_NSS_CERTS=1 -DUSE_X11=1 -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=\"298539-1\" -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -DCOMPONENT_BUILD -D_DEBUG -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -DV8_DEPRECATION_WARNINGS -D_FX_CPU_=_FX_X86_ -DOPJ_STATIC -DPNG_PREFIX -DPNG_USE_READ_MACROS -DPDF_ENABLE_V8 -DPDFIUM_PRINT_TEXT_WITH_GDI -I../.. -Igen -I../../third_party/pdfium -I../../build/linux/debian_jessie_i386-sysroot/usr/include/freetype2 -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -funwind-tables -fPIC -pipe -B../../third_party/binutils/Linux_x64/Release/bin -fcolor-diagnostics -m32 -msse2 -mfpmath=sse -mmmx -momit-leaf-frame-pointer -pthread -mstack-alignment=16 -mstackrealign -Wall -Werror -Wextra -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override -Wno-shift-negative-value -Wno-undefined-var-template -Wno-nonportable-include-path -Wno-address-of-packed-member -Wno-unused-lambda-capture -Wno-user-defined-warnings -O0 -fno-omit-frame-pointer -g2 -gsplit-dwarf --sysroot=../../build/linux/debian_jessie_i386-sysroot -fvisibility=hidden -Xclang -load -Xclang ../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so -Xclang -add-plugin -Xclang find-bad-constructs -Xclang -plugin-arg-find-bad-constructs -Xclang check-auto-raw-pointer -Xclang -plugin-arg-find-bad-constructs -Xclang check-ipc -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -fvisibility-inlines-hidden -Wno-undefined-bool-conversion -Wno-tautological-undefined-compare -std=gnu++11 -fno-rtti -fno-exceptions -c ../../third_party/pdfium/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp -o obj/third_party/pdfium/formfiller/cffl_interactiveformfiller.o
In file included from ../../third_party/pdfium/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp:12:
In file included from ../../third_party/pdfium/core/fxge/cfx_pathdata.h:14:
In file included from ../../third_party/pdfium/core/fxge/cfx_renderdevice.h:12:
In file included from ../../third_party/pdfium/core/fxge/cfx_gemodule.h:12:
In file included from ../../third_party/pdfium/core/fxge/cfx_fontmgr.h:13:
In file included from ../../third_party/pdfium/core/fxge/fx_font.h:17:
In file included from ../../third_party/pdfium/core/fxge/fx_freetype.h:11:
In file included from ../../build/linux/debian_jessie_i386-sysroot/usr/include/freetype2/freetype.h:33:
../../build/linux/debian_jessie_i386-sysroot/usr/include/freetype2/config/ftconfig.h:453:5: error: 'register' storage class specifier is deprecated and incompatible with C++1z [-Werror,-Wdeprecated-register]
    register FT_Int32  result;
    ^~~~~~~~~
1 error generated.

Original issue's description:
> Support for OpenType Font Variations on Windows
>
> Enable support for variable fonts on Windows through using
> SkFontMgr_Custom_Empty, a FreeType backed font-blob only SkTypeface
> factory (no access to system fonts). We will use a hybrid DirectWrite
> and FreeType font stack on Windows for at least as long as most of
> Windows versions we support with Chrome still do not have native support
> for font variations.
>
> Thanks to Ben Wagner for the help with enabling and prototyping this.
>
> BUG= 700926 
>
> Review-Url: https://codereview.chromium.org/2780133002
> Cr-Commit-Position: refs/heads/master@{#460895}
> Committed: https://chromium.googlesource.com/chromium/src/+/5265e65e5ea1ae3987c27f76914c9dd9a41506cd

TBR=bungeman@chromium.org,bungeman@google.com,eae@chromium.org,kjellander@chromium.org,machenbach@chromium.org,drott@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 700926 

Review-Url: https://codereview.chromium.org/2786263002
Cr-Commit-Position: refs/heads/master@{#460909}

[modify] https://crrev.com/b0e1e4e71a556604c95d25562f9447c175e541be/DEPS
[modify] https://crrev.com/b0e1e4e71a556604c95d25562f9447c175e541be/build_overrides/pdfium.gni
[modify] https://crrev.com/b0e1e4e71a556604c95d25562f9447c175e541be/skia/BUILD.gn
[modify] https://crrev.com/b0e1e4e71a556604c95d25562f9447c175e541be/third_party/WebKit/LayoutTests/NeverFixTests
[modify] https://crrev.com/b0e1e4e71a556604c95d25562f9447c175e541be/third_party/WebKit/LayoutTests/fast/text/variable-fonts/variable-box-font-expected.html
[modify] https://crrev.com/b0e1e4e71a556604c95d25562f9447c175e541be/third_party/WebKit/LayoutTests/platform/linux/fast/text/chromium-linux-fontconfig-renderstyle-expected.png
[modify] https://crrev.com/b0e1e4e71a556604c95d25562f9447c175e541be/third_party/WebKit/Source/platform/fonts/FontCustomPlatformData.cpp
[modify] https://crrev.com/b0e1e4e71a556604c95d25562f9447c175e541be/third_party/freetype/BUILD.gn
[modify] https://crrev.com/b0e1e4e71a556604c95d25562f9447c175e541be/third_party/freetype/include/freetype-custom-config/ftmodule.h
[modify] https://crrev.com/b0e1e4e71a556604c95d25562f9447c175e541be/third_party/freetype/include/freetype-custom-config/ftoption.h

Project Member

Comment 18 by bugdroid1@chromium.org, Mar 30 2017

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

commit 27b5c368e1cd8535b26aacc593e544e18d04d29f
Author: stgao <stgao@chromium.org>
Date: Thu Mar 30 23:26:51 2017

Reland of Support for OpenType Font Variations on Windows (patchset #1 id:1 of https://codereview.chromium.org/2786263002/ )

Reason for revert:
The revert didn't help.

https://luci-milo.appspot.com/buildbot/chromium.linux/Linux%20Builder%20(dbg)(32)/65398

Original issue's description:
> Revert of Support for OpenType Font Variations on Windows (patchset #8 id:140001 of https://codereview.chromium.org/2780133002/ )
>
> Reason for revert:
> Compile failure on linux bot: https://uberchromegw.corp.google.com/i/chromium.linux/builders/Linux%20Builder%20%28dbg%29%2832%29/builds/65396
>
> [6410/39636] CXX obj/third_party/pdfium/formfiller/cffl_interactiveformfiller.o
> FAILED: obj/third_party/pdfium/formfiller/cffl_interactiveformfiller.o
> /b/c/goma_client/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF obj/third_party/pdfium/formfiller/cffl_interactiveformfiller.o.d -DV8_DEPRECATION_WARNINGS -DUSE_UDEV -DUSE_AURA=1 -DUSE_PANGO=1 -DUSE_CAIRO=1 -DUSE_GLIB=1 -DUSE_NSS_CERTS=1 -DUSE_X11=1 -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=\"298539-1\" -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -DCOMPONENT_BUILD -D_DEBUG -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -DV8_DEPRECATION_WARNINGS -D_FX_CPU_=_FX_X86_ -DOPJ_STATIC -DPNG_PREFIX -DPNG_USE_READ_MACROS -DPDF_ENABLE_V8 -DPDFIUM_PRINT_TEXT_WITH_GDI -I../.. -Igen -I../../third_party/pdfium -I../../build/linux/debian_jessie_i386-sysroot/usr/include/freetype2 -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -funwind-tables -fPIC -pipe -B../../third_party/binutils/Linux_x64/Release/bin -fcolor-diagnostics -m32 -msse2 -mfpmath=sse -mmmx -momit-leaf-frame-pointer -pthread -mstack-alignment=16 -mstackrealign -Wall -Werror -Wextra -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override -Wno-shift-negative-value -Wno-undefined-var-template -Wno-nonportable-include-path -Wno-address-of-packed-member -Wno-unused-lambda-capture -Wno-user-defined-warnings -O0 -fno-omit-frame-pointer -g2 -gsplit-dwarf --sysroot=../../build/linux/debian_jessie_i386-sysroot -fvisibility=hidden -Xclang -load -Xclang ../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so -Xclang -add-plugin -Xclang find-bad-constructs -Xclang -plugin-arg-find-bad-constructs -Xclang check-auto-raw-pointer -Xclang -plugin-arg-find-bad-constructs -Xclang check-ipc -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -fvisibility-inlines-hidden -Wno-undefined-bool-conversion -Wno-tautological-undefined-compare -std=gnu++11 -fno-rtti -fno-exceptions -c ../../third_party/pdfium/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp -o obj/third_party/pdfium/formfiller/cffl_interactiveformfiller.o
> In file included from ../../third_party/pdfium/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp:12:
> In file included from ../../third_party/pdfium/core/fxge/cfx_pathdata.h:14:
> In file included from ../../third_party/pdfium/core/fxge/cfx_renderdevice.h:12:
> In file included from ../../third_party/pdfium/core/fxge/cfx_gemodule.h:12:
> In file included from ../../third_party/pdfium/core/fxge/cfx_fontmgr.h:13:
> In file included from ../../third_party/pdfium/core/fxge/fx_font.h:17:
> In file included from ../../third_party/pdfium/core/fxge/fx_freetype.h:11:
> In file included from ../../build/linux/debian_jessie_i386-sysroot/usr/include/freetype2/freetype.h:33:
> ../../build/linux/debian_jessie_i386-sysroot/usr/include/freetype2/config/ftconfig.h:453:5: error: 'register' storage class specifier is deprecated and incompatible with C++1z [-Werror,-Wdeprecated-register]
>     register FT_Int32  result;
>     ^~~~~~~~~
> 1 error generated.
>
> Original issue's description:
> > Support for OpenType Font Variations on Windows
> >
> > Enable support for variable fonts on Windows through using
> > SkFontMgr_Custom_Empty, a FreeType backed font-blob only SkTypeface
> > factory (no access to system fonts). We will use a hybrid DirectWrite
> > and FreeType font stack on Windows for at least as long as most of
> > Windows versions we support with Chrome still do not have native support
> > for font variations.
> >
> > Thanks to Ben Wagner for the help with enabling and prototyping this.
> >
> > BUG= 700926 
> >
> > Review-Url: https://codereview.chromium.org/2780133002
> > Cr-Commit-Position: refs/heads/master@{#460895}
> > Committed: https://chromium.googlesource.com/chromium/src/+/5265e65e5ea1ae3987c27f76914c9dd9a41506cd
>
> TBR=bungeman@chromium.org,bungeman@google.com,eae@chromium.org,kjellander@chromium.org,machenbach@chromium.org,drott@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG= 700926 
>
> Review-Url: https://codereview.chromium.org/2786263002
> Cr-Commit-Position: refs/heads/master@{#460909}
> Committed: https://chromium.googlesource.com/chromium/src/+/b0e1e4e71a556604c95d25562f9447c175e541be

TBR=bungeman@chromium.org,bungeman@google.com,eae@chromium.org,kjellander@chromium.org,machenbach@chromium.org,drott@chromium.org,ortuno@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 700926 

Review-Url: https://codereview.chromium.org/2792453002
Cr-Commit-Position: refs/heads/master@{#460926}

[modify] https://crrev.com/27b5c368e1cd8535b26aacc593e544e18d04d29f/DEPS
[modify] https://crrev.com/27b5c368e1cd8535b26aacc593e544e18d04d29f/build_overrides/pdfium.gni
[modify] https://crrev.com/27b5c368e1cd8535b26aacc593e544e18d04d29f/skia/BUILD.gn
[modify] https://crrev.com/27b5c368e1cd8535b26aacc593e544e18d04d29f/third_party/WebKit/LayoutTests/NeverFixTests
[modify] https://crrev.com/27b5c368e1cd8535b26aacc593e544e18d04d29f/third_party/WebKit/LayoutTests/fast/text/variable-fonts/variable-box-font-expected.html
[modify] https://crrev.com/27b5c368e1cd8535b26aacc593e544e18d04d29f/third_party/WebKit/LayoutTests/platform/linux/fast/text/chromium-linux-fontconfig-renderstyle-expected.png
[modify] https://crrev.com/27b5c368e1cd8535b26aacc593e544e18d04d29f/third_party/WebKit/Source/platform/fonts/FontCustomPlatformData.cpp
[modify] https://crrev.com/27b5c368e1cd8535b26aacc593e544e18d04d29f/third_party/freetype/BUILD.gn
[modify] https://crrev.com/27b5c368e1cd8535b26aacc593e544e18d04d29f/third_party/freetype/include/freetype-custom-config/ftmodule.h
[modify] https://crrev.com/27b5c368e1cd8535b26aacc593e544e18d04d29f/third_party/freetype/include/freetype-custom-config/ftoption.h

Comment 19 by drott@chromium.org, Mar 31 2017

Labels: -Restrict-View-Google
Status: Fixed (was: Started)
Labels: -Pri-2 Pri-1
The last commit broke gn gen --ide=vs because https://chromium.googlesource.com/chromium/src/+/27b5c368e1cd8535b26aacc593e544e18d04d29f/skia/BUILD.gn#47

The project files .vcxproj are generated with for example ((FREETYPE_MINOR) << 16) as their PreprocessorDefinitions but << becomes invalid (see screenshot).

Now you can't use Visual Studio to develop on Windows.
vs-breakage.PNG
273 KB View Download
According to https://msdn.microsoft.com/en-us/library/ms228186.aspx you can escape the << using % and the hex of the ASCII code. I tried it and it seems to work (at least the project load, can't try to build it yet). If there is no other alternative than using << for the defines then gn needs to add some extra steps to convert these special characters in hexa before it writes it in the final project file. See https://chromium.googlesource.com/chromium/src/tools/gn/+/master/visual_studio_writer.cc#545


Cc: brettw@chromium.org
Assuming SK_FREETYPE_MINIMUM_RUNTIME_VERSION as a build define is absolutely required (not familiar with the code using it) then here is something crude to fix the problem with few assumptions :

- std:regex is allowed in Chromium/gn code
- gn gen --ide=vs is not time critical

https://gist.github.com/darktears/7988e2ee71df2d7f65314c950f02e408

I put the fix in visual_studio_writer because that's the only instance where << would need to be converted (per https://msdn.microsoft.com/en-us/library/ms228186.aspx)

With the patch it takes :
Generating Visual Studio projects took 4722ms

Without :
Generating Visual Studio projects took 3285ms

We can probably make it almost as fast as before by only looking and converting SK defines but that's uglier than my proposal.

Open to suggestions, right now hacking in Visual is broken because a lot of solutions do not load due to this.




also see https://codereview.chromium.org/2794893003/ as a potential fix for the visualstudio problem
Project Member

Comment 24 by bugdroid1@chromium.org, Apr 3 2017

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

commit 32fda6433bcc281eb4794b8715cf44edb3dd1ce2
Author: michaeln <michaeln@chromium.org>
Date: Mon Apr 03 23:05:50 2017

Fix visual studio project files around SK_FREETYPE_MINIMUM_RUNTIME_VERSION

Replaced use of bit shifting operators with
multiplication because visual studio fails to open projects using
the '<<' operator in preprocessor definitions.

BUG= 700926 

Review-Url: https://codereview.chromium.org/2794893003
Cr-Commit-Position: refs/heads/master@{#461572}

[modify] https://crrev.com/32fda6433bcc281eb4794b8715cf44edb3dd1ce2/skia/BUILD.gn

Sign in to add a comment