SkMultiPictureDocument & SkMultiPictureDocumentReader

SkMultiPictureDocument & SkMultiPictureDocumentReader

also, a new DM::Src.

motivation: To be used to test the printing pipeline in Chromium.

BUG=skia:5370
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2023593002

Committed: https://skia.googlesource.com/skia/+/45420a945cfce1b83da954ab5fcb63516f646c0f

[hal.canary, 2016-05-28 01:27:43]

Description was changed from

==========
SkMultiPictureDocument & kMultiPictureDocumentReader

also, a new DM::Src.
==========

to

==========
SkMultiPictureDocument & kMultiPictureDocumentReader

also, a new DM::Src.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2023593002
==========

[hal.canary, 2016-05-28 01:30:42]

Description was changed from

==========
SkMultiPictureDocument & kMultiPictureDocumentReader

also, a new DM::Src.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2023593002
==========

to

==========
SkMultiPictureDocument & kMultiPictureDocumentReader

also, a new DM::Src.

motivation: To be used to test the printing pipeline in Chromium.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2023593002
==========

[hal.canary, 2016-05-28 01:44:48]

halcanary@google.com changed reviewers:
+ mtklein@google.com, reed@google.com, tomhudson@google.com

[hal.canary, 2016-05-28 01:44:48]

PTAL

mtklein@: extenstions to DM::Src
reed@: public API to be used by Chromium. 
tomhudson@: everything else

[hal.canary, 2016-05-28 01:45:05]

Description was changed from

==========
SkMultiPictureDocument & kMultiPictureDocumentReader

also, a new DM::Src.

motivation: To be used to test the printing pipeline in Chromium.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2023593002
==========

to

==========
SkMultiPictureDocument & SkMultiPictureDocumentReader

also, a new DM::Src.

motivation: To be used to test the printing pipeline in Chromium.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2023593002
==========

[hal.canary, 2016-05-31 13:04:09]

The CQ bit was checked by halcanary@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-31 13:04:17]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2023593002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2023593002/1

[hal.canary, 2016-05-31 13:15:28]

The CQ bit was checked by halcanary@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-31 13:15:39]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2023593002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2023593002/20001

[commit-bot: I haz the power, 2016-05-31 13:29:49]

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

[commit-bot: I haz the power, 2016-05-31 13:29:50]

Dry run: This issue passed the CQ dry run.

[reed1, 2016-05-31 22:36:09]

Need more dox and/or a bug or doc describe its uses, longevity, etc.

[hal.canary, 2016-06-01 01:49:43]

https://bugs.skia.org/5370

[hal.canary, 2016-06-01 01:50:06]

Description was changed from

==========
SkMultiPictureDocument & SkMultiPictureDocumentReader

also, a new DM::Src.

motivation: To be used to test the printing pipeline in Chromium.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2023593002
==========

to

==========
SkMultiPictureDocument & SkMultiPictureDocumentReader

also, a new DM::Src.

motivation: To be used to test the printing pipeline in Chromium.

BUG=skia:5370

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2023593002
==========

[hal.canary, 2016-06-01 16:49:44]

moved out of include/core to src/utils

[tomhudson, 2016-06-01 16:55:00]

https://codereview.chromium.org/2023593002/diff/1/dm/DM.cpp
File dm/DM.cpp (right):

https://codereview.chromium.org/2023593002/diff/1/dm/DM.cpp#newcode716
dm/DM.cpp:716: push_src("mskp", "", new MSKPSrc(path));
The documentation says mskps is a directory, but we also allow it to be an
individual file?

https://codereview.chromium.org/2023593002/diff/1/dm/DMSrcSink.cpp
File dm/DMSrcSink.cpp (right):

https://codereview.chromium.org/2023593002/diff/1/dm/DMSrcSink.cpp#newcode1019
dm/DMSrcSink.cpp:1019: std::unique_ptr<SkStreamAsset>
stream(SkStream::NewFromFile(fPath.c_str()));
We have to recreate this stream with every draw call? Wasn't it set up in the
constructor?

https://codereview.chromium.org/2023593002/diff/1/dm/DMSrcSink.cpp#newcode1022
dm/DMSrcSink.cpp:1022: return SkStringPrintf("SkMultiPictureDocumentReader
failed: %s",
Do we also want the page # in this error message?

https://codereview.chromium.org/2023593002/diff/1/src/core/SkMultiPictureDocu...
File src/core/SkMultiPictureDocument.cpp (right):

https://codereview.chromium.org/2023593002/diff/1/src/core/SkMultiPictureDocu...
src/core/SkMultiPictureDocument.cpp:39: if (trimBox && *trimBox != size) {
I can't intuit your intent here. This is surprising?

https://codereview.chromium.org/2023593002/diff/1/src/core/SkMultiPictureDocu...
src/core/SkMultiPictureDocument.cpp:73: good &=
wStream->writeText(SkMultiPictureDocumentProtocol::kMagic);
&&=?

https://codereview.chromium.org/2023593002/diff/1/src/core/SkMultiPictureDocu...
src/core/SkMultiPictureDocument.cpp:81: offset, (float)cullRect.right(),
(float)cullRect.bottom()};
It's an inviolable fact that cullRects are always originating from the origin?

https://codereview.chromium.org/2023593002/diff/1/src/core/SkMultiPictureDocu...
src/core/SkMultiPictureDocument.cpp:90: good &=
wStream->writeText("\nEndOfMultiPicture\n");
Any reason not to move this to another constexpr on the protocol object?

[hal.canary, 2016-06-01 18:09:16]

https://codereview.chromium.org/2023593002/diff/1/src/core/SkMultiPictureDocu...
File src/core/SkMultiPictureDocument.cpp (right):

https://codereview.chromium.org/2023593002/diff/1/src/core/SkMultiPictureDocu...
src/core/SkMultiPictureDocument.cpp:39: if (trimBox && *trimBox != size) {
On 2016/06/01 16:55:00, tomhudson wrote:
> I can't intuit your intent here. This is surprising?

Only trim if necessary.  SkPictures already have a trimBox.  All SkDocuments
follow this logic.

https://codereview.chromium.org/2023593002/diff/1/src/core/SkMultiPictureDocu...
src/core/SkMultiPictureDocument.cpp:73: good &=
wStream->writeText(SkMultiPictureDocumentProtocol::kMagic);
On 2016/06/01 16:55:00, tomhudson wrote:
> &&=?

Logically, yes.  C++ does not have a `&&=` operator.  for the bool type, &= does
what you want.

I'm tired of doing 

    if (!foo()) {
      return false;
    }

when 
    good &= foo();

is one third as many lines.

https://codereview.chromium.org/2023593002/diff/1/src/core/SkMultiPictureDocu...
src/core/SkMultiPictureDocument.cpp:81: offset, (float)cullRect.right(),
(float)cullRect.bottom()};
On 2016/06/01 16:54:59, tomhudson wrote:
> It's an inviolable fact that cullRects are always originating from the origin?

If the picture was created by the code in the other file, yes.

Otherwise, I could do a translate().

https://codereview.chromium.org/2023593002/diff/1/src/core/SkMultiPictureDocu...
src/core/SkMultiPictureDocument.cpp:90: good &=
wStream->writeText("\nEndOfMultiPicture\n");
On 2016/06/01 16:54:59, tomhudson wrote:
> Any reason not to move this to another constexpr on the protocol object?

I actually use it.  It's there more for human debugging.

[hal.canary, 2016-06-02 13:24:04]

https://codereview.chromium.org/2023593002/diff/1/dm/DM.cpp
File dm/DM.cpp (right):

https://codereview.chromium.org/2023593002/diff/1/dm/DM.cpp#newcode716
dm/DM.cpp:716: push_src("mskp", "", new MSKPSrc(path));
On 2016/06/01 16:54:59, tomhudson wrote:
> The documentation says mskps is a directory, but we also allow it to be an
> individual file?

Done.  (doc fixed)

https://codereview.chromium.org/2023593002/diff/1/dm/DMSrcSink.cpp
File dm/DMSrcSink.cpp (right):

https://codereview.chromium.org/2023593002/diff/1/dm/DMSrcSink.cpp#newcode1019
dm/DMSrcSink.cpp:1019: std::unique_ptr<SkStreamAsset>
stream(SkStream::NewFromFile(fPath.c_str()));
On 2016/06/01 16:54:59, tomhudson wrote:
> We have to recreate this stream with every draw call? Wasn't it set up in the
> constructor?

This is how the SKPSrc works too.  These objects are long-lived and we don't
want to leave a million file descriptors open at once.

https://codereview.chromium.org/2023593002/diff/1/dm/DMSrcSink.cpp#newcode1022
dm/DMSrcSink.cpp:1022: return SkStringPrintf("SkMultiPictureDocumentReader
failed: %s",
On 2016/06/01 16:54:59, tomhudson wrote:
> Do we also want the page # in this error message?

good idea.  I've improved all error messages and conditions.

https://codereview.chromium.org/2023593002/diff/1/src/core/SkMultiPictureDocu...
File src/core/SkMultiPictureDocument.cpp (right):

https://codereview.chromium.org/2023593002/diff/1/src/core/SkMultiPictureDocu...
src/core/SkMultiPictureDocument.cpp:90: good &=
wStream->writeText("\nEndOfMultiPicture\n");
On 2016/06/01 18:09:16, Hal Canary wrote:
> On 2016/06/01 16:54:59, tomhudson wrote:
> > Any reason not to move this to another constexpr on the protocol object?
> 
> I actually use it.  It's there more for human debugging.

I DON'T actually use it.  It's there more for human debugging.

[tomhudson, 2016-06-02 17:46:08]

Still needs Mike & maybe Mike, but LGTM.

[hal.canary, 2016-06-02 17:54:31]

On 2016/06/02 17:46:08, tomhudson wrote:
> Still needs Mike & maybe Mike, but LGTM.

Yesterday, reed@ said it was okay to put the header in src/ for now.

mtklein@?  Any objections to extending DM::Src?  Otherwise we can land and
iterate.

[hal.canary, 2016-06-02 17:54:39]

The CQ bit was checked by halcanary@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-02 17:54:45]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2023593002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2023593002/60001

[commit-bot: I haz the power, 2016-06-02 18:20:44]

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

[commit-bot: I haz the power, 2016-06-02 18:20:45]

Dry run: This issue passed the CQ dry run.

[hal.canary, 2016-06-02 19:39:53]

The CQ bit was checked by halcanary@google.com

[commit-bot: I haz the power, 2016-06-02 19:40:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2023593002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2023593002/60001

[commit-bot: I haz the power, 2016-06-02 19:41:18]

Description was changed from

==========
SkMultiPictureDocument & SkMultiPictureDocumentReader

also, a new DM::Src.

motivation: To be used to test the printing pipeline in Chromium.

BUG=skia:5370

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2023593002
==========

to

==========
SkMultiPictureDocument & SkMultiPictureDocumentReader

also, a new DM::Src.

motivation: To be used to test the printing pipeline in Chromium.

BUG=skia:5370

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2023593002

Committed:
https://skia.googlesource.com/skia/+/45420a945cfce1b83da954ab5fcb63516f646c0f
==========

[commit-bot: I haz the power, 2016-06-02 19:41:19]

Committed patchset #4 (id:60001) as
https://skia.googlesource.com/skia/+/45420a945cfce1b83da954ab5fcb63516f646c0f