[Question] CCRect::containsPoint

[progsource]

• cocos2d-x version: v3

cocos2d-x/cocos/math/CCGeometry.cpp

Lines 176 to 187 in 18a6d82

	bool Rect::containsPoint(const Vec2& point) const
	{
	bool bRet = false;
	if (point.x >= getMinX() && point.x <= getMaxX()
	&& point.y >= getMinY() && point.y <= getMaxY())
	{
	bRet = true;
	}
	return bRet;
	}

As issue #2423 was closed as far as I can see with somehow unrelated commits, I simply wanted to ask, if it is really wanted behavior to have an inclusive containsPoint method. My expectation would have been, that it should be exclusive.

[Priority10]

Please explain why do you think so?

This method is often used for simple collision detection. It seems to me quite logical that this is inclusive.

[progsource]

So - let's say we have a Rect(1, 1, 2, 2) - blue in the image.

And now we have the following results from containsPoint:

input	output
Vec2(0, 0)	false
Vec2(1, 0)	false
Vec2(2, 0)	false
Vec2(3, 0)	false
Vec2(0, 1)	false
Vec2(1, 1)	true
Vec2/(2, 1)	true
Vec2(3, 1)	true
Vec2(0, 2)	false
Vec2(1, 2)	true
Vec2(2, 2)	true
Vec2(3, 2)	true
Vec2(0, 3)	false
Vec2(1, 3)	true
Vec2(2, 3)	true
Vec2(3, 3)	true

So what I think is, that the bold marked outputs (red in the image) should actually not be inside of the rectangle, because the rectangle is only 2x2 and not 3x3. So it's like some kind of border added to it, which is now "inside" of the rectangle.

[progsource]

getMaxXand getMaxY return 3 in the above case. Maybe that should be instead 2.

cocos2d-x/cocos/math/CCGeometry.cpp

Lines 146 to 149 in b26e1bb

	float Rect::getMaxX() const
	{
	return origin.x + size.width;
	}

cocos2d-x/cocos/math/CCGeometry.cpp

Lines 161 to 164 in b26e1bb

	float Rect::getMaxY() const
	{
	return origin.y + size.height;
	}

[stevetranby]

The reason for inclusiveness is that the Rect/Vec2 are mathematical concepts. You’re thinking in rasterized pixels. If you take the mathematical rectangle {Vec2{1,1},Size{2,2}} is effectively your CYAN rectangle and doesn’t include the red area. However, when you rasterize a mathematical Rect into pixels then that is where the maxX/maxY pixel row/column is exclusive (instead of inclusive). I could probably explain better with some diagrams, but hope that makes enough sense.

[progsource]

Ok, got it. But then, would it maybe make sense to clarify the function a little bit (for example renaming it to containsPointInclusive and setting containsPoint to deprecated) , or maybe even adding a containsPointExclusive function?

[Priority10]

@progsource I am sorry for the late reply.

As @stevetranby perfectly explained, mathematically this is right.

Maybe it is a good idea to have containsPointInclusive and containsPointExclusive. But then the mathematically correct version of containsPointExclusive will look like

 bool Rect::containsPointExclusive(const Vec2& point) const 
 { 
     bool bRet = false; 
  
     if (point.x > getMinX() && point.x < getMaxX() 
         && point.y > getMinY() && point.y < getMaxY()) 
     { 
         bRet = true; 
     } 
  
     return bRet; 
 } 

and with your image it also will not give you the desired result.

To my shame I do not know how rasterization works in the engine and I can not tell if this is an eternal conflict of mathematics vs rasterization, or just rasterization does not work as it does on your image.

[v1993]

By the way, why we use if there at all?! Just write this function as

bool Rect::containsPoint(const Vec2& point) const
{
    return (point.x >= getMinX() && point.x <= getMaxX() 
         && point.y >= getMinY() && point.y <= getMaxY())
}

It is much easier!

[Priority10]

@v1993 Good point.

[progsource]

So - in the end I would be fine with just making it clear in code (either with changing the method name or adding a comment), that the current method is inclusive by intention. And well - what v1993 said ;)

[minggo]

I created #19155 to make the codes more clean and make the comment more clear.