Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 17b1d2e

Browse files
author
Emmanuel Garcia
committed
Address Chinmay comments
1 parent 4440e96 commit 17b1d2e

File tree

6 files changed

+134
-38
lines changed

6 files changed

+134
-38
lines changed

BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ group("flutter") {
7979
"//flutter/shell/platform/embedder:embedder_unittests",
8080
"//flutter/shell/platform/glfw/client_wrapper:client_wrapper_glfw_unittests",
8181
"//flutter/testing:testing_unittests",
82+
"//flutter/third_party/skia:skia_unittests",
8283
"//flutter/third_party/txt:txt_unittests",
8384
]
8485

testing/run_tests.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,8 @@ def RunCCTests(build_dir, filter):
147147
if IsLinux():
148148
RunEngineExecutable(build_dir, 'txt_unittests', filter, shuffle_flags)
149149

150+
RunEngineExecutable(build_dir, 'skia_unittests', filter, shuffle_flags)
151+
150152

151153
def RunEngineBenchmarks(build_dir, filter):
152154
print("Running Engine Benchmarks.")

third_party/skia/BUILD.gn

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
# Use of this source code is governed by a BSD-style license that can be
44
# found in the LICENSE file.
55

6+
import("//flutter/testing/testing.gni")
7+
68
source_set("skia") {
79
sources = [
810
"platform_view_rtree.cc",
@@ -13,3 +15,22 @@ source_set("skia") {
1315
"//third_party/skia",
1416
]
1517
}
18+
19+
test_fixtures("skia_fixtures") {
20+
fixtures = []
21+
}
22+
23+
executable("skia_unittests") {
24+
testonly = true
25+
sources = [
26+
"platform_view_rtree_unittests.cc",
27+
"main_unittests.cc",
28+
]
29+
deps = [
30+
":skia",
31+
":skia_fixtures",
32+
"//flutter/testing:testing_lib",
33+
"//third_party/dart/runtime:libdart_jit", # For logging.
34+
"//third_party/skia",
35+
]
36+
}

third_party/skia/main_unittests.cc

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/*
2+
* Copyright 2012 Google Inc.
3+
*
4+
* Use of this source code is governed by a BSD-style license that can be
5+
* found in the LICENSE file.
6+
*/
7+
8+
#include "flutter/testing/testing.h"
9+
10+
#include <cassert>
11+
12+
int main(int argc, char** argv) {
13+
testing::InitGoogleTest(&argc, argv);
14+
return RUN_ALL_TESTS();
15+
}

third_party/skia/platform_view_rtree.cc

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* found in the LICENSE file.
66
*/
77

8-
#include "skia/platform_view_rtree.h"
8+
#include "platform_view_rtree.h"
99

1010
PlatformViewRTree::PlatformViewRTree() : fCount(0) {}
1111

@@ -25,27 +25,23 @@ void PlatformViewRTree::insert(const SkRect boundsArray[],
2525
Branch b;
2626
b.fBounds = bounds;
2727
b.fOpIndex = i;
28+
b.isDraw = (metadata == nullptr) ? false : metadata[i].isDraw;
2829
branches.push_back(b);
2930
}
30-
Branch b;
31-
b.fBounds = bounds;
32-
b.fOpIndex = i;
33-
b.isDraw = (metadata == nullptr) ? false : metadata[i].isDraw;
34-
branches.push_back(b);
35-
}
3631

37-
fCount = (int)branches.size();
38-
if (fCount) {
39-
if (1 == fCount) {
40-
fNodes.reserve(1);
41-
Node* n = this->allocateNodeAtLevel(0);
42-
n->fNumChildren = 1;
43-
n->fChildren[0] = branches[0];
44-
fRoot.fSubtree = n;
45-
fRoot.fBounds = branches[0].fBounds;
46-
} else {
47-
fNodes.reserve(CountNodes(fCount));
48-
fRoot = this->bulkLoad(&branches);
32+
fCount = (int)branches.size();
33+
if (fCount) {
34+
if (1 == fCount) {
35+
fNodes.reserve(1);
36+
Node* n = this->allocateNodeAtLevel(0);
37+
n->fNumChildren = 1;
38+
n->fChildren[0] = branches[0];
39+
fRoot.fSubtree = n;
40+
fRoot.fBounds = branches[0].fBounds;
41+
} else {
42+
fNodes.reserve(CountNodes(fCount));
43+
fRoot = this->bulkLoad(&branches);
44+
}
4945
}
5046
}
5147

@@ -204,8 +200,8 @@ void PlatformViewRTree::searchRects(Node* node,
204200
SkRect* currentRecordRect = &node->fChildren[i].fBounds;
205201
std::vector<SkRect*> currentResults = *results;
206202
bool replacedExistingRect = false;
207-
// If the current record rect intersects with any of the rects in the
208-
// result vector, then join them, and update the rect in results.
203+
// // If the current record rect intersects with any of the rects in the
204+
// // result vector, then join them, and update the rect in results.
209205
size_t joiningRectIdx = currentResults.size();
210206
size_t resultIdx = 0;
211207
while (resultIdx < results->size()) {

third_party/skia/platform_view_rtree_unittests.cc

Lines changed: 78 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,17 @@
66
*/
77

88
#include "flutter/testing/testing.h"
9-
#include "flutter/testing/thread_test.h"
10-
#include "skia/platform_view_rtree.h"
9+
#include "platform_view_rtree.h"
1110

1211
#include "third_party/skia/include/core/SkCanvas.h"
1312
#include "third_party/skia/include/core/SkPictureRecorder.h"
1413

1514
namespace flutter {
1615
namespace testing {
1716

18-
TEST_F(PlatformViewRTree, NoIntersection) {
19-
auto r_tree = sk_make_sp<FlutterRTree>();
20-
auto rtree_factory = FlutterRTreeFactory(r_tree);
17+
TEST(PlatformViewRTree, NoIntersection) {
18+
auto r_tree = sk_make_sp<PlatformViewRTree>();
19+
auto rtree_factory = PlatformViewRTreeFactory(r_tree);
2120
auto recorder = std::make_unique<SkPictureRecorder>();
2221
auto recording_canvas = recorder->beginRecording(SkRect::MakeIWH(1000, 1000), &rtree_factory);
2322

@@ -36,9 +35,9 @@ TEST_F(PlatformViewRTree, NoIntersection) {
3635
ASSERT_TRUE(hits.empty());
3736
}
3837

39-
TEST_F(PlatformViewRTree, Intersection) {
40-
auto r_tree = sk_make_sp<FlutterRTree>();
41-
auto rtree_factory = FlutterRTreeFactory(r_tree);
38+
TEST(PlatformViewRTree, SingleRectIntersection) {
39+
auto r_tree = sk_make_sp<PlatformViewRTree>();
40+
auto rtree_factory = PlatformViewRTreeFactory(r_tree);
4241
auto recorder = std::make_unique<SkPictureRecorder>();
4342
auto recording_canvas = recorder->beginRecording(SkRect::MakeIWH(1000, 1000), &rtree_factory);
4443

@@ -60,9 +59,71 @@ TEST_F(PlatformViewRTree, Intersection) {
6059
ASSERT_EQ(*hits[0], SkRect::MakeLTRB(120, 120, 160, 160));
6160
}
6261

63-
TEST_F(PlatformViewRTree, JoinRectsWhenIntersectedCase1) {
64-
auto r_tree = sk_make_sp<FlutterRTree>();
65-
auto rtree_factory = FlutterRTreeFactory(r_tree);
62+
TEST(PlatformViewRTree, IgnoresNonDrawingRecords) {
63+
auto r_tree = sk_make_sp<PlatformViewRTree>();
64+
auto rtree_factory = PlatformViewRTreeFactory(r_tree);
65+
auto recorder = std::make_unique<SkPictureRecorder>();
66+
auto recording_canvas = recorder->beginRecording(SkRect::MakeIWH(1000, 1000), &rtree_factory);
67+
68+
auto rect_paint = SkPaint();
69+
rect_paint.setColor(SkColors::kCyan);
70+
rect_paint.setStyle(SkPaint::Style::kFill_Style);
71+
72+
// Creates two non drawing records.
73+
recording_canvas->translate(100, 100);
74+
// The result vector should only contain the clipping rect.
75+
recording_canvas->clipRect(SkRect::MakeLTRB(40, 40, 50, 50), SkClipOp::kIntersect);
76+
recording_canvas->drawRect(SkRect::MakeLTRB(20, 20, 80, 80), rect_paint);
77+
78+
recorder->finishRecordingAsPicture();
79+
80+
// The rtree has a translate, a clip and a rect record.
81+
ASSERT_EQ(3, r_tree->getCount());
82+
83+
auto query = SkRect::MakeLTRB(0, 0, 1000, 1000);
84+
auto hits = std::vector<SkRect*>();
85+
86+
r_tree->searchRects(query, &hits);
87+
ASSERT_EQ(1UL, hits.size());
88+
ASSERT_EQ(*hits[0], SkRect::MakeLTRB(120, 120, 180, 180));
89+
}
90+
91+
TEST(PlatformViewRTree, MultipleRectIntersection) {
92+
auto r_tree = sk_make_sp<PlatformViewRTree>();
93+
auto rtree_factory = PlatformViewRTreeFactory(r_tree);
94+
auto recorder = std::make_unique<SkPictureRecorder>();
95+
auto recording_canvas = recorder->beginRecording(SkRect::MakeIWH(1000, 1000), &rtree_factory);
96+
97+
auto rect_paint = SkPaint();
98+
rect_paint.setColor(SkColors::kCyan);
99+
rect_paint.setStyle(SkPaint::Style::kFill_Style);
100+
101+
// Given the A, B that intersect with the query rect,
102+
// there should be A and B in the result vector since
103+
// they don't intersect with each other.
104+
//
105+
// +-----+ +-----+
106+
// | A | | B |
107+
// +-----+ +-----+
108+
// A
109+
recording_canvas->drawRect(SkRect::MakeLTRB(100, 100, 200, 200), rect_paint);
110+
// B
111+
recording_canvas->drawRect(SkRect::MakeLTRB(300, 100, 400, 200), rect_paint);
112+
113+
recorder->finishRecordingAsPicture();
114+
115+
auto query = SkRect::MakeLTRB(0, 0, 1000, 1050);
116+
auto hits = std::vector<SkRect*>();
117+
118+
r_tree->searchRects(query, &hits);
119+
ASSERT_EQ(2UL, hits.size());
120+
ASSERT_EQ(*hits[0], SkRect::MakeLTRB(100, 100, 200, 200));
121+
ASSERT_EQ(*hits[1], SkRect::MakeLTRB(300, 100, 400, 200));
122+
}
123+
124+
TEST(PlatformViewRTree, JoinRectsWhenIntersectedCase1) {
125+
auto r_tree = sk_make_sp<PlatformViewRTree>();
126+
auto rtree_factory = PlatformViewRTreeFactory(r_tree);
66127
auto recorder = std::make_unique<SkPictureRecorder>();
67128
auto recording_canvas = recorder->beginRecording(SkRect::MakeIWH(1000, 1000), &rtree_factory);
68129

@@ -96,9 +157,9 @@ TEST_F(PlatformViewRTree, JoinRectsWhenIntersectedCase1) {
96157
ASSERT_EQ(*hits[0], SkRect::MakeLTRB(100, 100, 175, 175));
97158
}
98159

99-
TEST_F(PlatformViewRTree, JoinRectsWhenIntersectedCase2) {
100-
auto r_tree = sk_make_sp<FlutterRTree>();
101-
auto rtree_factory = FlutterRTreeFactory(r_tree);
160+
TEST(PlatformViewRTree, JoinRectsWhenIntersectedCase2) {
161+
auto r_tree = sk_make_sp<PlatformViewRTree>();
162+
auto rtree_factory = PlatformViewRTreeFactory(r_tree);
102163
auto recorder = std::make_unique<SkPictureRecorder>();
103164
auto recording_canvas = recorder->beginRecording(SkRect::MakeIWH(1000, 1000), &rtree_factory);
104165

@@ -139,9 +200,9 @@ TEST_F(PlatformViewRTree, JoinRectsWhenIntersectedCase2) {
139200
ASSERT_EQ(*hits[0], SkRect::MakeLTRB(50, 50, 500, 250));
140201
}
141202

142-
TEST_F(PlatformViewRTree, JoinRectsWhenIntersectedCase3) {
143-
auto r_tree = sk_make_sp<FlutterRTree>();
144-
auto rtree_factory = FlutterRTreeFactory(r_tree);
203+
TEST(PlatformViewRTree, JoinRectsWhenIntersectedCase3) {
204+
auto r_tree = sk_make_sp<PlatformViewRTree>();
205+
auto rtree_factory = PlatformViewRTreeFactory(r_tree);
145206
auto recorder = std::make_unique<SkPictureRecorder>();
146207
auto recording_canvas = recorder->beginRecording(SkRect::MakeIWH(1000, 1000), &rtree_factory);
147208

0 commit comments

Comments
 (0)