Skip to content

Commit 2d02c41

Browse files
fsamuelCommit bot
fsamuel
authored and
Commit bot
committed
Revert of Perform BSP polygon splitting and orientation selection in a single step. (patchset Pissandshittium#4 id:60001 of https://codereview.chromium.org/2043283002/ )
Reason for revert: Broke Windows 7 cc_unittests: DrawPolygonSplitTest.AngledSplit https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%281%29/builds/53692 Original issue's description: > Perform BSP polygon splitting and orientation selection in a single step. > > This eliminates redundant testing of vertices for orientation with > respect to the splitting polygon. Previously, up to 3 sets of tests > were made (once to determine whether the polygon was split, then once > during the split, and then finally to determine the orientation of the > split polygons. > > Merging these steps in order to reuse calculated values also eliminates > the possibility that different calculations in testing and splitting > could be inconsistent. > > BUG=606984 > CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel > > Committed: https://crrev.com/89c62c8f69d76471e866f21ee4f1ae5e0c5bca48 > Cr-Commit-Position: refs/heads/master@{#399459} TBR=enne@chromium.org,tobiasjs@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=606984 Review-Url: https://codereview.chromium.org/2060183003 Cr-Commit-Position: refs/heads/master@{#399472}
1 parent da29fd8 commit 2d02c41

File tree

7 files changed

+281
-260
lines changed

7 files changed

+281
-260
lines changed

cc/output/bsp_tree.cc

+43-16
Original file line numberDiff line numberDiff line change
@@ -44,24 +44,46 @@ void BspTree::BuildTree(
4444
// find a splitting plane, then classify polygons as either in front of
4545
// or behind that splitting plane.
4646
while (!polygon_list->empty()) {
47-
std::unique_ptr<DrawPolygon> polygon;
48-
std::unique_ptr<DrawPolygon> new_front;
49-
std::unique_ptr<DrawPolygon> new_back;
50-
// Time to split this geometry, *it needs to be split by node_data.
51-
polygon = PopFront(polygon_list);
52-
bool is_coplanar;
53-
node->node_data->SplitPolygon(std::move(polygon), &new_front, &new_back,
54-
&is_coplanar);
55-
if (is_coplanar) {
56-
if (new_front)
57-
node->coplanars_front.push_back(std::move(new_front));
58-
if (new_back)
59-
node->coplanars_back.push_back(std::move(new_back));
60-
} else {
61-
if (new_front)
47+
// Is this particular polygon in front of or behind our splitting polygon.
48+
BspCompareResult comparer_result =
49+
GetNodePositionRelative(*polygon_list->front(), *(node->node_data));
50+
51+
// If it's clearly behind or in front of the splitting plane, we use the
52+
// heuristic to decide whether or not we should put it at the back
53+
// or front of the list.
54+
switch (comparer_result) {
55+
case BSP_FRONT:
56+
front_list.push_back(PopFront(polygon_list));
57+
break;
58+
case BSP_BACK:
59+
back_list.push_back(PopFront(polygon_list));
60+
break;
61+
case BSP_SPLIT:
62+
{
63+
std::unique_ptr<DrawPolygon> polygon;
64+
std::unique_ptr<DrawPolygon> new_front;
65+
std::unique_ptr<DrawPolygon> new_back;
66+
// Time to split this geometry, *it needs to be split by node_data.
67+
polygon = PopFront(polygon_list);
68+
bool split_result =
69+
polygon->Split(*(node->node_data), &new_front, &new_back);
70+
DCHECK(split_result);
71+
if (!split_result) {
72+
break;
73+
}
6274
front_list.push_back(std::move(new_front));
63-
if (new_back)
6475
back_list.push_back(std::move(new_back));
76+
break;
77+
}
78+
case BSP_COPLANAR_FRONT:
79+
node->coplanars_front.push_back(PopFront(polygon_list));
80+
break;
81+
case BSP_COPLANAR_BACK:
82+
node->coplanars_back.push_back(PopFront(polygon_list));
83+
break;
84+
default:
85+
NOTREACHED();
86+
break;
6587
}
6688
}
6789

@@ -78,6 +100,11 @@ void BspTree::BuildTree(
78100
}
79101
}
80102

103+
BspCompareResult BspTree::GetNodePositionRelative(const DrawPolygon& node_a,
104+
const DrawPolygon& node_b) {
105+
return DrawPolygon::SideCompare(node_a, node_b);
106+
}
107+
81108
// The base comparer with 0,0,0 as camera position facing forward
82109
BspCompareResult BspTree::GetCameraPositionRelative(const DrawPolygon& node) {
83110
if (node.normal().z() > 0.0f) {

cc/output/bsp_tree.h

+4
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,10 @@ class CC_EXPORT BspTree {
101101
}
102102
}
103103

104+
// Returns whether or not nodeA is on one or the other side of nodeB,
105+
// coplanar, or whether it crosses nodeB's plane and needs to be split
106+
static BspCompareResult GetNodePositionRelative(const DrawPolygon& node_a,
107+
const DrawPolygon& node_b);
104108
// Returns whether or not our viewer is in front of or behind the plane
105109
// defined by this polygon/node
106110
static BspCompareResult GetCameraPositionRelative(const DrawPolygon& node);

cc/output/bsp_tree_unittest.cc

+8-34
Original file line numberDiff line numberDiff line change
@@ -45,50 +45,24 @@ class BspTreeTest {
4545
EXPECT_TRUE(VerifySidedness(bsp_tree.root()));
4646
}
4747

48-
static BspCompareResult SideCompare(const DrawPolygon& a,
49-
const DrawPolygon& b) {
50-
const float split_threshold = 0.05f;
51-
bool pos = false;
52-
bool neg = false;
53-
for (const auto& pt : a.points()) {
54-
float dist = b.SignedPointDistance(pt);
55-
neg |= dist < -split_threshold;
56-
pos |= dist > split_threshold;
57-
}
58-
if (pos && neg)
59-
return BSP_SPLIT;
60-
if (neg)
61-
return BSP_BACK;
62-
if (pos)
63-
return BSP_FRONT;
64-
double dot = gfx::DotProduct(a.normal(), b.normal());
65-
if ((dot >= 0.0f && a.order_index() >= b.order_index()) ||
66-
(dot <= 0.0f && a.order_index() <= b.order_index())) {
67-
// The sign of the dot product of the normals along with document order
68-
// determine which side it goes on, the vertices are ambiguous.
69-
return BSP_COPLANAR_BACK;
70-
}
71-
return BSP_COPLANAR_FRONT;
72-
}
73-
7448
static bool VerifySidedness(const std::unique_ptr<BspNode>& node) {
7549
// We check if both the front and back child nodes have geometry that is
7650
// completely on the expected side of the current node.
7751
bool front_ok = true;
7852
bool back_ok = true;
7953
if (node->back_child) {
8054
// Make sure the back child lies entirely behind this node.
81-
BspCompareResult result =
82-
SideCompare(*(node->back_child->node_data), *(node->node_data));
55+
BspCompareResult result = DrawPolygon::SideCompare(
56+
*(node->back_child->node_data), *(node->node_data));
8357
if (result != BSP_BACK) {
8458
return false;
8559
}
8660
back_ok = VerifySidedness(node->back_child);
8761
}
8862
// Make sure the front child lies entirely in front of this node.
8963
if (node->front_child) {
90-
BspCompareResult result =
91-
SideCompare(*(node->front_child->node_data), *(node->node_data));
64+
BspCompareResult result = DrawPolygon::SideCompare(
65+
*(node->front_child->node_data), *(node->node_data));
9266
if (result != BSP_FRONT) {
9367
return false;
9468
}
@@ -100,15 +74,15 @@ class BspTreeTest {
10074

10175
// Now we need to make sure our coplanar geometry is all actually coplanar.
10276
for (size_t i = 0; i < node->coplanars_front.size(); i++) {
103-
BspCompareResult result =
104-
SideCompare(*(node->coplanars_front[i]), *(node->node_data));
77+
BspCompareResult result = DrawPolygon::SideCompare(
78+
*(node->coplanars_front[i]), *(node->node_data));
10579
if (result != BSP_COPLANAR_FRONT) {
10680
return false;
10781
}
10882
}
10983
for (size_t i = 0; i < node->coplanars_back.size(); i++) {
110-
BspCompareResult result =
111-
SideCompare(*(node->coplanars_back[i]), *(node->node_data));
84+
BspCompareResult result = DrawPolygon::SideCompare(
85+
*(node->coplanars_back[i]), *(node->node_data));
11286
if (result != BSP_COPLANAR_BACK) {
11387
return false;
11488
}

0 commit comments

Comments
 (0)