[PDF] Fix font embedding restrictions.

[PDF] Fix font embedding restrictions.

Stop using restricted font outlines and honor don't subset restriction.

Committed: http://code.google.com/p/skia/source/detail?r=12600

[vandebo (ex-Chrome), 2013-12-06 05:52:41]

https://codereview.chromium.org/107863002/diff/1/src/pdf/SkPDFDocument.cpp
File src/pdf/SkPDFDocument.cpp (right):

https://codereview.chromium.org/107863002/diff/1/src/pdf/SkPDFDocument.cpp#ne...
src/pdf/SkPDFDocument.cpp:261: // Deprecated.
Deprecated just long enough to make the two sided change.

[reed1, 2013-12-06 14:22:10]

https://codereview.chromium.org/107863002/diff/1/include/core/SkAdvancedTypef...
File include/core/SkAdvancedTypefaceMetrics.h (right):

https://codereview.chromium.org/107863002/diff/1/include/core/SkAdvancedTypef...
include/core/SkAdvancedTypefaceMetrics.h:45: kEmpty_Flag          = 0x0,  //!<No
flags set
General skia convention is for the suffix to match (modulo plurals sometimes)
the enum name.

e.g.

kMultiMaster_FontFlag

https://codereview.chromium.org/107863002/diff/1/include/core/SkAdvancedTypef...
include/core/SkAdvancedTypefaceMetrics.h:58: kFixedPitch_Style  = 0x00001,
Same comment about enum / suffix matching

[vandebo (ex-Chrome), 2013-12-09 17:56:05]

https://codereview.chromium.org/107863002/diff/1/include/core/SkAdvancedTypef...
File include/core/SkAdvancedTypefaceMetrics.h (right):

https://codereview.chromium.org/107863002/diff/1/include/core/SkAdvancedTypef...
include/core/SkAdvancedTypefaceMetrics.h:45: kEmpty_Flag          = 0x0,  //!<No
flags set
On 2013/12/06 14:22:10, reed1 wrote:
> General skia convention is for the suffix to match (modulo plurals sometimes)
> the enum name.
> 
> e.g.
> 
> kMultiMaster_FontFlag

Done.

https://codereview.chromium.org/107863002/diff/1/include/core/SkAdvancedTypef...
include/core/SkAdvancedTypefaceMetrics.h:58: kFixedPitch_Style  = 0x00001,
It looks like fixing this wouldn't be too bad, but I'll do it in a different CL.

[reed1, 2013-12-09 17:59:25]

lgtm -- handing off to bungeman

[bungeman-skia, 2013-12-09 18:32:59]

https://codereview.chromium.org/107863002/diff/20001/src/core/SkTypeface.cpp
File src/core/SkTypeface.cpp (right):

https://codereview.chromium.org/107863002/diff/20001/src/core/SkTypeface.cpp#...
src/core/SkTypeface.cpp:266: if (result && result->fType ==
SkAdvancedTypefaceMetrics::kTrueType_Font) {
It seems you want to test for 'empty flags' here? Otherwise the extra code in
FreeType / GDI is going to just be recomputed here 99.99% of the time.

https://codereview.chromium.org/107863002/diff/20001/src/core/SkTypeface.cpp#...
src/core/SkTypeface.cpp:270: if (os2table.version.v4.fsType.field.Restricted ||
It looks like all of the fields you're looking for here exist in v2. Any reason
to use v4? (Note that there is now a v5, I just haven't added it yet). Of
course, all of this works with any version, so it isn't a big deal.

[vandebo (ex-Chrome), 2013-12-09 18:42:58]

https://codereview.chromium.org/107863002/diff/20001/src/core/SkTypeface.cpp
File src/core/SkTypeface.cpp (right):

https://codereview.chromium.org/107863002/diff/20001/src/core/SkTypeface.cpp#...
src/core/SkTypeface.cpp:266: if (result && result->fType ==
SkAdvancedTypefaceMetrics::kTrueType_Font) {
On 2013/12/09 18:32:59, bungeman1 wrote:
> It seems you want to test for 'empty flags' here? Otherwise the extra code in
> FreeType / GDI is going to just be recomputed here 99.99% of the time.

Windows otm interface has a flag for not embeddable, but not for not
subsettable, so I can't just test for empty flags.  I considered removing the
per platform implementations, but I figured it would be safer to check for
licensing restrictions in all possible ways.  I could possibly by an argument
for removing the freetype implementation, but it doesn't seem like a lot of CPU
overhead.  What do you think?

https://codereview.chromium.org/107863002/diff/20001/src/core/SkTypeface.cpp#...
src/core/SkTypeface.cpp:270: if (os2table.version.v4.fsType.field.Restricted ||
On 2013/12/09 18:32:59, bungeman1 wrote:
> It looks like all of the fields you're looking for here exist in v2. Any
reason
> to use v4? (Note that there is now a v5, I just haven't added it yet). Of
> course, all of this works with any version, so it isn't a big deal.

Sure, v2 is the first version with all the used flags defined.  Changed to that.
 Since they're not all in V0/VA I used the newest version.

[bungeman-skia, 2013-12-09 18:46:30]

lgtm

https://codereview.chromium.org/107863002/diff/20001/src/core/SkTypeface.cpp
File src/core/SkTypeface.cpp (right):

https://codereview.chromium.org/107863002/diff/20001/src/core/SkTypeface.cpp#...
src/core/SkTypeface.cpp:266: if (result && result->fType ==
SkAdvancedTypefaceMetrics::kTrueType_Font) {
On 2013/12/09 18:42:59, vandebo wrote:
> On 2013/12/09 18:32:59, bungeman1 wrote:
> > It seems you want to test for 'empty flags' here? Otherwise the extra code
in
> > FreeType / GDI is going to just be recomputed here 99.99% of the time.
> 
> Windows otm interface has a flag for not embeddable, but not for not
> subsettable, so I can't just test for empty flags.  I considered removing the
> per platform implementations, but I figured it would be safer to check for
> licensing restrictions in all possible ways.  I could possibly by an argument
> for removing the freetype implementation, but it doesn't seem like a lot of
CPU
> overhead.  What do you think?

We're already doing a ton of work, and just getting a pdf going isn't really all
that time sensitive, so it isn't a biggie.

[commit-bot: I haz the power, 2013-12-10 16:03:28]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/vandebo@chromium.org/107863002/40001

[commit-bot: I haz the power, 2013-12-10 16:16:38]

Retried try job too often on Build-Mac10.7-Clang-x86-Release-Trybot for step(s)
BuildSkiaLib, BuildTests
http://skiabot-master.pogerlabs.com:10117/buildstatus?builder=Build-Mac10.7-C...

[commit-bot: I haz the power, 2013-12-10 16:23:54]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/vandebo@chromium.org/107863002/60001

[commit-bot: I haz the power, 2013-12-10 16:32:13]

Change committed as 12600

[bungeman-skia, 2013-12-10 16:43:50]

https://codereview.chromium.org/107863002/diff/60001/src/ports/SkFontHost_win...
File src/ports/SkFontHost_win.cpp (right):

https://codereview.chromium.org/107863002/diff/60001/src/ports/SkFontHost_win...
src/ports/SkFontHost_win.cpp:1864:
SkAdvancedTypefaceMetrics::kNotEmbeddable_FontFlag)
Looks like there's a missing ';' here.

[vandebo (ex-Chrome), 2013-12-10 16:48:36]

On 2013/12/10 16:43:50, bungeman1 wrote:
>
https://codereview.chromium.org/107863002/diff/60001/src/ports/SkFontHost_win...
> File src/ports/SkFontHost_win.cpp (right):
> 
>
https://codereview.chromium.org/107863002/diff/60001/src/ports/SkFontHost_win...
> src/ports/SkFontHost_win.cpp:1864:
> SkAdvancedTypefaceMetrics::kNotEmbeddable_FontFlag)
> Looks like there's a missing ';' here.

Oops, fixed. No windows compile bot in the CQ?