Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 21 additions & 6 deletions impeller/scene/animation/animation_player.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::maps insert or insert_or_assign returns an iterator to element already. So you don't need to insert and then in turn find the inserted element.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

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() {
Expand All @@ -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();
}
Expand Down
8 changes: 5 additions & 3 deletions impeller/scene/animation/animation_player.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@

#pragma once

#include <map>
#include <memory>
#include <optional>
#include <unordered_map>
#include <vector>

#include "flutter/fml/hash_combine.h"
Expand All @@ -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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing const correctness here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


/// @brief Advanced all clips and updates animated properties in the scene.
void Update();

Expand All @@ -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_;

Expand Down
2 changes: 1 addition & 1 deletion impeller/scene/geometry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
57 changes: 55 additions & 2 deletions impeller/scene/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <inttypes.h>
#include <atomic>
#include <memory>
#include <vector>

#include "flutter/fml/logging.h"
#include "impeller/base/strings.h"
Expand All @@ -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) {
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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();
}
Expand All @@ -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
42 changes: 40 additions & 2 deletions impeller/scene/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#pragma once

#include <memory>
#include <mutex>
#include <optional>
#include <vector>

Expand All @@ -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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

impeller::Scalar here and default initialize the rest?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

float time_scale;
};

struct SeekAnimationEntry {
std::string animation_name;
float time;
};

using Entry = std::

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this just calls for inheritance instead of variants?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm on the fence, but I think that's where it will end up. Will do this in a later patch.

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_;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use impeller::Mutex, impeller::RWMutex, etc. and the associated static thread safety analysis macros here. That way, the compiler will tell us attempt a data race.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


friend Node;
};

static std::shared_ptr<Node> MakeFromFlatbuffer(
const fml::Mapping& ipscene_mapping,
Allocator& allocator);
Expand All @@ -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;
Expand All @@ -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(
Expand All @@ -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_;
Expand Down
5 changes: 2 additions & 3 deletions impeller/scene/scene.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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()));
}
Expand Down
4 changes: 2 additions & 2 deletions impeller/scene/scene.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down
19 changes: 10 additions & 9 deletions impeller/scene/scene_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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();
Expand Down
Loading