[not a bug] Error in Rect Class?

[Kimundi]

I'm a bit confused about the behavior in the Rect class. Why? Well at the moment the Rect definition consist of two Positions, UpperLeft and LowerRight and the Width and Height are the difference of them. This means, if we have this Rect: (7w x 5h)

Code: Select all

+-----+
|     |
|     |
|     |
+-----+

..the two Points are at this Positions: (x,y ; x+7,y+5 )

Code: Select all

#-----+
|     |
|     |
|     |
+-----+
       #

So the Lowerright Corner is not actual Inside the Rect.
That's because counting starts with one but indicing starts with zero, for example: An array with just one element would have the lenght 1 but you have to accces array(0) because indicing start at zero.
But that don't have to be an error, you don't have to change this behavior.
But then the error is in this function:

Code: Select all

  110 		bool isPointInside(const position2d<T>& pos) const
  111 		{
  112 			return (UpperLeftCorner.X <= pos.X &&
  113 				UpperLeftCorner.Y <= pos.Y &&
  114 				LowerRightCorner.X >= pos.X &&
  115 				LowerRightCorner.Y >= pos.Y);
  116 		}

It would also return True if pos is one Point to far right or low, which is wrong. I think we should either change the function to this:

Code: Select all

  110 		bool isPointInside(const position2d<T>& pos) const
  111 		{
  112 			return (UpperLeftCorner.X <= pos.X &&
  113 				UpperLeftCorner.Y <= pos.Y &&
  114 				(LowerRightCorner.X - 1) >= pos.X &&
  115 				(LowerRightCorner.Y - 1) >= pos.Y);
  116 		}

Or the definition of Width and Height to this:

Code: Select all

  185 		//! Returns width of rectangle.
  186 		T getWidth() const
  187 		{
  188 			return LowerRightCorner.X - 1 - UpperLeftCorner.X;
  189 		}
  190 
  191 		//! Returns height of rectangle.
  192 		T getHeight() const
  193 		{
  194 			return LowerRightCorner.Y - 1 - UpperLeftCorner.Y;
  195 		}

Which would mean that we define the LowerRightCorner as the lowest right Corner INSIDE the rect.

Code: Select all

#-----+
|     |
|     |
|     |
+-----#

But unless my second suggestion describes the original intention (which I don't know), I would suggest the first fix, because it would be easier to implement.
EDIT: Sorry if my writing is a bit unclear, but I hope you understand the basic Problem.

[vitek]

I'm confused by your example, and I don't really see the problem. If you have a rect that is represented by the points (0, 0) and (7, 5), the points (0, 0) and (7, 5) are technically not inside the rectangle, they are on the edge. To be totally accurate about this, the series of points ([0-7], 5), (7, [0-5]), (0, [0-5]) and ([0-7], 0) are all on the edge.

As it is currently written, the isPointInside() function returns true if the position2d is inside or on the edge of the given rect. If you want to know if a point is actually inside, I suggest you add a isPointTotalInside() that is implemented something like this...

Code: Select all

bool isPointTotalInside(core::position2d<T>& pos) const
{
  return UpperLeftCorner.X < pos.X &&
         UpperLeftCorner.Y < pos.Y &&
         pos.X < LowerRightCorner.X &&
         pos.Y < LowerRightCorner.Y;
}

Also, Since rect is a template, you can't just go and subtract 1 from the lower right corner value. If the template is instantiated on a f32 then the point (6.99, 4.99) would be outside the rectangle, and that would be totally wrong.

Travis

[Kimundi]

You're right
My problem was that I thougt about the Points as Pixels, which are actual small areas...
And about the templates: There is nothing like this in the .NET Wrapper, which I'm using