From 12f436f7702dc8360e64d6e6f70edddb23137e9f Mon Sep 17 00:00:00 2001
From: Brandon DeRosier <bdero@google.com>
Date: Tue, 10 Jan 2023 21:45:32 -0800
Subject: [PATCH 01/10] [Impeller Scene] Add mutation log to nodes

---
 impeller/scene/animation/animation_player.cc | 27 +++++++---
 impeller/scene/animation/animation_player.h  |  8 +--
 impeller/scene/geometry.cc                   |  2 +-
 impeller/scene/node.cc                       | 57 +++++++++++++++++++-
 impeller/scene/node.h                        | 42 ++++++++++++++-
 impeller/scene/scene.cc                      |  5 +-
 impeller/scene/scene.h                       |  4 +-
 impeller/scene/scene_unittests.cc            | 19 +++----
 lib/ui/experiments/scene.dart                | 27 +++++-----
 lib/ui/painting/scene/scene_node.cc          | 27 +++++++---
 10 files changed, 170 insertions(+), 48 deletions(-)

diff --git a/impeller/scene/animation/animation_player.cc b/impeller/scene/animation/animation_player.cc
index 01119fb521f61..8c0030a37a866 100644
--- a/impeller/scene/animation/animation_player.cc
+++ b/impeller/scene/animation/animation_player.cc
@@ -19,10 +19,15 @@ AnimationPlayer::~AnimationPlayer() = default;
 AnimationPlayer::AnimationPlayer(AnimationPlayer&&) = default;
 AnimationPlayer& AnimationPlayer::operator=(AnimationPlayer&&) = default;
 
-AnimationClip& AnimationPlayer::AddAnimation(
-    std::shared_ptr<Animation> animation,
+AnimationClip* AnimationPlayer::AddAnimation(
+    const std::shared_ptr<Animation>& animation,
     Node* bind_target) {
-  AnimationClip clip(std::move(animation), bind_target);
+  if (!animation) {
+    VALIDATION_LOG << "Cannot add null animation.";
+    return nullptr;
+  }
+
+  AnimationClip clip(animation, bind_target);
 
   // Record all of the unique default transforms that this AnimationClip
   // will mutate.
@@ -31,8 +36,18 @@ AnimationClip& AnimationPlayer::AddAnimation(
         {binding.node, binding.node->GetLocalTransform()});
   }
 
-  clips_.push_back(std::move(clip));
-  return clips_.back();
+  clips_.insert({animation->GetName(), std::move(clip)});
+  auto found = clips_.find(animation->GetName());
+  FML_DCHECK(found != clips_.end());
+  return &found->second;
+}
+
+AnimationClip* AnimationPlayer::GetClip(const std::string& name) {
+  auto result = clips_.find(name);
+  if (result == clips_.end()) {
+    return nullptr;
+  }
+  return &result->second;
 }
 
 void AnimationPlayer::Update() {
@@ -46,7 +61,7 @@ void AnimationPlayer::Update() {
   Reset();
 
   // Update and apply all clips.
-  for (auto& clip : clips_) {
+  for (auto& [_, clip] : clips_) {
     clip.Advance(delta_time);
     clip.ApplyToBindings();
   }
diff --git a/impeller/scene/animation/animation_player.h b/impeller/scene/animation/animation_player.h
index 23c7336aed371..72e90a4e6f848 100644
--- a/impeller/scene/animation/animation_player.h
+++ b/impeller/scene/animation/animation_player.h
@@ -4,9 +4,9 @@
 
 #pragma once
 
+#include <map>
 #include <memory>
 #include <optional>
-#include <unordered_map>
 #include <vector>
 
 #include "flutter/fml/hash_combine.h"
@@ -29,9 +29,11 @@ class AnimationPlayer final {
   AnimationPlayer(AnimationPlayer&&);
   AnimationPlayer& operator=(AnimationPlayer&&);
 
-  AnimationClip& AddAnimation(std::shared_ptr<Animation> animation,
+  AnimationClip* AddAnimation(const std::shared_ptr<Animation>& animation,
                               Node* bind_target);
 
+  AnimationClip* GetClip(const std::string& name);
+
   /// @brief  Advanced all clips and updates animated properties in the scene.
   void Update();
 
@@ -41,7 +43,7 @@ class AnimationPlayer final {
  private:
   std::unordered_map<Node*, Matrix> default_target_transforms_;
 
-  std::vector<AnimationClip> clips_;
+  std::map<std::string, AnimationClip> clips_;
 
   std::optional<TimePoint> previous_time_;
 
diff --git a/impeller/scene/geometry.cc b/impeller/scene/geometry.cc
index 099476d14202d..739d60923ba3b 100644
--- a/impeller/scene/geometry.cc
+++ b/impeller/scene/geometry.cc
@@ -95,7 +95,7 @@ std::shared_ptr<Geometry> Geometry::MakeFromFlatbuffer(
   }
 
   DeviceBufferDescriptor buffer_desc;
-  buffer_desc.size = vertices_bytes * indices_bytes;
+  buffer_desc.size = vertices_bytes + indices_bytes;
   buffer_desc.storage_mode = StorageMode::kHostVisible;
 
   auto buffer = allocator.CreateBuffer(buffer_desc);
diff --git a/impeller/scene/node.cc b/impeller/scene/node.cc
index 7343adb8fa98e..9626dbd189f16 100644
--- a/impeller/scene/node.cc
+++ b/impeller/scene/node.cc
@@ -7,6 +7,7 @@
 #include <inttypes.h>
 #include <atomic>
 #include <memory>
+#include <vector>
 
 #include "flutter/fml/logging.h"
 #include "impeller/base/strings.h"
@@ -24,6 +25,24 @@ namespace scene {
 
 static std::atomic_uint64_t kNextNodeID = 0;
 
+void Node::MutationLog::Append(const Entry& entry) {
+  std::lock_guard<std::mutex> lock(write_mutex_);
+  dirty_ = true;
+  entries_.push_back(entry);
+}
+
+std::optional<std::vector<Node::MutationLog::Entry>>
+Node::MutationLog::Flush() {
+  if (!dirty_) {
+    return std::nullopt;
+  }
+  std::lock_guard<std::mutex> lock(write_mutex_);
+  dirty_ = false;
+  auto result = entries_;
+  entries_ = {};
+  return result;
+}
+
 std::shared_ptr<Node> Node::MakeFromFlatbuffer(
     const fml::Mapping& ipscene_mapping,
     Allocator& allocator) {
@@ -257,7 +276,7 @@ std::shared_ptr<Animation> Node::FindAnimationByName(
   return nullptr;
 }
 
-AnimationClip& Node::AddAnimation(const std::shared_ptr<Animation>& animation) {
+AnimationClip* Node::AddAnimation(const std::shared_ptr<Animation>& animation) {
   if (!animation_player_.has_value()) {
     animation_player_ = AnimationPlayer();
   }
@@ -287,6 +306,11 @@ Matrix Node::GetGlobalTransform() const {
 }
 
 bool Node::AddChild(std::shared_ptr<Node> node) {
+  if (!node) {
+    VALIDATION_LOG << "Cannot add null child to node.";
+    return false;
+  }
+
   // TODO(bdero): Figure out a better paradigm/rules for nodes with multiple
   //              parents. We should probably disallow this, make deep
   //              copying of nodes cheap and easy, add mesh instancing, etc.
@@ -328,7 +352,32 @@ bool Node::IsJoint() const {
 
 bool Node::Render(SceneEncoder& encoder,
                   Allocator& allocator,
-                  const Matrix& parent_transform) const {
+                  const Matrix& parent_transform) {
+  std::optional<std::vector<MutationLog::Entry>> log = mutation_log_.Flush();
+  if (log.has_value()) {
+    for (const auto& entry : log.value()) {
+      if (auto e = std::get_if<MutationLog::SetTransformEntry>(&entry)) {
+        local_transform_ = e->transform;
+      } else if (auto e =
+                     std::get_if<MutationLog::SetAnimationStateEntry>(&entry)) {
+        auto* clip = animation_player_->GetClip(e->animation_name);
+        if (!clip) {
+          continue;
+        }
+        clip->SetPlaying(e->playing);
+        clip->SetWeight(e->weight);
+        clip->SetPlaybackTimeScale(e->time_scale);
+      } else if (auto e =
+                     std::get_if<MutationLog::SeekAnimationEntry>(&entry)) {
+        auto* clip = animation_player_->GetClip(e->animation_name);
+        if (!clip) {
+          continue;
+        }
+        clip->Seek(SecondsF(e->time));
+      }
+    }
+  }
+
   if (animation_player_.has_value()) {
     animation_player_->Update();
   }
@@ -345,5 +394,9 @@ bool Node::Render(SceneEncoder& encoder,
   return true;
 }
 
+void Node::AddMutation(const MutationLog::Entry& entry) {
+  mutation_log_.Append(entry);
+}
+
 }  // namespace scene
 }  // namespace impeller
diff --git a/impeller/scene/node.h b/impeller/scene/node.h
index 8b79e5f90fd81..29901b6eead4d 100644
--- a/impeller/scene/node.h
+++ b/impeller/scene/node.h
@@ -5,6 +5,7 @@
 #pragma once
 
 #include <memory>
+#include <mutex>
 #include <optional>
 #include <vector>
 
@@ -25,6 +26,39 @@ namespace scene {
 
 class Node final {
  public:
+  class MutationLog {
+   public:
+    struct SetTransformEntry {
+      Matrix transform;
+    };
+
+    struct SetAnimationStateEntry {
+      std::string animation_name;
+      bool playing;
+      float weight;
+      float time_scale;
+    };
+
+    struct SeekAnimationEntry {
+      std::string animation_name;
+      float time;
+    };
+
+    using Entry = std::
+        variant<SetTransformEntry, SetAnimationStateEntry, SeekAnimationEntry>;
+
+    void Append(const Entry& entry);
+
+   private:
+    std::optional<std::vector<Entry>> Flush();
+
+    bool dirty_ = false;
+    std::vector<Entry> entries_;
+    std::mutex write_mutex_;
+
+    friend Node;
+  };
+
   static std::shared_ptr<Node> MakeFromFlatbuffer(
       const fml::Mapping& ipscene_mapping,
       Allocator& allocator);
@@ -44,7 +78,7 @@ class Node final {
       bool exclude_animation_players = false) const;
 
   std::shared_ptr<Animation> FindAnimationByName(const std::string& name) const;
-  AnimationClip& AddAnimation(const std::shared_ptr<Animation>& animation);
+  AnimationClip* AddAnimation(const std::shared_ptr<Animation>& animation);
 
   void SetLocalTransform(Matrix transform);
   Matrix GetLocalTransform() const;
@@ -63,7 +97,9 @@ class Node final {
 
   bool Render(SceneEncoder& encoder,
               Allocator& allocator,
-              const Matrix& parent_transform) const;
+              const Matrix& parent_transform);
+
+  void AddMutation(const MutationLog::Entry& entry);
 
  private:
   void UnpackFromFlatbuffer(
@@ -72,6 +108,8 @@ class Node final {
       const std::vector<std::shared_ptr<Texture>>& textures,
       Allocator& allocator);
 
+  mutable MutationLog mutation_log_;
+
   Matrix local_transform_;
 
   std::string name_;
diff --git a/impeller/scene/scene.cc b/impeller/scene/scene.cc
index d1c8cae5002b2..ca7202afb04b4 100644
--- a/impeller/scene/scene.cc
+++ b/impeller/scene/scene.cc
@@ -31,7 +31,7 @@ Node& Scene::GetRoot() {
 }
 
 bool Scene::Render(const RenderTarget& render_target,
-                   const Matrix& camera_transform) const {
+                   const Matrix& camera_transform) {
   // Collect the render commands from the scene.
   SceneEncoder encoder;
   if (!root_.Render(encoder,
@@ -57,8 +57,7 @@ bool Scene::Render(const RenderTarget& render_target,
   return true;
 }
 
-bool Scene::Render(const RenderTarget& render_target,
-                   const Camera& camera) const {
+bool Scene::Render(const RenderTarget& render_target, const Camera& camera) {
   return Render(render_target,
                 camera.GetTransform(render_target.GetRenderTargetSize()));
 }
diff --git a/impeller/scene/scene.h b/impeller/scene/scene.h
index 9ff0745befaa0..210cbd5b850c0 100644
--- a/impeller/scene/scene.h
+++ b/impeller/scene/scene.h
@@ -28,9 +28,9 @@ class Scene {
   Node& GetRoot();
 
   bool Render(const RenderTarget& render_target,
-              const Matrix& camera_transform) const;
+              const Matrix& camera_transform);
 
-  bool Render(const RenderTarget& render_target, const Camera& camera) const;
+  bool Render(const RenderTarget& render_target, const Camera& camera);
 
  private:
   std::shared_ptr<SceneContext> scene_context_;
diff --git a/impeller/scene/scene_unittests.cc b/impeller/scene/scene_unittests.cc
index f2a048f8a7f4f..72696089845f1 100644
--- a/impeller/scene/scene_unittests.cc
+++ b/impeller/scene/scene_unittests.cc
@@ -126,9 +126,10 @@ TEST_P(SceneTest, TwoTriangles) {
   auto animation = gltf_scene->FindAnimationByName("Metronome");
   ASSERT_NE(animation, nullptr);
 
-  AnimationClip& metronome_clip = gltf_scene->AddAnimation(animation);
-  metronome_clip.SetLoop(true);
-  metronome_clip.Play();
+  AnimationClip* metronome_clip = gltf_scene->AddAnimation(animation);
+  ASSERT_NE(metronome_clip, nullptr);
+  metronome_clip->SetLoop(true);
+  metronome_clip->Play();
 
   auto scene_context = std::make_shared<SceneContext>(GetContext());
   auto scene = Scene(scene_context);
@@ -145,18 +146,18 @@ TEST_P(SceneTest, TwoTriangles) {
       ImGui::SliderFloat("Weight", &weight, -2, 2);
       ImGui::Checkbox("Loop", &loop);
       if (ImGui::Button("Play")) {
-        metronome_clip.Play();
+        metronome_clip->Play();
       }
       if (ImGui::Button("Pause")) {
-        metronome_clip.Pause();
+        metronome_clip->Pause();
       }
       if (ImGui::Button("Stop")) {
-        metronome_clip.Stop();
+        metronome_clip->Stop();
       }
 
-      metronome_clip.SetPlaybackTimeScale(playback_time_scale);
-      metronome_clip.SetWeight(weight);
-      metronome_clip.SetLoop(loop);
+      metronome_clip->SetPlaybackTimeScale(playback_time_scale);
+      metronome_clip->SetWeight(weight);
+      metronome_clip->SetLoop(loop);
     }
 
     ImGui::End();
diff --git a/lib/ui/experiments/scene.dart b/lib/ui/experiments/scene.dart
index 2b2e477a4c41e..db8b0698ec0bb 100644
--- a/lib/ui/experiments/scene.dart
+++ b/lib/ui/experiments/scene.dart
@@ -22,16 +22,17 @@ class SceneNode extends NativeFieldWrapperClass1 {
     // encoded paths (replacing ' ' with '%20', for example). We perform
     // the same encoding here so that users can load assets with the same
     // key they have written in the pubspec.
-      final String encodedKey = Uri(path: Uri.encodeFull(assetKey)).path;
+    final String encodedKey = Uri(path: Uri.encodeFull(assetKey)).path;
     {
-      final SceneNode? sceneNode = _ipsceneRegistry[encodedKey]?.target;
-      if (sceneNode != null) {
-        return Future<SceneNode>.value(sceneNode);
+      final Future<SceneNode>? futureSceneNode = _ipsceneRegistry[encodedKey]?.target;
+      if (futureSceneNode != null) {
+        return futureSceneNode;
       }
     }
 
     final SceneNode sceneNode = SceneNode._create();
-    return _futurize((_Callback<void> callback) {
+
+    Future<SceneNode> futureSceneNode = _futurize((_Callback<void> callback) {
       final String error = sceneNode._initFromAsset(assetKey, callback);
       if (error.isNotEmpty) {
         return error;
@@ -41,10 +42,11 @@ class SceneNode extends NativeFieldWrapperClass1 {
         return true;
       }());
 
-      _ipsceneRegistry[encodedKey] = WeakReference<SceneNode>(sceneNode);
-
       return null;
     }).then((_) => sceneNode);
+
+    _ipsceneRegistry[encodedKey] = WeakReference<Future<SceneNode>>(futureSceneNode);
+    return futureSceneNode;
   }
 
   static SceneNode fromTransform(Float64List matrix4) {
@@ -73,11 +75,11 @@ class SceneNode extends NativeFieldWrapperClass1 {
   // SceneNode.fromAsset. It holds weak references to the SceneNodes so that the
   // case where an in-use ipscene is requested again can be fast, but scenes
   // that are no longer referenced are not retained because of the cache.
-  static final Map<String, WeakReference<SceneNode>> _ipsceneRegistry =
-      <String, WeakReference<SceneNode>>{};
+  static final Map<String, WeakReference<Future<SceneNode>>> _ipsceneRegistry =
+      <String, WeakReference<Future<SceneNode>>>{};
 
   static Future<void> _reinitializeScene(String assetKey) async {
-    final WeakReference<SceneNode>? sceneRef = _ipsceneRegistry == null
+    final WeakReference<Future<SceneNode>>? sceneRef = _ipsceneRegistry == null
       ? null
       : _ipsceneRegistry[assetKey];
 
@@ -87,10 +89,11 @@ class SceneNode extends NativeFieldWrapperClass1 {
       return;
     }
 
-    final SceneNode? sceneNode = sceneRef.target;
-    if (sceneNode == null) {
+    final Future<SceneNode>? sceneNodeFuture = sceneRef.target;
+    if (sceneNodeFuture == null) {
       return;
     }
+    SceneNode sceneNode = await sceneNodeFuture;
 
     await _futurize((_Callback<void> callback) {
       final String error = sceneNode._initFromAsset(assetKey, callback);
diff --git a/lib/ui/painting/scene/scene_node.cc b/lib/ui/painting/scene/scene_node.cc
index cc0b2bae1f04b..6e2f106f8e2cd 100644
--- a/lib/ui/painting/scene/scene_node.cc
+++ b/lib/ui/painting/scene/scene_node.cc
@@ -21,6 +21,7 @@
 #include "third_party/tonic/dart_args.h"
 #include "third_party/tonic/dart_binding_macros.h"
 #include "third_party/tonic/dart_library_natives.h"
+#include "third_party/tonic/typed_data/typed_list.h"
 
 namespace flutter {
 
@@ -98,13 +99,16 @@ std::string SceneNode::initFromAsset(const std::string& asset_name,
   return "";
 }
 
+static impeller::Matrix ToMatrix(const tonic::Float64List& matrix4) {
+  return impeller::Matrix(matrix4[0], matrix4[1], matrix4[2], matrix4[3],    //
+                          matrix4[4], matrix4[5], matrix4[6], matrix4[7],    //
+                          matrix4[8], matrix4[9], matrix4[10], matrix4[11],  //
+                          matrix4[12], matrix4[13], matrix4[14], matrix4[15]);
+}
+
 void SceneNode::initFromTransform(const tonic::Float64List& matrix4) {
   node_ = std::make_shared<impeller::scene::Node>();
-  node_->SetLocalTransform(
-      impeller::Matrix(matrix4[0], matrix4[1], matrix4[2], matrix4[3],    //
-                       matrix4[4], matrix4[5], matrix4[6], matrix4[7],    //
-                       matrix4[8], matrix4[9], matrix4[10], matrix4[11],  //
-                       matrix4[12], matrix4[13], matrix4[14], matrix4[15]));
+  node_->SetLocalTransform(ToMatrix(matrix4));
 }
 
 void SceneNode::AddChild(Dart_Handle scene_node_handle) {
@@ -121,18 +125,25 @@ void SceneNode::AddChild(Dart_Handle scene_node_handle) {
 }
 
 void SceneNode::SetTransform(const tonic::Float64List& matrix4) {
-  // TODO(bdero): Implement mutation log.
+  impeller::scene::Node::MutationLog::SetTransformEntry entry = {
+      ToMatrix(matrix4)};
+  node_->AddMutation(entry);
 }
 
 void SceneNode::SetAnimationState(const std::string& animation_name,
                                   bool playing,
                                   double weight,
                                   double time_scale) {
-  // TODO(bdero): Implement mutation log.
+  impeller::scene::Node::MutationLog::SetAnimationStateEntry entry = {
+      animation_name, playing, static_cast<float>(weight),
+      static_cast<float>(time_scale)};
+  node_->AddMutation(entry);
 }
 
 void SceneNode::SeekAnimation(const std::string& animation_name, double time) {
-  // TODO(bdero): Implement mutation log.
+  impeller::scene::Node::MutationLog::SeekAnimationEntry entry = {
+      animation_name, static_cast<float>(time)};
+  node_->AddMutation(entry);
 }
 
 SceneNode::SceneNode() = default;

From 507d39eb425f085505f8225906351d82145eb897 Mon Sep 17 00:00:00 2001
From: Brandon DeRosier <bdero@google.com>
Date: Wed, 11 Jan 2023 01:12:37 -0800
Subject: [PATCH 02/10] Apply the bind pose matrix when the skin is initialized

---
 impeller/scene/animation/animation_player.cc  |   2 +-
 impeller/scene/animation/property_resolver.cc |  32 +++++-
 impeller/scene/scene_unittests.cc             | 107 +++++++++++++++++-
 impeller/scene/skin.cc                        |  14 ++-
 4 files changed, 140 insertions(+), 15 deletions(-)

diff --git a/impeller/scene/animation/animation_player.cc b/impeller/scene/animation/animation_player.cc
index 8c0030a37a866..64da764d813ef 100644
--- a/impeller/scene/animation/animation_player.cc
+++ b/impeller/scene/animation/animation_player.cc
@@ -69,7 +69,7 @@ void AnimationPlayer::Update() {
 
 void AnimationPlayer::Reset() {
   for (auto& [node, transform] : default_target_transforms_) {
-    node->SetLocalTransform(Matrix());
+    node->SetLocalTransform(transform);
   }
 }
 
diff --git a/impeller/scene/animation/property_resolver.cc b/impeller/scene/animation/property_resolver.cc
index d9d5db94e44e6..d82e044156c1e 100644
--- a/impeller/scene/animation/property_resolver.cc
+++ b/impeller/scene/animation/property_resolver.cc
@@ -89,8 +89,18 @@ void TranslationTimelineResolver::Apply(Node& target,
   if (key.lerp < 1) {
     value = values_[key.index - 1].Lerp(value, key.lerp);
   }
-  target.SetLocalTransform(target.GetLocalTransform() *
-                           Matrix::MakeTranslation(value * weight));
+
+  // TODO(bdero): Instead of decomposing the matrix and lerping properties here,
+  //              keep track of the decomposed version of the joints in the
+  //              animation player and pass the decompositions around. Then, the
+  //              animation player can apply the joint matrices at the end of
+  //              the update.
+  auto decomp = target.GetLocalTransform().Decompose();
+  if (!decomp.has_value()) {
+    return;
+  }
+  decomp->translation = decomp->translation.Lerp(value, weight);
+  target.SetLocalTransform(Matrix(decomp.value()));
 }
 
 RotationTimelineResolver::RotationTimelineResolver() = default;
@@ -108,8 +118,13 @@ void RotationTimelineResolver::Apply(Node& target,
   if (key.lerp < 1) {
     value = values_[key.index - 1].Slerp(value, key.lerp);
   }
-  target.SetLocalTransform(target.GetLocalTransform() *
-                           Matrix::MakeRotation(value * weight));
+
+  auto decomp = target.GetLocalTransform().Decompose();
+  if (!decomp.has_value()) {
+    return;
+  }
+  decomp->rotation = decomp->rotation.Slerp(value, weight);
+  target.SetLocalTransform(Matrix(decomp.value()));
 }
 
 ScaleTimelineResolver::ScaleTimelineResolver() = default;
@@ -125,8 +140,13 @@ void ScaleTimelineResolver::Apply(Node& target, SecondsF time, Scalar weight) {
   if (key.lerp < 1) {
     value = values_[key.index - 1].Lerp(value, key.lerp);
   }
-  target.SetLocalTransform(target.GetLocalTransform() *
-                           Matrix::MakeScale(value * weight));
+
+  auto decomp = target.GetLocalTransform().Decompose();
+  if (!decomp.has_value()) {
+    return;
+  }
+  decomp->scale = decomp->scale.Lerp(value, weight);
+  target.SetLocalTransform(Matrix(decomp.value()));
 }
 
 }  // namespace scene
diff --git a/impeller/scene/scene_unittests.cc b/impeller/scene/scene_unittests.cc
index 72696089845f1..d822c1497a8ca 100644
--- a/impeller/scene/scene_unittests.cc
+++ b/impeller/scene/scene_unittests.cc
@@ -27,8 +27,6 @@
 #include "third_party/flatbuffers/include/flatbuffers/verifier.h"
 #include "third_party/imgui/imgui.h"
 
-// #include "third_party/tinygltf/tiny_gltf.h"
-
 namespace impeller {
 namespace scene {
 namespace testing {
@@ -194,6 +192,111 @@ TEST_P(SceneTest, TwoTriangles) {
   OpenPlaygroundHere(callback);
 }
 
+TEST_P(SceneTest, Dash) {
+  auto allocator = GetContext()->GetResourceAllocator();
+
+  auto mapping = flutter::testing::OpenFixtureAsMapping("dash.glb.ipscene");
+  if (!mapping) {
+    // TODO(bdero): Just skip this playground is the dash asset isn't found. I
+    //              haven't checked it in because it's way too big right now,
+    //              but this is still useful to keep around for debugging
+    //              purposes.
+    return;
+  }
+  ASSERT_NE(mapping, nullptr);
+
+  std::shared_ptr<Node> gltf_scene =
+      Node::MakeFromFlatbuffer(*mapping, *allocator);
+  ASSERT_NE(gltf_scene, nullptr);
+
+  auto walk_anim = gltf_scene->FindAnimationByName("Walk");
+  ASSERT_NE(walk_anim, nullptr);
+
+  AnimationClip* walk_clip = gltf_scene->AddAnimation(walk_anim);
+  ASSERT_NE(walk_clip, nullptr);
+  walk_clip->SetLoop(true);
+  walk_clip->Play();
+
+  auto run_anim = gltf_scene->FindAnimationByName("Run");
+  ASSERT_NE(walk_anim, nullptr);
+
+  AnimationClip* run_clip = gltf_scene->AddAnimation(run_anim);
+  ASSERT_NE(run_clip, nullptr);
+  run_clip->SetLoop(true);
+  run_clip->Play();
+
+  auto scene_context = std::make_shared<SceneContext>(GetContext());
+  auto scene = Scene(scene_context);
+  scene.GetRoot().AddChild(std::move(gltf_scene));
+
+  Renderer::RenderCallback callback = [&](RenderTarget& render_target) {
+    ImGui::Begin("Controls", nullptr, ImGuiWindowFlags_AlwaysAutoResize);
+    {
+      static Scalar playback_time_scale = 1;
+      static Scalar walk = 0.5;
+      static Scalar run = 0.5;
+      static bool loop = true;
+
+      ImGui::SliderFloat("Playback time scale", &playback_time_scale, -5, 5);
+      ImGui::SliderFloat("Walk weight", &walk, 0, 1);
+      ImGui::SliderFloat("Run weight", &run, 0, 1);
+      ImGui::Checkbox("Loop", &loop);
+      if (ImGui::Button("Play")) {
+        walk_clip->Play();
+        run_clip->Play();
+      }
+      if (ImGui::Button("Pause")) {
+        walk_clip->Pause();
+        run_clip->Pause();
+      }
+      if (ImGui::Button("Stop")) {
+        walk_clip->Stop();
+        run_clip->Stop();
+      }
+
+      walk_clip->SetPlaybackTimeScale(playback_time_scale);
+      walk_clip->SetWeight(walk);
+      walk_clip->SetLoop(loop);
+
+      run_clip->SetPlaybackTimeScale(playback_time_scale);
+      run_clip->SetWeight(run);
+      run_clip->SetLoop(loop);
+    }
+
+    ImGui::End();
+    Node& node = *scene.GetRoot().GetChildren()[0];
+    node.SetLocalTransform(node.GetLocalTransform() *
+                           Matrix::MakeRotation(0.02, {0, 1, 0, 0}));
+
+    static ImVec2 mouse_pos_prev = ImGui::GetMousePos();
+    ImVec2 mouse_pos = ImGui::GetMousePos();
+    Vector2 mouse_diff =
+        Vector2(mouse_pos.x - mouse_pos_prev.x, mouse_pos.y - mouse_pos_prev.y);
+
+    static Vector3 position(0, 1, -5);
+    static Vector3 cam_position = position;
+    auto strafe =
+        Vector3(ImGui::IsKeyDown(ImGuiKey_D) - ImGui::IsKeyDown(ImGuiKey_A),
+                ImGui::IsKeyDown(ImGuiKey_E) - ImGui::IsKeyDown(ImGuiKey_Q),
+                ImGui::IsKeyDown(ImGuiKey_W) - ImGui::IsKeyDown(ImGuiKey_S));
+    position += strafe * 0.5;
+    cam_position = cam_position.Lerp(position, 0.02);
+
+    // Face towards the +Z direction (+X right, +Y up).
+    auto camera = Camera::MakePerspective(
+                      /* fov */ Degrees(60),
+                      /* position */ cam_position)
+                      .LookAt(
+                          /* target */ cam_position + Vector3(0, 0, 1),
+                          /* up */ {0, 1, 0});
+
+    scene.Render(render_target, camera);
+    return true;
+  };
+
+  OpenPlaygroundHere(callback);
+}
+
 }  // namespace testing
 }  // namespace scene
 }  // namespace impeller
diff --git a/impeller/scene/skin.cc b/impeller/scene/skin.cc
index 4a4c614195289..4407eeb5bf76f 100644
--- a/impeller/scene/skin.cc
+++ b/impeller/scene/skin.cc
@@ -41,12 +41,14 @@ std::unique_ptr<Skin> Skin::MakeFromFlatbuffer(
   }
 
   result.inverse_bind_matrices_.reserve(skin.inverse_bind_matrices()->size());
-  for (auto matrix : *skin.inverse_bind_matrices()) {
-    if (!matrix) {
-      result.inverse_bind_matrices_.push_back(Matrix());
-      continue;
-    }
-    result.inverse_bind_matrices_.push_back(importer::ToMatrix(*matrix));
+  for (size_t matrix_i = 0; matrix_i < skin.inverse_bind_matrices()->size();
+       matrix_i++) {
+    const auto* ip_matrix = skin.inverse_bind_matrices()->Get(matrix_i);
+    Matrix matrix = ip_matrix ? importer::ToMatrix(*ip_matrix) : Matrix();
+
+    result.inverse_bind_matrices_.push_back(matrix);
+    // Overwrite the joint transforms with the inverse bind pose.
+    result.joints_[matrix_i]->SetGlobalTransform(matrix.Invert());
   }
 
   return std::make_unique<Skin>(std::move(result));

From 5f75ed8ad633fdef60410bb764c8a1bd432c7350 Mon Sep 17 00:00:00 2001
From: Brandon DeRosier <bdero@google.com>
Date: Wed, 11 Jan 2023 02:01:02 -0800
Subject: [PATCH 03/10] Refactor property resolver to modify decomposed
 properties

---
 impeller/scene/animation/animation_clip.cc    |  9 +++--
 impeller/scene/animation/animation_clip.h     |  3 +-
 impeller/scene/animation/animation_player.cc  | 18 ++++++----
 impeller/scene/animation/animation_player.h   |  3 --
 impeller/scene/animation/property_resolver.cc | 35 +++++--------------
 impeller/scene/animation/property_resolver.h  | 17 ++++++---
 6 files changed, 43 insertions(+), 42 deletions(-)

diff --git a/impeller/scene/animation/animation_clip.cc b/impeller/scene/animation/animation_clip.cc
index 10019e8f52bd1..57ded397f3a10 100644
--- a/impeller/scene/animation/animation_clip.cc
+++ b/impeller/scene/animation/animation_clip.cc
@@ -110,9 +110,14 @@ void AnimationClip::Advance(SecondsF delta_time) {
   }
 }
 
-void AnimationClip::ApplyToBindings() const {
+void AnimationClip::ApplyToBindings(
+    std::unordered_map<Node*, MatrixDecomposition>& transform_decomps) const {
   for (auto& binding : bindings_) {
-    binding.channel.resolver->Apply(*binding.node, playback_time_, weight_);
+    auto decomp = transform_decomps.find(binding.node);
+    if (decomp == transform_decomps.end()) {
+      continue;
+    }
+    binding.channel.resolver->Apply(decomp->second, playback_time_, weight_);
   }
 }
 
diff --git a/impeller/scene/animation/animation_clip.h b/impeller/scene/animation/animation_clip.h
index cf584ae64620a..4dfa8eacb37fb 100644
--- a/impeller/scene/animation/animation_clip.h
+++ b/impeller/scene/animation/animation_clip.h
@@ -60,7 +60,8 @@ class AnimationClip final {
   void Advance(SecondsF delta_time);
 
   /// @brief  Applies the animation to all binded properties in the scene.
-  void ApplyToBindings() const;
+  void ApplyToBindings(
+      std::unordered_map<Node*, MatrixDecomposition>& transform_decomps) const;
 
  private:
   void BindToTarget(Node* node);
diff --git a/impeller/scene/animation/animation_player.cc b/impeller/scene/animation/animation_player.cc
index 64da764d813ef..eea1bfd90b73d 100644
--- a/impeller/scene/animation/animation_player.cc
+++ b/impeller/scene/animation/animation_player.cc
@@ -5,6 +5,7 @@
 #include "impeller/scene/animation/animation_player.h"
 
 #include <memory>
+#include <unordered_map>
 
 #include "flutter/fml/time/time_point.h"
 #include "impeller/base/timing.h"
@@ -58,18 +59,23 @@ void AnimationPlayer::Update() {
   auto delta_time = new_time - previous_time_.value();
   previous_time_ = new_time;
 
-  Reset();
+  std::unordered_map<Node*, MatrixDecomposition> transform_decomps;
+  for (auto& [node, transform] : default_target_transforms_) {
+    auto decomp = transform.Decompose();
+    if (!decomp.has_value()) {
+      continue;
+    }
+    transform_decomps[node] = decomp.value();
+  }
 
   // Update and apply all clips.
   for (auto& [_, clip] : clips_) {
     clip.Advance(delta_time);
-    clip.ApplyToBindings();
+    clip.ApplyToBindings(transform_decomps);
   }
-}
 
-void AnimationPlayer::Reset() {
-  for (auto& [node, transform] : default_target_transforms_) {
-    node->SetLocalTransform(transform);
+  for (auto& [node, decomp] : transform_decomps) {
+    node->SetLocalTransform(Matrix(decomp));
   }
 }
 
diff --git a/impeller/scene/animation/animation_player.h b/impeller/scene/animation/animation_player.h
index 72e90a4e6f848..c5222835080b6 100644
--- a/impeller/scene/animation/animation_player.h
+++ b/impeller/scene/animation/animation_player.h
@@ -37,9 +37,6 @@ class AnimationPlayer final {
   /// @brief  Advanced all clips and updates animated properties in the scene.
   void Update();
 
-  /// @brief  Reset all bound animation target transforms.
-  void Reset();
-
  private:
   std::unordered_map<Node*, Matrix> default_target_transforms_;
 
diff --git a/impeller/scene/animation/property_resolver.cc b/impeller/scene/animation/property_resolver.cc
index d82e044156c1e..aec9a993c7331 100644
--- a/impeller/scene/animation/property_resolver.cc
+++ b/impeller/scene/animation/property_resolver.cc
@@ -8,6 +8,7 @@
 #include <iterator>
 #include <memory>
 
+#include "impeller/geometry/matrix_decomposition.h"
 #include "impeller/geometry/point.h"
 #include "impeller/scene/node.h"
 
@@ -78,7 +79,7 @@ TranslationTimelineResolver::TranslationTimelineResolver() = default;
 
 TranslationTimelineResolver::~TranslationTimelineResolver() = default;
 
-void TranslationTimelineResolver::Apply(Node& target,
+void TranslationTimelineResolver::Apply(MatrixDecomposition& target,
                                         SecondsF time,
                                         Scalar weight) {
   if (values_.empty()) {
@@ -90,24 +91,14 @@ void TranslationTimelineResolver::Apply(Node& target,
     value = values_[key.index - 1].Lerp(value, key.lerp);
   }
 
-  // TODO(bdero): Instead of decomposing the matrix and lerping properties here,
-  //              keep track of the decomposed version of the joints in the
-  //              animation player and pass the decompositions around. Then, the
-  //              animation player can apply the joint matrices at the end of
-  //              the update.
-  auto decomp = target.GetLocalTransform().Decompose();
-  if (!decomp.has_value()) {
-    return;
-  }
-  decomp->translation = decomp->translation.Lerp(value, weight);
-  target.SetLocalTransform(Matrix(decomp.value()));
+  target.translation = target.translation.Lerp(value, weight);
 }
 
 RotationTimelineResolver::RotationTimelineResolver() = default;
 
 RotationTimelineResolver::~RotationTimelineResolver() = default;
 
-void RotationTimelineResolver::Apply(Node& target,
+void RotationTimelineResolver::Apply(MatrixDecomposition& target,
                                      SecondsF time,
                                      Scalar weight) {
   if (values_.empty()) {
@@ -119,19 +110,16 @@ void RotationTimelineResolver::Apply(Node& target,
     value = values_[key.index - 1].Slerp(value, key.lerp);
   }
 
-  auto decomp = target.GetLocalTransform().Decompose();
-  if (!decomp.has_value()) {
-    return;
-  }
-  decomp->rotation = decomp->rotation.Slerp(value, weight);
-  target.SetLocalTransform(Matrix(decomp.value()));
+  target.rotation = target.rotation.Slerp(value, weight);
 }
 
 ScaleTimelineResolver::ScaleTimelineResolver() = default;
 
 ScaleTimelineResolver::~ScaleTimelineResolver() = default;
 
-void ScaleTimelineResolver::Apply(Node& target, SecondsF time, Scalar weight) {
+void ScaleTimelineResolver::Apply(MatrixDecomposition& target,
+                                  SecondsF time,
+                                  Scalar weight) {
   if (values_.empty()) {
     return;
   }
@@ -141,12 +129,7 @@ void ScaleTimelineResolver::Apply(Node& target, SecondsF time, Scalar weight) {
     value = values_[key.index - 1].Lerp(value, key.lerp);
   }
 
-  auto decomp = target.GetLocalTransform().Decompose();
-  if (!decomp.has_value()) {
-    return;
-  }
-  decomp->scale = decomp->scale.Lerp(value, weight);
-  target.SetLocalTransform(Matrix(decomp.value()));
+  target.scale = target.scale.Lerp(value, weight);
 }
 
 }  // namespace scene
diff --git a/impeller/scene/animation/property_resolver.h b/impeller/scene/animation/property_resolver.h
index e0cd1888a857a..56e111e82742e 100644
--- a/impeller/scene/animation/property_resolver.h
+++ b/impeller/scene/animation/property_resolver.h
@@ -11,6 +11,7 @@
 #include "flutter/fml/hash_combine.h"
 #include "flutter/fml/macros.h"
 #include "impeller/base/timing.h"
+#include "impeller/geometry/matrix_decomposition.h"
 #include "impeller/geometry/quaternion.h"
 #include "impeller/geometry/scalar.h"
 #include "impeller/geometry/vector.h"
@@ -46,7 +47,9 @@ class PropertyResolver {
   ///         many different PropertyResolvers prior to rendering. For example,
   ///         an AnimationPlayer may blend multiple Animations together by
   ///         applying several AnimationClips.
-  virtual void Apply(Node& target, SecondsF time, Scalar weight) = 0;
+  virtual void Apply(MatrixDecomposition& target,
+                     SecondsF time,
+                     Scalar weight) = 0;
 };
 
 class TimelineResolver : public PropertyResolver {
@@ -74,7 +77,9 @@ class TranslationTimelineResolver final : public TimelineResolver {
   ~TranslationTimelineResolver();
 
   // |Resolver|
-  void Apply(Node& target, SecondsF time, Scalar weight) override;
+  void Apply(MatrixDecomposition& target,
+             SecondsF time,
+             Scalar weight) override;
 
  private:
   TranslationTimelineResolver();
@@ -91,7 +96,9 @@ class RotationTimelineResolver final : public TimelineResolver {
   ~RotationTimelineResolver();
 
   // |Resolver|
-  void Apply(Node& target, SecondsF time, Scalar weight) override;
+  void Apply(MatrixDecomposition& target,
+             SecondsF time,
+             Scalar weight) override;
 
  private:
   RotationTimelineResolver();
@@ -108,7 +115,9 @@ class ScaleTimelineResolver final : public TimelineResolver {
   ~ScaleTimelineResolver();
 
   // |Resolver|
-  void Apply(Node& target, SecondsF time, Scalar weight) override;
+  void Apply(MatrixDecomposition& target,
+             SecondsF time,
+             Scalar weight) override;
 
  private:
   ScaleTimelineResolver();

From da7026058dabb0692cd4f1a24d889f66a27eeb24 Mon Sep 17 00:00:00 2001
From: Brandon DeRosier <bdero@google.com>
Date: Wed, 11 Jan 2023 03:16:56 -0800
Subject: [PATCH 04/10] Make the property resolvers additive again to support
 complex blending

---
 ci/licenses_golden/licenses_flutter           |  2 ++
 impeller/geometry/quaternion.h                |  2 ++
 impeller/scene/BUILD.gn                       |  1 +
 impeller/scene/animation/animation_clip.cc    |  9 ++++---
 impeller/scene/animation/animation_clip.h     |  3 ++-
 impeller/scene/animation/animation_player.cc  | 27 ++++++++++---------
 impeller/scene/animation/animation_player.h   |  3 ++-
 .../scene/animation/animation_transforms.h    | 18 +++++++++++++
 impeller/scene/animation/property_resolver.cc | 16 ++++++-----
 impeller/scene/animation/property_resolver.h  |  9 ++++---
 10 files changed, 61 insertions(+), 29 deletions(-)
 create mode 100644 impeller/scene/animation/animation_transforms.h

diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter
index 2a31b9f945b4b..6c8128d46e6a5 100644
--- a/ci/licenses_golden/licenses_flutter
+++ b/ci/licenses_golden/licenses_flutter
@@ -1614,6 +1614,7 @@ ORIGIN: ../../../flutter/impeller/scene/animation/animation_clip.cc + ../../../f
 ORIGIN: ../../../flutter/impeller/scene/animation/animation_clip.h + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/scene/animation/animation_player.cc + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/scene/animation/animation_player.h + ../../../flutter/LICENSE
+ORIGIN: ../../../flutter/impeller/scene/animation/animation_transforms.h + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/scene/animation/property_resolver.cc + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/scene/animation/property_resolver.h + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/scene/camera.cc + ../../../flutter/LICENSE
@@ -4094,6 +4095,7 @@ FILE: ../../../flutter/impeller/scene/animation/animation_clip.cc
 FILE: ../../../flutter/impeller/scene/animation/animation_clip.h
 FILE: ../../../flutter/impeller/scene/animation/animation_player.cc
 FILE: ../../../flutter/impeller/scene/animation/animation_player.h
+FILE: ../../../flutter/impeller/scene/animation/animation_transforms.h
 FILE: ../../../flutter/impeller/scene/animation/property_resolver.cc
 FILE: ../../../flutter/impeller/scene/animation/property_resolver.h
 FILE: ../../../flutter/impeller/scene/camera.cc
diff --git a/impeller/geometry/quaternion.h b/impeller/geometry/quaternion.h
index 0270dab9dc814..0dacf0428fa43 100644
--- a/impeller/geometry/quaternion.h
+++ b/impeller/geometry/quaternion.h
@@ -45,6 +45,8 @@ struct Quaternion {
     return {x * m, y * m, z * m, w * m};
   }
 
+  Quaternion Invert() const { return {-x, -y, -z, w}; }
+
   Quaternion Slerp(const Quaternion& to, double time) const;
 
   Quaternion operator*(const Quaternion& o) const {
diff --git a/impeller/scene/BUILD.gn b/impeller/scene/BUILD.gn
index dbd531cf8e17d..d937166b1ff21 100644
--- a/impeller/scene/BUILD.gn
+++ b/impeller/scene/BUILD.gn
@@ -12,6 +12,7 @@ impeller_component("scene") {
     "animation/animation_clip.h",
     "animation/animation_player.cc",
     "animation/animation_player.h",
+    "animation/animation_transforms.h",
     "animation/property_resolver.cc",
     "animation/property_resolver.h",
     "camera.cc",
diff --git a/impeller/scene/animation/animation_clip.cc b/impeller/scene/animation/animation_clip.cc
index 57ded397f3a10..c5a9656a14c34 100644
--- a/impeller/scene/animation/animation_clip.cc
+++ b/impeller/scene/animation/animation_clip.cc
@@ -111,13 +111,14 @@ void AnimationClip::Advance(SecondsF delta_time) {
 }
 
 void AnimationClip::ApplyToBindings(
-    std::unordered_map<Node*, MatrixDecomposition>& transform_decomps) const {
+    std::unordered_map<Node*, AnimationTransforms>& transform_decomps) const {
   for (auto& binding : bindings_) {
-    auto decomp = transform_decomps.find(binding.node);
-    if (decomp == transform_decomps.end()) {
+    auto transforms = transform_decomps.find(binding.node);
+    if (transforms == transform_decomps.end()) {
       continue;
     }
-    binding.channel.resolver->Apply(decomp->second, playback_time_, weight_);
+    binding.channel.resolver->Apply(transforms->second, playback_time_,
+                                    weight_);
   }
 }
 
diff --git a/impeller/scene/animation/animation_clip.h b/impeller/scene/animation/animation_clip.h
index 4dfa8eacb37fb..8d8af880bc932 100644
--- a/impeller/scene/animation/animation_clip.h
+++ b/impeller/scene/animation/animation_clip.h
@@ -9,6 +9,7 @@
 
 #include "flutter/fml/macros.h"
 #include "impeller/scene/animation/animation.h"
+#include "impeller/scene/animation/animation_transforms.h"
 
 namespace impeller {
 namespace scene {
@@ -61,7 +62,7 @@ class AnimationClip final {
 
   /// @brief  Applies the animation to all binded properties in the scene.
   void ApplyToBindings(
-      std::unordered_map<Node*, MatrixDecomposition>& transform_decomps) const;
+      std::unordered_map<Node*, AnimationTransforms>& transform_decomps) const;
 
  private:
   void BindToTarget(Node* node);
diff --git a/impeller/scene/animation/animation_player.cc b/impeller/scene/animation/animation_player.cc
index eea1bfd90b73d..aa5d4c1ca8fc7 100644
--- a/impeller/scene/animation/animation_player.cc
+++ b/impeller/scene/animation/animation_player.cc
@@ -33,8 +33,12 @@ AnimationClip* AnimationPlayer::AddAnimation(
   // Record all of the unique default transforms that this AnimationClip
   // will mutate.
   for (const auto& binding : clip.bindings_) {
-    default_target_transforms_.insert(
-        {binding.node, binding.node->GetLocalTransform()});
+    auto decomp = binding.node->GetLocalTransform().Decompose();
+    if (!decomp.has_value()) {
+      continue;
+    }
+    target_transforms_.insert(
+        {binding.node, AnimationTransforms{.bind_pose = decomp.value()}});
   }
 
   clips_.insert({animation->GetName(), std::move(clip)});
@@ -59,23 +63,20 @@ void AnimationPlayer::Update() {
   auto delta_time = new_time - previous_time_.value();
   previous_time_ = new_time;
 
-  std::unordered_map<Node*, MatrixDecomposition> transform_decomps;
-  for (auto& [node, transform] : default_target_transforms_) {
-    auto decomp = transform.Decompose();
-    if (!decomp.has_value()) {
-      continue;
-    }
-    transform_decomps[node] = decomp.value();
+  // Reset the animated pose state.
+  for (auto& [node, transforms] : target_transforms_) {
+    transforms.animated_pose = transforms.bind_pose;
   }
 
-  // Update and apply all clips.
+  // Update and apply all clips to the animation pose state.
   for (auto& [_, clip] : clips_) {
     clip.Advance(delta_time);
-    clip.ApplyToBindings(transform_decomps);
+    clip.ApplyToBindings(target_transforms_);
   }
 
-  for (auto& [node, decomp] : transform_decomps) {
-    node->SetLocalTransform(Matrix(decomp));
+  // Apply the animated pose to the bound joints.
+  for (auto& [node, transforms] : target_transforms_) {
+    node->SetLocalTransform(Matrix(transforms.animated_pose));
   }
 }
 
diff --git a/impeller/scene/animation/animation_player.h b/impeller/scene/animation/animation_player.h
index c5222835080b6..2b2e85fd17eba 100644
--- a/impeller/scene/animation/animation_player.h
+++ b/impeller/scene/animation/animation_player.h
@@ -14,6 +14,7 @@
 #include "flutter/fml/time/time_delta.h"
 #include "impeller/base/timing.h"
 #include "impeller/geometry/matrix.h"
+#include "impeller/geometry/matrix_decomposition.h"
 #include "impeller/scene/animation/animation_clip.h"
 
 namespace impeller {
@@ -38,7 +39,7 @@ class AnimationPlayer final {
   void Update();
 
  private:
-  std::unordered_map<Node*, Matrix> default_target_transforms_;
+  std::unordered_map<Node*, AnimationTransforms> target_transforms_;
 
   std::map<std::string, AnimationClip> clips_;
 
diff --git a/impeller/scene/animation/animation_transforms.h b/impeller/scene/animation/animation_transforms.h
new file mode 100644
index 0000000000000..b15c9817781d4
--- /dev/null
+++ b/impeller/scene/animation/animation_transforms.h
@@ -0,0 +1,18 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#pragma once
+
+#include "impeller/geometry/matrix_decomposition.h"
+
+namespace impeller {
+namespace scene {
+
+struct AnimationTransforms {
+  MatrixDecomposition bind_pose;
+  MatrixDecomposition animated_pose;
+};
+
+}  // namespace scene
+}  // namespace impeller
diff --git a/impeller/scene/animation/property_resolver.cc b/impeller/scene/animation/property_resolver.cc
index aec9a993c7331..a43a308efdb75 100644
--- a/impeller/scene/animation/property_resolver.cc
+++ b/impeller/scene/animation/property_resolver.cc
@@ -79,7 +79,7 @@ TranslationTimelineResolver::TranslationTimelineResolver() = default;
 
 TranslationTimelineResolver::~TranslationTimelineResolver() = default;
 
-void TranslationTimelineResolver::Apply(MatrixDecomposition& target,
+void TranslationTimelineResolver::Apply(AnimationTransforms& target,
                                         SecondsF time,
                                         Scalar weight) {
   if (values_.empty()) {
@@ -91,14 +91,15 @@ void TranslationTimelineResolver::Apply(MatrixDecomposition& target,
     value = values_[key.index - 1].Lerp(value, key.lerp);
   }
 
-  target.translation = target.translation.Lerp(value, weight);
+  target.animated_pose.translation +=
+      (value - target.bind_pose.translation) * weight;
 }
 
 RotationTimelineResolver::RotationTimelineResolver() = default;
 
 RotationTimelineResolver::~RotationTimelineResolver() = default;
 
-void RotationTimelineResolver::Apply(MatrixDecomposition& target,
+void RotationTimelineResolver::Apply(AnimationTransforms& target,
                                      SecondsF time,
                                      Scalar weight) {
   if (values_.empty()) {
@@ -110,14 +111,16 @@ void RotationTimelineResolver::Apply(MatrixDecomposition& target,
     value = values_[key.index - 1].Slerp(value, key.lerp);
   }
 
-  target.rotation = target.rotation.Slerp(value, weight);
+  target.animated_pose.rotation =
+      target.animated_pose.rotation *
+      Quaternion().Slerp(target.bind_pose.rotation.Invert() * value, weight);
 }
 
 ScaleTimelineResolver::ScaleTimelineResolver() = default;
 
 ScaleTimelineResolver::~ScaleTimelineResolver() = default;
 
-void ScaleTimelineResolver::Apply(MatrixDecomposition& target,
+void ScaleTimelineResolver::Apply(AnimationTransforms& target,
                                   SecondsF time,
                                   Scalar weight) {
   if (values_.empty()) {
@@ -129,7 +132,8 @@ void ScaleTimelineResolver::Apply(MatrixDecomposition& target,
     value = values_[key.index - 1].Lerp(value, key.lerp);
   }
 
-  target.scale = target.scale.Lerp(value, weight);
+  target.animated_pose.scale *=
+      Vector3(1, 1, 1).Lerp(value / target.bind_pose.scale, weight);
 }
 
 }  // namespace scene
diff --git a/impeller/scene/animation/property_resolver.h b/impeller/scene/animation/property_resolver.h
index 56e111e82742e..f7b5ce2abfdff 100644
--- a/impeller/scene/animation/property_resolver.h
+++ b/impeller/scene/animation/property_resolver.h
@@ -15,6 +15,7 @@
 #include "impeller/geometry/quaternion.h"
 #include "impeller/geometry/scalar.h"
 #include "impeller/geometry/vector.h"
+#include "impeller/scene/animation/animation_transforms.h"
 
 namespace impeller {
 namespace scene {
@@ -47,7 +48,7 @@ class PropertyResolver {
   ///         many different PropertyResolvers prior to rendering. For example,
   ///         an AnimationPlayer may blend multiple Animations together by
   ///         applying several AnimationClips.
-  virtual void Apply(MatrixDecomposition& target,
+  virtual void Apply(AnimationTransforms& target,
                      SecondsF time,
                      Scalar weight) = 0;
 };
@@ -77,7 +78,7 @@ class TranslationTimelineResolver final : public TimelineResolver {
   ~TranslationTimelineResolver();
 
   // |Resolver|
-  void Apply(MatrixDecomposition& target,
+  void Apply(AnimationTransforms& target,
              SecondsF time,
              Scalar weight) override;
 
@@ -96,7 +97,7 @@ class RotationTimelineResolver final : public TimelineResolver {
   ~RotationTimelineResolver();
 
   // |Resolver|
-  void Apply(MatrixDecomposition& target,
+  void Apply(AnimationTransforms& target,
              SecondsF time,
              Scalar weight) override;
 
@@ -115,7 +116,7 @@ class ScaleTimelineResolver final : public TimelineResolver {
   ~ScaleTimelineResolver();
 
   // |Resolver|
-  void Apply(MatrixDecomposition& target,
+  void Apply(AnimationTransforms& target,
              SecondsF time,
              Scalar weight) override;
 

From f430cb530edd38bd0c1f5bdc5bb9c995425cddec Mon Sep 17 00:00:00 2001
From: Brandon DeRosier <bdero@google.com>
Date: Wed, 11 Jan 2023 03:38:19 -0800
Subject: [PATCH 05/10] Normalize clip weights

---
 impeller/scene/animation/animation_clip.cc   | 7 ++++---
 impeller/scene/animation/animation_clip.h    | 3 ++-
 impeller/scene/animation/animation_player.cc | 9 ++++++++-
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/impeller/scene/animation/animation_clip.cc b/impeller/scene/animation/animation_clip.cc
index c5a9656a14c34..4a03bf58ba5b5 100644
--- a/impeller/scene/animation/animation_clip.cc
+++ b/impeller/scene/animation/animation_clip.cc
@@ -67,7 +67,7 @@ Scalar AnimationClip::GetWeight() const {
 }
 
 void AnimationClip::SetWeight(Scalar weight) {
-  weight_ = weight;
+  weight_ = std::max(0.0f, weight);
 }
 
 SecondsF AnimationClip::GetPlaybackTime() const {
@@ -111,14 +111,15 @@ void AnimationClip::Advance(SecondsF delta_time) {
 }
 
 void AnimationClip::ApplyToBindings(
-    std::unordered_map<Node*, AnimationTransforms>& transform_decomps) const {
+    std::unordered_map<Node*, AnimationTransforms>& transform_decomps,
+    Scalar weight_multiplier) const {
   for (auto& binding : bindings_) {
     auto transforms = transform_decomps.find(binding.node);
     if (transforms == transform_decomps.end()) {
       continue;
     }
     binding.channel.resolver->Apply(transforms->second, playback_time_,
-                                    weight_);
+                                    weight_ * weight_multiplier);
   }
 }
 
diff --git a/impeller/scene/animation/animation_clip.h b/impeller/scene/animation/animation_clip.h
index 8d8af880bc932..8fe150e2061ca 100644
--- a/impeller/scene/animation/animation_clip.h
+++ b/impeller/scene/animation/animation_clip.h
@@ -62,7 +62,8 @@ class AnimationClip final {
 
   /// @brief  Applies the animation to all binded properties in the scene.
   void ApplyToBindings(
-      std::unordered_map<Node*, AnimationTransforms>& transform_decomps) const;
+      std::unordered_map<Node*, AnimationTransforms>& transform_decomps,
+      Scalar weight_multiplier) const;
 
  private:
   void BindToTarget(Node* node);
diff --git a/impeller/scene/animation/animation_player.cc b/impeller/scene/animation/animation_player.cc
index aa5d4c1ca8fc7..0fadaf545343f 100644
--- a/impeller/scene/animation/animation_player.cc
+++ b/impeller/scene/animation/animation_player.cc
@@ -68,10 +68,17 @@ void AnimationPlayer::Update() {
     transforms.animated_pose = transforms.bind_pose;
   }
 
+  // Compute a weight multiplier for normalizing the animation.
+  Scalar total_weight = 0;
+  for (auto& [_, clip] : clips_) {
+    total_weight += clip.GetWeight();
+  }
+  Scalar weight_multiplier = total_weight > 1 ? 1 / total_weight : 1;
+
   // Update and apply all clips to the animation pose state.
   for (auto& [_, clip] : clips_) {
     clip.Advance(delta_time);
-    clip.ApplyToBindings(target_transforms_);
+    clip.ApplyToBindings(target_transforms_, weight_multiplier);
   }
 
   // Apply the animated pose to the bound joints.

From 84fc9ee30e9e0474f16611a2270e4582dd74fd7d Mon Sep 17 00:00:00 2001
From: Brandon DeRosier <bdero@google.com>
Date: Wed, 11 Jan 2023 08:30:57 -0800
Subject: [PATCH 06/10] Fix UI-side animation bugs and add looping config

---
 impeller/scene/node.cc              | 31 +++++++++++++++++++++++++----
 impeller/scene/node.h               |  9 +++++----
 lib/ui/experiments/scene.dart       | 10 +++++-----
 lib/ui/painting/scene/scene_node.cc | 13 +++++++++---
 lib/ui/painting/scene/scene_node.h  |  1 +
 5 files changed, 48 insertions(+), 16 deletions(-)

diff --git a/impeller/scene/node.cc b/impeller/scene/node.cc
index 9626dbd189f16..9686f9775e58b 100644
--- a/impeller/scene/node.cc
+++ b/impeller/scene/node.cc
@@ -360,19 +360,42 @@ bool Node::Render(SceneEncoder& encoder,
         local_transform_ = e->transform;
       } else if (auto e =
                      std::get_if<MutationLog::SetAnimationStateEntry>(&entry)) {
-        auto* clip = animation_player_->GetClip(e->animation_name);
+        AnimationClip* clip =
+            animation_player_.has_value()
+                ? animation_player_->GetClip(e->animation_name)
+                : nullptr;
         if (!clip) {
-          continue;
+          auto animation = FindAnimationByName(e->animation_name);
+          if (!animation) {
+            continue;
+          }
+          clip = AddAnimation(animation);
+          if (!clip) {
+            continue;
+          }
         }
+
         clip->SetPlaying(e->playing);
+        clip->SetLoop(e->loop);
         clip->SetWeight(e->weight);
         clip->SetPlaybackTimeScale(e->time_scale);
       } else if (auto e =
                      std::get_if<MutationLog::SeekAnimationEntry>(&entry)) {
-        auto* clip = animation_player_->GetClip(e->animation_name);
+        AnimationClip* clip =
+            animation_player_.has_value()
+                ? animation_player_->GetClip(e->animation_name)
+                : nullptr;
         if (!clip) {
-          continue;
+          auto animation = FindAnimationByName(e->animation_name);
+          if (!animation) {
+            continue;
+          }
+          clip = AddAnimation(animation);
+          if (!clip) {
+            continue;
+          }
         }
+
         clip->Seek(SecondsF(e->time));
       }
     }
diff --git a/impeller/scene/node.h b/impeller/scene/node.h
index 29901b6eead4d..07dc6b66704a3 100644
--- a/impeller/scene/node.h
+++ b/impeller/scene/node.h
@@ -34,14 +34,15 @@ class Node final {
 
     struct SetAnimationStateEntry {
       std::string animation_name;
-      bool playing;
-      float weight;
-      float time_scale;
+      bool playing = false;
+      bool loop = false;
+      Scalar weight = 0;
+      Scalar time_scale = 1;
     };
 
     struct SeekAnimationEntry {
       std::string animation_name;
-      float time;
+      float time = 0;
     };
 
     using Entry = std::
diff --git a/lib/ui/experiments/scene.dart b/lib/ui/experiments/scene.dart
index db8b0698ec0bb..c18f356b05939 100644
--- a/lib/ui/experiments/scene.dart
+++ b/lib/ui/experiments/scene.dart
@@ -63,8 +63,8 @@ class SceneNode extends NativeFieldWrapperClass1 {
     _setTransform(matrix4);
   }
 
-  void setAnimationState(String animationName, bool playing, double weight, double timeScale) {
-    _setAnimationState(animationName, playing, weight, timeScale);
+  void setAnimationState(String animationName, bool playing, bool loop, double weight, double timeScale) {
+    _setAnimationState(animationName, playing, loop, weight, timeScale);
   }
 
   void seekAnimation(String animationName, double time) {
@@ -119,10 +119,10 @@ class SceneNode extends NativeFieldWrapperClass1 {
   @FfiNative<Void Function(Pointer<Void>, Handle)>('SceneNode::SetTransform')
   external void _setTransform(Float64List matrix4);
 
-  @FfiNative<Void Function(Pointer<Void>, Handle, Handle, Handle, Handle)>('SceneScene::SetAnimationState')
-  external void _setAnimationState(String animationName, bool playing, double weight, double timeScale);
+  @FfiNative<Void Function(Pointer<Void>, Handle, Bool, Bool, Double, Double)>('SceneNode::SetAnimationState')
+  external void _setAnimationState(String animationName, bool playing, bool loop, double weight, double timeScale);
 
-  @FfiNative<Void Function(Pointer<Void>, Handle, Handle)>('SceneNode::SeekAnimation')
+  @FfiNative<Void Function(Pointer<Void>, Handle, Double)>('SceneNode::SeekAnimation')
   external void _seekAnimation(String animationName, double time);
 
   /// Returns a fresh instance of [SceneShader].
diff --git a/lib/ui/painting/scene/scene_node.cc b/lib/ui/painting/scene/scene_node.cc
index 6e2f106f8e2cd..914dd2c94a5f6 100644
--- a/lib/ui/painting/scene/scene_node.cc
+++ b/lib/ui/painting/scene/scene_node.cc
@@ -132,17 +132,24 @@ void SceneNode::SetTransform(const tonic::Float64List& matrix4) {
 
 void SceneNode::SetAnimationState(const std::string& animation_name,
                                   bool playing,
+                                  bool loop,
                                   double weight,
                                   double time_scale) {
   impeller::scene::Node::MutationLog::SetAnimationStateEntry entry = {
-      animation_name, playing, static_cast<float>(weight),
-      static_cast<float>(time_scale)};
+      .animation_name = animation_name,
+      .playing = playing,
+      .loop = loop,
+      .weight = static_cast<float>(weight),
+      .time_scale = static_cast<float>(time_scale),
+  };
   node_->AddMutation(entry);
 }
 
 void SceneNode::SeekAnimation(const std::string& animation_name, double time) {
   impeller::scene::Node::MutationLog::SeekAnimationEntry entry = {
-      animation_name, static_cast<float>(time)};
+      .animation_name = animation_name,
+      .time = static_cast<float>(time),
+  };
   node_->AddMutation(entry);
 }
 
diff --git a/lib/ui/painting/scene/scene_node.h b/lib/ui/painting/scene/scene_node.h
index 563b0f9232c55..ee44e801f7de6 100644
--- a/lib/ui/painting/scene/scene_node.h
+++ b/lib/ui/painting/scene/scene_node.h
@@ -48,6 +48,7 @@ class SceneNode : public RefCountedDartWrappable<SceneNode> {
 
   void SetAnimationState(const std::string& animation_name,
                          bool playing,
+                         bool loop,
                          double weight,
                          double time_scale);
 

From 55bb7716ef3346281d599f2e3cb5330c83f69deb Mon Sep 17 00:00:00 2001
From: Brandon DeRosier <bdero@google.com>
Date: Wed, 11 Jan 2023 08:58:38 -0800
Subject: [PATCH 07/10] Backface culling

---
 impeller/scene/scene_context.cc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/impeller/scene/scene_context.cc b/impeller/scene/scene_context.cc
index 284d6da6b3ac1..1ee1a7a5b3733 100644
--- a/impeller/scene/scene_context.cc
+++ b/impeller/scene/scene_context.cc
@@ -28,6 +28,9 @@ void SceneContextOptions::ApplyToPipelineDescriptor(
 
   desc.SetSampleCount(sample_count);
   desc.SetPrimitiveType(primitive_type);
+
+  desc.SetWindingOrder(WindingOrder::kCounterClockwise);
+  desc.SetCullMode(CullMode::kBackFace);
 }
 
 SceneContext::SceneContext(std::shared_ptr<Context> context)

From ffc366620f834ff790fdd6cfca81d64bab2f08a6 Mon Sep 17 00:00:00 2001
From: Brandon DeRosier <bdero@google.com>
Date: Wed, 11 Jan 2023 09:00:03 -0800
Subject: [PATCH 08/10] Make dart analyzer happy

---
 lib/ui/experiments/scene.dart | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/ui/experiments/scene.dart b/lib/ui/experiments/scene.dart
index c18f356b05939..abe610caccff7 100644
--- a/lib/ui/experiments/scene.dart
+++ b/lib/ui/experiments/scene.dart
@@ -32,7 +32,7 @@ class SceneNode extends NativeFieldWrapperClass1 {
 
     final SceneNode sceneNode = SceneNode._create();
 
-    Future<SceneNode> futureSceneNode = _futurize((_Callback<void> callback) {
+    final Future<SceneNode> futureSceneNode = _futurize((_Callback<void> callback) {
       final String error = sceneNode._initFromAsset(assetKey, callback);
       if (error.isNotEmpty) {
         return error;
@@ -93,7 +93,7 @@ class SceneNode extends NativeFieldWrapperClass1 {
     if (sceneNodeFuture == null) {
       return;
     }
-    SceneNode sceneNode = await sceneNodeFuture;
+    final SceneNode sceneNode = await sceneNodeFuture;
 
     await _futurize((_Callback<void> callback) {
       final String error = sceneNode._initFromAsset(assetKey, callback);

From ae9e48dd9b151f9bc35cd00e7db33356cbec396d Mon Sep 17 00:00:00 2001
From: Brandon DeRosier <bdero@google.com>
Date: Wed, 11 Jan 2023 18:19:32 -0800
Subject: [PATCH 09/10] Incorporate vertex colors for imported meshes again

---
 impeller/scene/material.cc | 2 --
 1 file changed, 2 deletions(-)

diff --git a/impeller/scene/material.cc b/impeller/scene/material.cc
index 73ce339babe7f..87eb92e198013 100644
--- a/impeller/scene/material.cc
+++ b/impeller/scene/material.cc
@@ -81,13 +81,11 @@ std::unique_ptr<UnlitMaterial> UnlitMaterial::MakeFromFlatbuffer(
 
   if (material.base_color_factor()) {
     result->SetColor(importer::ToColor(*material.base_color_factor()));
-    result->SetVertexColorWeight(0);
   }
 
   if (material.base_color_texture() >= 0 &&
       material.base_color_texture() < static_cast<int32_t>(textures.size())) {
     result->SetColorTexture(textures[material.base_color_texture()]);
-    result->SetVertexColorWeight(0);
   }
 
   return result;

From 942635b9df69636cda3136288fc421b2978f9169 Mon Sep 17 00:00:00 2001
From: Brandon DeRosier <bdero@google.com>
Date: Wed, 11 Jan 2023 19:14:18 -0800
Subject: [PATCH 10/10] Address comments

---
 impeller/scene/animation/animation_player.cc | 10 ++++------
 impeller/scene/animation/animation_player.h  |  2 +-
 impeller/scene/node.cc                       |  5 +++--
 impeller/scene/node.h                        |  8 +++++---
 4 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/impeller/scene/animation/animation_player.cc b/impeller/scene/animation/animation_player.cc
index 0fadaf545343f..9fcf53306879f 100644
--- a/impeller/scene/animation/animation_player.cc
+++ b/impeller/scene/animation/animation_player.cc
@@ -41,18 +41,16 @@ AnimationClip* AnimationPlayer::AddAnimation(
         {binding.node, AnimationTransforms{.bind_pose = decomp.value()}});
   }
 
-  clips_.insert({animation->GetName(), std::move(clip)});
-  auto found = clips_.find(animation->GetName());
-  FML_DCHECK(found != clips_.end());
-  return &found->second;
+  auto result = clips_.insert({animation->GetName(), std::move(clip)});
+  return &result.first->second;
 }
 
-AnimationClip* AnimationPlayer::GetClip(const std::string& name) {
+AnimationClip* AnimationPlayer::GetClip(const std::string& name) const {
   auto result = clips_.find(name);
   if (result == clips_.end()) {
     return nullptr;
   }
-  return &result->second;
+  return const_cast<AnimationClip*>(&result->second);
 }
 
 void AnimationPlayer::Update() {
diff --git a/impeller/scene/animation/animation_player.h b/impeller/scene/animation/animation_player.h
index 2b2e85fd17eba..bcbb678d4c619 100644
--- a/impeller/scene/animation/animation_player.h
+++ b/impeller/scene/animation/animation_player.h
@@ -33,7 +33,7 @@ class AnimationPlayer final {
   AnimationClip* AddAnimation(const std::shared_ptr<Animation>& animation,
                               Node* bind_target);
 
-  AnimationClip* GetClip(const std::string& name);
+  AnimationClip* GetClip(const std::string& name) const;
 
   /// @brief  Advanced all clips and updates animated properties in the scene.
   void Update();
diff --git a/impeller/scene/node.cc b/impeller/scene/node.cc
index 9686f9775e58b..8f1da38f1e559 100644
--- a/impeller/scene/node.cc
+++ b/impeller/scene/node.cc
@@ -11,6 +11,7 @@
 
 #include "flutter/fml/logging.h"
 #include "impeller/base/strings.h"
+#include "impeller/base/thread.h"
 #include "impeller/base/validation.h"
 #include "impeller/geometry/matrix.h"
 #include "impeller/scene/animation/animation_player.h"
@@ -26,17 +27,17 @@ namespace scene {
 static std::atomic_uint64_t kNextNodeID = 0;
 
 void Node::MutationLog::Append(const Entry& entry) {
-  std::lock_guard<std::mutex> lock(write_mutex_);
+  WriterLock lock(write_mutex_);
   dirty_ = true;
   entries_.push_back(entry);
 }
 
 std::optional<std::vector<Node::MutationLog::Entry>>
 Node::MutationLog::Flush() {
+  WriterLock lock(write_mutex_);
   if (!dirty_) {
     return std::nullopt;
   }
-  std::lock_guard<std::mutex> lock(write_mutex_);
   dirty_ = false;
   auto result = entries_;
   entries_ = {};
diff --git a/impeller/scene/node.h b/impeller/scene/node.h
index 07dc6b66704a3..2f144397500fd 100644
--- a/impeller/scene/node.h
+++ b/impeller/scene/node.h
@@ -10,6 +10,8 @@
 #include <vector>
 
 #include "flutter/fml/macros.h"
+#include "impeller/base/thread.h"
+#include "impeller/base/thread_safety.h"
 #include "impeller/geometry/matrix.h"
 #include "impeller/renderer/render_target.h"
 #include "impeller/renderer/texture.h"
@@ -53,9 +55,9 @@ class Node final {
    private:
     std::optional<std::vector<Entry>> Flush();
 
-    bool dirty_ = false;
-    std::vector<Entry> entries_;
-    std::mutex write_mutex_;
+    RWMutex write_mutex_;
+    bool dirty_ IPLR_GUARDED_BY(write_mutex_) = false;
+    std::vector<Entry> entries_ IPLR_GUARDED_BY(write_mutex_);
 
     friend Node;
   };