Skip to content

Commit

Permalink
Fix bug in Bounds2i iterators.
Browse files Browse the repository at this point in the history
For degenerate bounds, they would end up going on forever (or at least a
very long time, visiting all sorts of points that weren't inside the
bounds).

Added unit tests to test both their regular operation as well as degenerate
cases.
  • Loading branch information
mmp committed Oct 16, 2016
1 parent 31dda0e commit 6651675
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 1 deletion.
10 changes: 9 additions & 1 deletion src/core/geometry.h
Original file line number Diff line number Diff line change
Expand Up @@ -1299,7 +1299,15 @@ inline Bounds2iIterator begin(const Bounds2i &b) {
}

inline Bounds2iIterator end(const Bounds2i &b) {
return Bounds2iIterator(b, Point2i(b.pMin.x, b.pMax.y));
// Normally, the ending point is at the minimum x value and one past
// the last valid y value.
Point2i pEnd(b.pMin.x, b.pMax.y);
// However, if the bounds are degenerate, override the end point to
// equal the start point so that any attempt to iterate over the bounds
// exits out immediately.
if (b.pMin.x >= b.pMax.x || b.pMin.y >= b.pMax.y)
pEnd = b.pMin;
return Bounds2iIterator(b, pEnd);
}

template <typename T>
Expand Down
42 changes: 42 additions & 0 deletions src/tests/bounds.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@

#include "tests/gtest/gtest.h"
#include "pbrt.h"
#include "geometry.h"

TEST(Bounds2, IteratorBasic) {
Bounds2i b{{0, 1}, {2, 3}};
Point2i e[] = { { 0, 1 }, { 1, 1 }, { 0, 2 }, { 1, 2 } };
int offset = 0;
for (auto p : b) {
EXPECT_LT(offset, sizeof(e) / sizeof(e[0]));
EXPECT_EQ(e[offset], p) << "offset = " << offset;
++offset;
}
}

TEST(Bounds2, IteratorDegenerate) {
Bounds2i b{{0, 0}, {0, 10}};
for (auto p : b) {
// This loop should never run.
bool reached = true;
EXPECT_FALSE(reached) << "p = " << p;
break;
}

Bounds2i b2{{0, 0}, {4, 0}};
for (auto p : b2) {
// This loop should never run.
bool reached = true;
EXPECT_FALSE(reached) << "p = " << p;
break;
}

Bounds2i b3;
for (auto p : b3) {
// This loop should never run.
bool reached = true;
EXPECT_FALSE(reached) << "p = " << p;
break;
}
}

0 comments on commit 6651675

Please sign in to comment.