Introduce bounding box #199
Conversation
All unit tests green
| template <typename T> | ||
| bool IsContained(T point) const | ||
| { |
There was a problem hiding this comment.
Same as above, perhaps change to contains.
| RTree m_facesRTree; ///< Spatial R-Tree used to inquire face circumcenters | ||
| BoundingBox m_boundingBoxCache; ///< Caches the last bounding box used for selecting the locations | ||
|
|
There was a problem hiding this comment.
Will this caching restrict future threading posibilities
If no threading is planned, then its ok
There was a problem hiding this comment.
Now we operate on a single thread. This member will need some protection (mutex) in case of multithreading
| BoundingBox ComputeBoundingBox(UInt i) const | ||
| { | ||
| std::vector<Point> points; | ||
| if (IsEmpty()) | ||
| { | ||
| return BoundingBox(points); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Seems to be something missing.
The point values are not set.
There was a problem hiding this comment.
removed function
| /// @param[in] points The point values | ||
| /// @returns A tuple with bottom left and upper right corners of the bounding box | ||
| template <typename T> |
There was a problem hiding this comment.
Comment is not quite right
| BoundingBox(const Point& m_lower_left, const Point& m_upper_right) | ||
| : m_lowerLeft(m_lower_left), | ||
| m_upperRight(m_upper_right) | ||
| { | ||
| } |
There was a problem hiding this comment.
Should we check that the lower left is really less than the upper right, otherwise throw an exception
|
|
||
| if (nodes[n].x != constants::missing::doubleValue && nodes[n].y != constants::missing::doubleValue) | ||
| { |
There was a problem hiding this comment.
Is there an IsValid function for points?
| // FindEdgeCloseToAPoint builds m_edgesRTree for searching the edges | ||
| mesh->BuildTree(meshkernel::Mesh::Location::Edges); | ||
| const size_t index = mesh->FindEdgeCloseToAPoint({1.5, 1.5}); | ||
| ASSERT_TRUE(static_cast<long long>(index) >= 0); // Luca, need a better test here: ASSERT_EQ(index, actual_closest_edge_index); |
There was a problem hiding this comment.
Are there any testing of building trees with a bounding box?
Also, are there any specific BB tests, I may have missed them
…sed on the emptiness of trees Remove RtreeTriggers tests Explicitly call build tree when necessary
…ub.com/Deltares/MeshKernel into feature/GRIDEDIT-604_improve_searching
lucacarniato
requested review from
ahmad-el-sayed
and removed request for
ahmad-el-sayed
All unit tests green