Closed Bug 934157 Opened 6 years ago Closed 6 years ago

Fix BaseRect::IsFinite() to actually work

Categories

(Core :: Graphics, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: jwatt, Assigned: jwatt)

Details

Attachments

(2 files)

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
Attachment #826347 - Flags: review?
Attachment #826348 - Flags: review?(bas)
Attachment #826347 - Flags: review? → review?(bas)
Alternative 2 seems superior to me.
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.)
MFBT should be fine, AIUI.
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.
Attachment #826348 - Flags: review?(bas) → review+
(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.
Attachment #826347 - Flags: review?(bas)
https://hg.mozilla.org/mozilla-central/rev/c435419ae121
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
(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.
Perhaps it's worth trying to enable TestRect.cpp on windows as well?
You need to log in before you can comment on or make changes to this bug.