Bug 34297 – [Win] Fix a bug of round() with huge integral numbers

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Kent Tamura]

[Win] Fix a bug of round() with huge integral numbers

[Kent Tamura]

Created attachment 47665 [details]
Proposed patch

[Brian Weinstein]

There are a big number of tests in svg that are being Skipped on Windows due to rounding issues. Can you see if this patch fixes them?

# <rdar://problem/5663632> TextStream::operator<<(float) rounding differs between Mac OS X and Windows
svg/batik/paints/patternRegions.svg
svg/batik/text/textAnchor.svg
svg/carto.net/button.svg
svg/carto.net/colourpicker.svg
svg/carto.net/combobox.svg
svg/carto.net/selectionlist.svg
svg/carto.net/slider.svg
svg/carto.net/textbox.svg
svg/carto.net/window.svg
svg/custom/circular-marker-reference-1.svg
svg/custom/circular-marker-reference-3.svg
svg/custom/circular-marker-reference-4.svg
svg/custom/coords-relative-units-transforms.svg
svg/custom/marker-changes.svg
svg/custom/use-referencing-nonexisting-symbol.svg
svg/custom/width-full-percentage.svg
svg/W3C-SVG-1.1/coords-viewattr-01-b.svg
svg/W3C-SVG-1.1/fonts-elem-01-t.svg
svg/W3C-SVG-1.1/fonts-elem-02-t.svg
svg/W3C-SVG-1.1/fonts-elem-03-b.svg
svg/W3C-SVG-1.1/fonts-elem-04-b.svg
svg/W3C-SVG-1.1/fonts-elem-07-b.svg

Thanks!

[Kent Tamura]

(In reply to comment #2)
> There are a big number of tests in svg that are being Skipped on Windows due to
> rounding issues. Can you see if this patch fixes them?
> 
> # <rdar://problem/5663632> TextStream::operator<<(float) rounding differs
> between Mac OS X and Windows

This patch won't change the behavior of snprintf() in TextStream::operator<<(float).  Unfortunately, they won't be fixed.

[Darin Adler]

Comment on attachment 47665 [details]
Proposed patch

> +    if (!_finite(num))
> +        return num;

Seems bad for performance to do this. Can't we just leave this out? It seems that infinity or NAN would be unchanged by the code below.

> +    double tmp = ceil(num);

I think we can come up with a better name than "tmp" for this. I suggest "ceiling" or "integer"?

[Kent Tamura]

(In reply to comment #4)
> (From update of attachment 47665 [details])
> > +    if (!_finite(num))
> > +        return num;
> 
> Seems bad for performance to do this. Can't we just leave this out? It seems
> that infinity or NAN would be unchanged by the code below.

ok, removed.

> 
> > +    double tmp = ceil(num);
> 
> I think we can come up with a better name than "tmp" for this. I suggest
> "ceiling" or "integer"?

Renamed it to "integer".

Landed as r54121 <http://trac.webkit.org/changeset/54121>