IntersectRect bug

[PatmanSan]

Currently InterSectRect allows an empty rect to pass as valid (Right = Left, Bottom = Top):
Result := (Dst.Right >= Dst.Left) and (Dst.Bottom >= Dst.Top);

Should be
Result := (Dst.Right > Dst.Left) and (Dst.Bottom > Dst.Top);
or
Result := not IsRectEmpty(Dst);

[PatmanSan]

Sorry for the bug in the title. Might as well be a feature required by the library. [Humbled]

[andersmelander]

The > vs >= doesn't matter. The result is the same.
The missing check for an empty rect however is unfortunate since this alone means the behavior is different from the standard RTL function.

It is unclear however what the correct behavior is. The RTL function originally mirrored the Win32 function of the same name and in this function the handling of empty rects is undefined - or at least undocumented.

One can of course argue that if one of the rects are empty, then any intersection must also be empty. But I guess it could also be argued that even though the intersection of two empty rects is an empty rect, they do in fact intersect. I don't know. I guess that's a question for the philosophers :-)

Anyhow, although I would love to bring the function into line with the Delphi RTL implementation, I'm hesitant to change such a low level function that has had this flaw for a very long time. We might break peoples code by doing so.

I'll have to investigate the history of this function a bit more before I take a stand.

Btw, I think it's a bug. If this was a feature request I would decline it on grounds of the preservation of backward compatibility.

[CuriousKit]

Looking at this logically...

If the two input rects do not intersect at all, then Dst has at least one negative dimension and this is caught by the Result := (Dst.Right >= Dst.Left) and (Dst.Bottom >= Dst.Top); line. This also holds for two distinct empty rects (two empty rects that coincide will produce an empty rect in Dst).

On the other hand, if two rects touch on one of their edges, then the intersecting rect is a degenerate line (if they touch by a shared corner, then it's a degenerate point). Whether this should count as an intersection or not is up for debate. For the purposes of collision detection, for example, counting this as an intersection would be preferred.

However, let us look at another degenerate case... an empty rect inside a non-empty rect, or what is essentially a degenerate point or line inside a rect. Logically, this is a perfectly valid intersection; however, changing the criteria to Result := (Dst.Right > Dst.Left) and (Dst.Bottom > Dst.Top); would cause IntersectRect to return False in this case.

Long story short, my personal stand is "keep as is" due to the above.

[andersmelander]

It's good to get a second opinion on this. FWIW, I don't think there really is a good solution.

As far as I can tell the function was originally introduced as a native (and thus faster) alternative to the Delphi RTL function which at that point was a call to the Win32 API function. Since then Delphi has gotten its own implementation(s) and if the behavior of the two had been identical then I would just have deprecated Graphic32's in favor of Delphi's.

I'm mostly leaning towards renaming or marking it deprecated to avoid the ambiguity. That at least would be hard-breaking change (meaning the compiler would fail or warn on it) which would force the developer to decide, case by case, if the RTL IntersectRect was a suitable replacement.

[CuriousKit]

That's fine by me. As long as the behaviour is well-defined or at least well-documented for this function, that's all that matters in the end. Edge cases are always fun!