From b67ca7473fa2a78543c978c06dc004360663ad39 Mon Sep 17 00:00:00 2001 From: Florin Malita Date: Mon, 27 Jan 2020 16:13:18 -0500 Subject: [PATCH] [skottie] Switch SkMatrix44 -> SkM44 Change-Id: If58516a0dad5b51debf497b069713fb6f37999e6 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/266940 Reviewed-by: Mike Reed Commit-Queue: Florin Malita --- modules/skottie/src/Camera.cpp | 67 +++++++++++----------------- modules/skottie/src/Camera.h | 4 +- modules/skottie/src/SkottieValue.cpp | 6 +-- modules/skottie/src/Transform.cpp | 42 +++++++---------- modules/skottie/src/Transform.h | 16 +++---- modules/sksg/include/SkSGTransform.h | 18 ++++---- modules/sksg/src/SkSGTransform.cpp | 64 +++++++++++++++++++------- modules/sksg/src/SkSGTransformPriv.h | 8 ++-- 8 files changed, 115 insertions(+), 110 deletions(-) diff --git a/modules/skottie/src/Camera.cpp b/modules/skottie/src/Camera.cpp index 53f285605a..2f41449bf6 100644 --- a/modules/skottie/src/Camera.cpp +++ b/modules/skottie/src/Camera.cpp @@ -7,7 +7,6 @@ #include "modules/skottie/src/Camera.h" -#include "include/utils/Sk3D.h" #include "modules/skottie/src/SkottieJson.h" #include "modules/skottie/src/SkottiePriv.h" #include "modules/sksg/include/SkSGTransform.h" @@ -17,31 +16,20 @@ namespace internal { namespace { -SkMatrix44 ComputeCameraMatrix(const SkPoint3& position, - const SkPoint3& poi, - const SkPoint3& rotation, - const SkSize& viewport_size, - float zoom) { - const auto pos = SkPoint3{ position.fX, position.fY, -position.fZ }, - up = SkPoint3{ 0, 1, 0 }; +SkM44 ComputeCameraMatrix(const SkV3& position, + const SkV3& poi, + const SkV3& rotation, + const SkSize& viewport_size, + float zoom) { // Initial camera vector. - SkMatrix44 cam_t; - Sk3LookAt(&cam_t, pos, poi, up); - - // Rotation origin is camera position. - { - SkMatrix44 rot; - rot.setRotateDegreesAbout(1, 0, 0, rotation.fX); - cam_t.postConcat(rot); - rot.setRotateDegreesAbout(0, 1, 0, rotation.fY); - cam_t.postConcat(rot); - rot.setRotateDegreesAbout(0, 0, 1, -rotation.fZ); - cam_t.postConcat(rot); - } - - // Flip world Z, as it is opposite of what Sk3D expects. - cam_t.preScale(1, 1, -1); + const auto cam_t = SkM44::Rotate({0, 0, 1}, SkDegreesToRadians(-rotation.z)) + * SkM44::Rotate({0, 1, 0}, SkDegreesToRadians( rotation.y)) + * SkM44::Rotate({1, 0, 0}, SkDegreesToRadians( rotation.x)) + * Sk3LookAt({ position.x, position.y, -position.z }, + { poi.x, poi.y, poi.z }, + { 0, 1, 0 }) + * SkM44::Scale(1, 1, -1); // View parameters: // @@ -52,16 +40,13 @@ SkMatrix44 ComputeCameraMatrix(const SkPoint3& position, view_distance = zoom, view_angle = std::atan(sk_ieee_float_divide(view_size * 0.5f, view_distance)); - SkMatrix44 persp_t; - Sk3Perspective(&persp_t, 0, view_distance, 2 * view_angle); - persp_t.postScale(view_size * 0.5f, view_size * 0.5f, 1); + const auto persp_t = SkM44::Scale(view_size * 0.5f, view_size * 0.5f, 1) + * Sk3Perspective(0, view_distance, 2 * view_angle); - SkMatrix44 m; - m.setTranslate(viewport_size.width() * 0.5f, viewport_size.height() * 0.5f, 0); - m.preConcat(persp_t); - m.preConcat(cam_t); - - return m; + return SkM44::Translate(viewport_size.width() * 0.5f, + viewport_size.height() * 0.5f, + 0) + * persp_t * cam_t; } } // namespace @@ -82,7 +67,7 @@ CameraAdaper::CameraAdaper(const skjson::ObjectValue& jlayer, CameraAdaper::~CameraAdaper() = default; -SkMatrix44 CameraAdaper::totalMatrix() const { +SkM44 CameraAdaper::totalMatrix() const { // Camera parameters: // // * location -> position attribute @@ -98,7 +83,7 @@ SkMatrix44 CameraAdaper::totalMatrix() const { fZoom); } -SkPoint3 CameraAdaper::poi(const SkPoint3& pos) const { +SkV3 CameraAdaper::poi(const SkV3& pos) const { // AE supports two camera types: // // - one-node camera: does not auto-orient, and starts off perpendicular @@ -108,12 +93,12 @@ SkPoint3 CameraAdaper::poi(const SkPoint3& pos) const { // and auto-orients to point in its direction. if (fType == CameraType::kOneNode) { - return { pos.fX, pos.fY, -pos.fZ - 1}; + return { pos.x, pos.y, -pos.z - 1}; } const auto ap = this->anchor_point(); - return { ap.fX, ap.fY, -ap.fZ }; + return { ap.x, ap.y, -ap.z }; } sk_sp CameraAdaper::DefaultCameraTransform(const SkSize& viewport_size) { @@ -122,11 +107,11 @@ sk_sp CameraAdaper::DefaultCameraTransform(const SkSize& viewpo static constexpr float kDefaultAEZoom = 879.13f; - const SkPoint3 pos = { center.fX, center.fY, -kDefaultAEZoom }, - poi = { pos.fX, pos.fY, -pos.fZ - 1 }, - rot = { 0, 0, 0 }; + const SkV3 pos = { center.fX, center.fY, -kDefaultAEZoom }, + poi = { pos.x, pos.y, -pos.z - 1 }, + rot = { 0, 0, 0 }; - return sksg::Matrix::Make( + return sksg::Matrix::Make( ComputeCameraMatrix(pos, poi, rot, viewport_size, kDefaultAEZoom)); } diff --git a/modules/skottie/src/Camera.h b/modules/skottie/src/Camera.h index ba6668aeaf..36090483b9 100644 --- a/modules/skottie/src/Camera.h +++ b/modules/skottie/src/Camera.h @@ -24,7 +24,7 @@ public: // Used in the absence of an explicit camera layer. static sk_sp DefaultCameraTransform(const SkSize& viewport_size); - SkMatrix44 totalMatrix() const override; + SkM44 totalMatrix() const override; private: enum class CameraType { @@ -32,7 +32,7 @@ private: kTwoNode, // explicitly facing a POI (the anchor point), auto-orients }; - SkPoint3 poi(const SkPoint3& pos) const; + SkV3 poi(const SkV3& pos) const; const SkSize fViewportSize; const CameraType fType; diff --git a/modules/skottie/src/SkottieValue.cpp b/modules/skottie/src/SkottieValue.cpp index d9f5dbb4d1..ba833e78da 100644 --- a/modules/skottie/src/SkottieValue.cpp +++ b/modules/skottie/src/SkottieValue.cpp @@ -9,8 +9,8 @@ #include "include/core/SkColor.h" #include "include/core/SkPoint.h" -#include "include/core/SkPoint3.h" #include "include/core/SkSize.h" +#include "include/private/SkM44.h" #include "include/private/SkNx.h" #include "modules/skottie/src/SkottieJson.h" #include "modules/skottie/src/SkottiePriv.h" @@ -103,9 +103,9 @@ SkPoint ValueTraits::As(const VectorValue& vec) { template <> template <> -SkPoint3 ValueTraits::As(const VectorValue& vec) { +SkV3 ValueTraits::As(const VectorValue& vec) { // best effort to turn this into a 3D point - return SkPoint3 { + return SkV3 { vec.size() > 0 ? vec[0] : 0, vec.size() > 1 ? vec[1] : 0, vec.size() > 2 ? vec[2] : 0, diff --git a/modules/skottie/src/Transform.cpp b/modules/skottie/src/Transform.cpp index e0a666d1ca..c36056bc71 100644 --- a/modules/skottie/src/Transform.cpp +++ b/modules/skottie/src/Transform.cpp @@ -128,7 +128,7 @@ sk_sp AnimationBuilder::attachMatrix2D(const skjson::ObjectValu TransformAdapter3D::TransformAdapter3D(const skjson::ObjectValue& jtransform, const AnimationBuilder& abuilder) - : INHERITED(sksg::Matrix::Make(SkMatrix44::I())) { + : INHERITED(sksg::Matrix::Make(SkM44())) { this->bind(abuilder, jtransform["a"], fAnchorPoint); this->bind(abuilder, jtransform["p"], fPosition); @@ -148,40 +148,31 @@ void TransformAdapter3D::onSync() { this->node()->setMatrix(this->totalMatrix()); } -SkPoint3 TransformAdapter3D::anchor_point() const { - return ValueTraits::As(fAnchorPoint); +SkV3 TransformAdapter3D::anchor_point() const { + return ValueTraits::As(fAnchorPoint); } -SkPoint3 TransformAdapter3D::position() const { - return ValueTraits::As(fPosition); +SkV3 TransformAdapter3D::position() const { + return ValueTraits::As(fPosition); } -SkVector3 TransformAdapter3D::rotation() const { +SkV3 TransformAdapter3D::rotation() const { // orientation and axis-wise rotation map onto the same property. - return ValueTraits::As(fOrientation) + SkPoint3{ fRx, fRy, fRz }; + return ValueTraits::As(fOrientation) + SkV3{ fRx, fRy, fRz }; } -SkMatrix44 TransformAdapter3D::totalMatrix() const { +SkM44 TransformAdapter3D::totalMatrix() const { const auto anchor_point = this->anchor_point(), positon = this->position(), - scale = ValueTraits::As(fScale), + scale = ValueTraits::As(fScale), rotation = this->rotation(); - SkMatrix44 m; - m.setTranslate(-anchor_point.fX, -anchor_point.fY, -anchor_point.fZ); - m.postScale(scale.fX / 100, scale.fY / 100, scale.fZ / 100); - - SkMatrix44 r; - r.setRotateDegreesAbout(0, 0, 1, rotation.fZ); - m.postConcat(r); - r.setRotateDegreesAbout(0, 1, 0, rotation.fY); - m.postConcat(r); - r.setRotateDegreesAbout(1, 0, 0, rotation.fX); - m.postConcat(r); - - m.postTranslate(positon.fX, positon.fY, positon.fZ); - - return m; + return SkM44::Translate(positon.x, positon.y, positon.z) + * SkM44::Rotate({ 1, 0, 0 }, SkDegreesToRadians(rotation.x)) + * SkM44::Rotate({ 0, 1, 0 }, SkDegreesToRadians(rotation.y)) + * SkM44::Rotate({ 0, 0, 1 }, SkDegreesToRadians(rotation.z)) + * SkM44::Scale(scale.x / 100, scale.y / 100, scale.z / 100) + * SkM44::Translate(-anchor_point.x, -anchor_point.y, -anchor_point.z); } sk_sp AnimationBuilder::attachMatrix3D(const skjson::ObjectValue& jtransform, @@ -190,7 +181,8 @@ sk_sp AnimationBuilder::attachMatrix3D(const skjson::ObjectValu SkASSERT(adapter); if (adapter->isStatic()) { - if (adapter->totalMatrix().isIdentity()) { + // TODO: SkM44::isIdentity? + if (adapter->totalMatrix() == SkM44()) { // The transform has no observable effects - we can discard. return parent; } diff --git a/modules/skottie/src/Transform.h b/modules/skottie/src/Transform.h index 3f26fde17b..e5abf94589 100644 --- a/modules/skottie/src/Transform.h +++ b/modules/skottie/src/Transform.h @@ -11,9 +11,8 @@ #include "modules/skottie/src/SkottieAdapter.h" #include "include/core/SkMatrix.h" -#include "include/core/SkMatrix44.h" #include "include/core/SkPoint.h" -#include "include/core/SkPoint3.h" +#include "include/private/SkM44.h" #include "modules/skottie/src/Adapter.h" namespace skjson { @@ -72,18 +71,17 @@ private: using INHERITED = DiscardableAdapterBase>; }; -class TransformAdapter3D : public DiscardableAdapterBase> { +class TransformAdapter3D : public DiscardableAdapterBase> { public: TransformAdapter3D(const skjson::ObjectValue&, const AnimationBuilder&); ~TransformAdapter3D() override; - virtual SkMatrix44 totalMatrix() const; + virtual SkM44 totalMatrix() const; protected: - SkPoint3 anchor_point() const; - SkPoint3 position() const; - SkVector3 rotation() const; + SkV3 anchor_point() const; + SkV3 position() const; + SkV3 rotation() const; private: void onSync() final; @@ -96,7 +94,7 @@ private: fRy = 0, fRz = 0; - using INHERITED = DiscardableAdapterBase>; + using INHERITED = DiscardableAdapterBase>; }; } // namespace internal diff --git a/modules/sksg/include/SkSGTransform.h b/modules/sksg/include/SkSGTransform.h index da6b3c5bb8..c835eae2e8 100644 --- a/modules/sksg/include/SkSGTransform.h +++ b/modules/sksg/include/SkSGTransform.h @@ -11,7 +11,7 @@ #include "modules/sksg/include/SkSGEffectNode.h" #include "include/core/SkMatrix.h" -#include "include/core/SkMatrix44.h" +#include "include/private/SkM44.h" namespace sksg { @@ -31,8 +31,8 @@ protected: virtual bool is44() const = 0; - virtual SkMatrix asMatrix () const = 0; - virtual SkMatrix44 asMatrix44() const = 0; + virtual SkMatrix asMatrix() const = 0; + virtual SkM44 asM44 () const = 0; private: friend class TransformPriv; @@ -43,7 +43,7 @@ private: /** * Concrete, matrix-backed Transform. * - * Supported instantiations: SkMatrix, SkMatrix44. + * Supported instantiations: SkMatrix, SkM44. * * Sample use: * @@ -55,8 +55,8 @@ private: template class Matrix final : public Transform { public: - template ::value || - std::is_same::value>> + template ::value || + std::is_same::value>> static sk_sp Make(const T& m) { return sk_sp(new Matrix(m)); } SG_ATTRIBUTE(Matrix, T, fMatrix) @@ -68,10 +68,10 @@ protected: return SkRect::MakeEmpty(); } - bool is44() const override { return std::is_same::value; } + bool is44() const override { return std::is_same::value; } - SkMatrix asMatrix () const override { return SkMatrix(fMatrix); } - SkMatrix44 asMatrix44() const override { return fMatrix; } + SkMatrix asMatrix() const override; + SkM44 asM44 () const override; private: T fMatrix; diff --git a/modules/sksg/src/SkSGTransform.cpp b/modules/sksg/src/SkSGTransform.cpp index 9131dc00b4..2fdf596801 100644 --- a/modules/sksg/src/SkSGTransform.cpp +++ b/modules/sksg/src/SkSGTransform.cpp @@ -14,11 +14,29 @@ namespace sksg { namespace { +template +SkMatrix AsSkMatrix(const T&); + +template <> +SkMatrix AsSkMatrix(const SkMatrix& m) { return m; } + +template <> +SkMatrix AsSkMatrix(const SkM44& m) { return m.asM33(); } + +template +SkM44 AsSkM44(const T&); + +template <> +SkM44 AsSkM44(const SkMatrix& m) { return SkM44(m); } + +template <> +SkM44 AsSkM44(const SkM44& m) { return m; } + template class Concat final : public Transform { public: - template ::value || - std::is_same::value >> + template ::value || + std::is_same::value >> Concat(sk_sp a, sk_sp b) : fA(std::move(a)), fB(std::move(b)) { SkASSERT(fA); @@ -43,16 +61,16 @@ protected: return SkRect::MakeEmpty(); } - bool is44() const override { return std::is_same::value; } + bool is44() const override { return std::is_same::value; } SkMatrix asMatrix() const override { SkASSERT(!this->hasInval()); - return SkMatrix(fComposed); + return AsSkMatrix(fComposed); } - SkMatrix44 asMatrix44() const override { + SkM44 asM44() const override { SkASSERT(!this->hasInval()); - return fComposed; + return AsSkM44(fComposed); } private: @@ -65,8 +83,8 @@ private: template class Inverse final : public Transform { public: - template ::value || - std::is_same::value >> + template ::value || + std::is_same::value >> explicit Inverse(sk_sp t) : fT(std::move(t)) { SkASSERT(fT); @@ -83,22 +101,22 @@ protected: fT->revalidate(ic, ctm); if (!TransformPriv::As(fT).invert(&fInverted)) { - fInverted.reset(); + fInverted.setIdentity(); } return SkRect::MakeEmpty(); } - bool is44() const override { return std::is_same::value; } + bool is44() const override { return std::is_same::value; } SkMatrix asMatrix() const override { SkASSERT(!this->hasInval()); - return SkMatrix(fInverted); + return AsSkMatrix(fInverted); } - SkMatrix44 asMatrix44() const override { + SkM44 asM44() const override { SkASSERT(!this->hasInval()); - return fInverted; + return AsSkM44(fInverted); } private: @@ -110,6 +128,18 @@ private: } // namespace +template <> +SkMatrix Matrix::asMatrix() const { return fMatrix; } + +template <> +SkM44 Matrix::asM44() const { return SkM44(fMatrix); } + +template <> +SkMatrix Matrix::asMatrix() const { return fMatrix.asM33(); } + +template <> +SkM44 Matrix::asM44() const { return fMatrix; } + // Transform nodes don't generate damage on their own, but via ancestor TransformEffects. Transform::Transform() : INHERITED(kBubbleDamage_Trait) {} @@ -123,8 +153,8 @@ sk_sp Transform::MakeConcat(sk_sp a, sk_sp b) { } return TransformPriv::Is44(a) || TransformPriv::Is44(b) - ? sk_sp(new Concat(std::move(a), std::move(b))) - : sk_sp(new Concat(std::move(a), std::move(b))); + ? sk_sp(new Concat(std::move(a), std::move(b))) + : sk_sp(new Concat(std::move(a), std::move(b))); } sk_sp Transform::MakeInverse(sk_sp t) { @@ -133,8 +163,8 @@ sk_sp Transform::MakeInverse(sk_sp t) { } return TransformPriv::Is44(t) - ? sk_sp(new Inverse(std::move(t))) - : sk_sp(new Inverse(std::move(t))); + ? sk_sp(new Inverse(std::move(t))) + : sk_sp(new Inverse(std::move(t))); } TransformEffect::TransformEffect(sk_sp child, sk_sp transform) diff --git a/modules/sksg/src/SkSGTransformPriv.h b/modules/sksg/src/SkSGTransformPriv.h index d0c98183e0..099fbafe89 100644 --- a/modules/sksg/src/SkSGTransformPriv.h +++ b/modules/sksg/src/SkSGTransformPriv.h @@ -18,8 +18,8 @@ public: static bool Is44(const sk_sp&t) { return t->is44(); } - template ::value || - std::is_same::value >> + template ::value || + std::is_same::value >> static T As(const sk_sp&); private: @@ -32,8 +32,8 @@ inline SkMatrix TransformPriv::As(const sk_sp& t) { } template <> -inline SkMatrix44 TransformPriv::As(const sk_sp& t) { - return t->asMatrix44(); +inline SkM44 TransformPriv::As(const sk_sp& t) { + return t->asM44(); } } // namespace sksg