530372 - Replace PR_MIN/PR_MAX with NS_MIN/NS_MAX in SVG SMIL

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jae-Seong Lee-Russo]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20091120 Minefield/3.7a1pre (.NET CLR 3.5.30729)
Build Identifier: 

Please see Bug #512106.

Reproducible: Always

Comment 1

[Jae-Seong Lee-Russo]

Attachment: Search&Replace, 0

Comment 2

[Phil Ringnalda (:philor)]

Sorry, rather badly bit-rotted and no longer comes close to applying.

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: Same patch, using unix-style line-endings

Applies fine actually (no fuzz needed).  It's got dos-style line-endings, though.

Here's the patch passed through the "dos2unix" tool.

Comment 4

[Daniel Holbert [:dholbert]]

(In reply to comment #3)
> Applies fine actually (no fuzz needed).  It's got dos-style line-endings,
> though.

(The dos line endings apparently make "hg import" barf.  The "patch" tool handles them better, though it spams the warning "Stripping trailing CRs from patch.")

This is still checkin-needed, but use the unix-style one for more successful patch-application.

Comment 5

[Phil Ringnalda (:philor)]

That does indeed apply better, and I someday I'll manage to remember that.

However, if you look at try-eef9251596ce in http://tinderbox.mozilla.org/showbuilds.cgi?tree=MozillaTry&maxdate=1261360140&hours=2 that's just this patch, which burned on Linux, Mac, WinMo and WinCE, only building on WinNT, so I contend it's still not checkin-needed.

Comment 6

[Daniel Holbert [:dholbert]]

Indeed -- the build error is:
> nsSVGFilters.cpp:4806: error: no matching function for call to ‘NS_MAX(double, float)’

That's an error from by this chunk of the patch:
>-      cosConeAngle = PR_MAX(cos(limitingConeAngle * radPerDeg), 0);
>+      cosConeAngle = NS_MAX(cos(limitingConeAngle * radPerDeg), 0.f);

I think "0.f" needs to be "0.0" there.

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: Same patch, with build failure fixed

Here's the same patch, with the fix from comment 6.  It builds fine on my linux machine.

Carrying forward r=jwatt

Comment 8

[Phil Ringnalda (:philor)]

http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1261364064.1261366611.11219.gz

An amusing option, to have it work on Linux and OS X and WinMo and WinCE, and only fail on WinNT, but I still think a version which will actually compile on every platform would be nicer.

Comment 9

[Daniel Holbert [:dholbert]]

Attachment: Same patch, with build failure fixed & hand-holding for windows compiler

I guess Windows needs some hand-holding to figure out that it needs to use the |double| version of NS_MAX for two double arguments.

Added an explicit "<double>" in this one -- carrying forward r=jwatt.

Comment 10

[Phil Ringnalda (:philor)]

tryserver agrees with you about that version

Comment 11

[Nochum Sossonko [:Natch]]

Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/15d6851096b9.