change write-image to use pipeverbs like everyone else, and add test
authorreed <reed@google.com>
Wed, 14 Sep 2016 00:25:19 +0000 (17:25 -0700)
committerCommit bot <commit-bot@chromium.org>
Wed, 14 Sep 2016 00:25:19 +0000 (17:25 -0700)
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2335973003

Review-Url: https://codereview.chromium.org/2335973003

src/pipe/SkPipeCanvas.cpp
src/pipe/SkPipeCanvas.h
src/pipe/SkPipeFormat.h
src/pipe/SkPipeReader.cpp
tests/PipeTest.cpp

index 062d416..97c31ce 100644 (file)
@@ -1037,9 +1037,13 @@ void SkPipeSerializer::write(SkPicture* picture, SkWStream* stream) {
 }
 
 void SkPipeSerializer::write(SkImage* image, SkWStream* stream) {
-    stream->write32(kDefineImage_ExtPipeVerb);
-    SkPipeWriter writer(stream, &fImpl->fDeduper);
-    writer.writeImage(image);
+    int index = fImpl->fDeduper.findImage(image);
+    if (0 == index) {
+        // Try to define the image
+        fImpl->fDeduper.setStream(stream);
+        index = fImpl->fDeduper.findOrDefineImage(image);
+    }
+    stream->write32(pack_verb(SkPipeVerb::kWriteImage, index));
 }
 
 SkCanvas* SkPipeSerializer::beginWrite(const SkRect& cull, SkWStream* stream) {
@@ -1053,4 +1057,5 @@ SkCanvas* SkPipeSerializer::beginWrite(const SkRect& cull, SkWStream* stream) {
 void SkPipeSerializer::endWrite() {
     fImpl->fCanvas->restoreToCount(1);
     fImpl->fCanvas.reset(nullptr);
+    fImpl->fDeduper.setCanvas(nullptr);
 }
index 25353c9..8dbb83f 100644 (file)
@@ -23,7 +23,7 @@ public:
     void reset() { fArray.reset(); }
 
     // returns the found index or 0
-    int find(const T& key) {
+    int find(const T& key) const {
         const Rec* stop = fArray.end();
         for (const Rec* curr = fArray.begin(); curr < stop; ++curr) {
             if (key == curr->fKey) {
@@ -64,6 +64,9 @@ public:
     void setStream(SkWStream* stream) { fStream = stream; }
     void setTypefaceSerializer(SkTypefaceSerializer* tfs) { fTFSerializer = tfs; }
 
+    // returns 0 if not found
+    int findImage(SkImage* image) const { return fImages.find(image->uniqueID()); }
+
     int findOrDefineImage(SkImage*) override;
     int findOrDefinePicture(SkPicture*) override;
     int findOrDefineTypeface(SkTypeface*) override;
index f521657..539d385 100644 (file)
@@ -11,7 +11,6 @@
 #include "SkTypes.h"
 
 #define kDefinePicture_ExtPipeVerb  SkSetFourByteTag('s', 'k', 'p', 'i')
-#define kDefineImage_ExtPipeVerb    SkSetFourByteTag('s', 'k', 'i', 'm')
 
 enum class SkPipeVerb : uint8_t {
     kSave,              // extra == 0
@@ -57,6 +56,7 @@ enum class SkPipeVerb : uint8_t {
     kDefineFactory,     // extra == factory_index (followed by padded getTypeName string)
     kDefinePicture,     // extra == 0 or forget_index + 1 (0 means we're defining a new picture)
     kEndPicture,        // extra == picture_index
+    kWriteImage,        // extra == image_index
 };
 
 enum PaintUsage {
index d437548..2edc2df 100644 (file)
@@ -689,7 +689,7 @@ static sk_sp<SkImage> make_from_encoded(const sk_sp<SkData>& data) {
     return image;
 }
 
-static void defineImage_handler(SkPipeReader& reader, uint32_t packedVerb, SkCanvas* canvas) {
+static void defineImage_handler(SkPipeReader& reader, uint32_t packedVerb, SkCanvas*) {
     SkASSERT(SkPipeVerb::kDefineImage == unpack_verb(packedVerb));
     SkPipeInflator* inflator = (SkPipeInflator*)reader.getInflator();
     uint32_t extra = unpack_verb_extra(packedVerb);
@@ -867,21 +867,32 @@ sk_sp<SkPicture> SkPipeDeserializer::readPicture(const void* data, size_t size)
 
 sk_sp<SkImage> SkPipeDeserializer::readImage(const void* data, size_t size) {
     if (size < sizeof(uint32_t)) {
+        SkDebugf("-------- data length too short for readImage %d\n", size);
         return nullptr;
     }
 
-    uint32_t header;
-    memcpy(&header, data, 4); size -= 4; data = (const char*)data + 4;
-    if (kDefineImage_ExtPipeVerb != header) {
+    const uint32_t* ptr = (const uint32_t*)data;
+    uint32_t packedVerb = *ptr++;
+    size -= 4;
+
+    if (SkPipeVerb::kDefineImage == unpack_verb(packedVerb)) {
+        SkPipeInflator inflator(&fImpl->fImages, &fImpl->fPictures,
+                                &fImpl->fTypefaces, &fImpl->fFactories,
+                                fImpl->fTFDeserializer);
+        SkPipeReader reader(this, ptr, size);
+        reader.setInflator(&inflator);
+        defineImage_handler(reader, packedVerb, nullptr);
+        packedVerb = reader.read32();  // read the next verb
+    }
+    if (SkPipeVerb::kWriteImage != unpack_verb(packedVerb)) {
+        SkDebugf("-------- unexpected verb for readImage %d\n", unpack_verb(packedVerb));
         return nullptr;
     }
-
-    SkPipeInflator inflator(&fImpl->fImages, &fImpl->fPictures,
-                            &fImpl->fTypefaces, &fImpl->fFactories,
-                            fImpl->fTFDeserializer);
-    SkPipeReader reader(this, data, size);
-    reader.setInflator(&inflator);
-    return sk_sp<SkImage>(reader.readImage());
+    int index = unpack_verb_extra(packedVerb);
+    if (0 == index) {
+        return nullptr; // writer failed
+    }
+    return sk_ref_sp(fImpl->fImages.get(index - 1));
 }
 
 static bool do_playback(SkPipeReader& reader, SkCanvas* canvas, int* endPictureIndex) {
index f345deb..7f42071 100644 (file)
@@ -49,7 +49,7 @@ static bool deep_equal(SkImage* a, SkImage* b) {
     return true;
 }
 
-DEF_TEST(Pipe_image, reporter) {
+DEF_TEST(Pipe_image_draw_first, reporter) {
     sk_sp<SkImage> img = GetResourceAsImage("mandrill_128.png");
     SkASSERT(img.get());
 
@@ -86,3 +86,30 @@ DEF_TEST(Pipe_image, reporter) {
     auto img2 = drain_as_image(&deserializer, &stream);
     REPORTER_ASSERT(reporter, img1.get() == img2.get());
 }
+
+DEF_TEST(Pipe_image_draw_second, reporter) {
+    sk_sp<SkImage> img = GetResourceAsImage("mandrill_128.png");
+    SkASSERT(img.get());
+
+    SkPipeSerializer serializer;
+    SkPipeDeserializer deserializer;
+    SkDynamicMemoryWStream stream;
+
+    serializer.write(img.get(), &stream);
+    size_t offset0 = stream.bytesWritten();
+    REPORTER_ASSERT(reporter, offset0 > 100);   // the raw image must be sorta big
+    drain_as_image(&deserializer, &stream);
+
+    // The 2nd image should be nice and small
+    serializer.write(img.get(), &stream);
+    size_t offset1 = stream.bytesWritten();
+    REPORTER_ASSERT(reporter, offset1 <= 32);
+    drain_as_image(&deserializer, &stream);
+
+    // Now try drawing the image, it should also be small
+    SkCanvas* wc = serializer.beginWrite(SkRect::MakeWH(100, 100), &stream);
+    wc->drawImage(img, 0, 0, nullptr);
+    serializer.endWrite();
+    size_t offset2 = stream.bytesWritten();
+    REPORTER_ASSERT(reporter, offset2 <= 32);
+}