Runtime: System.Drawing.Rectangle Contains methods have misleading documentation

[TobiasBreuer]

This issue is true for the good old Framework as well as for the "new" .NetCore implementation (corefx/src/System.Drawing.Primitives/src/System/Drawing/Rectangle.cs)

The Rectangle class provides the following overloaded versions of the contains method:

public bool Contains(int x, int y);
public bool Contains(Point pt) // forwards to Contains(int, int);
public bool Contains(Rectangle rect);

While the contains method with the Rectangle parameter includes the border points on the right and the bottom, the contains method with the two integer parameter will categorize a point on the border as not being contained.

Assume the following cases:

Rectangle test = new Rectangle (0, 0, 100, 20);
Rectangle test2 = new Rectangle(0, 0, 100, 20);

bool covered = test.Contains(test2) // will return true as expected
bool covered2 = test.Contains(0, 0) // will return true as expected
bool covered3 = test.Contains(100, 20) // will return false NOT expected
bool covered4 = test.Contains(100, 0) // will return false NOT expected

The last cases in my eyes are unexpected, as the documentation for Rectangle.Contains (with the rectangle parameter) states it will return true if it fully covers the given rectangle.

In my eyes it would be more intuitive and semantically consistent if the implementation would be changed from

public bool Contains(int x, int y) => X <= x && x < X + Width && Y <= y && y < Y + Height;

to

public bool Contains(int x, int y) => X <= x && x <= X + Width && Y <= y && y <= Y + Height;

I know it's a matter of definition what "Contains" semantically means. One could find some good arguments why the last two test cases actually are correct. But then we should also discuss the first test case which should be then false as well. So either a Rectangle is fully contained which means also the points (i.e. Rectangle.Right and Rectangle.Bottom) or if those points are not contained then the rectangle itself cannot (should not) be contained as well. (I personally prefer the way i mentioned in the comments in code above).

Best regards,

Tobias

[mikedn]

I know it's a matter of definition what "Contains" semantically means.

Well, the System.Drawing.Rectangle is used for graphics. Pixel oriented graphics to be precise. No matter how you look at it there's no way that pixel (100, 0) can be considered to be contained by rectangle (0, 0, 100, 20) which covers pixel columns 0 to 99.

[TobiasBreuer]

Yes, this is of course totally correct. But still there is some inconsistency there in my opinion because a Rectangle defined as above has also the properties
Right = 100;
Bottom = 20;
So from your argumentation (which I fully agree to), either the Rectangle class has to reflect it's actual column coverage (Right = 99 [inclusive]) or the contains should be adapted. Or we all agree that Right = 100 [exclusive] and we can close this issue.

The documentation for the Right property actually states:

/// Gets the x-coordinate of the lower-right corner of the
/// rectangular region defined by this \

which suggests that the Right coordinate is included within the Rectangle...

[mikedn]

The documentation for the Right property actually states:

Where? MSDN says:

Gets the x-coordinate that is the sum of X and Width property values of this Rectangle structure.

and

The value of the Right property represents the x-coordinate of the first point at the right edge of the rectangle that is not contained in the rectangle.

The fact that Right and Bottom are exclusive or inclusive is IMO a different issue, unrelated to the behavior of Contains.

[TobiasBreuer]

Well... Shame on me - that pretty much clarifies the situation. I was referring to the xml documentation in the source code (given on both, the ReferenceSource and here within the file). That is where my quote is copied from.

With this additional statement from MSDN you quoted, the situation is clear and the behaviour matches the definition. I'm sorry for unnecesserily causing any trouble. This issue can be closed with the given information.

[willdean]

Sounds like a PR on the XML docs would be worthwhile...