[PDF] Fix font embedding restrictions.

[PDF] Fix font embedding restrictions.

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

Resubmit of r12600.

Committed: https://skia.googlesource.com/skia/+/0f9bad01b0e7ad592ffb342dcf1d238b15329be1

[vandebo (ex-Chrome), 2014-06-11 17:14:09]

This was reverted because it caused issues with poppler on Mac, but we don't do
that anymore.

[bungeman-skia, 2014-06-11 17:39:28]

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

https://codereview.chromium.org/334443002/diff/1/src/core/SkTypeface.cpp#newc...
src/core/SkTypeface.cpp:278: if
(this->getTableData(SkEndian_SwapBE32(SkOTTableOS2::TAG), 0,
SkOTTableOS2::TAG is a compile time constant, so SkTEndian_SwapBE32 might be
better here.

[vandebo (ex-Chrome), 2014-06-11 18:46:53]

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

https://codereview.chromium.org/334443002/diff/1/src/core/SkTypeface.cpp#newc...
src/core/SkTypeface.cpp:278: if
(this->getTableData(SkEndian_SwapBE32(SkOTTableOS2::TAG), 0,
On 2014/06/11 17:39:28, bungeman1 wrote:
> SkOTTableOS2::TAG is a compile time constant, so SkTEndian_SwapBE32 might be
> better here.

Done.

[bungeman-skia, 2014-06-11 21:24:45]

So I think the code mostly looks fine, except for there being no "fair use" flag
for the user to state they know what they're doing and take responsibility for
staying within the license they have for their fonts (not that anyone actually
knows what that means).

On the other hand, I think this is the wrong place to start. If we want to
enforce this sort of thing, Blink (and Android) should learn a mode where when
they're printing they try to avoid fonts which ask not to be embedded. With this
CL if a font cannot be found then we will wind up with gibberish, since Blink
and Android are always giving us glyph codes. This will be done silently without
providing any feedback to the user that their PDF probably won't work and might
even print like gibberish even if it does display correctly (depending on the
system print method).

[vandebo (ex-Chrome), 2014-06-11 21:38:30]

On 2014/06/11 21:24:45, bungeman1 wrote:
> So I think the code mostly looks fine, except for there being no "fair use"
flag
> for the user to state they know what they're doing and take responsibility for
> staying within the license they have for their fonts (not that anyone actually
> knows what that means).

I don't think there's a fair use exception - if the font declares it's not
embeddable, then it should not be embedded.  If the the user has a license
beyond that, then the owner should give them a different version of the font
without the bit set.

> On the other hand, I think this is the wrong place to start. If we want to
> enforce this sort of thing, Blink (and Android) should learn a mode where when
> they're printing they try to avoid fonts which ask not to be embedded. With
this
> CL if a font cannot be found then we will wind up with gibberish, since Blink
> and Android are always giving us glyph codes. This will be done silently
without
> providing any feedback to the user that their PDF probably won't work and
might
> even print like gibberish even if it does display correctly (depending on the
> system print method).

Advice from our lawyer is to not embed shapes of non-embeddable fonts.  I'm
happy to connect you if you have particular knowledge or precedent about the
matter.  I'll refrain from mentioned my current and previous thoughts on the
matter.

I looked at it six months ago and again just now - not embeddable fonts are not
encountered very often in Chrome.  Current stat is 0.02% of fonts in print
previews.

For that 0.02% of fonts, I would generally expect it to work on the same system
- where the font is available - but not on machines without the font.

[bungeman-skia, 2014-06-11 22:45:51]

On 2014/06/11 21:38:30, vandebo wrote:
> On 2014/06/11 21:24:45, bungeman1 wrote:
> > So I think the code mostly looks fine, except for there being no "fair use"
> flag
> > for the user to state they know what they're doing and take responsibility
for
> > staying within the license they have for their fonts (not that anyone
actually
> > knows what that means).
> 
> I don't think there's a fair use exception - if the font declares it's not
> embeddable, then it should not be embedded.  If the the user has a license
> beyond that, then the owner should give them a different version of the font
> without the bit set.
> 
> > On the other hand, I think this is the wrong place to start. If we want to
> > enforce this sort of thing, Blink (and Android) should learn a mode where
when
> > they're printing they try to avoid fonts which ask not to be embedded. With
> this
> > CL if a font cannot be found then we will wind up with gibberish, since
Blink
> > and Android are always giving us glyph codes. This will be done silently
> without
> > providing any feedback to the user that their PDF probably won't work and
> might
> > even print like gibberish even if it does display correctly (depending on
the
> > system print method).
> 
> Advice from our lawyer is to not embed shapes of non-embeddable fonts.  I'm
> happy to connect you if you have particular knowledge or precedent about the
> matter.  I'll refrain from mentioned my current and previous thoughts on the
> matter.
> 
> I looked at it six months ago and again just now - not embeddable fonts are
not
> encountered very often in Chrome.  Current stat is 0.02% of fonts in print
> previews.
> 
> For that 0.02% of fonts, I would generally expect it to work on the same
system
> - where the font is available - but not on machines without the font.

I'm not sure why you bring up embedding shapes, as I didn't mention that. My
argument is that you're dropping the fonts silently, and then ending up with
gibberish. I don't think these currently work even on the current system most of
the time since I think we usually end up using the wrong name (not the
PostScript name, this needs to be fixed). Beyond that, the silence on dropping
font data without feedback when the biggest users of this haven't been updated
to avoid such fonts when they know that they're trying to produce a stand-alone
document seems a bit backward. I have a hard time believing that this isn't
going to be a big issue given that all the mac bots failed gms on this change.
At the very least there needs to be some kind of immediate feedback so that the
user knows that the generated PDF probably isn't going to print correctly. Even
better to have Blink/Android re-do the layout with fonts which will work. While
I support your efforts, you're springing a rather nasty surprise on Blink
without any means for them to work around it. My comment was more that Blink (at
least) should get fixed up first (with somewhere for the user to state which
level they're comfortable with) so that the user has some out. In other words, I
have a hard time saying that this looks good when it is an instant Blink bug,
and if we had any real printing tests, they would fail. This will also be a
regression against other browsers, which will print something sane.

However, since you brought up embedding shapes, see the XPS specification at
http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-388.pdf which
states "For fonts with “Restricted license embedding” licensing intent,
producers SHOULD generate a path filled with an image brush referencing an image
of rendered characters and SHOULD include the actual text in the
AutomationProperties.Name attribute of the <Path> element." (S2.18). Maybe our
lawyer should talk to theirs.

[bungeman-skia, 2014-06-11 22:49:24]

On 2014/06/11 22:45:51, bungeman1 wrote:
> On 2014/06/11 21:38:30, vandebo wrote:
> > On 2014/06/11 21:24:45, bungeman1 wrote:
> > > So I think the code mostly looks fine, except for there being no "fair
use"
> > flag
> > > for the user to state they know what they're doing and take responsibility
> for
> > > staying within the license they have for their fonts (not that anyone
> actually
> > > knows what that means).
> > 
> > I don't think there's a fair use exception - if the font declares it's not
> > embeddable, then it should not be embedded.  If the the user has a license
> > beyond that, then the owner should give them a different version of the font
> > without the bit set.
> > 
> > > On the other hand, I think this is the wrong place to start. If we want to
> > > enforce this sort of thing, Blink (and Android) should learn a mode where
> when
> > > they're printing they try to avoid fonts which ask not to be embedded.
With
> > this
> > > CL if a font cannot be found then we will wind up with gibberish, since
> Blink
> > > and Android are always giving us glyph codes. This will be done silently
> > without
> > > providing any feedback to the user that their PDF probably won't work and
> > might
> > > even print like gibberish even if it does display correctly (depending on
> the
> > > system print method).
> > 
> > Advice from our lawyer is to not embed shapes of non-embeddable fonts.  I'm
> > happy to connect you if you have particular knowledge or precedent about the
> > matter.  I'll refrain from mentioned my current and previous thoughts on the
> > matter.
> > 
> > I looked at it six months ago and again just now - not embeddable fonts are
> not
> > encountered very often in Chrome.  Current stat is 0.02% of fonts in print
> > previews.
> > 
> > For that 0.02% of fonts, I would generally expect it to work on the same
> system
> > - where the font is available - but not on machines without the font.
> 
> I'm not sure why you bring up embedding shapes, as I didn't mention that. My
> argument is that you're dropping the fonts silently, and then ending up with
> gibberish. I don't think these currently work even on the current system most
of
> the time since I think we usually end up using the wrong name (not the
> PostScript name, this needs to be fixed). Beyond that, the silence on dropping
> font data without feedback when the biggest users of this haven't been updated
> to avoid such fonts when they know that they're trying to produce a
stand-alone
> document seems a bit backward. I have a hard time believing that this isn't
> going to be a big issue given that all the mac bots failed gms on this change.
> At the very least there needs to be some kind of immediate feedback so that
the
> user knows that the generated PDF probably isn't going to print correctly.
Even
> better to have Blink/Android re-do the layout with fonts which will work.
While
> I support your efforts, you're springing a rather nasty surprise on Blink
> without any means for them to work around it. My comment was more that Blink
(at
> least) should get fixed up first (with somewhere for the user to state which
> level they're comfortable with) so that the user has some out. In other words,
I
> have a hard time saying that this looks good when it is an instant Blink bug,
> and if we had any real printing tests, they would fail. This will also be a
> regression against other browsers, which will print something sane.
> 
> However, since you brought up embedding shapes, see the XPS specification at
> http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-388.pdf
which
> states "For fonts with “Restricted license embedding” licensing intent,
> producers SHOULD generate a path filled with an image brush referencing an
image
> of rendered characters and SHOULD include the actual text in the
> AutomationProperties.Name attribute of the <Path> element." (S2.18). Maybe our
> lawyer should talk to theirs.

Just wanted to clarify, that last bit in XPS is stating that you should just
rasterize to a bitmap and embed the bitmap. It avoids the shapes. I'm assuming
this shouldn't be too difficult to do in the PDF back-end?

[vandebo (ex-Chrome), 2014-06-13 23:52:02]

On 2014/06/11 22:49:24, bungeman1 wrote:
> On 2014/06/11 22:45:51, bungeman1 wrote:
> > On 2014/06/11 21:38:30, vandebo wrote:
> > > On 2014/06/11 21:24:45, bungeman1 wrote:
> > > > So I think the code mostly looks fine, except for there being no "fair
> use"
> > > flag
> > > > for the user to state they know what they're doing and take
responsibility
> > for
> > > > staying within the license they have for their fonts (not that anyone
> > actually
> > > > knows what that means).
> > > 
> > > I don't think there's a fair use exception - if the font declares it's not
> > > embeddable, then it should not be embedded.  If the the user has a license
> > > beyond that, then the owner should give them a different version of the
font
> > > without the bit set.
> > > 
> > > > On the other hand, I think this is the wrong place to start. If we want
to
> > > > enforce this sort of thing, Blink (and Android) should learn a mode
where
> > when
> > > > they're printing they try to avoid fonts which ask not to be embedded.
> With
> > > this
> > > > CL if a font cannot be found then we will wind up with gibberish, since
> > Blink
> > > > and Android are always giving us glyph codes. This will be done silently
> > > without
> > > > providing any feedback to the user that their PDF probably won't work
and
> > > might
> > > > even print like gibberish even if it does display correctly (depending
on
> > the
> > > > system print method).
> > > 
> > > Advice from our lawyer is to not embed shapes of non-embeddable fonts. 
I'm
> > > happy to connect you if you have particular knowledge or precedent about
the
> > > matter.  I'll refrain from mentioned my current and previous thoughts on
the
> > > matter.
> > > 
> > > I looked at it six months ago and again just now - not embeddable fonts
are
> > not
> > > encountered very often in Chrome.  Current stat is 0.02% of fonts in print
> > > previews.
> > > 
> > > For that 0.02% of fonts, I would generally expect it to work on the same
> > system
> > > - where the font is available - but not on machines without the font.
> > 

> > I'm not sure why you bring up embedding shapes, as I didn't mention that. My

Aside from fixing the subsetting bit, that's all that this CL should change. If
the
code currently encounters an embed restricted font, it uses the paths from the
font
to synthesize a type 3 font, thus embedding the shapes.

> > argument is that you're dropping the fonts silently, and then ending up with
> > gibberish. I don't think these currently work even on the current system
most
> of
> > the time since I think we usually end up using the wrong name (not the
> > PostScript name, this needs to be fixed). Beyond that, the silence on
dropping
> > font data without feedback when the biggest users of this haven't been
updated
> > to avoid such fonts when they know that they're trying to produce a
> stand-alone

We could inform consumers and have them draw with a fallback font instead, but
it seems better to me to correctly encode to font information (it's not clear to
me if you think there's currently a bug here). A system with the font will
display
it as intended otherwise the PDF renderer will choose a fallback font.

Given that we currently only have a glyph encoding, then having the consumer
fall
back to embeddable fonts when generating PDFs seems ok, though I think someone
using a non-embeddable font is likely to be aware of it and will treat output
using it with care.  That category of people is likely to be very particular
about
font choice and probably would consider a font substitution a bug. 

> > document seems a bit backward. I have a hard time believing that this isn't
> > going to be a big issue given that all the mac bots failed gms on this
change.

That was a bug, which I've fixed in patch set 3.  As I said, it's currently
0.02% of
font usage while printing.

> > At the very least there needs to be some kind of immediate feedback so that
> the
> > user knows that the generated PDF probably isn't going to print correctly.
> Even
> > better to have Blink/Android re-do the layout with fonts which will work.
> While
> > I support your efforts, you're springing a rather nasty surprise on Blink
> > without any means for them to work around it. My comment was more that Blink
> (at
> > least) should get fixed up first (with somewhere for the user to state which
> > level they're comfortable with) so that the user has some out. In other
words,
> I
> > have a hard time saying that this looks good when it is an instant Blink
bug,
> > and if we had any real printing tests, they would fail. This will also be a
> > regression against other browsers, which will print something sane.
> > 
> > However, since you brought up embedding shapes, see the XPS specification at
> > http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-388.pdf
> which
> > states "For fonts with “Restricted license embedding” licensing intent,
> > producers SHOULD generate a path filled with an image brush referencing an
> image
> > of rendered characters and SHOULD include the actual text in the
> > AutomationProperties.Name attribute of the <Path> element." (S2.18). Maybe
our
> > lawyer should talk to theirs.

I think that's consistent with not doing what the current code does.

> Just wanted to clarify, that last bit in XPS is stating that you should just
> rasterize to a bitmap and embed the bitmap. It avoids the shapes. I'm assuming
> this shouldn't be too difficult to do in the PDF back-end?

Yes, we could generate images and embed them, but that isn't necessarily
desirable
in some cases. Consider the use case of someone who intends to use a not
embeddable
font within a single system, or shop with the same fonts installed everywhere.

I did consider rasterizing not embeddable fonts, but there is an engineering
cost
as well as a file size cost.  Considering the frequency of this case, my best
judgement at the time was to let the PDF renderer choose a fallback font.
Though as you point out, the glyph encoding makes this unlikely to have a
reasonable result. I also did not have the information that things don't work
correctly in direct write mode.

What's your suggestion for the best path forward?

[vandebo (ex-Chrome), 2014-06-17 15:31:32]

On 2014/06/13 23:52:02, vandebo wrote:
> On 2014/06/11 22:49:24, bungeman1 wrote:
> > On 2014/06/11 22:45:51, bungeman1 wrote:
> > > On 2014/06/11 21:38:30, vandebo wrote:
> > > > On 2014/06/11 21:24:45, bungeman1 wrote:
> > > > > So I think the code mostly looks fine, except for there being no "fair
> > use"
> > > > flag
> > > > > for the user to state they know what they're doing and take
> responsibility
> > > for
> > > > > staying within the license they have for their fonts (not that anyone
> > > actually
> > > > > knows what that means).
> > > > 
> > > > I don't think there's a fair use exception - if the font declares it's
not
> > > > embeddable, then it should not be embedded.  If the the user has a
license
> > > > beyond that, then the owner should give them a different version of the
> font
> > > > without the bit set.
> > > > 
> > > > > On the other hand, I think this is the wrong place to start. If we
want
> to
> > > > > enforce this sort of thing, Blink (and Android) should learn a mode
> where
> > > when
> > > > > they're printing they try to avoid fonts which ask not to be embedded.
> > With
> > > > this
> > > > > CL if a font cannot be found then we will wind up with gibberish,
since
> > > Blink
> > > > > and Android are always giving us glyph codes. This will be done
silently
> > > > without
> > > > > providing any feedback to the user that their PDF probably won't work
> and
> > > > might
> > > > > even print like gibberish even if it does display correctly (depending
> on
> > > the
> > > > > system print method).
> > > > 
> > > > Advice from our lawyer is to not embed shapes of non-embeddable fonts. 
> I'm
> > > > happy to connect you if you have particular knowledge or precedent about
> the
> > > > matter.  I'll refrain from mentioned my current and previous thoughts on
> the
> > > > matter.
> > > > 
> > > > I looked at it six months ago and again just now - not embeddable fonts
> are
> > > not
> > > > encountered very often in Chrome.  Current stat is 0.02% of fonts in
print
> > > > previews.
> > > > 
> > > > For that 0.02% of fonts, I would generally expect it to work on the same
> > > system
> > > > - where the font is available - but not on machines without the font.
> > > 
> 
> > > I'm not sure why you bring up embedding shapes, as I didn't mention that.
My
> 
> Aside from fixing the subsetting bit, that's all that this CL should change.
If
> the
> code currently encounters an embed restricted font, it uses the paths from the
> font
> to synthesize a type 3 font, thus embedding the shapes.
> 
> > > argument is that you're dropping the fonts silently, and then ending up
with
> > > gibberish. I don't think these currently work even on the current system
> most
> > of
> > > the time since I think we usually end up using the wrong name (not the
> > > PostScript name, this needs to be fixed). Beyond that, the silence on
> dropping
> > > font data without feedback when the biggest users of this haven't been
> updated
> > > to avoid such fonts when they know that they're trying to produce a
> > stand-alone
> 
> We could inform consumers and have them draw with a fallback font instead, but
> it seems better to me to correctly encode to font information (it's not clear
to
> me if you think there's currently a bug here). A system with the font will
> display
> it as intended otherwise the PDF renderer will choose a fallback font.
> 
> Given that we currently only have a glyph encoding, then having the consumer
> fall
> back to embeddable fonts when generating PDFs seems ok, though I think someone
> using a non-embeddable font is likely to be aware of it and will treat output
> using it with care.  That category of people is likely to be very particular
> about
> font choice and probably would consider a font substitution a bug. 
> 
> > > document seems a bit backward. I have a hard time believing that this
isn't
> > > going to be a big issue given that all the mac bots failed gms on this
> change.
> 
> That was a bug, which I've fixed in patch set 3.  As I said, it's currently
> 0.02% of
> font usage while printing.
> 
> > > At the very least there needs to be some kind of immediate feedback so
that
> > the
> > > user knows that the generated PDF probably isn't going to print correctly.
> > Even
> > > better to have Blink/Android re-do the layout with fonts which will work.
> > While
> > > I support your efforts, you're springing a rather nasty surprise on Blink
> > > without any means for them to work around it. My comment was more that
Blink
> > (at
> > > least) should get fixed up first (with somewhere for the user to state
which
> > > level they're comfortable with) so that the user has some out. In other
> words,
> > I
> > > have a hard time saying that this looks good when it is an instant Blink
> bug,
> > > and if we had any real printing tests, they would fail. This will also be
a
> > > regression against other browsers, which will print something sane.
> > > 
> > > However, since you brought up embedding shapes, see the XPS specification
at
> > > http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-388.pdf
> > which
> > > states "For fonts with “Restricted license embedding” licensing intent,
> > > producers SHOULD generate a path filled with an image brush referencing an
> > image
> > > of rendered characters and SHOULD include the actual text in the
> > > AutomationProperties.Name attribute of the <Path> element." (S2.18). Maybe
> our
> > > lawyer should talk to theirs.
> 
> I think that's consistent with not doing what the current code does.
> 
> > Just wanted to clarify, that last bit in XPS is stating that you should just
> > rasterize to a bitmap and embed the bitmap. It avoids the shapes. I'm
assuming
> > this shouldn't be too difficult to do in the PDF back-end?
> 
> Yes, we could generate images and embed them, but that isn't necessarily
> desirable
> in some cases. Consider the use case of someone who intends to use a not
> embeddable
> font within a single system, or shop with the same fonts installed everywhere.
> 
> I did consider rasterizing not embeddable fonts, but there is an engineering
> cost
> as well as a file size cost.  Considering the frequency of this case, my best
> judgement at the time was to let the PDF renderer choose a fallback font.
> Though as you point out, the glyph encoding makes this unlikely to have a
> reasonable result. I also did not have the information that things don't work
> correctly in direct write mode.
> 
> What's your suggestion for the best path forward?

ping - any thoughts on best path forward?

[bungeman-skia, 2014-06-18 17:28:14]

On 2014/06/17 15:31:32, vandebo wrote:
> ping - any thoughts on best path forward?

Now that this has some chance of doing the right thing with DirectWrite, eh,
lgtm. (Though the change for that came rather close to this change, this might
need to be rebased.) I wish we had some actual tests for this.

[vandebo (ex-Chrome), 2014-06-18 22:30:39]

On 2014/06/18 17:28:14, bungeman1 wrote:
> On 2014/06/17 15:31:32, vandebo wrote:
> > ping - any thoughts on best path forward?
> 
> Now that this has some chance of doing the right thing with DirectWrite, eh,
> lgtm. (Though the change for that came rather close to this change, this might
> need to be rebased.) I wish we had some actual tests for this.

Testing would be nice, yea, but it would require some framework for using
specific fonts.

[vandebo (ex-Chrome), 2014-06-18 22:30:44]

The CQ bit was checked by vandebo@chromium.org

[commit-bot: I haz the power, 2014-06-18 22:31:05]

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

[commit-bot: I haz the power, 2014-06-18 22:31:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-18 22:31:11]

Presubmit check for 334443002-60001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Since the CL is editing public API, you must have an LGTM from one of:
('reed@chromium.org', 'reed@google.com', 'bsalomon@chromium.org',
'bsalomon@google.com')

Presubmit checks took 1.4s to calculate.

[vandebo (ex-Chrome), 2014-06-18 22:34:05]

TBRing mike for API changes since he already approved them here:
https://codereview.chromium.org/107863002

[vandebo (ex-Chrome), 2014-06-18 22:34:12]

The CQ bit was checked by vandebo@chromium.org

[commit-bot: I haz the power, 2014-06-18 22:35:05]

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

[commit-bot: I haz the power, 2014-06-18 22:35:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-18 22:35:10]

Presubmit check for 334443002-60001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Since the CL is editing public API, you must have an LGTM from one of:
('reed@chromium.org', 'reed@google.com', 'bsalomon@chromium.org',
'bsalomon@google.com')

[vandebo (ex-Chrome), 2014-06-18 22:36:49]

Mike - that didn't work, can you stamp this please.

[reed1, 2014-06-19 13:03:09]

lgtm

[vandebo (ex-Chrome), 2014-06-19 15:54:26]

The CQ bit was checked by vandebo@chromium.org

[commit-bot: I haz the power, 2014-06-19 15:54:33]

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

[commit-bot: I haz the power, 2014-06-19 18:05:47]

Change committed as 0f9bad01b0e7ad592ffb342dcf1d238b15329be1