Rectangle::contains() bugs?

General JUCE discussion
1

these two versions use < and > in some comparisons:

but Rectangle::contains (Rectangle other) uses <= and >= everywhere.

this gives seemingly contradictory results:

  juce::Rectangle<float> a{0,0,10,10};
  juce::Rectangle<float> b{5,0,5,10};

  a.contains(b); // true
  a.contains(b.getTopLeft()); // true
  a.contains(b.getBottomLeft()); // false??
  a.contains(b.getTopRight()); // false??
  a.contains(b.getBottomRight()); // false??
2 Likes
2

I’ve just encountered this and can vouch for it being a bug in the versions of Rectangle::contains() that take Points or x/y as parameters.

The version that takes a Point, for instance, looks like this:

bool contains (Point<ValueType> point) const noexcept
{
    return point.x >= pos.x && point.y >= pos.y && point.x < pos.x + w && point.y < pos.y + h;
}

but Rectangle::getRight() and Rectangle::getBottom() are defined like this:

inline ValueType getRight() const noexcept { return pos.x + w; }
inline ValueType getBottom() const noexcept { return pos.y + h; }

…which means that contains() will return false if point.x == getRight() or point.y == getBottom().

The documentation indicates that getRight() and getBottom() return the rightmost and bottommost points of the rectangle, not points just outside it:

https://docs.juce.com/master/classRectangle.html#aeea826946e996fc0b3d8f44d21b181c0

https://docs.juce.com/master/classRectangle.html#af6b325feb4f7b107bcc51aee56a5fbc1

So it looks like the two < in contains() should be <= instead.

3

Imagine a rect of width=1 and height=1. That means exactly one point should be inside.
With your changes, this rectangle would contain 4 pixels, even though it’s just 1 pixel wide and 1 pixel high.

There is some logic to the current implementation.

4

That makes sense, but in that case getRight() should return pos.x + w - 1 rather than pos.x + w. Possibly the bug is in getRight() and getBottom() rather than contains(). Or else the documentation is wrong and getRight() is actually supposed to be 1 pixel outside the rightmost pixel, a la Windows APIs like GetWindowRect().

5

Rectangle::setRight and Rectangle::setBottom are also problematic. setRight looks like this:

void setRight (ValueType newRight) noexcept { pos.x = jmin (pos.x, newRight); w = newRight - pos.x; }

So if pos.x was, say, three, and you called setRight(3), the rectangle’s width would get set to zero, causing contains() to always return false. This seems incorrect unless setRight() is actually supposed to be setting one beyond the rightmost pixel and a rectangle with left = right is supposed to be zero pixels wide. In that case, it’s just that the documentation could stand to be made more clear.

6

So the consistent view would be to imagine it as two ranges of X and Y, where the lower bound is included and the upper bound excluded.

Just a word of warning, even if it is found to be inconsistent with the expectation, changing it will break almost every project, beginning with Component…