SkMultiPictureDocument : clean up MultiPictureDocumentReader interface

Motivation:  We may want to make SkMultiPictureDocument.h public in the
future.

Change-Id: Ie97b88d51a179c2283155d65bcadee32178115ca
Reviewed-on: https://skia-review.googlesource.com/11402
Commit-Queue: Hal Canary <halcanary@google.com>
Reviewed-by: Herb Derby <herb@google.com>

BUILD.gn

@@ -921,7 +921,6 @@ if (skia_enable_tools) {
       "tools/timer",
     ]
     sources = [
-      "src/utils/SkMultiPictureDocumentReader.cpp",  # TODO(halcanary): move to tools?
       "tools/AndroidSkDebugToStdOut.cpp",
       "tools/CrashHandler.cpp",
       "tools/LsanSuppressions.cpp",

dm/DMSrcSink.cpp

@@ -26,6 +26,7 @@
 #include "SkLiteDL.h"
 #include "SkLiteRecorder.h"
 #include "SkMallocPixelRef.h"
+#include "SkMultiPictureDocumentPriv.h"
 #include "SkMultiPictureDraw.h"
 #include "SkNullCanvas.h"
 #include "SkOSFile.h"
@@ -1194,30 +1195,39 @@ bool SVGSrc::veto(SinkFlags flags) const {
 
 MSKPSrc::MSKPSrc(Path path) : fPath(path) {
     std::unique_ptr<SkStreamAsset> stream = SkStream::MakeFromFile(fPath.c_str());
-    (void)fReader.init(stream.get());
+    int count = SkMultiPictureDocumentReadPageCount(stream.get());
+    if (count > 0) {
+        fPages.reset(count);
+        (void)SkMultiPictureDocumentReadPageSizes(stream.get(), &fPages[0], fPages.count());
+    }
 }
 
-int MSKPSrc::pageCount() const { return fReader.pageCount(); }
+int MSKPSrc::pageCount() const { return fPages.count(); }
 
 SkISize MSKPSrc::size() const { return this->size(0); }
-SkISize MSKPSrc::size(int i) const { return fReader.pageSize(i).toCeil(); }
+SkISize MSKPSrc::size(int i) const {
+    return i >= 0 && i < fPages.count() ? fPages[i].fSize.toCeil() : SkISize::Make(0, 0);
+}
 
 Error MSKPSrc::draw(SkCanvas* c) const { return this->draw(0, c); }
 Error MSKPSrc::draw(int i, SkCanvas* canvas) const {
-    std::unique_ptr<SkStreamAsset> stream = SkStream::MakeFromFile(fPath.c_str());
+    if (this->pageCount() == 0) {
-    if (!stream) {
-        return SkStringPrintf("Unable to open file: %s", fPath.c_str());
-    }
-    if (fReader.pageCount() == 0) {
         return SkStringPrintf("Unable to parse MultiPictureDocument file: %s", fPath.c_str());
     }
-    if (i >= fReader.pageCount()) {
+    if (i >= fPages.count() || i < 0) {
         return SkStringPrintf("MultiPictureDocument page number out of range: %d", i);
     }
-    sk_sp<SkPicture> page = fReader.readPage(stream.get(), i);
+    SkPicture* page = fPages[i].fPicture.get();
     if (!page) {
-        return SkStringPrintf("SkMultiPictureDocumentReader failed on page %d: %s",
+        std::unique_ptr<SkStreamAsset> stream = SkStream::MakeFromFile(fPath.c_str());
-                              i, fPath.c_str());
+        if (!stream) {
+            return SkStringPrintf("Unable to open file: %s", fPath.c_str());
+        }
+        if (!SkMultiPictureDocumentRead(stream.get(), &fPages[0], fPages.count())) {
+            return SkStringPrintf("SkMultiPictureDocument reader failed on page %d: %s", i,
+                                  fPath.c_str());
+        }
+        page = fPages[i].fPicture.get();
     }
     canvas->drawPicture(page);
     return "";

dm/DMSrcSink.h

@@ -15,7 +15,7 @@
 #include "SkBitmapRegionDecoder.h"
 #include "SkCanvas.h"
 #include "SkData.h"
-#include "SkMultiPictureDocumentReader.h"
+#include "SkMultiPictureDocument.h"
 #include "SkPicture.h"
 #include "gm.h"
 
@@ -288,7 +288,7 @@ public:
 
 private:
     Path fPath;
-    SkMultiPictureDocumentReader fReader;
+    mutable SkTArray<SkDocumentPage> fPages;
 };
 
 /*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/

src/utils/SkMultiPictureDocument.cpp

@@ -7,11 +7,14 @@
 
 #include "SkMultiPictureDocument.h"
 #include "SkMultiPictureDocumentPriv.h"
+#include "SkNWayCanvas.h"
 #include "SkPicture.h"
 #include "SkPictureRecorder.h"
 #include "SkStream.h"
 #include "SkTArray.h"
 
+#include <limits.h>
+
 /*
   File format:
       BEGINNING_OF_FILE:
@@ -26,6 +29,22 @@
 */
 
 namespace {
+// The unique file signature for this file type.
+static constexpr char kMagic[] = "Skia Multi-Picture Doc\n\n";
+
+static constexpr char kEndPage[] = "SkMultiPictureEndPage";
+
+const uint32_t kVersion = 2;
+
+static SkSize join(const SkTArray<SkSize>& sizes) {
+    SkSize joined = SkSize::Make(0, 0);
+    for (SkSize s : sizes) {
+        joined = SkSize::Make(SkTMax(joined.width(), s.width()),
+                              SkTMax(joined.height(), s.height()));
+    }
+    return joined;
+}
+
 static SkCanvas* trim(SkCanvas* canvas,
                       SkScalar w, SkScalar h,
                       const SkRect& trimBox) {
@@ -59,19 +78,17 @@ struct MultiPictureDocument final : public SkDocument {
     void onClose(SkWStream* wStream) override {
         SkASSERT(wStream);
         SkASSERT(wStream->bytesWritten() == 0);
-        wStream->writeText(SkMultiPictureDocumentProtocol::kMagic);
+        wStream->writeText(kMagic);
-        wStream->write32(SkMultiPictureDocumentProtocol::kVersion);
+        wStream->write32(kVersion);
         wStream->write32(SkToU32(fPages.count()));
         for (SkSize s : fSizes) {
             wStream->write(&s, sizeof(s));
         }
-        SkSize bigsize = SkMultiPictureDocumentProtocol::Join(fSizes);
+        SkSize bigsize = join(fSizes);
         SkCanvas* c = fPictureRecorder.beginRecording(SkRect::MakeSize(bigsize));
         for (const sk_sp<SkPicture>& page : fPages) {
             c->drawPicture(page);
-            c->drawAnnotation(SkRect::MakeEmpty(),
+            c->drawAnnotation(SkRect::MakeEmpty(), kEndPage, nullptr);
-                              SkMultiPictureDocumentProtocol::kEndPage,
-                              nullptr);
         }
         sk_sp<SkPicture> p = fPictureRecorder.finishRecordingAsPicture();
         p->serialize(wStream);
@@ -89,3 +106,103 @@ struct MultiPictureDocument final : public SkDocument {
 sk_sp<SkDocument> SkMakeMultiPictureDocument(SkWStream* wStream) {
     return sk_make_sp<MultiPictureDocument>(wStream, nullptr);
 }
+
+////////////////////////////////////////////////////////////////////////////////
+
+int SkMultiPictureDocumentReadPageCount(SkStreamSeekable* stream) {
+    if (!stream) {
+        return 0;
+    }
+    stream->seek(0);
+    const size_t size = sizeof(kMagic) - 1;
+    char buffer[size];
+    if (size != stream->read(buffer, size) || 0 != memcmp(kMagic, buffer, size)) {
+        stream = nullptr;
+        return 0;
+    }
+    uint32_t versionNumber = stream->readU32();
+    if (versionNumber != kVersion) {
+        return 0;
+    }
+    uint32_t pageCount = stream->readU32();
+    if (pageCount > INT_MAX) {
+        return 0;
+    }
+    // leave stream position right here.
+    return (int)pageCount;
+}
+
+bool SkMultiPictureDocumentReadPageSizes(SkStreamSeekable* stream,
+                                         SkDocumentPage* dstArray,
+                                         int dstArrayCount) {
+    if (!dstArray || dstArrayCount < 1) {
+        return false;
+    }
+    int pageCount = SkMultiPictureDocumentReadPageCount(stream);
+    if (pageCount < 1 || pageCount != dstArrayCount) {
+        return false;
+    }
+    for (int i = 0; i < pageCount; ++i) {
+        SkSize& s = dstArray[i].fSize;
+        if (sizeof(s) != stream->read(&s, sizeof(s))) {
+            return false;
+        }
+    }
+    // leave stream position right here.
+    return true;
+}
+
+namespace {
+struct PagerCanvas : public SkNWayCanvas {
+    SkPictureRecorder fRecorder;
+    SkDocumentPage* fDst;
+    int fCount;
+    int fIndex = 0;
+    PagerCanvas(SkISize wh, SkDocumentPage* dst, int count)
+            : SkNWayCanvas(wh.width(), wh.height()), fDst(dst), fCount(count) {
+        this->nextCanvas();
+    }
+    void nextCanvas() {
+        if (fIndex < fCount) {
+            SkRect bounds = SkRect::MakeSize(fDst[fIndex].fSize);
+            this->addCanvas(fRecorder.beginRecording(bounds));
+        }
+    }
+    void onDrawAnnotation(const SkRect& r, const char* key, SkData* d) override {
+        if (0 == strcmp(key, kEndPage)) {
+            this->removeAll();
+            if (fIndex < fCount) {
+                fDst[fIndex].fPicture = fRecorder.finishRecordingAsPicture();
+                ++fIndex;
+            }
+            this->nextCanvas();
+        } else {
+            this->SkNWayCanvas::onDrawAnnotation(r, key, d);
+        }
+    }
+};
+}  // namespace
+
+bool SkMultiPictureDocumentRead(SkStreamSeekable* stream,
+                                SkDocumentPage* dstArray,
+                                int dstArrayCount) {
+    if (!SkMultiPictureDocumentReadPageSizes(stream, dstArray, dstArrayCount)) {
+        return false;
+    }
+    SkSize joined = SkSize::Make(0.0f, 0.0f);
+    for (int i = 0; i < dstArrayCount; ++i) {
+        joined = SkSize::Make(SkTMax(joined.width(), dstArray[i].fSize.width()),
+                              SkTMax(joined.height(), dstArray[i].fSize.height()));
+    }
+
+    auto picture = SkPicture::MakeFromStream(stream);
+
+    PagerCanvas canvas(joined.toCeil(), dstArray, dstArrayCount);
+    // Must call playback(), not drawPicture() to reach
+    // PagerCanvas::onDrawAnnotation().
+    picture->playback(&canvas);
+    if (canvas.fIndex != dstArrayCount) {
+        SkDEBUGF(("Malformed SkMultiPictureDocument\n"));
+    }
+    return true;
+}

src/utils/SkMultiPictureDocument.h

@@ -7,42 +7,32 @@
 #ifndef SkMultiPictureDocument_DEFINED
 #define SkMultiPictureDocument_DEFINED
 
-/*
-  This format is not intended to be used in production.
-
-  For clients looking for a way to represent a document in memory,
-
-    struct Doc {
-        std::vector<sk_sp<SkPicture>> fPages;
-        std::vector<SkSize> fPageSizes;
-    };
-
-  or
-
-    struct Page {
-        sk_sp<SkPicture> fPage;
-        SkSize fPageSize;
-    };
-    std::vector<Page> pages;
-
-  would work much better.
-
-  Multi-SkPicture (MSKP) files are still useful for debugging and
-  testing.
-
-  The downsides of this format are currently:
-  - no way to extract a single page; must read the entire file at once.
-  - must use `dm` to convert to another format before passing into
-    standard skp tools.
-  - `dm` can extract the first page to skp, but no others.
-
-  TODO(halcanary): replace with somthing that addresses these issues.
- */
-
 #include "SkDocument.h"
 
-/** Writes into an experimental, undocumented file format that is
+class SkStreamSeekable;
-    useful for debugging documents printed via Skia. */
+
+/**
+ *  Writes into a file format that is similar to SkPicture::serialize()
+ */
 SK_API sk_sp<SkDocument> SkMakeMultiPictureDocument(SkWStream* dst);
 
+struct SkDocumentPage {
+    sk_sp<SkPicture> fPicture;
+    SkSize fSize;
+};
+
+/**
+ *  Returns the number of pages in the SkMultiPictureDocument.
+ */
+SK_API int SkMultiPictureDocumentReadPageCount(SkStreamSeekable* src);
+
+/**
+ *  Read the SkMultiPictureDocument into the provided array of pages.
+ *  dstArrayCount must equal SkMultiPictureDocumentReadPageCount().
+ *  Return false on error.
+ */
+SK_API bool SkMultiPictureDocumentRead(SkStreamSeekable* src,
+                                       SkDocumentPage* dstArray,
+                                       int dstArrayCount);
+
 #endif  // SkMultiPictureDocument_DEFINED

src/utils/SkMultiPictureDocumentPriv.h

@@ -8,25 +8,14 @@
 #ifndef SkMultiPictureDocumentPriv_DEFINED
 #define SkMultiPictureDocumentPriv_DEFINED
 
-#include "SkTArray.h"
+#include "SkMultiPictureDocument.h"
-#include "SkSize.h"
 
-namespace SkMultiPictureDocumentProtocol {
+/**
-static constexpr char kMagic[] = "Skia Multi-Picture Doc\n\n";
+ *  Additional API allows one to read the array of page-sizes without parsing
-
+ *  the entire file.  Used by DM.
-static constexpr char kEndPage[] = "SkMultiPictureEndPage";
+ */
-
+bool SkMultiPictureDocumentReadPageSizes(SkStreamSeekable* src,
-const uint32_t kVersion = 2;
+                                         SkDocumentPage* dstArray,
-
+                                         int dstArrayCount);
-inline SkSize Join(const SkTArray<SkSize>& sizes) {
-    SkSize joined = SkSize::Make(0, 0);
-    for (SkSize s : sizes) {
-        joined = SkSize::Make(SkTMax(joined.width(), s.width()),
-                              SkTMax(joined.height(), s.height()));
-    }
-    return joined;
-}
-
-}
 
 #endif  // SkMultiPictureDocumentPriv_DEFINED

src/utils/SkMultiPictureDocumentReader.cpp

@@ -1,93 +0,0 @@
-/*
- * Copyright 2016 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#include "SkMultiPictureDocumentPriv.h"
-#include "SkMultiPictureDocumentReader.h"
-#include "SkPicture.h"
-#include "SkStream.h"
-#include "SkPictureRecorder.h"
-#include "SkNWayCanvas.h"
-
-bool SkMultiPictureDocumentReader::init(SkStreamSeekable* stream) {
-    if (!stream) {
-        return false;
-    }
-    stream->seek(0);
-    const size_t size = sizeof(SkMultiPictureDocumentProtocol::kMagic) - 1;
-    char buffer[size];
-    if (size != stream->read(buffer, size) ||
-        0 != memcmp(SkMultiPictureDocumentProtocol::kMagic, buffer, size)) {
-        stream = nullptr;
-        return false;
-    }
-    bool good = true;
-    uint32_t versionNumber = stream->readU32();
-    if (versionNumber != SkMultiPictureDocumentProtocol::kVersion) {
-        return false;
-    }
-    uint32_t pageCount = stream->readU32();
-    fSizes.reset(pageCount);
-    for (uint32_t i = 0; i < pageCount; ++i) {
-        SkSize size;
-        good &= sizeof(size) == stream->read(&size, sizeof(size));
-        fSizes[i] = size;
-    }
-    fOffset = stream->getPosition();
-    return good;
-}
-
-namespace {
-struct PagerCanvas : public SkNWayCanvas {
-    SkPictureRecorder fRecorder;
-    const SkTArray<SkSize>* fSizes;
-    SkTArray<sk_sp<SkPicture>>* fDest;
-    PagerCanvas(SkISize  wh,
-                const SkTArray<SkSize>* s,
-                SkTArray<sk_sp<SkPicture>>* d)
-        : SkNWayCanvas(wh.width(), wh.height()), fSizes(s), fDest(d) {
-        this->nextCanvas();
-    }
-    void nextCanvas() {
-        int i = fDest->count();
-        if (i < fSizes->count()) {
-            SkRect bounds = SkRect::MakeSize((*fSizes)[i]);
-            this->addCanvas(fRecorder.beginRecording(bounds));
-        }
-    }
-    void onDrawAnnotation(const SkRect& r, const char* key, SkData* d) override {
-        if (0 == strcmp(key, SkMultiPictureDocumentProtocol::kEndPage)) {
-            this->removeAll();
-            if (fRecorder.getRecordingCanvas()) {
-                fDest->emplace_back(fRecorder.finishRecordingAsPicture());
-            }
-            this->nextCanvas();
-        } else {
-            this->SkNWayCanvas::onDrawAnnotation(r, key, d);
-        }
-    }
-};
-}  // namespace
-
-sk_sp<SkPicture> SkMultiPictureDocumentReader::readPage(SkStreamSeekable* stream,
-                                                        int pageNumber) const {
-    SkASSERT(pageNumber >= 0);
-    SkASSERT(pageNumber < fSizes.count());
-    if (0 == fPages.count()) {
-        stream->seek(fOffset); // jump to beginning of skp
-        auto picture = SkPicture::MakeFromStream(stream);
-        SkISize size = SkMultiPictureDocumentProtocol::Join(fSizes).toCeil();
-        PagerCanvas canvas(size, &fSizes, &this->fPages);
-        // Must call playback(), not drawPicture() to reach
-        // PagerCanvas::onDrawAnnotation().
-        picture->playback(&canvas);
-        if (fPages.count() != fSizes.count()) {
-            SkDEBUGF(("Malformed SkMultiPictureDocument\n"));
-        }
-    }
-    // Allow for malformed document.
-    return pageNumber < fPages.count() ? fPages[pageNumber] : nullptr;
-}

src/utils/SkMultiPictureDocumentReader.h

@@ -1,46 +0,0 @@
-/*
- * Copyright 2016 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-#ifndef SkMultiPictureDocumentReader_DEFINED
-#define SkMultiPictureDocumentReader_DEFINED
-
-#include "../private/SkTArray.h"
-#include "SkPicture.h"
-#include "SkSize.h"
-#include "SkStream.h"
-
-/** A lightweight helper class for reading a Skia MultiPictureDocument. */
-class SkMultiPictureDocumentReader {
-public:
-    /** Initialize the MultiPictureDocument.  Does not take ownership
-        of the SkStreamSeekable. */
-    bool init(SkStreamSeekable*);
-
-    /** Return to factory settings. */
-    void reset() {
-        fSizes.reset();
-        fPages.reset();
-    }
-
-    /** Call this after calling init() (otherwise you'll always get zero). */
-    int pageCount() const { return fSizes.count(); }
-
-    /** Deserialize a page from the stream.  Call init() first.  The
-        SkStreamSeekable doesn't need to be the same object, but
-        should point to the same information as before. */
-    sk_sp<SkPicture> readPage(SkStreamSeekable*, int) const;
-
-    /** Fetch the size of the given page, without deserializing the
-        entire page. */
-    SkSize pageSize(int i) const { return fSizes[i]; }
-
-private:
-    SkTArray<SkSize> fSizes;
-    size_t fOffset;
-    mutable SkTArray<sk_sp<SkPicture>> fPages;
-};
-
-#endif  // SkMultiPictureDocumentReader_DEFINED