Re: [geometry] DOMRect: use of unrestricted doubles

> On Oct 17, 2016, at 7:03 am, Rik Cabanier <cabanier@gmail.com> wrote:
> 
> On Sun, Oct 16, 2016 at 11:15 PM, Rik Cabanier <cabanier@gmail.com <mailto:cabanier@gmail.com>> wrote:
> 
> 
> On Sun, Oct 16, 2016 at 9:21 PM, Simon Fraser <smfr@me.com <mailto:smfr@me.com>> wrote:
>> On Oct 16, 2016, at 9:34 AM, Rik Cabanier <cabanier@gmail.com <mailto:cabanier@gmail.com>> wrote:
>> 
>> On Sat, Oct 15, 2016 at 2:47 PM, Simon Fraser <smfr@me.com <mailto:smfr@me.com>> wrote:
>> https://drafts.fxtf.org/geometry/#DOMRect <https://drafts.fxtf.org/geometry/#DOMRect>
>> 
>> Using unrestricted doubles forces implementors to handle NaN and Inf values for x, y, width and height, and to correctly propagate NaN and Inf through steps used to calculate left, top, right, bottom (assuming IEEE rules, though the spec does not make this explicit). This is non-trivial, since std::min<> and  std::max<> do not follow the same NaN propagation rules as Math.min()/Math.max(). Implementors have to add isnan() checks to left, top, right, bottom implementations. Is the complexity worth it?
>> 
>> yes. We allowed this so NaN and Inf would signal when matrix calculations hit edge conditions.
>> Instead of throwing or giving inaccurate result, it was decided to allow the values so authors can check for those. The alternative would be to throw and we feared that this would break a lot of code since people don't test with exceptions.
>> 
>> See also the thread here: https://lists.mozilla.org/pipermail/dev-platform/2014-June/005091.html <https://lists.mozilla.org/pipermail/dev-platform/2014-June/005091.html>
>> 
>> When I did the implementation in Firefox, this actually made the code easier to implement since I didn't have to add a bunch of conditionals and could just rely on the FPU to do the correct thing.
> 
> Well, Firefox does the wrong thing for DOMRect.left() when, for example. width is NaN (if you assume the spec follows JS Math rules).
> 
> roc implemented DOMRect and it seems that it was a simple rename of something that Firefox already had [1]. It likely doesn't follow the spec to the letter.
> 
> There's a test in the patch in https://bugs.webkit.org/show_bug.cgi?id=163464 <https://bugs.webkit.org/show_bug.cgi?id=163464> (which needs to be converted to a web platform test).
> 
> Why do you need to add the special case handling? If width or x is NaN, shouldn't left always returns x? 
> 
> Sorry, I meant to say NaN
> min(x, x+NaN) = min(x, NaN) = (x<NaN?x:NaN) = NaN 
>  
> 1: https://bugzilla.mozilla.org/show_bug.cgi?id=916520  <https://bugzilla.mozilla.org/show_bug.cgi?id=916520>
Correct implementation of left, top, right and bottom require two NaN checks per call, which is an unfortunate side-effect of allowing NaN through the interface:

https://trac.webkit.org/browser/trunk/Source/WebCore/dom/DOMRectReadOnly.h?rev=207438#L47
https://trac.webkit.org/browser/trunk/Source/WTF/wtf/MathExtras.h?rev=207438#L403 <https://trac.webkit.org/browser/trunk/Source/WTF/wtf/MathExtras.h?rev=207438#L403>

Simon