Skip to content

Commit

Permalink
feat(dom): fix vue3 list item position (Tencent#3826)
Browse files Browse the repository at this point in the history
* feat(dom): fix vue3 list sort

* feat(dom): fix vue3 list item position

* feat(dom): perf code && adjust needSortByIndex

* feat(dom): fix dom manager unit test

* feat(dom): avoid get self depth when skip sortByIndex
  • Loading branch information
zealotchen0 authored and wwwcg committed Apr 12, 2024
1 parent b99a215 commit 446417d
Show file tree
Hide file tree
Showing 11 changed files with 63 additions and 20 deletions.
2 changes: 1 addition & 1 deletion dom/include/dom/dom_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class DomManager : public std::enable_shared_from_this<DomManager> {
uint32_t id) ;

static void CreateDomNodes(const std::weak_ptr<RootNode>& weak_root_node,
std::vector<std::shared_ptr<DomInfo>>&& nodes);
std::vector<std::shared_ptr<DomInfo>>&& nodes, bool needSortByIndex);
static void UpdateDomNodes(const std::weak_ptr<RootNode>& weak_root_node,
std::vector<std::shared_ptr<DomInfo>>&& nodes);
static void MoveDomNodes(const std::weak_ptr<RootNode>& weak_root_node,
Expand Down
2 changes: 2 additions & 0 deletions dom/include/dom/dom_node.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ class DomNode : public std::enable_shared_from_this<DomNode> {
uint32_t id = kInvalidId; // RenderNode的id
uint32_t pid = kInvalidId; // 父RenderNode的id
int32_t index = kInvalidIndex; // 本节点在父RenderNode上的索引
int32_t depth = kInvalidIndex; // 本节点在父RenderNode上的深度
};

inline std::shared_ptr<DomNode> GetParent() { return parent_.lock(); }
Expand Down Expand Up @@ -142,6 +143,7 @@ class DomNode : public std::enable_shared_from_this<DomNode> {
void MarkWillChange(bool flag);
int32_t GetSelfIndex();
int32_t GetChildIndex(uint32_t id);
int32_t GetSelfDepth();

int32_t IndexOf(const std::shared_ptr<DomNode>& child);
std::shared_ptr<DomNode> GetChildAt(size_t index);
Expand Down
2 changes: 1 addition & 1 deletion dom/include/dom/root_node.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class RootNode : public DomNode {
virtual void RemoveEventListener(const std::string& name, uint64_t listener_id) override;

void ReleaseResources();
void CreateDomNodes(std::vector<std::shared_ptr<DomInfo>>&& nodes);
void CreateDomNodes(std::vector<std::shared_ptr<DomInfo>>&& nodes, bool needSortByIndex);
void UpdateDomNodes(std::vector<std::shared_ptr<DomInfo>>&& nodes);
void MoveDomNodes(std::vector<std::shared_ptr<DomInfo>>&& nodes);
void DeleteDomNodes(std::vector<std::shared_ptr<DomInfo>>&& nodes);
Expand Down
3 changes: 2 additions & 1 deletion dom/include/dom/scene_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ class SceneBuilder {

static void Create(const std::weak_ptr<DomManager>& dom_manager,
const std::weak_ptr<RootNode>& root_node,
std::vector<std::shared_ptr<DomInfo>>&& nodes);
std::vector<std::shared_ptr<DomInfo>>&& nodes,
bool needSortByIndex);
static void Update(const std::weak_ptr<DomManager>& dom_manager,
const std::weak_ptr<RootNode>& root_node,
std::vector<std::shared_ptr<DomInfo>>&& nodes);
Expand Down
7 changes: 4 additions & 3 deletions dom/src/dom/dom_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,14 @@ std::shared_ptr<DomNode> DomManager::GetNode(const std::weak_ptr<RootNode>& weak
}

void DomManager::CreateDomNodes(const std::weak_ptr<RootNode>& weak_root_node,
std::vector<std::shared_ptr<DomInfo>>&& nodes) {
std::vector<std::shared_ptr<DomInfo>>&& nodes,
bool needSortByIndex) {
auto root_node = weak_root_node.lock();
if (!root_node) {
return;
}
size_t create_size = nodes.size();
root_node->CreateDomNodes(std::move(nodes));
root_node->CreateDomNodes(std::move(nodes), needSortByIndex);
FOOTSTONE_DLOG(INFO) << "[Hippy Statistic] create node size = " << create_size << ", total node size = " << root_node->GetChildCount();
}

Expand Down Expand Up @@ -265,7 +266,7 @@ bool DomManager::SetSnapShot(const std::shared_ptr<RootNode>& root_node, const b
nodes.push_back(std::make_shared<DomInfo>(dom_node, nullptr, nullptr));
}

CreateDomNodes(root_node, std::move(nodes));
CreateDomNodes(root_node, std::move(nodes), false);
EndBatch(root_node);

return true;
Expand Down
6 changes: 3 additions & 3 deletions dom/src/dom/dom_manager_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ TEST(DomManagerTest, CreateDomNodes) {
std::shared_ptr<hippy::DomNode> root_node = manager->GetNode(10);
root_node->SetDomManager(manager);
std::vector<std::shared_ptr<hippy::DomInfo>> infos = ParserFile("create_node.json", manager);
manager->CreateDomNodes(std::move(infos));
manager->CreateDomNodes(std::move(infos), false);

ASSERT_EQ(root_node->GetChildren().size(), 1);
auto child = root_node->GetChildren();
Expand All @@ -156,7 +156,7 @@ TEST(DomManagerTest, UpdateDomNodes) {
std::shared_ptr<hippy::DomNode> root_node = manager->GetNode(10);
root_node->SetDomManager(manager);
std::vector<std::shared_ptr<hippy::DomInfo>> infos = ParserFile("create_node.json", manager);
manager->CreateDomNodes(std::move(infos));
manager->CreateDomNodes(std::move(infos), false);
std::string json =
"[[{\"id\":59,\"pId\":61,\"name\":\"Text\",\"props\":{\"numberOfLines\":1,\"text\":\"本地调试\","
"\"style\":{\"color\":4280558628,\"fontSize\":26}}},{}]]";
Expand All @@ -174,7 +174,7 @@ TEST(DomManagerTest, DeleteDomNodes) {
std::shared_ptr<hippy::DomNode> root_node = manager->GetNode(10);
root_node->SetDomManager(manager);
std::vector<std::shared_ptr<hippy::DomInfo>> infos = ParserFile("create_node.json", manager);
manager->CreateDomNodes(std::move(infos));
manager->CreateDomNodes(std::move(infos), false);

std::string json = "[[{\"id\":63,\"pId\":10,\"name\":\"View\"},{}]]";
std::vector<std::shared_ptr<hippy::DomInfo>> delete_nodes = ParserJson(json, manager);
Expand Down
7 changes: 7 additions & 0 deletions dom/src/dom/dom_node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,13 @@ int32_t DomNode::GetSelfIndex() {
return -1;
}

int32_t DomNode::GetSelfDepth() {
if (auto parent = parent_.lock()) {
return 1 + parent->GetSelfDepth();
}
return 1;
}

std::shared_ptr<DomNode> DomNode::RemoveChildAt(int32_t index) {
auto child = children_[footstone::check::checked_numeric_cast<int32_t, unsigned long>(index)];
child->SetParent(nullptr);
Expand Down
37 changes: 31 additions & 6 deletions dom/src/dom/root_node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,13 @@ footstone::utils::PersistentObjectMap<uint32_t, std::shared_ptr<RootNode>> RootN
// | 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.
// 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.
// 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,
Expand Down Expand Up @@ -122,7 +122,7 @@ void RootNode::RemoveEventListener(const std::string& name, uint64_t listener_id

void RootNode::ReleaseResources() {}

void RootNode::CreateDomNodes(std::vector<std::shared_ptr<DomInfo>>&& nodes) {
void RootNode::CreateDomNodes(std::vector<std::shared_ptr<DomInfo>>&& nodes, bool needSortByIndex) {
for (const auto& interceptor : interceptors_) {
interceptor->OnDomNodeCreate(nodes);
}
Expand All @@ -142,8 +142,33 @@ void RootNode::CreateDomNodes(std::vector<std::shared_ptr<DomInfo>>&& nodes) {
OnDomNodeCreated(node);
}
for (const auto& node : nodes_to_create) {
node->SetRenderInfo({node->GetId(), node->GetPid(), node->GetSelfIndex()});
if (needSortByIndex) {
node->SetRenderInfo({node->GetId(), node->GetPid(), node->GetSelfIndex(), node->GetSelfDepth()});
} else {
// 如果不需要对 index 排序,其他场景目前没有用到 depth,避免冗余计算
node->SetRenderInfo({node->GetId(), node->GetPid(), node->GetSelfIndex(), -1});
}
}

if (needSortByIndex) {
// 针对反向插入的场景 (比如先查 index = 15的节点,再插入 index = 14,13,12.. 的节点),先做排序。否则会导致 renderNode 节点位置错乱。详见:
// https://doc.weixin.qq.com/doc/w3_ANsAsgZ1ACckOPazHXERJqKHOCbP1?scode=AJEAIQdfAAogJJ2RicAMgAvQZ1ACc
// 排序要保证两个原则:1. 父节点在子节点前;2. 同一父节点的子节点,必须按照 index 从小到大的顺序排序
// 同一层级,不同父节点的子节点,位置可以交叉,但要保证原则2,即同一父节点子节点 index 是从小到大的顺序
std::stable_sort(
nodes_to_create.begin(),
nodes_to_create.end(),
[](const std::shared_ptr<hippy::DomNode>& a, const std::shared_ptr<hippy::DomNode>& b)
{
auto render_info_a = a->GetRenderInfo();
auto render_info_b = b->GetRenderInfo();
if (render_info_a.depth == render_info_b.depth) {
return render_info_a.index < render_info_b.index;
}
return render_info_a.depth < render_info_b.depth;
});
}

auto event = std::make_shared<DomEvent>(kDomTreeCreated, weak_from_this(), nullptr);
HandleEvent(event);

Expand Down
5 changes: 3 additions & 2 deletions dom/src/dom/scene_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,12 @@ inline namespace dom {

void SceneBuilder::Create(const std::weak_ptr<DomManager>& weak_dom_manager,
const std::weak_ptr<RootNode>& root_node,
std::vector<std::shared_ptr<DomInfo>>&& nodes) {
std::vector<std::shared_ptr<DomInfo>>&& nodes,
bool needSortByIndex) {
auto dom_manager = weak_dom_manager.lock();
if (dom_manager) {
dom_manager->RecordDomStartTimePoint();
dom_manager->CreateDomNodes(root_node, std::move(nodes));
dom_manager->CreateDomNodes(root_node, std::move(nodes), needSortByIndex);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,14 @@ function endBatch() {
const { rootViewId } = getHippyCachedInstance();
// create Scene Builder with rootView id
const sceneBuilder = new global.Hippy.SceneBuilder(rootViewId);
// nodes need sort by index
const needSortByIndex = true;
// batch operations on nodes based on operation type
chunks.forEach((chunk) => {
switch (chunk.type) {
case NodeOperateType.CREATE:
printNodeOperation(chunk.printedNodes, 'createNode');
sceneBuilder.create(chunk.nodes);
sceneBuilder.create(chunk.nodes, needSortByIndex);
handleEventListeners(chunk.eventNodes, sceneBuilder);
break;
case NodeOperateType.UPDATE:
Expand Down
8 changes: 6 additions & 2 deletions driver/js/src/modules/scene_builder_module.cc
Original file line number Diff line number Diff line change
Expand Up @@ -435,8 +435,12 @@ std::shared_ptr<ClassTemplate<SceneBuilder>> RegisterSceneBuilder(const std::wea
if (!scope) {
return nullptr;
}
auto ret = HandleJsValue(scope->GetContext(), arguments[0], scope);
SceneBuilder::Create(scope->GetDomManager(), scope->GetRootNode(), std::move(std::get<2>(ret)));
auto nodes = HandleJsValue(scope->GetContext(), arguments[0], scope);
bool needSortByIndex = false;
if (argument_count == 2) {
scope->GetContext()->GetValueBoolean(arguments[1], &needSortByIndex);
}
SceneBuilder::Create(scope->GetDomManager(), scope->GetRootNode(), std::move(std::get<2>(nodes)), needSortByIndex);
return nullptr;
};
class_template.functions.emplace_back(std::move(create_func_def));
Expand Down

0 comments on commit 446417d

Please sign in to comment.