rombust

11-29-2012, 07:47 AM

This post is for both ClanLib 2.3 and ClanLib 3.0 (Devel), but we cannot change CL_Rect on ClanLib 2.3 without breaking existing code.

CL_Rect (or clan::Rect ) is currently defined as follows:

Using the following rectangle containing these pixels:

XXXXXXXXXX

XX####XXX

XX####XXX

XX####XXX

XXXXXXXXXX

The CL_Rect will contain:

left (X1) = 2, top (Y1) = 1, right (X2) = 6, bottom (Y2) = 4

So X2 and Y2 are exclusive points (outside the rectangle).

CL_Rect calculates:

Type get_width() const { return right - left; }

Type get_height() const { return bottom - top; }

So, width = 4 and height = 3, as expected

However, these following functions are treating X2 and Y2 as inclusive points (inside the rectange)

"contains, is_overlapped, is_inside"

These functions do not make it clear that the right and bottom are exclusive points:

"(set/get)_top_right, (set/get)_bottom_right, (set/get)_bottom_left)"

An additional comment about "contains"

bool contains(const Vec2<Type> &p) const

{

return ((p.x >= left && p.x <= right) || (p.x <= left && p.x >= right))

&& ((p.y >= top && p.y <= bottom) || (p.y <= top && p.y >= bottom));

}

Note "p.x <= right" should be "p.x < right" etc.

Also that function is also handling the situation where X2 is less than X1 and treating it as a valid rectangle (swapping left and right). I don't think that it should.

The "normalize" function can reverse backward rectangles. (Although, I don't think that function should be there either)

Comments?

CL_Rect (or clan::Rect ) is currently defined as follows:

Using the following rectangle containing these pixels:

XXXXXXXXXX

XX####XXX

XX####XXX

XX####XXX

XXXXXXXXXX

The CL_Rect will contain:

left (X1) = 2, top (Y1) = 1, right (X2) = 6, bottom (Y2) = 4

So X2 and Y2 are exclusive points (outside the rectangle).

CL_Rect calculates:

Type get_width() const { return right - left; }

Type get_height() const { return bottom - top; }

So, width = 4 and height = 3, as expected

However, these following functions are treating X2 and Y2 as inclusive points (inside the rectange)

"contains, is_overlapped, is_inside"

These functions do not make it clear that the right and bottom are exclusive points:

"(set/get)_top_right, (set/get)_bottom_right, (set/get)_bottom_left)"

An additional comment about "contains"

bool contains(const Vec2<Type> &p) const

{

return ((p.x >= left && p.x <= right) || (p.x <= left && p.x >= right))

&& ((p.y >= top && p.y <= bottom) || (p.y <= top && p.y >= bottom));

}

Note "p.x <= right" should be "p.x < right" etc.

Also that function is also handling the situation where X2 is less than X1 and treating it as a valid rectangle (swapping left and right). I don't think that it should.

The "normalize" function can reverse backward rectangles. (Although, I don't think that function should be there either)

Comments?