From e36510cdeb7b847462ed722b9be25c75f3760ba6 Mon Sep 17 00:00:00 2001 From: "Michael P. Gerlek" Date: Fri, 8 May 2015 08:20:56 -0400 Subject: [PATCH 1/3] added note and test case for #897 Also took the liberty of moving the 2D ctor from cpp file to the hpp file, to be consistent with the other ctors. --- include/pdal/util/Bounds.hpp | 9 +++++++-- src/util/Bounds.cpp | 5 ----- test/unit/BoundsTest.cpp | 17 +++++++++++++++++ 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/include/pdal/util/Bounds.hpp b/include/pdal/util/Bounds.hpp index 203cd835e9..7130cb1a5b 100644 --- a/include/pdal/util/Bounds.hpp +++ b/include/pdal/util/Bounds.hpp @@ -79,7 +79,13 @@ class PDAL_DLL BOX3D minz(box.minz), maxz(box.maxz) {} - BOX3D(double minx, double miny, double maxx, double maxy) ; + // Note: a "2D" bbox is explicitly constructed to have invalid Z values, + // which might lead to some problems when used in conjuction with a "3D" + // bbox. See, for example, issue #897. + BOX3D(double minx, double miny, double maxx, double maxy) : + minx(minx), maxx(maxx), miny(miny), maxy(maxy), + minz(HIGHEST), maxz(LOWEST) + {} BOX3D(double minx, double miny, double minz, double maxx, double maxy, double maxz) : @@ -251,4 +257,3 @@ inline std::ostream& operator << (std::ostream& ostr, const BOX3D& bounds) extern PDAL_DLL std::istream& operator>>(std::istream& istr, BOX3D& bounds); } // namespace pdal - diff --git a/src/util/Bounds.cpp b/src/util/Bounds.cpp index 73a704454d..6ac7cc60cc 100644 --- a/src/util/Bounds.cpp +++ b/src/util/Bounds.cpp @@ -67,11 +67,6 @@ namespace pdal const double BOX3D::LOWEST = (std::numeric_limits::lowest)(); const double BOX3D::HIGHEST = (std::numeric_limits::max)(); - -BOX3D::BOX3D(double minx, double miny, double maxx, double maxy) : - minx(minx), maxx(maxx), miny(miny), maxy(maxy), - minz(HIGHEST), maxz(LOWEST) -{} void BOX3D::clear() { diff --git a/test/unit/BoundsTest.cpp b/test/unit/BoundsTest.cpp index c915c683f2..589601814b 100644 --- a/test/unit/BoundsTest.cpp +++ b/test/unit/BoundsTest.cpp @@ -234,3 +234,20 @@ TEST(BoundsTest, test_2d_input) BOX3D r(1.1,2.2,101.1,102.2); EXPECT_EQ(r, rr); } + +TEST(BoundsTest, test_issue_897) +{ + BOX3D boxA(0.0, 0.0, 100.0, 100.0); // a "2D" box + BOX3D boxB(50.0, 50.0, 3.1, 51.0, 51.0, 3.14); // a "3D" box, wholly inside boxA + + // Currently aContainsB is false: see issue #387. + const bool aContainsB = boxA.contains(boxB); + const bool bContainsA = boxB.contains(boxA); + const bool aContainsA = boxA.contains(boxA); + const bool bContainsB = boxB.contains(boxB); + + EXPECT_FALSE(aContainsB); + EXPECT_FALSE(bContainsA); + EXPECT_TRUE(aContainsA); + EXPECT_TRUE(bContainsB); +} From c48f819681fcccc9786078957b60f6d168127e73 Mon Sep 17 00:00:00 2001 From: "Michael P. Gerlek" Date: Fri, 8 May 2015 11:28:28 -0400 Subject: [PATCH 2/3] (fails on MSVC, see #897 and #900) Revert "added note and test case for #897" This reverts commit e36510cdeb7b847462ed722b9be25c75f3760ba6. --- include/pdal/util/Bounds.hpp | 9 ++------- src/util/Bounds.cpp | 5 +++++ test/unit/BoundsTest.cpp | 17 ----------------- 3 files changed, 7 insertions(+), 24 deletions(-) diff --git a/include/pdal/util/Bounds.hpp b/include/pdal/util/Bounds.hpp index 7130cb1a5b..203cd835e9 100644 --- a/include/pdal/util/Bounds.hpp +++ b/include/pdal/util/Bounds.hpp @@ -79,13 +79,7 @@ class PDAL_DLL BOX3D minz(box.minz), maxz(box.maxz) {} - // Note: a "2D" bbox is explicitly constructed to have invalid Z values, - // which might lead to some problems when used in conjuction with a "3D" - // bbox. See, for example, issue #897. - BOX3D(double minx, double miny, double maxx, double maxy) : - minx(minx), maxx(maxx), miny(miny), maxy(maxy), - minz(HIGHEST), maxz(LOWEST) - {} + BOX3D(double minx, double miny, double maxx, double maxy) ; BOX3D(double minx, double miny, double minz, double maxx, double maxy, double maxz) : @@ -257,3 +251,4 @@ inline std::ostream& operator << (std::ostream& ostr, const BOX3D& bounds) extern PDAL_DLL std::istream& operator>>(std::istream& istr, BOX3D& bounds); } // namespace pdal + diff --git a/src/util/Bounds.cpp b/src/util/Bounds.cpp index 6ac7cc60cc..73a704454d 100644 --- a/src/util/Bounds.cpp +++ b/src/util/Bounds.cpp @@ -67,6 +67,11 @@ namespace pdal const double BOX3D::LOWEST = (std::numeric_limits::lowest)(); const double BOX3D::HIGHEST = (std::numeric_limits::max)(); + +BOX3D::BOX3D(double minx, double miny, double maxx, double maxy) : + minx(minx), maxx(maxx), miny(miny), maxy(maxy), + minz(HIGHEST), maxz(LOWEST) +{} void BOX3D::clear() { diff --git a/test/unit/BoundsTest.cpp b/test/unit/BoundsTest.cpp index 589601814b..c915c683f2 100644 --- a/test/unit/BoundsTest.cpp +++ b/test/unit/BoundsTest.cpp @@ -234,20 +234,3 @@ TEST(BoundsTest, test_2d_input) BOX3D r(1.1,2.2,101.1,102.2); EXPECT_EQ(r, rr); } - -TEST(BoundsTest, test_issue_897) -{ - BOX3D boxA(0.0, 0.0, 100.0, 100.0); // a "2D" box - BOX3D boxB(50.0, 50.0, 3.1, 51.0, 51.0, 3.14); // a "3D" box, wholly inside boxA - - // Currently aContainsB is false: see issue #387. - const bool aContainsB = boxA.contains(boxB); - const bool bContainsA = boxB.contains(boxA); - const bool aContainsA = boxA.contains(boxA); - const bool bContainsB = boxB.contains(boxB); - - EXPECT_FALSE(aContainsB); - EXPECT_FALSE(bContainsA); - EXPECT_TRUE(aContainsA); - EXPECT_TRUE(bContainsB); -} From 4aa5b27a65a01ad3c7b5cdcc8c0aced5dd48b831 Mon Sep 17 00:00:00 2001 From: "Michael P. Gerlek" Date: Fri, 8 May 2015 11:42:41 -0400 Subject: [PATCH 3/3] committing again, this time without gratuitous actor fix --- include/pdal/util/Bounds.hpp | 5 ++++- test/unit/BoundsTest.cpp | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/include/pdal/util/Bounds.hpp b/include/pdal/util/Bounds.hpp index 203cd835e9..7c150c2cb5 100644 --- a/include/pdal/util/Bounds.hpp +++ b/include/pdal/util/Bounds.hpp @@ -79,6 +79,10 @@ class PDAL_DLL BOX3D minz(box.minz), maxz(box.maxz) {} + // Note: a "2D" bbox is explicitly constructed to have invalid Z values, + // which might lead to some problems when used in conjuction with a "3D" + // bbox. See, for example, issue #897. + // Note: ctor body not inlined due to MSVC, see #900 BOX3D(double minx, double miny, double maxx, double maxy) ; BOX3D(double minx, double miny, double minz, double maxx, double maxy, @@ -251,4 +255,3 @@ inline std::ostream& operator << (std::ostream& ostr, const BOX3D& bounds) extern PDAL_DLL std::istream& operator>>(std::istream& istr, BOX3D& bounds); } // namespace pdal - diff --git a/test/unit/BoundsTest.cpp b/test/unit/BoundsTest.cpp index c915c683f2..589601814b 100644 --- a/test/unit/BoundsTest.cpp +++ b/test/unit/BoundsTest.cpp @@ -234,3 +234,20 @@ TEST(BoundsTest, test_2d_input) BOX3D r(1.1,2.2,101.1,102.2); EXPECT_EQ(r, rr); } + +TEST(BoundsTest, test_issue_897) +{ + BOX3D boxA(0.0, 0.0, 100.0, 100.0); // a "2D" box + BOX3D boxB(50.0, 50.0, 3.1, 51.0, 51.0, 3.14); // a "3D" box, wholly inside boxA + + // Currently aContainsB is false: see issue #387. + const bool aContainsB = boxA.contains(boxB); + const bool bContainsA = boxB.contains(boxA); + const bool aContainsA = boxA.contains(boxA); + const bool bContainsB = boxB.contains(boxB); + + EXPECT_FALSE(aContainsB); + EXPECT_FALSE(bContainsA); + EXPECT_TRUE(aContainsA); + EXPECT_TRUE(bContainsB); +}