One binary serialization path for SkSamplingOptions

Bug: skia:13036
Change-Id: I31ccc3d1cfb271a2c2000476b91a695390cef4a6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526518
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>

src/core/BUILD.bazel

@@ -5383,6 +5383,7 @@ generated_cc_atom(
     deps = [
         ":SkMatrixPriv_hdr",
         ":SkWriter32_hdr",
+        "//include/core:SkSamplingOptions_hdr",
         "//include/private:SkTo_hdr",
     ],
 )

src/core/SkMatrixImageFilter.cpp

@@ -48,7 +48,7 @@ sk_sp<SkFlattenable> SkMatrixImageFilter::CreateProc(SkReadBuffer& buffer) {
         if (buffer.isVersionLT(SkPicturePriv::kMatrixImageFilterSampling_Version)) {
             return SkSamplingPriv::FromFQ(buffer.read32LE(kLast_SkLegacyFQ), kLinear_SkMediumAs);
         } else {
-            return SkSamplingPriv::Read(buffer);
+            return buffer.readSampling();
         }
     }();
     return Make(matrix, sampling, common.getInput(0));
@@ -57,7 +57,7 @@ sk_sp<SkFlattenable> SkMatrixImageFilter::CreateProc(SkReadBuffer& buffer) {
 void SkMatrixImageFilter::flatten(SkWriteBuffer& buffer) const {
     this->INHERITED::flatten(buffer);
     buffer.writeMatrix(fTransform);
-    SkSamplingPriv::Write(buffer, fSampling);
+    buffer.writeSampling(fSampling);
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////

src/core/SkPictureRecord.cpp

@@ -921,14 +921,7 @@ void SkPictureRecord::addRegion(const SkRegion& region) {
 }
 
 void SkPictureRecord::addSampling(const SkSamplingOptions& sampling) {
-    fWriter.writeBool(sampling.useCubic);
+    fWriter.writeSampling(sampling);
-    if (sampling.useCubic) {
-        fWriter.writeScalar(sampling.cubic.B);
-        fWriter.writeScalar(sampling.cubic.C);
-    } else {
-        fWriter.writeInt(static_cast<uint32_t>(sampling.filter));
-        fWriter.writeInt(static_cast<uint32_t>(sampling.mipmap));
-    }
 }
 
 void SkPictureRecord::addText(const void* text, size_t byteLength) {

src/core/SkSamplingPriv.h

@@ -43,9 +43,6 @@ public:
         return !sampling.useCubic || sampling.cubic.B == 0;
     }
 
-    static SkSamplingOptions Read(SkReadBuffer&);
-    static void Write(SkWriteBuffer&, const SkSamplingOptions&);
-
     static SkSamplingOptions FromFQ(SkLegacyFQ, SkMediumAs = kNearest_SkMediumAs);
 };
 

src/core/SkWriteBuffer.cpp

@@ -124,6 +124,10 @@ void SkBinaryWriteBuffer::writeRegion(const SkRegion& region) {
     fWriter.writeRegion(region);
 }
 
+void SkBinaryWriteBuffer::writeSampling(const SkSamplingOptions& sampling) {
+    fWriter.writeSampling(sampling);
+}
+
 void SkBinaryWriteBuffer::writePath(const SkPath& path) {
     fWriter.writePath(path);
 }

src/core/SkWriteBuffer.h

@@ -60,6 +60,7 @@ public:
     virtual void writeIRect(const SkIRect& rect) = 0;
     virtual void writeRect(const SkRect& rect) = 0;
     virtual void writeRegion(const SkRegion& region) = 0;
+    virtual void writeSampling(const SkSamplingOptions&) = 0;
     virtual void writePath(const SkPath& path) = 0;
     virtual size_t writeStream(SkStream* stream, size_t length) = 0;
     virtual void writeImage(const SkImage*) = 0;
@@ -122,6 +123,7 @@ public:
     void writeIRect(const SkIRect& rect) override;
     void writeRect(const SkRect& rect) override;
     void writeRegion(const SkRegion& region) override;
+    void writeSampling(const SkSamplingOptions&) override;
     void writePath(const SkPath& path) override;
     size_t writeStream(SkStream* stream, size_t length) override;
     void writeImage(const SkImage*) override;

src/core/SkWriter32.cpp

@@ -5,9 +5,11 @@
  * found in the LICENSE file.
  */
 
+#include "src/core/SkWriter32.h"
+
+#include "include/core/SkSamplingOptions.h"
 #include "include/private/SkTo.h"
 #include "src/core/SkMatrixPriv.h"
-#include "src/core/SkWriter32.h"
 
 void SkWriter32::writeMatrix(const SkMatrix& matrix) {
     size_t size = SkMatrixPriv::WriteToMemory(matrix, nullptr);
@@ -15,6 +17,17 @@ void SkWriter32::writeMatrix(const SkMatrix& matrix) {
     SkMatrixPriv::WriteToMemory(matrix, this->reserve(size));
 }
 
+void SkWriter32::writeSampling(const SkSamplingOptions& sampling) {
+    this->writeBool(sampling.useCubic);
+    if (sampling.useCubic) {
+        this->writeScalar(sampling.cubic.B);
+        this->writeScalar(sampling.cubic.C);
+    } else {
+        this->write32((unsigned)sampling.filter);
+        this->write32((unsigned)sampling.mipmap);
+    }
+}
+
 void SkWriter32::writeString(const char str[], size_t len) {
     if (nullptr == str) {
         str = "";

src/core/SkWriter32.h

@@ -23,6 +23,8 @@
 #include "include/private/SkTemplates.h"
 #include "include/private/SkTo.h"
 
+struct SkSamplingOptions;
+
 class SkWriter32 : SkNoncopyable {
 public:
     /**
@@ -148,6 +150,8 @@ public:
         rgn.writeToMemory(this->reserve(size));
     }
 
+    void writeSampling(const SkSamplingOptions& sampling);
+
     // write count bytes (must be a multiple of 4)
     void writeMul4(const void* values, size_t size) {
         this->write(values, size);

src/effects/imagefilters/SkImageImageFilter.cpp

@@ -92,7 +92,7 @@ sk_sp<SkFlattenable> SkImageImageFilter::CreateProc(SkReadBuffer& buffer) {
 }
 
 void SkImageImageFilter::flatten(SkWriteBuffer& buffer) const {
-    SkSamplingPriv::Write(buffer, fSampling);
+    buffer.writeSampling(fSampling);
     buffer.writeRect(fSrcRect);
     buffer.writeRect(fDstRect);
     buffer.writeImage(fImage.get());

src/image/BUILD.bazel

@@ -294,10 +294,8 @@ generated_cc_atom(
         "//src/core:SkMipmapBuilder_hdr",
         "//src/core:SkMipmap_hdr",
         "//src/core:SkNextID_hdr",
-        "//src/core:SkReadBuffer_hdr",
         "//src/core:SkSamplingPriv_hdr",
         "//src/core:SkSpecialImage_hdr",
-        "//src/core:SkWriteBuffer_hdr",
         "//src/gpu:GrDirectContextPriv_hdr",
         "//src/gpu:GrFragmentProcessor_hdr",
         "//src/gpu:GrImageContextPriv_hdr",

src/image/SkImage.cpp

@@ -21,6 +21,7 @@
 #include "src/core/SkMipmap.h"
 #include "src/core/SkMipmapBuilder.h"
 #include "src/core/SkNextID.h"
+#include "src/core/SkSamplingPriv.h"
 #include "src/core/SkSpecialImage.h"
 #include "src/image/SkImage_Base.h"
 #include "src/image/SkReadPixelsRec.h"
@@ -738,10 +739,6 @@ sk_sp<SkImage> SkMipmapBuilder::attachTo(const SkImage* src) {
 
 //////////////////////////////////////////////////////////////////////////////////////////////////
 
-#include "src/core/SkReadBuffer.h"
-#include "src/core/SkSamplingPriv.h"
-#include "src/core/SkWriteBuffer.h"
-
 SkSamplingOptions SkSamplingPriv::FromFQ(SkLegacyFQ fq, SkMediumAs behavior) {
     switch (fq) {
         case kHigh_SkLegacyFQ:
@@ -757,26 +754,3 @@ SkSamplingOptions SkSamplingPriv::FromFQ(SkLegacyFQ fq, SkMediumAs behavior) {
     }
     return SkSamplingOptions(SkFilterMode::kNearest, SkMipmapMode::kNone);
 }
-
-SkSamplingOptions SkSamplingPriv::Read(SkReadBuffer& buffer) {
-    if (buffer.readBool()) {
-        SkScalar B = buffer.readScalar(),
-                 C = buffer.readScalar();
-        return SkSamplingOptions({B,C});
-    } else {
-        auto filter = buffer.read32LE<SkFilterMode>(SkFilterMode::kLinear);
-        auto mipmap = buffer.read32LE<SkMipmapMode>(SkMipmapMode::kLinear);
-        return SkSamplingOptions(filter, mipmap);
-    }
-}
-
-void SkSamplingPriv::Write(SkWriteBuffer& buffer, const SkSamplingOptions& sampling) {
-    buffer.writeBool(sampling.useCubic);
-    if (sampling.useCubic) {
-        buffer.writeScalar(sampling.cubic.B);
-        buffer.writeScalar(sampling.cubic.C);
-    } else {
-        buffer.writeUInt((unsigned)sampling.filter);
-        buffer.writeUInt((unsigned)sampling.mipmap);
-    }
-}

src/shaders/BUILD.bazel

@@ -183,8 +183,6 @@ generated_cc_atom(
         "//src/core:SkOpts_hdr",
         "//src/core:SkRasterPipeline_hdr",
         "//src/core:SkReadBuffer_hdr",
-        "//src/core:SkSamplingPriv_hdr",
-        "//src/core:SkScopeExit_hdr",
         "//src/core:SkVM_hdr",
         "//src/core:SkWriteBuffer_hdr",
         "//src/gpu:GrColorInfo_hdr",

src/shaders/SkImageShader.cpp

@@ -17,8 +17,6 @@
 #include "src/core/SkOpts.h"
 #include "src/core/SkRasterPipeline.h"
 #include "src/core/SkReadBuffer.h"
-#include "src/core/SkSamplingPriv.h"
-#include "src/core/SkScopeExit.h"
 #include "src/core/SkVM.h"
 #include "src/core/SkWriteBuffer.h"
 #include "src/image/SkImage_Base.h"
@@ -138,7 +136,7 @@ sk_sp<SkFlattenable> SkImageShader::CreateProc(SkReadBuffer& buffer) {
         // we just default to Nearest in sampling
     }
     if (readSampling) {
-        sampling = SkSamplingPriv::Read(buffer);
+        sampling = buffer.readSampling();
     }
 
     SkMatrix localMatrix;
@@ -162,7 +160,7 @@ void SkImageShader::flatten(SkWriteBuffer& buffer) const {
     buffer.writeUInt((unsigned)fTileModeX);
     buffer.writeUInt((unsigned)fTileModeY);
 
-    SkSamplingPriv::Write(buffer, fSampling);
+    buffer.writeSampling(fSampling);
 
     buffer.writeMatrix(this->getLocalMatrix());
     buffer.writeImage(fImage.get());

tools/debugger/BUILD.bazel

@@ -165,6 +165,7 @@ generated_cc_atom(
         "//include/core:SkPixmap_hdr",
         "//include/core:SkPoint3_hdr",
         "//include/core:SkRSXform_hdr",
+        "//include/core:SkSamplingOptions_hdr",
         "//include/core:SkSize_hdr",
         "//include/core:SkStream_hdr",
         "//include/core:SkTypeface_hdr",

tools/debugger/DrawCommand.cpp

@@ -25,6 +25,7 @@
 #include "include/core/SkPixmap.h"
 #include "include/core/SkPoint3.h"
 #include "include/core/SkRSXform.h"
+#include "include/core/SkSamplingOptions.h"
 #include "include/core/SkSize.h"
 #include "include/core/SkStream.h"
 #include "include/core/SkTypeface.h"
@@ -74,6 +75,7 @@ class GrDirectContext;
 #define DEBUGCANVAS_ATTRIBUTE_COLOR "color"
 #define DEBUGCANVAS_ATTRIBUTE_ALPHA "alpha"
 #define DEBUGCANVAS_ATTRIBUTE_BLENDMODE "blendMode"
+#define DEBUGCANVAS_ATTRIBUTE_SAMPLING "sampling"
 #define DEBUGCANVAS_ATTRIBUTE_STYLE "style"
 #define DEBUGCANVAS_ATTRIBUTE_STROKEWIDTH "strokeWidth"
 #define DEBUGCANVAS_ATTRIBUTE_STROKEMITER "strokeMiter"
@@ -568,6 +570,16 @@ void DrawCommand::MakeJsonRegion(SkJSONWriter& writer, const SkRegion& region) {
     MakeJsonPath(writer, path);
 }
 
+void DrawCommand::MakeJsonSampling(SkJSONWriter& writer, const SkSamplingOptions& sampling) {
+    writer.beginObject();
+    writer.appendBool("useCubic", sampling.useCubic);
+    writer.appendS32("filter", (int)sampling.filter);
+    writer.appendS32("mipmap", (int)sampling.mipmap);
+    writer.appendFloat("cubic.B", sampling.cubic.B);
+    writer.appendFloat("cubic.C", sampling.cubic.C);
+    writer.endObject();
+}
+
 static const char* clipop_name(SkClipOp op) {
     switch (op) {
         case SkClipOp::kDifference: return DEBUGCANVAS_CLIPOP_DIFFERENCE;
@@ -1213,6 +1225,8 @@ void DrawImageCommand::toJSON(SkJSONWriter& writer, UrlDataManager& urlDataManag
         writer.appendName(DEBUGCANVAS_ATTRIBUTE_PAINT);
         MakeJsonPaint(writer, *fPaint, urlDataManager);
     }
+    writer.appendName(DEBUGCANVAS_ATTRIBUTE_SAMPLING);
+    MakeJsonSampling(writer, fSampling);
 
     writer.appendU32(DEBUGCANVAS_ATTRIBUTE_UNIQUE_ID, fImage->uniqueID());
     writer.appendS32(DEBUGCANVAS_ATTRIBUTE_WIDTH, fImage->width());
@@ -1318,6 +1332,8 @@ void DrawImageRectCommand::toJSON(SkJSONWriter& writer, UrlDataManager& urlDataM
     MakeJsonRect(writer, fSrc);
     writer.appendName(DEBUGCANVAS_ATTRIBUTE_DST);
     MakeJsonRect(writer, fDst);
+    writer.appendName(DEBUGCANVAS_ATTRIBUTE_SAMPLING);
+    MakeJsonSampling(writer, fSampling);
     if (fPaint.isValid()) {
         writer.appendName(DEBUGCANVAS_ATTRIBUTE_PAINT);
         MakeJsonPaint(writer, *fPaint, urlDataManager);
@@ -1377,6 +1393,8 @@ void DrawImageRectLayerCommand::toJSON(SkJSONWriter& writer, UrlDataManager& url
 
     writer.appendName(DEBUGCANVAS_ATTRIBUTE_DST);
     MakeJsonRect(writer, fDst);
+    writer.appendName(DEBUGCANVAS_ATTRIBUTE_SAMPLING);
+    MakeJsonSampling(writer, fSampling);
     if (fPaint.isValid()) {
         writer.appendName(DEBUGCANVAS_ATTRIBUTE_PAINT);
         MakeJsonPaint(writer, *fPaint, urlDataManager);

tools/debugger/DrawCommand.h

@@ -126,6 +126,7 @@ public:
     static void MakeJsonMatrix44(SkJSONWriter&, const SkM44&);
     static void MakeJsonPath(SkJSONWriter&, const SkPath& path);
     static void MakeJsonRegion(SkJSONWriter&, const SkRegion& region);
+    static void MakeJsonSampling(SkJSONWriter&, const SkSamplingOptions& sampling);
     static void MakeJsonPaint(SkJSONWriter&, const SkPaint& paint, UrlDataManager& urlDataManager);
     static void MakeJsonLattice(SkJSONWriter&, const SkCanvas::Lattice& lattice);
 

tools/debugger/JsonWriteBuffer.cpp

@@ -192,6 +192,11 @@ void JsonWriteBuffer::writePath(const SkPath& path) {
     DrawCommand::MakeJsonPath(*fWriter, path);
 }
 
+void JsonWriteBuffer::writeSampling(const SkSamplingOptions& sampling) {
+    this->append("sampling");
+    DrawCommand::MakeJsonSampling(*fWriter, sampling);
+}
+
 size_t JsonWriteBuffer::writeStream(SkStream* stream, size_t length) {
     // Contents not supported
     this->append("stream");

tools/debugger/JsonWriteBuffer.h

@@ -21,6 +21,7 @@ class SkMatrix;
 class SkPaint;
 class SkPath;
 class SkRegion;
+struct SkSamplingOptions;
 class SkStream;
 class SkTypeface;
 class UrlDataManager;
@@ -58,6 +59,7 @@ public:
     void   writeRect(const SkRect& rect) override;
     void   writeRegion(const SkRegion& region) override;
     void   writePath(const SkPath& path) override;
+    void   writeSampling(const SkSamplingOptions&) override;
     size_t writeStream(SkStream* stream, size_t length) override;
     void   writeImage(const SkImage*) override;
     void   writeTypeface(SkTypeface* typeface) override;

tools/viewer/SKPSlide.cpp

@@ -46,7 +46,7 @@ void SKPSlide::load(SkScalar, SkScalar) {
     fStream->rewind();
     fPic = SkPicture::MakeFromStream(fStream.get());
     if (!fPic) {
-        SkDebugf("Could parse SkPicture from skp stream for slide %s.\n", fName.c_str());
+        SkDebugf("Could not parse SkPicture from skp stream for slide %s.\n", fName.c_str());
         return;
     }
     fCullRect = fPic->cullRect().roundOut();