Add Truetype and Type 1 font embedding support

Add Truetype and Type 1 font embedding support

Sorry this is such a large CL.  It was very exploratory for me to make this
work.

- Add an interface to SkFontHost to retrieve font information and provide NULL implementations on all platforms except Linux.
- Segment large Type 1 fonts into fonts with shared resources with 255 glyphs each.
- Convert the various Type 1 formats to the form PDF wants.
- Update font as we draw text instead of as part of the graphical state.
- Remove built-in font support, we can't really use it.

Other changes I can pull out to a separate CL if you like.

- Add SkTScopedPtr class.
- Fix double free of resources.
- Fix bug in resource unique-ifying code.
- Don't print anything for any empty clip path.
- Fix copy paste error - MiterLimit.
- Fix sign extension bug in SkPDFString
- Fix FlateTest rename that was missed on a previous commit.

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

[reed1, 2011-01-20 12:48:40]

Lets move SkPDFTypefaceInfo into its own header.

[reed1, 2011-01-20 12:53:57]

Do we need SkTScopedPtr and SkAutoTDelete? Can we combine these two classes
somehow? Perhaps we could make our class match the semantics of the new c++0x
std shared_ptr.

[reed1, 2011-01-20 13:53:03]

oops, not shared_ptr. I mean unique_ptr

[Steve VanDeBogart, 2011-01-20 22:25:01]

On 2011/01/20 12:48:40, reed1 wrote:
> Lets move SkPDFTypefaceInfo into its own header.

Done.

On 2011/01/20 12:53:57, reed1 wrote:
> Do we need SkTScopedPtr and SkAutoTDelete? Can we combine these two classes
> somehow? Perhaps we could make our class match the semantics of the new c++0x
> std shared_ptr.

SkTScopedPtr is designed to match the anticipated TR2 implementation of the
scoped_ptr class.

I could combine SkTScopedPtr and SkAutoTDelete, but they are used in very
different ways.  SkAutoTDelete is usually created and then forgotten about, with
another pointer to the object used through out the method.  There are 12 uses of
SkAutoTDelete.  I could pretty easily covert those all to the SkTScopedPtr usage
pattern.

Let me know if you'd like me to make that change and I'll pull it into a
separate CL.

[reed1, 2011-01-20 22:38:58]

Lets move SkTScopedPtr into its own header, just for clarity. We can continue
the exploration skia smart-ptrs outside of this particular CL.

LGTM

[Steve VanDeBogart, 2011-01-20 22:54:54]

On 2011/01/20 22:38:58, reed1 wrote:
> Lets move SkTScopedPtr into its own header, just for clarity. We can continue
> the exploration skia smart-ptrs outside of this particular CL.

Done.

AGL, I would still appreciate your review, especially of PDF and font stuff.

[Steve VanDeBogart, 2011-01-25 00:23:11]

agl, ping.  I see you will be offsite most of this week.  No rush on this CL as
long as it gets on your todo list.

[agl, 2011-01-25 15:37:27]

LGTM

http://codereview.appspot.com/4082042/diff/12001/include/core/SkFontHost.h
File include/core/SkFontHost.h (right):

http://codereview.appspot.com/4082042/diff/12001/include/core/SkFontHost.h#ne...
include/core/SkFontHost.h:2: * Copyright (C) 2011 The Android Open Source
Project
Not important, but we typically don't bump the copyright year.

http://codereview.appspot.com/4082042/diff/12001/include/core/SkTemplates.h
File include/core/SkTemplates.h (right):

http://codereview.appspot.com/4082042/diff/12001/include/core/SkTemplates.h#n...
include/core/SkTemplates.h:2: * Copyright (C) 2011 The Android Open Source
Project
ditto

http://codereview.appspot.com/4082042/diff/12001/include/core/SkTypeface.h
File include/core/SkTypeface.h (right):

http://codereview.appspot.com/4082042/diff/12001/include/core/SkTypeface.h#ne...
include/core/SkTypeface.h:2: * Copyright (C) 2011 The Android Open Source
Project
(won't bother with them all.)

http://codereview.appspot.com/4082042/diff/12001/include/pdf/SkPDFFont.h
File include/pdf/SkPDFFont.h (right):

http://codereview.appspot.com/4082042/diff/12001/include/pdf/SkPDFFont.h#newc...
include/pdf/SkPDFFont.h:101: // A fGlyphID of 0  with no fFont always matches.
s/  / /

http://codereview.appspot.com/4082042/diff/12001/src/pdf/SkPDFFont.cpp
File src/pdf/SkPDFFont.cpp (right):

http://codereview.appspot.com/4082042/diff/12001/src/pdf/SkPDFFont.cpp#newcode34
src/pdf/SkPDFFont.cpp:34: if (buf[0] != 0x80 || buf[1] != sectionType)
Needs a comment and it looks like it should check that the input is >= 6 bytes
first.

http://codereview.appspot.com/4082042/diff/12001/src/pdf/SkPDFFont.cpp#newcode61
src/pdf/SkPDFFont.cpp:61: const char* end = src + size;
Needs a function comment. Also, passing a size suggests that the string might
not be NUL terminated. In which case, strstr would be bad. If it is NUL
terminated, a comment to that effect would be good.

http://codereview.appspot.com/4082042/diff/12001/src/pdf/SkPDFFont.cpp#newcode68
src/pdf/SkPDFFont.cpp:68: dataPos < end)
If the input is NUL terminated then the "dataPos < end" is superfluous.

http://codereview.appspot.com/4082042/diff/12001/src/pdf/SkPDFFont.cpp#newcode77
src/pdf/SkPDFFont.cpp:77: if (*trailerPos == '\n' || *trailerPos == '\r' ||
*trailerPos == ' ') {
since trailerPos = strstr(dataPos, "cleartomark") then *trailerPos = 'c'. So I
think this loop will always return false.

http://codereview.appspot.com/4082042/diff/12001/src/pdf/SkPDFFont.cpp#newcod...
src/pdf/SkPDFFont.cpp:116: // NULL terminated so that we can use strstr() to
search it. So make as few
s/NULL/NUL/

http://codereview.appspot.com/4082042/diff/12001/src/pdf/SkPDFFont.cpp#newcod...
src/pdf/SkPDFFont.cpp:117: // copies as convenient.
The last sentence is unclear to me. s/convenient/possible/?

http://codereview.appspot.com/4082042/diff/12001/src/pdf/SkPDFFont.cpp#newcod...
src/pdf/SkPDFFont.cpp:131: while (srcLen - read > 0) {
while (read < srcLen) ?

http://codereview.appspot.com/4082042/diff/12001/src/pdf/SkPDFFont.cpp#newcod...
src/pdf/SkPDFFont.cpp:132: got = srcStream->read((void
*)staticStream->getAtPos(),
got could be declared in this block.

http://codereview.appspot.com/4082042/diff/12001/src/pdf/SkPDFFont.cpp#newcod...
src/pdf/SkPDFFont.cpp:182: char curNibble = hexToBin(*hexData);
You take some pains to support hex strings with bad characters and odd lengths.
Couldn't you ban them in the parsing function?

for (...; hexData += 2) {
  v = hexToBin(hexData[0]) << 8 | hexToBin(hexData[1]);
  ...
}

[Steve VanDeBogart, 2011-01-25 21:00:10]

Thanks for the review.

http://codereview.appspot.com/4082042/diff/12001/include/core/SkFontHost.h
File include/core/SkFontHost.h (right):

http://codereview.appspot.com/4082042/diff/12001/include/core/SkFontHost.h#ne...
include/core/SkFontHost.h:2: * Copyright (C) 2011 The Android Open Source
Project
On 2011/01/25 15:37:27, agl wrote:
> Not important, but we typically don't bump the copyright year.

I removed all gratuitous copyright changes; left in the ones where there wasn't
a copyright before and the pdf file ones.

http://codereview.appspot.com/4082042/diff/12001/src/pdf/SkPDFFont.cpp
File src/pdf/SkPDFFont.cpp (right):

http://codereview.appspot.com/4082042/diff/12001/src/pdf/SkPDFFont.cpp#newcode34
src/pdf/SkPDFFont.cpp:34: if (buf[0] != 0x80 || buf[1] != sectionType)
On 2011/01/25 15:37:27, agl wrote:
> Needs a comment and it looks like it should check that the input is >= 6 bytes
> first.

Done.

http://codereview.appspot.com/4082042/diff/12001/src/pdf/SkPDFFont.cpp#newcode61
src/pdf/SkPDFFont.cpp:61: const char* end = src + size;
On 2011/01/25 15:37:27, agl wrote:
> Needs a function comment. Also, passing a size suggests that the string might
> not be NUL terminated. In which case, strstr would be bad. If it is NUL
> terminated, a comment to that effect would be good.

Done.

http://codereview.appspot.com/4082042/diff/12001/src/pdf/SkPDFFont.cpp#newcode77
src/pdf/SkPDFFont.cpp:77: if (*trailerPos == '\n' || *trailerPos == '\r' ||
*trailerPos == ' ') {
On 2011/01/25 15:37:27, agl wrote:
> since trailerPos = strstr(dataPos, "cleartomark") then *trailerPos = 'c'. So I
> think this loop will always return false.

Indeed. I was missing a trailerPos-- in the loop initialization.

http://codereview.appspot.com/4082042/diff/12001/src/pdf/SkPDFFont.cpp#newcod...
src/pdf/SkPDFFont.cpp:116: // NULL terminated so that we can use strstr() to
search it. So make as few
On 2011/01/25 15:37:27, agl wrote:
> s/NULL/NUL/

Done.

http://codereview.appspot.com/4082042/diff/12001/src/pdf/SkPDFFont.cpp#newcod...
src/pdf/SkPDFFont.cpp:117: // copies as convenient.
On 2011/01/25 15:37:27, agl wrote:
> The last sentence is unclear to me. s/convenient/possible/?

Done.

http://codereview.appspot.com/4082042/diff/12001/src/pdf/SkPDFFont.cpp#newcod...
src/pdf/SkPDFFont.cpp:131: while (srcLen - read > 0) {
On 2011/01/25 15:37:27, agl wrote:
> while (read < srcLen) ?

Done.

http://codereview.appspot.com/4082042/diff/12001/src/pdf/SkPDFFont.cpp#newcod...
src/pdf/SkPDFFont.cpp:132: got = srcStream->read((void
*)staticStream->getAtPos(),
On 2011/01/25 15:37:27, agl wrote:
> got could be declared in this block.

Done.

http://codereview.appspot.com/4082042/diff/12001/src/pdf/SkPDFFont.cpp#newcod...
src/pdf/SkPDFFont.cpp:182: char curNibble = hexToBin(*hexData);
On 2011/01/25 15:37:27, agl wrote:
> You take some pains to support hex strings with bad characters and odd
lengths.
> Couldn't you ban them in the parsing function?
> 
> for (...; hexData += 2) {
>   v = hexToBin(hexData[0]) << 8 | hexToBin(hexData[1]);
>   ...
> }

I don't control the input files, which may contain arbitrary whitespace and have
well defined semantics for an odd number of Hex digits.  I could reformat the
hex section when I parse the file, but there are also line length limits so I
would still have to deal with white space.  Leaving as is.

[agl, 2011-01-26 13:51:06]

On Tue, Jan 25, 2011 at 4:00 PM,  <vandebo@chromium.org> wrote:
> Thanks for the review.

LGTM

[agl, 2011-01-26 13:51:08]

On Tue, Jan 25, 2011 at 4:00 PM,  <vandebo@chromium.org> wrote:
> Thanks for the review.

LGTM