Skip to content

Commit

Permalink
fix(dom): fix dom diff algorithm
Browse files Browse the repository at this point in the history
  • Loading branch information
@ilikethese @hippy-actions
ilikethese authored and hippy-actions[bot] committed Dec 15, 2023
1 parent 5b94924 commit 637d5e9
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 64 deletions.
36 changes: 28 additions & 8 deletions dom/include/dom/root_node.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,37 @@

#include <stack>

#include "dom/diff_utils.h"
#include "dom/dom_node.h"
#include "footstone/task_runner.h"
#include "footstone/persistent_object_map.h"
#include "footstone/task_runner.h"

namespace hippy {
inline namespace dom {

class RootNode;

/**
* In HippyVue/HippyReact, updating node styles can be intricate.
* This class is specifically designed to compute the differences when updating DOM node styles.
*/
class DomNodeStyleDiffer {
public:
DomNodeStyleDiffer() = default;
~DomNodeStyleDiffer() = default;

bool Calculate(const std::shared_ptr<hippy::dom::RootNode>& root_node, const std::shared_ptr<DomInfo>& dom_info,
hippy::dom::DiffValue& style_diff, hippy::dom::DiffValue& ext_style_diff);
void Reset() {
node_ext_style_map_.clear();
node_style_map_.clear();
}

private:
std::unordered_map<uint32_t, std::unordered_map<std::string, std::shared_ptr<HippyValue>>> node_style_map_;
std::unordered_map<uint32_t, std::unordered_map<std::string, std::shared_ptr<HippyValue>>> node_ext_style_map_;
};

class RootNode : public DomNode {
public:
using TaskRunner = footstone::runner::TaskRunner;
Expand Down Expand Up @@ -71,7 +95,6 @@ class RootNode : public DomNode {
void Traverse(const std::function<void(const std::shared_ptr<DomNode>&)>& on_traverse);
void AddInterceptor(const std::shared_ptr<DomActionInterceptor>& interceptor);


static footstone::utils::PersistentObjectMap<uint32_t, std::shared_ptr<RootNode>>& PersistentMap() {
return persistent_map_;
}
Expand All @@ -80,16 +103,12 @@ class RootNode : public DomNode {
static void MarkLayoutNodeDirty(const std::vector<std::shared_ptr<DomNode>>& nodes);

struct DomOperation {
enum class Op {
kOpCreate, kOpUpdate, kOpDelete, kOpMove
} op;
enum class Op { kOpCreate, kOpUpdate, kOpDelete, kOpMove } op;
std::vector<std::shared_ptr<DomNode>> nodes;
};

struct EventOperation {
enum class Op {
kOpAdd, kOpRemove
} op;
enum class Op { kOpAdd, kOpRemove } op;
uint32_t id;
std::string name;
};
Expand All @@ -107,6 +126,7 @@ class RootNode : public DomNode {
std::weak_ptr<DomManager> dom_manager_;
std::vector<std::shared_ptr<DomActionInterceptor>> interceptors_;
std::shared_ptr<AnimationManager> animation_manager_;
std::unique_ptr<DomNodeStyleDiffer> style_differ_;

static footstone::utils::PersistentObjectMap<uint32_t, std::shared_ptr<RootNode>> persistent_map_;
};
Expand Down
132 changes: 76 additions & 56 deletions dom/src/dom/root_node.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
#include <stack>

#include "dom/animation/animation_manager.h"
#include "dom/diff_utils.h"
#include "dom/render_manager.h"
#include "footstone/deserializer.h"
#include "footstone/hippy_value.h"
Expand All @@ -45,10 +44,67 @@ using Task = footstone::Task;

footstone::utils::PersistentObjectMap<uint32_t, std::shared_ptr<RootNode>> RootNode::persistent_map_;

// In Hippy Vue, there are some special cases where there are multiple update instructions for the same node. This can
// cause issues with the diff algorithm and lead to incorrect results.
// Example:
//
// Dom Node:
// |------|--------------------------------------|
// | id | style: {text: "a", color: "red"} |
// | 1 | diff: {} |
// |------|--------------------------------------|
//
// Previous update algorithm:
// |------|-----------------------| update instructions: |------|-----------------------| update instructions: |------|-------------------------------------|
// | id | style: {text: "a"} | { text: "b"} | id | style: {text: "b"} | { text: "b", fontsize: 12} | id | style: {text: "b", fontsize: 12} |
// | 1 | diff: {} | -------------------> | 1 | diff: {text: "b"} | --------------------------> | 1 | diff: {fontsize: "b"} |
// |------|-----------------------| |------|-----------------------| |------|-------------------------------------|
// In the previous diff algorithm, the differences were generated by comparing the DOM styles and update instructions.
// However, in Hippy Vue, two update instructions might be generated within the same batch. This can lead to incorrect diff results.
// The diff should be {text: "b", fontsize: 12}, but the previous diff algorithm cacluate {fontsize: "b"}
//
// To address this issue, the new update algorithm is as follows:
// 1. When a node's style needs to be updated for the first time, we save the current style.
// 2. Subsequent update differences are generated by comparing the saved styles with the update instructions.
// 3. At the end of the batch, we clear the saved styles.
bool DomNodeStyleDiffer::Calculate(const std::shared_ptr<hippy::dom::RootNode>& root_node,
const std::shared_ptr<DomInfo>& dom_info, hippy::dom::DiffValue& style_diff,
hippy::dom::DiffValue& ext_style_diff) {
if (!root_node) return false;
if (dom_info == nullptr || dom_info->dom_node == nullptr) return false;

auto dom_node = root_node->GetNode(dom_info->dom_node->GetId());
if (dom_node == nullptr) return false;
uint32_t dom_id = dom_node->GetId();

// 保存 batch 最早的 style 和 ext_style, 该批次中的所有的 diff 都由这个 style 比较产生
if (node_style_map_.find(dom_id) == node_style_map_.end()) {
std::unordered_map<std::string, std::shared_ptr<HippyValue>> style;
std::unordered_map<std::string, std::shared_ptr<HippyValue>> ext_style;
auto dom_style = dom_node->GetStyleMap();
for (const auto& pair : *dom_style) {
style[pair.first] = std::make_shared<HippyValue>(*pair.second);
}
node_style_map_.insert({dom_id, style});
auto dom_ext_style = dom_node->GetExtStyle();
for (const auto& pair : *dom_ext_style) {
ext_style[pair.first] = std::make_shared<HippyValue>(*pair.second);
}
node_ext_style_map_.insert({dom_id, ext_style});
}

auto base_style = node_style_map_.at(dom_id);
auto base_ext_style = node_ext_style_map_.at(dom_id);
style_diff = DiffUtils::DiffProps(base_style, *dom_info->dom_node->GetStyleMap(), false);
ext_style_diff = DiffUtils::DiffProps(base_ext_style, *dom_info->dom_node->GetExtStyle(), false);
return true;
}

RootNode::RootNode(uint32_t id) : DomNode(id, 0, 0, "", "", nullptr, nullptr, {}) {
SetRenderInfo({id, 0, 0});
animation_manager_ = std::make_shared<AnimationManager>();
interceptors_.push_back(animation_manager_);
style_differ_ = std::make_unique<DomNodeStyleDiffer>();
}

RootNode::RootNode() : RootNode(0) {}
Expand Down Expand Up @@ -101,70 +157,34 @@ void RootNode::UpdateDomNodes(std::vector<std::shared_ptr<DomInfo>>&& nodes) {
interceptor->OnDomNodeUpdate(nodes);
}

// In Hippy Vue, there are some special cases where there are multiple update instructions for the same node. This can
// cause issues with the diff algorithm and lead to incorrect results.
// Example:
//
// Dom Node Style:
// |------|----------------|
// | id | style |
// | 1 | text : {} |
// | 2 | some style |
//
// Update instructions:
// |------|-------------------------------|----------------|------------------------|
// | id | style | operation | diff style result |
// | 1 | text : { "color": "blue" } | compare | { "color": "blue" } |
// | 2 | some style | | |
// | 1 | text : { "color": "red" } | compare | { "color": "red" } |
// | 1 | text : { "color": "red" } | compare | { } |
// In last diff algroithm the diff_style = {}
//
// To Solve this case we should use the last update instruction to generate the diff style.
// Update instructions:
// |------|-------------------------------|----------------|------------------------|
// | id | style | operation | diff style result |
// | 1 | text : { "color": "blue" } | skip | { } |
// | 2 | some style | | |
// | 1 | text : { "color": "red" } | skip | { } |
// | 1 | text : { "color": "red" } | compare | { "color": "red" } |
// In new diff algroithm the diff_style = { "color": "red" }
std::unordered_map<uint32_t, std::shared_ptr<DomInfo>> skipped_instructions;
for (const auto& node_info : nodes) {
auto id = node_info->dom_node->GetId();
skipped_instructions[id] = node_info;
}

std::vector<std::shared_ptr<DomNode>> nodes_to_update;
for (const auto& [id, node_info] : skipped_instructions) {
std::shared_ptr<DomNode> dom_node = GetNode(node_info->dom_node->GetId());
for (const auto& node : nodes) {
std::shared_ptr<DomNode> dom_node = GetNode(node->dom_node->GetId());
if (dom_node == nullptr) {
continue;
}
auto skip_style_diff = false;
if (node_info->diff_info != nullptr) {
skip_style_diff = node_info->diff_info->skip_style_diff;

hippy::dom::DiffValue style_diff, ext_style_diff;
if (!style_differ_->Calculate(std::static_pointer_cast<RootNode>(shared_from_this()), node, style_diff,
ext_style_diff)) {
continue;
}
// diff props
auto style_diff_value = DiffUtils::DiffProps(*dom_node->GetStyleMap(), *node_info->dom_node->GetStyleMap(), skip_style_diff);
auto ext_diff_value = DiffUtils::DiffProps(*dom_node->GetExtStyle(), *node_info->dom_node->GetExtStyle(), false);
auto style_update = std::get<0>(style_diff_value);
auto ext_update = std::get<0>(ext_diff_value);

auto style_update = std::get<0>(style_diff);
auto ext_update = std::get<0>(ext_style_diff);
std::shared_ptr<DomValueMap> diff_value = std::make_shared<DomValueMap>();
if (!style_update->empty()) {
diff_value->insert(style_update->begin(), style_update->end());
}
if (!ext_update->empty()) {
diff_value->insert(ext_update->begin(), ext_update->end());
}
if (!skip_style_diff) {
dom_node->SetStyleMap(node_info->dom_node->GetStyleMap());
}
dom_node->SetExtStyleMap(node_info->dom_node->GetExtStyle());
dom_node->SetStyleMap(node->dom_node->GetStyleMap());
dom_node->SetExtStyleMap(node->dom_node->GetExtStyle());
dom_node->SetDiffStyle(diff_value);

auto style_delete = std::get<1>(style_diff_value);
auto ext_delete = std::get<1>(ext_diff_value);
auto style_delete = std::get<1>(style_diff);
auto ext_delete = std::get<1>(ext_style_diff);
std::shared_ptr<std::vector<std::string>> delete_value = std::make_shared<std::vector<std::string>>();
if (!style_delete->empty()) {
delete_value->insert(delete_value->end(), style_delete->begin(), style_delete->end());
Expand All @@ -174,8 +194,6 @@ void RootNode::UpdateDomNodes(std::vector<std::shared_ptr<DomInfo>>&& nodes) {
delete_value->insert(delete_value->end(), ext_delete->begin(), ext_delete->end());
}
dom_node->SetDeleteProps(delete_value);
node_info->dom_node->SetDiffStyle(diff_value);
node_info->dom_node->SetDeleteProps(delete_value);
if (!style_update->empty() || !style_delete->empty()) {
dom_node->UpdateLayoutStyleInfo(*style_update, *style_delete);
}
Expand Down Expand Up @@ -278,9 +296,11 @@ void RootNode::CallFunction(uint32_t id, const std::string& name, const DomArgum
}

void RootNode::SyncWithRenderManager(const std::shared_ptr<RenderManager>& render_manager) {
TDF_PERF_DO_STMT_AND_LOG(unsigned long domCnt = dom_operations_.size(); , "RootNode::SyncWithRenderManager");
TDF_PERF_DO_STMT_AND_LOG(unsigned long domCnt = dom_operations_.size();, "RootNode::SyncWithRenderManager");
if (style_differ_ != nullptr) style_differ_->Reset();
FlushDomOperations(render_manager);
TDF_PERF_DO_STMT_AND_LOG(unsigned long evCnt = event_operations_.size(); , "RootNode::FlushDomOperations Done, dom op count:%lld", domCnt);
TDF_PERF_DO_STMT_AND_LOG(unsigned long evCnt = event_operations_.size();
, "RootNode::FlushDomOperations Done, dom op count:%lld", domCnt);
FlushEventOperations(render_manager);
TDF_PERF_LOG("RootNode::FlushEventOperations Done, event op count:%d", evCnt);
DoAndFlushLayout(render_manager);
Expand Down Expand Up @@ -455,7 +475,7 @@ void RootNode::FlushDomOperations(const std::shared_ptr<RenderManager>& render_m
}

void RootNode::MarkLayoutNodeDirty(const std::vector<std::shared_ptr<DomNode>>& nodes) {
for (const auto& node: nodes) {
for (const auto& node : nodes) {
if (node && node->GetLayoutNode() && !node->GetLayoutNode()->HasParentEngineNode()) {
auto parent = node->GetParent();
while (parent) {
Expand Down

0 comments on commit 637d5e9

Please sign in to comment.