From e2341a4a681e3f63d2719af7dbb21fcd714a1fdb Mon Sep 17 00:00:00 2001 From: Tianer Zhou Date: Sat, 15 Jun 2024 10:58:24 +0800 Subject: [PATCH 1/2] optimize section update, add diff Signed-off-by: Tianer Zhou Change-Id: Ie2e8e59ba296014a52741752d20aecf1791c42d6 --- .../pattern/waterflow/water_flow_pattern.cpp | 8 +-- .../pattern/waterflow/water_flow_sections.cpp | 54 +++++++++---------- .../pattern/waterflow/water_flow_sections.h | 22 ++++---- .../native/node/water_flow_modifier.cpp | 2 +- 4 files changed, 40 insertions(+), 46 deletions(-) diff --git a/frameworks/core/components_ng/pattern/waterflow/water_flow_pattern.cpp b/frameworks/core/components_ng/pattern/waterflow/water_flow_pattern.cpp index 95b7c0b6463..85608c51597 100644 --- a/frameworks/core/components_ng/pattern/waterflow/water_flow_pattern.cpp +++ b/frameworks/core/components_ng/pattern/waterflow/water_flow_pattern.cpp @@ -455,7 +455,7 @@ RefPtr WaterFlowPattern::GetOrCreateWaterFlowSections() pattern->OnSectionChangedNow(start); }; sections_->SetOnDataChange(callback); - sections_->SetOnDataChangeNow(callbackNow); + sections_->SetOnDataChangeCAPI(callbackNow); return sections_; } @@ -467,12 +467,6 @@ void WaterFlowPattern::OnSectionChanged(int32_t start) } auto info = DynamicCast(layoutInfo_); CHECK_NULL_VOID(info); - auto host = GetHost(); - CHECK_NULL_VOID(host); - if (sections_->IsSpecialUpdate()) { - // optimize adding or removing children in the last section. Prevent complete reset of that section. - ++start; - } info->InitSegments(sections_->GetSectionInfo(), start); info->margins_.clear(); diff --git a/frameworks/core/components_ng/pattern/waterflow/water_flow_sections.cpp b/frameworks/core/components_ng/pattern/waterflow/water_flow_sections.cpp index c68fcf1dd70..a8197b2decd 100644 --- a/frameworks/core/components_ng/pattern/waterflow/water_flow_sections.cpp +++ b/frameworks/core/components_ng/pattern/waterflow/water_flow_sections.cpp @@ -16,37 +16,47 @@ #include "frameworks/core/components_ng/pattern/waterflow/water_flow_sections.h" namespace OHOS::Ace::NG { +// push: start, 0, newSection +// update: start, 0, newSection +// splice: start, deleteCount, newSections void WaterFlowSections::ChangeData( - int32_t start, int32_t deleteCount, const std::vector& newSections) + size_t start, int32_t deleteCount, const std::vector& newSections) { - if (deleteCount == 1 && newSections.size() == 1) { - // prepare for checking special case - prevSection_ = { sections_[start], start }; - } else { - prevSection_ = std::nullopt; - } + prevSections_ = sections_; TAG_LOGI(AceLogTag::ACE_WATERFLOW, "section changed, start:%{public}d, deleteCount:%{public}d, newSections:%{public}zu", start, deleteCount, newSections.size()); - if (static_cast(start) < sections_.size()) { - auto it = sections_.begin() + start; + if (start < sections_.size()) { + auto it = sections_.begin() + static_cast(start); sections_.erase(it, it + deleteCount); sections_.insert(it, newSections.begin(), newSections.end()); } else { sections_.insert(sections_.end(), newSections.begin(), newSections.end()); } + // perform diff to get actual [start] + for (; start < sections_.size(); ++start) { + if (start >= prevSections_.size()) { + break; + } + if (sections_[start] != prevSections_[start]) { + // can skip re-init the first modified section with only an itemCount diff + // common scenario when develop add/remove items at the end + if (sections_[start].OnlyCountDiff(prevSections_[start])) { + ++start; + } + break; + } + } + if (onSectionDataChange_) { - // push: start, 0, newSection - // update: start, 0, newSection - // splice: start, deleteCount, newSections onSectionDataChange_(start); } } // for c-api, replace all -void WaterFlowSections::ChangeDataNow( +void WaterFlowSections::ChangeDataCAPI( int32_t start, int32_t deleteCount, const std::vector& newSections) { prevSections_ = sections_; @@ -64,26 +74,12 @@ void WaterFlowSections::ChangeDataNow( sections_.insert(sections_.end(), newSections.begin(), newSections.end()); } - if (onSectionDataChangeNow_) { + if (onSectionDataChangeCAPI_) { // push: start, 0, newSection // update: start, 0, newSection // splice: start, deleteCount, newSections - onSectionDataChangeNow_(start); - } -} - -bool WaterFlowSections::IsSpecialUpdate() const -{ - if (sections_.empty() || !prevSection_) { - return false; - } - const int32_t start = prevSection_->second; - if (start >= static_cast(sections_.size())) { - return false; + onSectionDataChangeCAPI_(start); } - const auto& cur = sections_[start]; - const auto& prev = prevSection_->first; - return cur.OnlyCountDiff(prev); } bool WaterFlowSections::IsSpecialUpdateCAPI(int32_t updateIndex) const diff --git a/frameworks/core/components_ng/pattern/waterflow/water_flow_sections.h b/frameworks/core/components_ng/pattern/waterflow/water_flow_sections.h index 83c6d2c659b..4f08485d156 100644 --- a/frameworks/core/components_ng/pattern/waterflow/water_flow_sections.h +++ b/frameworks/core/components_ng/pattern/waterflow/water_flow_sections.h @@ -60,12 +60,20 @@ public: { onSectionDataChange_ = func; } - void SetOnDataChangeNow(std::function&& func) + void SetOnDataChangeCAPI(std::function&& func) { - onSectionDataChangeNow_ = func; + onSectionDataChangeCAPI_ = func; } - void ChangeData(int32_t start, int32_t deleteCount, const std::vector
& newSections); - void ChangeDataNow(int32_t start, int32_t deleteCount, const std::vector
& newSections); + + /** + * @brief Change section data. + * + * @param start first modified section index. + * @param deleteCount number of sections to delete at index [start]. + * @param newSections to insert at index [start]. + */ + void ChangeData(size_t start, int32_t deleteCount, const std::vector
& newSections); + void ChangeDataCAPI(int32_t start, int32_t deleteCount, const std::vector
& newSections); const std::vector
& GetSectionInfo() const { return sections_; @@ -76,17 +84,13 @@ public: * * @return true only if itemCount in the modified section has changed and everything else remains the same. */ - bool IsSpecialUpdate() const; bool IsSpecialUpdateCAPI(int32_t updateIndex) const; private: - // {first changed section, index of that section} - // for comparing and handling special update case - std::optional> prevSection_; std::vector
sections_; // for comparing and handling special case std::vector
prevSections_; std::function onSectionDataChange_; - std::function onSectionDataChangeNow_; + std::function onSectionDataChangeCAPI_; }; } // namespace OHOS::Ace::NG diff --git a/frameworks/core/interfaces/native/node/water_flow_modifier.cpp b/frameworks/core/interfaces/native/node/water_flow_modifier.cpp index 7a9cf07978b..41527b5fd22 100644 --- a/frameworks/core/interfaces/native/node/water_flow_modifier.cpp +++ b/frameworks/core/interfaces/native/node/water_flow_modifier.cpp @@ -457,7 +457,7 @@ void SetWaterFlowSectionOptions(ArkUINodeHandle node, ArkUI_Int32 start, ArkUIWa } } - waterFlowSections->ChangeDataNow(start, newSections.size(), newSections); + waterFlowSections->ChangeDataCAPI(start, newSections.size(), newSections); } void ResetWaterFlowSectionOptions(ArkUINodeHandle node) -- Gitee From 025b38c992fc360df5727b5bdfca47294e6ec9a9 Mon Sep 17 00:00:00 2001 From: Tianer Zhou Date: Sat, 15 Jun 2024 11:08:00 +0800 Subject: [PATCH 2/2] add test Signed-off-by: Tianer Zhou Change-Id: I64628a9573dffaba4d0a78bb2447b06d04dcd2da --- .../pattern/waterflow/water_flow_sections.cpp | 7 ++- .../pattern/waterflow/water_flow_sections.h | 2 +- test/unittest/core/pattern/waterflow/BUILD.gn | 1 + .../waterflow/water_flow_section_test.cpp | 58 +++++++++++++++++++ .../water_flow_segment_integrated.cpp | 5 +- 5 files changed, 67 insertions(+), 6 deletions(-) create mode 100644 test/unittest/core/pattern/waterflow/water_flow_section_test.cpp diff --git a/frameworks/core/components_ng/pattern/waterflow/water_flow_sections.cpp b/frameworks/core/components_ng/pattern/waterflow/water_flow_sections.cpp index a8197b2decd..0a7a065c8ac 100644 --- a/frameworks/core/components_ng/pattern/waterflow/water_flow_sections.cpp +++ b/frameworks/core/components_ng/pattern/waterflow/water_flow_sections.cpp @@ -25,7 +25,7 @@ void WaterFlowSections::ChangeData( prevSections_ = sections_; TAG_LOGI(AceLogTag::ACE_WATERFLOW, - "section changed, start:%{public}d, deleteCount:%{public}d, newSections:%{public}zu", start, deleteCount, + "section changed, start:%{public}zu, deleteCount:%{public}d, newSections:%{public}zu", start, deleteCount, newSections.size()); if (start < sections_.size()) { auto it = sections_.begin() + static_cast(start); @@ -42,16 +42,17 @@ void WaterFlowSections::ChangeData( } if (sections_[start] != prevSections_[start]) { // can skip re-init the first modified section with only an itemCount diff - // common scenario when develop add/remove items at the end + // to optimize the common scenario when developers add/remove items at the end if (sections_[start].OnlyCountDiff(prevSections_[start])) { ++start; } break; } } + prevSections_.clear(); if (onSectionDataChange_) { - onSectionDataChange_(start); + onSectionDataChange_(static_cast(start)); } } diff --git a/frameworks/core/components_ng/pattern/waterflow/water_flow_sections.h b/frameworks/core/components_ng/pattern/waterflow/water_flow_sections.h index 4f08485d156..804ecebc492 100644 --- a/frameworks/core/components_ng/pattern/waterflow/water_flow_sections.h +++ b/frameworks/core/components_ng/pattern/waterflow/water_flow_sections.h @@ -68,7 +68,7 @@ public: /** * @brief Change section data. * - * @param start first modified section index. + * @param start index of the first modified section. * @param deleteCount number of sections to delete at index [start]. * @param newSections to insert at index [start]. */ diff --git a/test/unittest/core/pattern/waterflow/BUILD.gn b/test/unittest/core/pattern/waterflow/BUILD.gn index 6de475f84d7..b74ac84a48a 100644 --- a/test/unittest/core/pattern/waterflow/BUILD.gn +++ b/test/unittest/core/pattern/waterflow/BUILD.gn @@ -32,6 +32,7 @@ ace_unittest("water_flow_test_segmented") { sources = [ "water_flow_layout_info_test.cpp", "water_flow_scroller_test_ng.cpp", + "water_flow_section_test.cpp", "water_flow_segment_integrated.cpp", "water_flow_segment_layout_test.cpp", "water_flow_test_ng.cpp", diff --git a/test/unittest/core/pattern/waterflow/water_flow_section_test.cpp b/test/unittest/core/pattern/waterflow/water_flow_section_test.cpp new file mode 100644 index 00000000000..ad26a32a0fd --- /dev/null +++ b/test/unittest/core/pattern/waterflow/water_flow_section_test.cpp @@ -0,0 +1,58 @@ +/* + * Copyright (c) 2024 Huawei Device Co., Ltd. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#include "test/unittest/core/pattern/test_ng.h" +#include "water_flow_item_maps.h" + +#include "frameworks/core/components_ng/pattern/waterflow/water_flow_sections.h" + +namespace OHOS::Ace::NG { +using namespace testing; +using namespace testing::ext; + +class WaterFlowSectionTest : public TestNG {}; + +/** + * @tc.name: Update001 + * @tc.desc: Test Update + * @tc.type: FUNC + */ +HWTEST_F(WaterFlowSectionTest, Update001, TestSize.Level1) +{ + WaterFlowSections s; + int32_t startIdx = -1; + s.SetOnDataChange([&startIdx](int32_t start) { startIdx = start; }); + s.ChangeData(0, 0, SECTION_5); + EXPECT_EQ(startIdx, 0); + + // update with the same data, should do nothing + s.ChangeData(0, 5, SECTION_5); + EXPECT_EQ(startIdx, 5); + + auto newSection = SECTION_5; + newSection[3].itemsCount = 100; + s.ChangeData(0, 5, newSection); + EXPECT_EQ(startIdx, 4); + + // replace last section. item count update can be skipped + newSection[4].itemsCount = 10; + s.ChangeData(4, 1, { newSection[4] }); + EXPECT_EQ(startIdx, 5); + + // updating other properties can't be skipped + newSection[4].crossCount = 3; + s.ChangeData(4, 1, { newSection[4] }); + EXPECT_EQ(startIdx, 4); +} +} // namespace OHOS::Ace::NG diff --git a/test/unittest/core/pattern/waterflow/water_flow_segment_integrated.cpp b/test/unittest/core/pattern/waterflow/water_flow_segment_integrated.cpp index 17b07631381..09d0b9770ef 100644 --- a/test/unittest/core/pattern/waterflow/water_flow_segment_integrated.cpp +++ b/test/unittest/core/pattern/waterflow/water_flow_segment_integrated.cpp @@ -513,8 +513,9 @@ HWTEST_F(WaterFlowSegmentIntegratedTest, Replace005, TestSize.Level1) newSection[0].itemsCount = 10; secObj->ChangeData(2, 1, newSection); MockPipelineContext::GetCurrent()->FlushBuildFinishCallbacks(); - EXPECT_EQ(info->endPosArray_.size(), 6); - EXPECT_EQ(info->segmentStartPos_.size(), 2); + // section 2 reset should be skipped + EXPECT_EQ(info->endPosArray_.size(), 16); + EXPECT_EQ(info->segmentStartPos_.size(), 3); EXPECT_EQ(info->segmentTails_.size(), 4); EXPECT_EQ(info->segmentTails_[2], 16); -- Gitee