Reland "[skottie] Add image sampling and transform options"

fmalita · Skia Commit-Bot · commit 38921cafe1bb · 2020-12-03T19:21:57.000Z
This reverts commit b81842a. Reason for revert: reland with fixes Original change's description: > Revert "[skottie] Add image sampling and transform options" > > This reverts commit 2f24405. > > Reason for revert: broke Win/shared > > Original change's description: > > [skottie] Add image sampling and transform options > > > > Expand the SkImageAsset API to support controlling sampling options and > > pass an additional transform. > > > > Bug: skia:10944, skia:10942 > > Change-Id: I7bad0b2ab58ed40fe4b425de0eb6970a4c7d7117 > > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340097 > > Commit-Queue: Florin Malita <fmalita@chromium.org> > > Commit-Queue: Florin Malita <fmalita@google.com> > > Reviewed-by: Mike Reed <reed@google.com> > > TBR=fmalita@chromium.org,fmalita@google.com,reed@google.com,aparchur@google.com > > Change-Id: I59d4161356ffdc20588f1bd3beb33c54e44807a2 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: skia:10944 > Bug: skia:10942 > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340619 > Reviewed-by: Florin Malita <fmalita@google.com> > Commit-Queue: Florin Malita <fmalita@google.com> TBR=fmalita@chromium.org,fmalita@google.com,reed@google.com,aparchur@google.com # Not skipping CQ checks because this is a reland. Bug: skia:10944 Bug: skia:10942 Change-Id: I91892f4db6366ceb07d1a49a7bc54da17cea5399 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340657 Reviewed-by: Mike Reed <reed@google.com> Reviewed-by: Brian Osman <brianosman@google.com> Commit-Queue: Florin Malita <fmalita@chromium.org> Commit-Queue: Florin Malita <fmalita@google.com>
diff --git a/include/core/SkSamplingOptions.h b/include/core/SkSamplingOptions.h
@@ -66,6 +66,15 @@ struct SK_API SkSamplingOptions {
         , cubic(c) {}
 
     explicit SkSamplingOptions(SkFilterQuality);
+
+    bool operator==(const SkSamplingOptions& other) const {
+        return useCubic == other.useCubic
+            && cubic.B  == other.cubic.B
+            && cubic.C  == other.cubic.C
+            && filter   == other.filter
+            && mipmap   == other.mipmap;
+    }
+    bool operator!=(const SkSamplingOptions& other) const { return !(*this == other); }
 };
 
 #endif
diff --git a/modules/skottie/BUILD.gn b/modules/skottie/BUILD.gn
@@ -55,6 +55,7 @@ if (skia_enable_skottie) {
         sources = [
           "src/SkottieTest.cpp",
           "tests/AudioLayer.cpp",
+          "tests/Image.cpp",
           "tests/Keyframe.cpp",
           "tests/Text.cpp",
         ]
diff --git a/modules/skottie/src/layers/FootageLayer.cpp b/modules/skottie/src/layers/FootageLayer.cpp
@@ -17,11 +17,14 @@ namespace internal {
 
 namespace  {
 
-SkMatrix image_matrix(const sk_sp<SkImage>& image, const SkISize& dest_size) {
-    return image ? SkMatrix::MakeRectToRect(SkRect::Make(image->bounds()),
-                                            SkRect::Make(dest_size),
-                                            SkMatrix::kCenter_ScaleToFit)
-                 : SkMatrix::I();
+SkMatrix image_matrix(const ImageAsset::FrameData& frame_data, const SkISize& dest_size) {
+    if (!frame_data.image) {
+        return SkMatrix::I();
+    }
+
+    return frame_data.matrix * SkMatrix::MakeRectToRect(SkRect::Make(frame_data.image->bounds()),
+                                                        SkRect::Make(dest_size),
+                                                        SkMatrix::kCenter_ScaleToFit);
 }
 
 class FootageAnimator final : public Animator {
@@ -45,10 +48,15 @@ class FootageAnimator final : public Animator {
             return false;
         }
 
-        auto frame = fAsset->getFrame((t + fTimeBias) * fTimeScale);
-        if (frame != fImageNode->getImage()) {
-            fImageTransformNode->setMatrix(image_matrix(frame, fAssetSize));
-            fImageNode->setImage(std::move(frame));
+        auto frame_data = fAsset->getFrameData((t + fTimeBias) * fTimeScale);
+        const auto m = image_matrix(frame_data, fAssetSize);
+        if (frame_data.image    != fImageNode->getImage() ||
+            frame_data.sampling != fImageNode->getSamplingOptions() ||
+            m                   != fImageTransformNode->getMatrix()) {
+
+            fImageNode->setImage(std::move(frame_data.image));
+            fImageNode->setSamplingOptions(frame_data.sampling);
+            fImageTransformNode->setMatrix(m);
             return true;
         }
 
@@ -102,7 +110,6 @@ sk_sp<sksg::RenderNode> AnimationBuilder::attachFootageAsset(const skjson::Objec
     SkASSERT(asset_info->fAsset);
 
     auto image_node = sksg::Image::Make(nullptr);
-    image_node->setQuality(kMedium_SkFilterQuality);
 
     // Optional image transform (mapping the intrinsic image size to declared asset size).
     sk_sp<sksg::Matrix<SkMatrix>> image_transform;
@@ -121,17 +128,19 @@ sk_sp<sksg::RenderNode> AnimationBuilder::attachFootageAsset(const skjson::Objec
                                                                      1 / fFrameRate));
     } else {
         // No animator needed, resolve the (only) frame upfront.
-        auto frame = asset_info->fAsset->getFrame(0);
-        if (!frame) {
+        auto frame_data = asset_info->fAsset->getFrameData(0);
+        if (!frame_data.image) {
             this->log(Logger::Level::kError, nullptr, "Could not load single-frame image asset.");
             return nullptr;
         }
 
-        if (frame->bounds().size() != asset_info->fSize) {
-            image_transform = sksg::Matrix<SkMatrix>::Make(image_matrix(frame, asset_info->fSize));
+        const auto m = image_matrix(frame_data, asset_info->fSize);
+        if (!m.isIdentity()) {
+            image_transform = sksg::Matrix<SkMatrix>::Make(m);
         }
 
-        image_node->setImage(std::move(frame));
+        image_node->setImage(std::move(frame_data.image));
+        image_node->setSamplingOptions(frame_data.sampling);
     }
 
     // Image layers are sized explicitly.
diff --git a/modules/skottie/tests/Image.cpp b/modules/skottie/tests/Image.cpp
@@ -0,0 +1,133 @@
+/*
+ * Copyright 2020 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "include/core/SkSurface.h"
+#include "modules/skottie/include/Skottie.h"
+#include "tests/Test.h"
+
+using namespace skottie;
+
+DEF_TEST(Skottie_Image_CustomTransform, r) {
+    static constexpr char json[] =
+        R"({
+             "v": "5.2.1",
+             "w": 100,
+             "h": 100,
+             "fr": 10,
+             "ip": 0,
+             "op": 100,
+             "assets": [{
+               "id": "img_0",
+               "p" : "img_0.png",
+               "u" : "images/",
+               "w" : 100,
+               "h" :  50
+             }],
+             "layers": [
+               {
+                 "ip": 0,
+                 "op": 100,
+                 "ty": 2,
+                 "refId": "img_0",
+                 "ks": {
+                   "p": { "a": 0, "k": [0,25] }
+                 }
+               }
+             ]
+           })";
+
+    SkMemoryStream stream(json, strlen(json));
+
+    static const struct TestData {
+        float    t;
+        SkMatrix m;
+        SkColor  c[5]; // expected color samples at center/L/T/R/B
+    } tests[] {
+        { 0, SkMatrix::I(),
+            {0xffff0000, 0xffff0000, 0xff00ff00, 0xffff0000, 0xff00ff00}},
+        { 1, SkMatrix::Translate(50,25) * SkMatrix::Scale(.5f,.5f) * SkMatrix::Translate(-50,-25),
+            {0xffff0000, 0xff00ff00, 0xff00ff00, 0xff00ff00, 0xff00ff00}},
+        { 2, SkMatrix::Translate(-50, 0),
+            {0xff00ff00, 0xffff0000, 0xff00ff00, 0xff00ff00, 0xff00ff00}},
+        { 3, SkMatrix::Translate(0, -25),
+            {0xff00ff00, 0xff00ff00, 0xffff0000, 0xff00ff00, 0xff00ff00}},
+        { 4, SkMatrix::Translate(50, 0),
+            {0xffff0000, 0xff00ff00, 0xff00ff00, 0xffff0000, 0xff00ff00}},
+        { 5, SkMatrix::Translate(0, 25),
+            {0xffff0000, 0xffff0000, 0xff00ff00, 0xffff0000, 0xffff0000}},
+    };
+
+    class TestImageAsset final : public ImageAsset {
+    public:
+        TestImageAsset(const TestData* tst, skiatest::Reporter* r)
+            : fTest(tst)
+            , fReporter(r) {
+
+            auto surf = SkSurface::MakeRasterN32Premul(200, 100);
+            surf->getCanvas()->drawColor(0xffff0000);
+            fImage = surf->makeImageSnapshot();
+        }
+
+    private:
+        bool isMultiFrame() override { return true; }
+
+        FrameData getFrameData(float t) override {
+            REPORTER_ASSERT(fReporter, t == fTest->t);
+
+            return { fImage, SkSamplingOptions(), fTest++->m };
+        }
+
+        sk_sp<SkImage>      fImage;
+        const TestData*     fTest;
+        skiatest::Reporter* fReporter;
+    };
+
+    class TestResourceProvider final : public ResourceProvider {
+    public:
+        TestResourceProvider(const TestData* tst, skiatest::Reporter* r)
+            : fTest(tst)
+            , fReporter(r) {}
+
+    private:
+        sk_sp<ImageAsset> loadImageAsset(const char[], const char[], const char[]) const override {
+            return sk_make_sp<TestImageAsset>(fTest, fReporter);
+        }
+
+        const TestData*      fTest;
+        skiatest::Reporter*  fReporter;
+    };
+
+    auto anim = Animation::Builder()
+                    .setResourceProvider(sk_make_sp<TestResourceProvider>(tests, r))
+                    .make(&stream);
+
+    REPORTER_ASSERT(r, anim);
+
+    static constexpr SkSize render_size{100, 100};
+    auto surf = SkSurface::MakeRasterN32Premul(render_size.width(), render_size.height());
+    auto rect = SkRect::MakeSize(render_size);
+
+    SkPixmap pmap;
+    surf->peekPixels(&pmap);
+
+    for (const auto& tst : tests) {
+        surf->getCanvas()->clear(0xff00ff00);
+        anim->seekFrameTime(tst.t);
+        anim->render(surf->getCanvas(), &rect);
+
+        REPORTER_ASSERT(r,
+            tst.c[0] == pmap.getColor(render_size.width() / 2, render_size.height() / 2));
+        REPORTER_ASSERT(r,
+            tst.c[1] == pmap.getColor(1                      , render_size.height() / 2));
+        REPORTER_ASSERT(r,
+            tst.c[2] == pmap.getColor(render_size.width() / 2, 1));
+        REPORTER_ASSERT(r,
+            tst.c[3] == pmap.getColor(render_size.width() - 1, render_size.height() / 2));
+        REPORTER_ASSERT(r,
+            tst.c[4] == pmap.getColor(render_size.width() /2 , render_size.height() - 1));
+    }
+}
diff --git a/modules/skresources/BUILD.gn b/modules/skresources/BUILD.gn
@@ -14,7 +14,10 @@ static_library("skresources") {
   public_configs = [ ":public_config" ]
   public = skia_skresources_public
   sources = skia_skresources_sources
-  configs += [ "../../:skia_private" ]
+  configs += [
+    "../../:skia_private",
+    "../../:skia_library",
+  ]
   deps = [
     "../..:skia",
     "../../experimental/ffmpeg:video_decoder",
diff --git a/modules/skresources/include/SkResources.h b/modules/skresources/include/SkResources.h
@@ -9,7 +9,9 @@
 #define SkResources_DEFINED
 
 #include "include/core/SkData.h"
+#include "include/core/SkMatrix.h"
 #include "include/core/SkRefCnt.h"
+#include "include/core/SkSamplingOptions.h"
 #include "include/core/SkString.h"
 #include "include/core/SkTypeface.h"
 #include "include/core/SkTypes.h"
@@ -34,17 +36,41 @@ class SK_API ImageAsset : public SkRefCnt {
     virtual bool isMultiFrame() = 0;
 
     /**
+     * DEPRECATED: override getFrameData() instead.
+     *
      * Returns the SkImage for a given frame.
      *
-     * If the image asset is static, getImage() is only called once, at animation load time.
+     * If the image asset is static, getFrame() is only called once, at animation load time.
+     * Otherwise, this gets invoked every time the animation time is adjusted (on every seek).
+     *
+     * Embedders should cache and serve the same SkImage whenever possible, for efficiency.
+     *
+     * @param t   Frame time code, in seconds, relative to the image layer timeline origin
+     *            (in-point).
+     */
+    virtual sk_sp<SkImage> getFrame(float t);
+
+    struct FrameData {
+        // SkImage payload.
+        sk_sp<SkImage>    image;
+        // Resampling parameters.
+        SkSamplingOptions sampling;
+        // Additional image transform to be applied before AE scaling rules.
+        SkMatrix          matrix = SkMatrix::I();
+    };
+
+    /**
+     * Returns the payload for a given frame.
+     *
+     * If the image asset is static, getFrameData() is only called once, at animation load time.
      * Otherwise, this gets invoked every time the animation time is adjusted (on every seek).
      *
      * Embedders should cache and serve the same SkImage whenever possible, for efficiency.
      *
      * @param t   Frame time code, in seconds, relative to the image layer timeline origin
      *            (in-point).
      */
-    virtual sk_sp<SkImage> getFrame(float t) = 0;
+    virtual FrameData getFrameData(float t);
 };
 
 class MultiFrameImageAsset final : public ImageAsset {
diff --git a/modules/skresources/src/SkResources.cpp b/modules/skresources/src/SkResources.cpp
@@ -88,6 +88,19 @@ class VideoAsset final : public ImageAsset {
 
 } // namespace
 
+sk_sp<SkImage> ImageAsset::getFrame(float t) {
+    return nullptr;
+}
+
+ImageAsset::FrameData ImageAsset::getFrameData(float t) {
+    // legacy behavior
+    return {
+        this->getFrame(t),
+        SkSamplingOptions(SkFilterMode::kLinear, SkMipmapMode::kNearest),
+        SkMatrix::I(),
+    };
+}
+
 sk_sp<MultiFrameImageAsset> MultiFrameImageAsset::Make(sk_sp<SkData> data, bool predecode) {
     if (auto codec = SkCodec::MakeFromData(std::move(data))) {
         return sk_sp<MultiFrameImageAsset>(
diff --git a/modules/sksg/include/SkSGImage.h b/modules/sksg/include/SkSGImage.h
@@ -10,7 +10,7 @@
 
 #include "modules/sksg/include/SkSGRenderNode.h"
 
-#include "include/core/SkFilterQuality.h"
+#include "include/core/SkSamplingOptions.h"
 
 class SkImage;
 
@@ -26,9 +26,9 @@ class Image final : public RenderNode {
         return sk_sp<Image>(new Image(std::move(image)));
     }
 
-    SG_ATTRIBUTE(Image,     sk_sp<SkImage> , fImage    )
-    SG_ATTRIBUTE(Quality  , SkFilterQuality, fQuality  )
-    SG_ATTRIBUTE(AntiAlias, bool           , fAntiAlias)
+    SG_ATTRIBUTE(Image          , sk_sp<SkImage>   , fImage          )
+    SG_ATTRIBUTE(SamplingOptions, SkSamplingOptions, fSamplingOptions)
+    SG_ATTRIBUTE(AntiAlias      , bool             , fAntiAlias      )
 
 protected:
     explicit Image(sk_sp<SkImage>);
@@ -39,9 +39,9 @@ class Image final : public RenderNode {
     SkRect onRevalidate(InvalidationController*, const SkMatrix&) override;
 
 private:
-    sk_sp<SkImage>  fImage;
-    SkFilterQuality fQuality   = kNone_SkFilterQuality;
-    bool            fAntiAlias = true;
+    SkSamplingOptions fSamplingOptions;
+    sk_sp<SkImage>    fImage;
+    bool              fAntiAlias = true;
 
     using INHERITED = RenderNode;
 };
diff --git a/modules/sksg/src/SkSGImage.cpp b/modules/sksg/src/SkSGImage.cpp
@@ -19,9 +19,19 @@ void Image::onRender(SkCanvas* canvas, const RenderContext* ctx) const {
         return;
     }
 
+    // Ignoring cubic params and trilerp for now.
+    // TODO: convert to drawImage(sampling options) when available.
+    auto legacy_quality = [](const SkSamplingOptions& sampling) {
+        return
+            sampling.useCubic                         ? SkFilterQuality::kHigh_SkFilterQuality :
+            sampling.filter == SkFilterMode::kNearest ? SkFilterQuality::kNone_SkFilterQuality :
+            sampling.mipmap == SkMipmapMode::kNone    ? SkFilterQuality::kLow_SkFilterQuality  :
+                                                        SkFilterQuality::kMedium_SkFilterQuality;
+    };
+
     SkPaint paint;
     paint.setAntiAlias(fAntiAlias);
-    paint.setFilterQuality(fQuality);
+    paint.setFilterQuality(legacy_quality(fSamplingOptions));
 
     sksg::RenderNode::ScopedRenderContext local_ctx(canvas, ctx);
     if (ctx) {

Original file line number	Diff line number	Diff line change
`@@ -55,6 +55,7 @@ if (skia_enable_skottie) {`
`55`	`55`	`sources = [`
`56`	`56`	`"src/SkottieTest.cpp",`
`57`	`57`	`"tests/AudioLayer.cpp",`
	`58`	`+ "tests/Image.cpp",`
`58`	`59`	`"tests/Keyframe.cpp",`
`59`	`60`	`"tests/Text.cpp",`
`60`	`61`	`]`