934157 - Fix BaseRect::IsFinite() to actually work

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jonathan Watt [:jwatt]]

It seems like nobody is actually calling BaseRect::IsFinite(), at least not in code that is compiled on Windows or B2G Desktop. When I tried using it I get compiler errors since MSVC doesn't define isfinite in std:

https://tbpl.mozilla.org/?tree=Try&rev=c4d27f9544de

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: patch alternative 1

Comment 2

[Jonathan Watt [:jwatt]]

Attachment: patch alternative 2

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Alternative 2 seems superior to me.

Comment 4

[Jonathan Watt [:jwatt]]

Whaaat?! You don't like my neat trick in alternative 1? ;)

(Being serious, yes indeed, but I'm not sure how much Moz2D is trying to eliminate external dependencies.)

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

MFBT should be fine, AIUI.

Comment 6

[Bas Schouten (:bas.schouten)]

Comment on attachment 826348 [details] [diff] [review]
patch alternative 2

Review of attachment 826348 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, this is fine, we'll have to update the MFBT in the Moz2D repository though. The one in there currently doesn't have IsFinite.

Comment 7

[Bas Schouten (:bas.schouten)]

(In reply to Bas Schouten (:bas.schouten) from comment #6)
> Comment on attachment 826348 [details] [diff] [review]
> patch alternative 2
> 
> Review of attachment 826348 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Yes, this is fine, we'll have to update the MFBT in the Moz2D repository
> though. The one in there currently doesn't have IsFinite.

I've updated the MFBT in the Moz2D repository. So we can now use IsFinite there.

Comment 8

[Jonathan Watt [:jwatt]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c435419ae121

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c435419ae121

Comment 10

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Jonathan Watt [:jwatt] from comment #0)
> It seems like nobody is actually calling BaseRect::IsFinite(), at least not
> in code that is compiled on Windows or B2G Desktop. When I tried using it I
> get compiler errors since MSVC doesn't define isfinite in std:
> 
> https://tbpl.mozilla.org/?tree=Try&rev=c4d27f9544de

Indeed, the gtest that's covering this is explicitly disabled for windows.  Didn't notice that.

Comment 11

[Milan Sreckovic [:milan] (needinfo for best results)]

Perhaps it's worth trying to enable TestRect.cpp on windows as well?