Fix rectangle behavior properly, minor improvements

This commit is contained in:
karl 2020-12-06 19:37:47 +01:00
parent 14e633d236
commit 82c0df1840
2 changed files with 9 additions and 16 deletions

3
Line.h
View File

@ -42,7 +42,8 @@ public:
other -= from(); other -= from();
// Cross product greater than zero? // Cross product greater than zero?
if (m_to_from_origin.x() * other.y() - m_to_from_origin.y() * other.x() > 0) // Note: We need ">=" here so that we include points which are directly on the existing hull.
if (m_to_from_origin.x() * other.y() - m_to_from_origin.y() * other.x() >= 0)
{ {
return true; return true;
} }

View File

@ -132,24 +132,20 @@ private:
// these properly? We definitely need to remove them all from input later; // these properly? We definitely need to remove them all from input later;
// do we also need to handle them all further? This hotfix works, but seems // do we also need to handle them all further? This hotfix works, but seems
// like an unnecessarily big performance hit for that edge case. // like an unnecessarily big performance hit for that edge case.
// FIXME: This workaround also causes problems with extremely large numbers
// of randomly generated numbers, causing random lines within the data!
// Points inside the hull are added because of random lines.
for (const Point &point : input) for (const Point &point : input)
{ {
float this_distance = line.distance_squared_to(point); float this_distance = line.distance_squared_to(point);
// TODO: Both are required, otherwise only one side of the rectangle is if (this_distance == furthest_distance)
// taken -- why?
if (this_distance == furthest_distance || line.distance_squared_to(point) == 0)
{ {
output.emplace_back(point); output.emplace_back(point);
} }
} }
// Remove the previously checked points from the input -- we can't do that in
// the previous loop because we can't delete from the list we're iterating over
input.remove_if([furthest_distance, line](Point point) input.remove_if([furthest_distance, line](Point point)
{ {
return furthest_distance == line.distance_squared_to(point); return furthest_distance == line.distance_squared_to(point);
}); });
// Hotfix end
output.emplace_back(furthest_point); output.emplace_back(furthest_point);
input.remove(furthest_point); input.remove(furthest_point);
@ -173,12 +169,9 @@ private:
new_line2 = Line(furthest_point, line.to()); new_line2 = Line(furthest_point, line.to());
} }
// TODO: Test if this improves performance // We don't need to remove points inside the triangle created by those lines.
//Triangle triangle(a, b, c); // That happens implicitly since they are not handled further due to the
//input.remove_if([triangle](Point point) // following step, which assigns each line its corresponding points to handle:
//{
// return triangle.is_point_inside(point);
//});
// Get points right of new_line1 and 2 // Get points right of new_line1 and 2
std::list<Point> left_of_line1, left_of_line2; std::list<Point> left_of_line1, left_of_line2;
@ -189,8 +182,7 @@ private:
{ {
left_of_line1.emplace_back(point); left_of_line1.emplace_back(point);
} }
// TODO: Can we do else if here, or could we then miss out on points? else if (!new_line2.is_point_right(point))
if (!new_line2.is_point_right(point))
{ {
left_of_line2.emplace_back(point); left_of_line2.emplace_back(point);
} }