BUG --> core::rect::isPointInside()

[Scrappi]

Look at file "rect.h"

The methods are:
1) ok, if the lower/right corner is outside the rectangle
2) ok, if the lower/right corner is outside the rectangle
3) ok, if the lower/right corner is inside the rectangle

I think (3) is wrong. It should be:

return (
UpperLeftCorner.X <= pos.X &&
UpperLeftCorner.Y <= pos.Y &&
LowerRightCorner.X > pos.X &&
LowerRightCorner.Y > pos.Y);


===========================================

1)
//! Constructor with upper left corner and dimension
template <class U>
rect(const position2d<T>& pos, const dimension2d<U>& size)
: UpperLeftCorner(pos), LowerRightCorner(pos.X + size.Width, pos.Y + size.Height) {}


2)
T getWidth() const
{
return LowerRightCorner.X - UpperLeftCorner.X;
}
T getHeight() const
{
return LowerRightCorner.Y - UpperLeftCorner.Y;
}


3)
bool isPointInside(const position2d<T>& pos) const
{
return (UpperLeftCorner.X <= pos.X &&
UpperLeftCorner.Y <= pos.Y &&
LowerRightCorner.X >= pos.X &&
LowerRightCorner.Y >= pos.Y);
}

[hybrid]

Any test numbers which show the problem?

[Scrappi]

Any test numbers which show the problem?

Please do some brain storming.
This is much easier than simple numbers.

If the upper/left corner is inside the rectangle
and the the lower/right corner is outside then

upper/left <= position is correct
lower/right >= position is incorrect

because both terms allows the position to be same as the corner
and remember that the lower/right corner is not inside the rectangle

the original source code will return true if a point
is one pixel below or right of the rectangle

normally it's not easy to do a mouse click exactly at one pixel
but it's still a error ...

[hybrid]

Please do some better bug reporting, otherwise this might stay in the engine forever. If you have a test case, please tell me. If not, I guess it's not that much of a problem here and we can live with or without. If it's really so easy, I guess you can come up with a few lines of code even, which show that there's something wrong.

[Sylence]

If the upper/left corner is inside the rectangle
and the the lower/right corner is outside then

How can the border of a rectangle be outside of the rectangle itself ?

Anyways I painted a bit and this looks ok to me:


If you would change to > instead of >= the blue dot on the lower right border would not be inside the rectangle.

Or did I get you wrong ?

[CuteAlien]

edit: Ok, I get the problem, I run into this myself once (or some more often...). You have to think about units/corners, not about pixels. Irrlicht rect isn't used to give pixel coordinates. So rect.LowerRightCorner for a 800x600 screen is indeed 800x600 and not 799x599.

Edit2: And yeah - in this case you are probably right. Puh... this was one of my very first questions when I started with Irrlicht: http://irrlicht.sourceforge.net/phpBB2/ ... hp?t=17831

Edit3: I'm really not sure now. When working with units the function makes sense as it is - when working with pixels (and that is most likely often the case) it is wrong. Should we add a second function with another name maybe? This could also make the problem more obvious. How should we name them?

[Scrappi]

The problem is that all GUI rectangles are using rectangles that have a lower/right corner outside of their drawing area.

The drawing methods will do same. So it's ok.

If you are using this rectangle to check for mouse clicks if they are inside this drawing area you may get a wrong answer.

This is a misleading software design. Classes should be consistent.

At least a good description should tell me about this 'feature'.
Or it's confusing ...

[CuteAlien]

Yes, that's pretty much the problem. Pixel-checks go wrong. Then again a point when working with units is indeed inside on the lower-right corner. That's why I propose maybe adding a second function - not sure how to call it, maybe isPixelInside or isCoordinateInside, or... ideas welcome. So the moment people see there are 2 nearly identical functions they are also warned automatically somewhat.

(And then I'll have to go over Irrlicht sources and check where isPointInside is used and decide in each case which one is correct...).