Major problems with rect<T>?

[wyrmmage]

Perhaps I'm missing something really obvious here, (and it seems like something that would be immediately noticed), but it looks like rect<T> is broken for quite a few functions. For instance, try out this code:

Code: Select all

#include <iostream>
#include "irrlicht/irrlicht.h"

int main()
{
using namespace irr;
using namespace core;

rect<f32>* validRect = new rect<f32>(position2d<f32>(0, 0), dimension2d<f32>(2, 2)); //works
rect<f32>* invalidRect = new rect<f32>(position2d<f32>(-1, 1), position2d<f32>(1, -1)); //doesn't work
std::cout << "validRect: " << validRect->isValid() << "\n";
std::cout << "invalidRect: " << invalidRect->isValid() << "\n";

std::cout << "\n";

std::cout << "validWidth: " << validRect->getWidth() << ", validHeight: " << validRect->getHeight() << "\n";
std::cout << "invalidWidth: " << invalidRect->getWidth() << ", invalidHeight: " << invalidRect->getHeight() << "\n";

int tempInt;
std::cin >> tempInt;
return 1;
}

You'll notice that the rectangle named invalidRect returns 0 for invalidRect->isValid(); the rectangle is perfectly valid, however, since a rectangle constructed with the upperLeft coordinate at (-1, 1) and the lowerRight coordinate at (1, -1) looks like this:

Code: Select all

(-1, 1)----------(1, 1)
|                  |
|                  |
|                  |
(-1, -1)---------(1, -1)

This isn't a perfect square, I realize, but you get the point

The problem is in rect.h, where isValid() tests if LowerRightCorner.X >= UpperLeftCorner.X (which is fine), but it's also testing for LowerRightCorner.Y >= UpperLeftCorner.Y. Why would we test if the lower y coordinate is greater than the upper y coordinate? :\

This problem is also found in several other fuctions.

The reason that the rectangle

Code: Select all

rect<f32>* validRect = new rect<f32>(position2d<f32>(0, 0), dimension2d<f32>(2, 2));

is found to be valid is because in the constructor rect(const position2d<T>& pos, const dimension2d<T>& size), for the y coordinate it's adding the height and the y-coordinate of the position instead of subtracting it.

I'll provide a full patch later so that if this is truly a bug it can be fixed quickly

Thanks
-wyrmmage

[Ion Dune]

That code looks right to me. The terms "UpperLeft" and "LowerRight" refer to the positions on the screen, with 0,0 being the upper left most pixel and 1280,1024 (for example) being the lower right most pixel.

The problem with your picture, it seems, is that you have positive y above negative y, and it is the opposite for irrlicht (IIRC).

[wyrmmage]

hehe, after I started building the patch I realized what was happening, thanks XD

I was expecting the rectangle to behave like a normal rectangle, instead of a Windows rectangle.

It would be nice if the documentation said something about this. It already hints about it by saying

Code: Select all

Mostly used by 2D GUI elements and for 2D drawing methods. It has 2 positions instead of position and dimension and a fast method for collision detection with other rectangles and points.

, but a little more explanation would be nice

While this thread is open, I might as well point out that the function isValid():

Code: Select all

bool isValid() const
{
return ((LowerRightCorner.X >= UpperLeftCorner.X) && (UpperCorner.Y >= LowerRightCorner.Y));
}

does not do what its description says:

Code: Select all

Returns if the rect is valid to draw. It could be invalid if the UpperLeftCorner is lower or more right than the LowerRightCorner, or if any dimension is 0.

since it happily lets rectangles with a width and/or height of 0 through

The isValid() function could easily be re-written to use the getHeight() and getWidth() functions, like so:

Code: Select all

bool isValid() const
{
return (getWidth() > 0 && getHeight() > 0);
}

, which seems more....proper, to me

Thanks,
-wyrmmage

[torleif]

Yes, creating rectangles in irrlicht is very messy. I would like to see an overridden constructor/ set method:

rect<T> (position p1, position p2) // construct a rect the traditional way
rect<T> (position p1, dimension d1) // would construct a rect at p1, with width/ height of d1


Implementing it would be very easy, and it would solve a lot of headaches

[Ion Dune]

rect<T> (position p1, dimension d1) // would construct a rect at p1, with width/ height of d1

You mean like this?