[PDF] Add support for Clear, Src, Dst, DstOver xfermodes.

[PDF] Add support for Clear, Src, Dst, DstOver xfermodes.

This uses the refactoring in http://codereview.appspot.com/4459041/ to add support for additional xfer modes.  Calling setupContentEntry may affect previous content entries (removing, reordering, or modifying their clip) and indicates to the caller if it should draw the new item or not.

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

[Steve VanDeBogart, 2011-05-05 18:40:39]

I also have support for Src/Dst-In/Out in the pipeline.

[reed1, 2011-05-05 18:48:32]

defers to chris

[Chris Guillory, 2011-05-06 19:18:15]

On 2011/05/05 18:48:32, reed1 wrote:
> defers to chris
Looking. Mike, mind taking a browse over the new xfermode logic.

[reed1, 2011-05-06 20:57:52]

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

http://codereview.appspot.com/4464043/diff/1/src/pdf/SkPDFDevice.cpp#newcode579
src/pdf/SkPDFDevice.cpp:579: // though we won't draw anything.
Not sure what the comment means. In the raster and gpu devices, count==0 is
always treated as do absolutely nothing, like an empty-clip.

http://codereview.appspot.com/4464043/diff/1/src/pdf/SkPDFDevice.cpp#newcode588
src/pdf/SkPDFDevice.cpp:588: SkScalar strokeWidth = paint.getStrokeWidth();
you want SkScalarHalf(strokeWidth) or strokeWidth / 2 here

http://codereview.appspot.com/4464043/diff/1/src/pdf/SkPDFDevice.cpp#newcode589
src/pdf/SkPDFDevice.cpp:589: SkScalar halfStroke = strokeWidth * SK_ScalarHalf;
How do you handle hairlines / hairpoints? In those case, skia-raster will just
draw a pixel, however they are specified as strokeWidth==0. (so you'll be
creating empty rects below.)

[Chris Guillory, 2011-05-06 21:09:20]

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

http://codereview.appspot.com/4464043/diff/1/include/pdf/SkPDFDevice.h#newcod...
include/pdf/SkPDFDevice.h:161: // If the paint or clip is such that we shouldn't
draw anything, this
Nit: Since this comment is referring to both functions can you rephrase: "these
return false and do not"

[Steve VanDeBogart, 2011-05-07 01:49:16]

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

http://codereview.appspot.com/4464043/diff/1/include/pdf/SkPDFDevice.h#newcod...
include/pdf/SkPDFDevice.h:161: // If the paint or clip is such that we shouldn't
draw anything, this
On 2011/05/06 21:09:20, Chris Guillory wrote:
> Nit: Since this comment is referring to both functions can you rephrase:
"these
> return false and do not"

Done.

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

http://codereview.appspot.com/4464043/diff/1/src/pdf/SkPDFDevice.cpp#newcode579
src/pdf/SkPDFDevice.cpp:579: // though we won't draw anything.
On 2011/05/06 20:57:52, reed1 wrote:
> Not sure what the comment means. In the raster and gpu devices, count==0 is
> always treated as do absolutely nothing, like an empty-clip.

Hmm, indeed.  Fixed.

http://codereview.appspot.com/4464043/diff/1/src/pdf/SkPDFDevice.cpp#newcode588
src/pdf/SkPDFDevice.cpp:588: SkScalar strokeWidth = paint.getStrokeWidth();
On 2011/05/06 20:57:52, reed1 wrote:
> you want SkScalarHalf(strokeWidth) or strokeWidth / 2 here

Done.

http://codereview.appspot.com/4464043/diff/1/src/pdf/SkPDFDevice.cpp#newcode589
src/pdf/SkPDFDevice.cpp:589: SkScalar halfStroke = strokeWidth * SK_ScalarHalf;
On 2011/05/06 20:57:52, reed1 wrote:
> How do you handle hairlines / hairpoints? In those case, skia-raster will just
> draw a pixel, however they are specified as strokeWidth==0. (so you'll be
> creating empty rects below.)

Indeed, this was broken.  PDF does draw a round cap point with stroke width 0,
so I added a special case to the special case... :-/

[Chris Guillory, 2011-05-07 02:42:08]

LGTM to your explanation. Some minor nits.

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

http://codereview.appspot.com/4464043/diff/9001/src/pdf/SkPDFDevice.cpp#newco...
src/pdf/SkPDFDevice.cpp:361: ContentEntry* entry = entries->get();
Nit: parentEntry.get();

http://codereview.appspot.com/4464043/diff/9001/src/pdf/SkPDFDevice.cpp#newco...
src/pdf/SkPDFDevice.cpp:378: entry = entry->fNext.get();
Nit: parentEntry.get()

[Steve VanDeBogart, 2011-05-07 18:49:36]

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

http://codereview.appspot.com/4464043/diff/9001/src/pdf/SkPDFDevice.cpp#newco...
src/pdf/SkPDFDevice.cpp:361: ContentEntry* entry = entries->get();
On 2011/05/07 02:42:08, Chris Guillory wrote:
> Nit: parentEntry.get();

I simplified even further, getting rid of entry and renameing parentEntry
(really entryPointer) to entry.