Concerns about rect<T>

You discovered a bug in the engine, and you are sure that it is not a problem of your code? Just post it in here. Please read the bug posting guidelines first.
Post Reply
gs

Concerns about rect<T>

Post by gs »

I have found some "interesting" behaviour for the rect<T> class.

Example 1:

rect<s32> r1(0,0,10,10);
rect<s32> r2(r1.UpperLeftCorner,dimension2d<s32>(r1.getWidth(),r1.getHeight()));

//At this point r1 does not equal r2, r2 is one pixel larger.

Example 2:

rect<s32> r1(0,0,1,1);

// two horizontal points test true to be inside the rect
r1.isPointInside(0,0) == true;
r1.isPointInside(1,0) == true;

// however the width of the rect is said to be one
r1.getWidth() == 1;

Same problem exists with getHeight()

Thanks,
-Dan
jox
Bug Slayer
Posts: 726
Joined: Thu Apr 22, 2004 6:55 pm
Location: Germany

Re: Concerns about rect<T>

Post by jox »

gs wrote:I have found some "interesting" behaviour for the rect<T> class.

Example 1:

rect<s32> r1(0,0,10,10);
rect<s32> r2(r1.UpperLeftCorner,dimension2d<s32>(r1.getWidth(),r1.getHeight()));

//At this point r1 does not equal r2, r2 is one pixel larger.
I can not confirm that! I executed the code and compared the rects and they equal!

gs wrote:Example 2:

rect<s32> r1(0,0,1,1);

// two horizontal points test true to be inside the rect
r1.isPointInside(0,0) == true;
r1.isPointInside(1,0) == true;

// however the width of the rect is said to be one
r1.getWidth() == 1;

Same problem exists with getHeight()
This one I can confirm!

If an empty rect (width and height equals zero) has UpperLeftCorner(0,0) and LowerRightCorner(0,0) then a rect with width == 3 and height == 3 and UpperLeftCorner(0,0) would have LowerRightCorner(3,3).

Code: Select all

  0 1 2 3
0 U---+
1 |   |
2 +---+
3       L

U - UpperRightCorner(0,0)
L - LowerRightCorner(3,3)
isPointInside(0,0) should equal true
isPointInside(3,3) should eqal false

If that logic is right, then the function isPointInside() has indeed a bug!

Original function (Irrlicht 0.6):

Code: Select all

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

Code: Select all

bool isPointInside(const position2d<T>& pos) const
{
	return UpperLeftCorner.X <= pos.X && UpperLeftCorner.Y <= pos.Y &&
		LowerRightCorner.X > pos.X && LowerRightCorner.Y > pos.Y;
}
Trip42
Posts: 10
Joined: Sun Jun 06, 2004 9:49 pm

Post by Trip42 »

It seems like the rect class functions as if the bottom right corner is outside of the rect in all cases except when checking isPointInside. Fixing isPointInside might take care of the problem as long as it's documented that the right and bottom sides of a rect are not a part of that rect.

This still seems broken to me, I would expect a Rect to be defined by the top left and bottom right corner, not the top left and one point beyond the bottom right corner.

-Brian
[/code]
jox
Bug Slayer
Posts: 726
Joined: Thu Apr 22, 2004 6:55 pm
Location: Germany

Post by jox »

Trip42 wrote:This still seems broken to me, I would expect a Rect to be defined by the top left and bottom right corner, not the top left and one point beyond the bottom right corner.
Yes that's right!

But perhaps mathematically it still makes sense...

1.) ...to define an empty rectangle as UpperLeftCorner(0,0) LowerRightCorner(0,0)

2.) ...if you visualize it as

Code: Select all

A 3x3 rectangle:

  0   1   2   3   4
0 U---+---+---+---+
  | P | P | P |   |
1 +---+---+---+---+
  | P | P | P |   |
2 +---+---+---+---+
  | P | P | P |   |
3 +---+---+---L---+
  |   |   |   |   |
4 +---+---+---+---+

U - UpperLeftCorner(0,0)
L - LowerRightCorner(3,3)
P - Point inside rectangle
(also because it's talking about corners, not points. Here all corners would actually be "outside" the rectangle)

3.) ...if you have to calculate things with it.

Of course it should be better documented!
gs

Post by gs »

That is a reasonable definition of a rectangle, however, if we assume that is the "official" definition, then the rest of irrlicht is misusing rectangles.

for Direct3D 8, using your suggested definition of a rectangle
draw2dRectangle and draw2dImage are broken. If you give them a rectangle of (0,0,1,1) they should draw nothing, but they draw 1 pixel. Since the GUI elements depend on these two functions, the error propogates to them.

I haven't confirmed this, but I suspect that the fonts treat rectangles like the rest of the library, which is not the way that you suggest we treat them.

Personally I don't care how they are defined, but it would be nice to know one way or the other, and have internal consistancy.

-Dan
gs

Post by gs »

sorry ignore my previous post
-Dan
Post Reply