First pieces of SkPDFDevice. Supports:

First pieces of SkPDFDevice. Supports:

Matrix transforms.
Rendering bitmaps.
Basic paint parameters.
Rendering rectangles, points, lines, polygons.
Render a paint to the page.

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

[agl, 2010-10-19 21:39:57]

LGTM

http://codereview.appspot.com/2584041/diff/1/include/pdf/SkPDFDevice.h
File include/pdf/SkPDFDevice.h (right):

http://codereview.appspot.com/2584041/diff/1/include/pdf/SkPDFDevice.h#newcode71
include/pdf/SkPDFDevice.h:71: virtual void drawPoints(const SkDraw&,
SkCanvas::PointMode mode, size_t count,
80 chars

http://codereview.appspot.com/2584041/diff/1/include/pdf/SkPDFDevice.h#newcode89
include/pdf/SkPDFDevice.h:89: virtual void drawVertices(const SkDraw&,
SkCanvas::VertexMode, int vertexCount,
80 chars

http://codereview.appspot.com/2584041/diff/1/src/pdf/SkPDFDevice.cpp
File src/pdf/SkPDFDevice.cpp (right):

http://codereview.appspot.com/2584041/diff/1/src/pdf/SkPDFDevice.cpp#newcode53
src/pdf/SkPDFDevice.cpp:53: return SkString("S\n");
I note that only one of these branches appends a newline.

http://codereview.appspot.com/2584041/diff/1/src/pdf/SkPDFDevice.cpp#newcode127
src/pdf/SkPDFDevice.cpp:127: strokePath();
Just a thought, if it's a polygon, does the caller expect us to close the path?

http://codereview.appspot.com/2584041/diff/1/src/pdf/SkPDFDevice.cpp#newcode131
src/pdf/SkPDFDevice.cpp:131: for (size_t i = 1; i < count; i += 2) {
I wonder if it wouldn't be smoother to read if the for loop went from 0 ...
count/2 and the indexes has "* 2" below. Your call.

http://codereview.appspot.com/2584041/diff/1/src/pdf/SkPDFDevice.cpp#newcode331
src/pdf/SkPDFDevice.cpp:331: } while(0)
indent two more spaces

http://codereview.appspot.com/2584041/diff/1/src/pdf/SkPDFGraphicState.cpp
File src/pdf/SkPDFGraphicState.cpp (right):

http://codereview.appspot.com/2584041/diff/1/src/pdf/SkPDFGraphicState.cpp#ne...
src/pdf/SkPDFGraphicState.cpp:40: SkTDArray<SkPDFGraphicState::GSCanonicalEntry>
static initializers make DaveM cry. Can you not make them pointers and init on
demand? Skia probably doesn't have support for pthread_once and friends, so you
might end up racing to create them, but that's fixable in the future.

http://codereview.appspot.com/2584041/diff/1/src/pdf/SkPDFImage.cpp
File src/pdf/SkPDFImage.cpp (right):

http://codereview.appspot.com/2584041/diff/1/src/pdf/SkPDFImage.cpp#newcode136
src/pdf/SkPDFImage.cpp:136: // Should this un-premultiply?
For PDF? Probably. You should check.

[Steve VanDeBogart, 2010-10-20 01:03:09]

http://codereview.appspot.com/2584041/diff/1/include/pdf/SkPDFDevice.h
File include/pdf/SkPDFDevice.h (right):

http://codereview.appspot.com/2584041/diff/1/include/pdf/SkPDFDevice.h#newcode71
include/pdf/SkPDFDevice.h:71: virtual void drawPoints(const SkDraw&,
SkCanvas::PointMode mode, size_t count,
On 2010/10/19 21:39:57, agl wrote:
> 80 chars

Done.

http://codereview.appspot.com/2584041/diff/1/include/pdf/SkPDFDevice.h#newcode89
include/pdf/SkPDFDevice.h:89: virtual void drawVertices(const SkDraw&,
SkCanvas::VertexMode, int vertexCount,
On 2010/10/19 21:39:57, agl wrote:
> 80 chars

Done.

http://codereview.appspot.com/2584041/diff/1/src/pdf/SkPDFDevice.cpp
File src/pdf/SkPDFDevice.cpp (right):

http://codereview.appspot.com/2584041/diff/1/src/pdf/SkPDFDevice.cpp#newcode53
src/pdf/SkPDFDevice.cpp:53: return SkString("S\n");
On 2010/10/19 21:39:57, agl wrote:
> I note that only one of these branches appends a newline.

Only (and all) branches that return append a newline

http://codereview.appspot.com/2584041/diff/1/src/pdf/SkPDFDevice.cpp#newcode127
src/pdf/SkPDFDevice.cpp:127: strokePath();
On 2010/10/19 21:39:57, agl wrote:
> Just a thought, if it's a polygon, does the caller expect us to close the
path?

I wondered the same thing.  The raster implementation does not.

http://codereview.appspot.com/2584041/diff/1/src/pdf/SkPDFDevice.cpp#newcode131
src/pdf/SkPDFDevice.cpp:131: for (size_t i = 1; i < count; i += 2) {
On 2010/10/19 21:39:57, agl wrote:
> I wonder if it wouldn't be smoother to read if the for loop went from 0 ...
> count/2 and the indexes has "* 2" below. Your call.

Done.

http://codereview.appspot.com/2584041/diff/1/src/pdf/SkPDFDevice.cpp#newcode331
src/pdf/SkPDFDevice.cpp:331: } while(0)
On 2010/10/19 21:39:57, agl wrote:
> indent two more spaces

Done.

http://codereview.appspot.com/2584041/diff/1/src/pdf/SkPDFGraphicState.cpp
File src/pdf/SkPDFGraphicState.cpp (right):

http://codereview.appspot.com/2584041/diff/1/src/pdf/SkPDFGraphicState.cpp#ne...
src/pdf/SkPDFGraphicState.cpp:40: SkTDArray<SkPDFGraphicState::GSCanonicalEntry>
On 2010/10/19 21:39:57, agl wrote:
> static initializers make DaveM cry. Can you not make them pointers and init on
> demand? Skia probably doesn't have support for pthread_once and friends, so
you
> might end up racing to create them, but that's fixable in the future.

I changed these to static methods with static varibles, which seems to do the
right thing with recent gcc versions, (see --fno-threadsafe-statics) but is not
thread safe on Windows.  There doesn't seem to be a good solution on Windows.

http://codereview.appspot.com/2584041/diff/1/src/pdf/SkPDFImage.cpp
File src/pdf/SkPDFImage.cpp (right):

http://codereview.appspot.com/2584041/diff/1/src/pdf/SkPDFImage.cpp#newcode136
src/pdf/SkPDFImage.cpp:136: // Should this un-premultiply?
On 2010/10/19 21:39:57, agl wrote:
> For PDF? Probably. You should check.

Done.