Bug 15518 - Replace == use in RectangleF.Contains (FP rounding)
Summary: Replace == use in RectangleF.Contains (FP rounding)
Status: RESOLVED FIXED
Alias: None
Product: iOS
Classification: Xamarin
Component: Xamarin.iOS.dll ()
Version: 7.0.4.x
Hardware: PC Mac OS
: --- normal
Target Milestone: Untriaged
Assignee: Sebastien Pouliot
URL:
Depends on:
Blocks:
 
Reported: 2013-10-19 17:20 UTC by Larry O'Brien
Modified: 2013-10-31 16:40 UTC (History)
4 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Larry O'Brien 2013-10-19 17:20:05 UTC
`RectangleF.Contains(RectangleF)` uses overloaded == to compare, but also may execute some FP math (Contains -> Intersect -> FromLTRB, which performs `right - left` and `bottom - top`. 

Those calcs can create FP rounding errors:

RectangleF r1 = new RectangleF (119.221f, -122.9433f, 646f, 646f); 
RectangleF r2 = new RectangleF (238.4419f, 0f, 84.55808f, 77.11342f);
bool contains = r1.Contains (r2); 

Returns false because 323f - 238.4419f = 84.55811f, which introduces a rounding error (84.55811 != 84.5581)

RectangleF.operator== uses == for FP compare. A "difference less than epsilon" technique might be more appropriate, either in the == overload or in the RectangleF.Contains() method.
Comment 1 Brian Rice 2013-10-19 22:04:22 UTC
Wouldn't this be more efficient anyway (and also not have the rounding errors):

public bool Contains(RectangleF rect) {
    if (this.Left <= rect.Left &&
        this.Top <= rect.Top &&
        this.Bottom >= rect.Bottom &&
        this.Right >= rect.Right)
        return true;
    return false;
}
Comment 2 Sebastien Pouliot 2013-10-22 08:03:51 UTC
Make sense. I'll look to see if XI implmentation match Syetm.Drawing code first. That version was field tested for a long time against MS version.
Comment 3 Sebastien Pouliot 2013-10-22 08:11:16 UTC
Interesting, we do fail one test (from System.Drawing unit tests)

2013-10-22 08:09:41.711 monotouchtest[1460:a0b] 	[FAIL] ContainsF :   a
  Expected: True
  But was:  False
2013-10-22 08:09:41.711 monotouchtest[1460:a0b] 		  at MonoTests.System.Drawing.TestRectangleF.ContainsF () [0x00044] in /Developer/MonoTouch/Source/mono/mcs/class/System.Drawing/Test/System.Drawing/TestRectangleF.cs:78 
2013-10-22 08:09:41.711 monotouchtest[1460:a0b] 		  at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (System.Reflection.MonoMethod,object,object[],System.Exception&)
2013-10-22 08:09:41.712 monotouchtest[1460:a0b] 		  at System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00044] in /Developer/MonoTouch/Source/mono/mcs/class/corlib/System.Reflection/MonoMethod.cs:230
Comment 4 Sebastien Pouliot 2013-10-22 08:28:56 UTC
Fixed in master 9d86706d72c5e7b7e6e8cde07b838df40394fc35

Update to use the same code:

   return x <= rect.x && Right >= rect.Right && y <= rect.y && Bottom >= rect.Bottom;

QA: System.Drawing unit tests enabled for Rectangle{F], Point[F] and Size[F] in the same commit