Generalize the flip origin argument to the PDF device constructor.

Generalize the flip origin argument to the PDF device constructor.

The argument still has a default value that does what most users will want, but provides more flexibility.
Chrome will use this change to support an initial translation of the origin to simulate a margin and to scale the entire content (needed on Windows).

When landing to Chrome, this will need http://codereview.chromium.org/6820038

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

[reed1, 2011-04-11 17:21:27]

The initialTransform should be a const parameter.

Can we make it required (i.e. const SkMatrix& instead of const SkMatrix*)? Are
there many places we create these devices outside of the factory, that we need
to support NULL (which has the logical, but non-obvious value of
flip+translate)?

[Steve VanDeBogart, 2011-04-11 17:32:31]

On 2011/04/11 17:21:27, reed1 wrote:
> The initialTransform should be a const parameter.
> 
> Can we make it required (i.e. const SkMatrix& instead of const SkMatrix*)? Are
> there many places we create these devices outside of the factory, that we need
> to support NULL (which has the logical, but non-obvious value of
> flip+translate)?

The casual user will want to flip+translate in order to make the coordinate
system the same.  One alternative would be to always flip and translate (to make
the PDF device consistent with the rest of Skia) and then the factory can also
flip & translate for the layer case, resulting in the identity transform.

[Chris Guillory, 2011-04-11 20:44:06]

LGTM with comments from Mike.

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

http://codereview.appspot.com/4373052/diff/1/include/pdf/SkPDFUtils.h#newcode43
include/pdf/SkPDFUtils.h:43: static void AppendMatrix(const SkMatrix& matrix,
SkWStream* content);
Does AppendTransform or AppendTransformMatrix make more sense?

[reed1, 2011-04-11 20:53:34]

I happen to like AppendMatrix, since that what it is. My 2cents

On 2011/04/11 20:44:06, Chris Guillory wrote:
> LGTM with comments from Mike.
> 
> http://codereview.appspot.com/4373052/diff/1/include/pdf/SkPDFUtils.h
> File include/pdf/SkPDFUtils.h (right):
> 
>
http://codereview.appspot.com/4373052/diff/1/include/pdf/SkPDFUtils.h#newcode43
> include/pdf/SkPDFUtils.h:43: static void AppendMatrix(const SkMatrix& matrix,
> SkWStream* content);
> Does AppendTransform or AppendTransformMatrix make more sense?

[reed1, 2011-04-11 20:54:44]

definitely think the matrix parameter (ptr or ref) should be const.

[Steve VanDeBogart, 2011-04-11 21:50:06]

On 2011/04/11 17:32:31, Steve VanDeBogart wrote:
> On 2011/04/11 17:21:27, reed1 wrote:
> > The initialTransform should be a const parameter.
> > 
> > Can we make it required (i.e. const SkMatrix& instead of const SkMatrix*)?
Are
> > there many places we create these devices outside of the factory, that we
need
> > to support NULL (which has the logical, but non-obvious value of
> > flip+translate)?
> 
> The casual user will want to flip+translate in order to make the coordinate
> system the same.  One alternative would be to always flip and translate (to
make
> the PDF device consistent with the rest of Skia) and then the factory can also
> flip & translate for the layer case, resulting in the identity transform.

I've done this to get a const ref parameter and remove the possibly surprising
difference between a NULL and identity initial transform.

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

http://codereview.appspot.com/4373052/diff/1/include/pdf/SkPDFUtils.h#newcode43
include/pdf/SkPDFUtils.h:43: static void AppendMatrix(const SkMatrix& matrix,
SkWStream* content);
On 2011/04/11 20:44:06, Chris Guillory wrote:
> Does AppendTransform or AppendTransformMatrix make more sense?

Because this includes the "cm" command, I think AppendTransform does make more
sense.  Changed.

[Chris Guillory, 2011-04-11 23:13:20]

LGTM. One comment.

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

http://codereview.appspot.com/4373052/diff/7001/src/pdf/SkPDFDevice.cpp#newco...
src/pdf/SkPDFDevice.cpp:147: fInitialTransform.setTranslate(0, height);
Comment here? Perhaps move the deleted commented above enum OriginTransform
here.

[Steve VanDeBogart, 2011-04-11 23:23:49]

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

http://codereview.appspot.com/4373052/diff/7001/src/pdf/SkPDFDevice.cpp#newco...
src/pdf/SkPDFDevice.cpp:147: fInitialTransform.setTranslate(0, height);
On 2011/04/11 23:13:20, Chris Guillory wrote:
> Comment here? Perhaps move the deleted commented above enum OriginTransform
> here.

Done.

[reed1, 2011-04-12 11:43:01]

LGTM

[zmcmay_gmail.com, 2012-06-04 08:17:25]

You can take a try of free trial version of Kvisoft PDF flip page
creator<http://www.kvisoft.com/flipbook-maker-pro/>to make your PDF files flip
page like a real book. No watermark printed.