Closed Bug 530372 Opened 15 years ago Closed 15 years ago

Replace PR_MIN/PR_MAX with NS_MIN/NS_MAX in SVG SMIL

Categories

(Core :: SVG, defect)

defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1

People

(Reporter: lusian, Assigned: lusian)

References

Details

Attachments

(2 files, 2 obsolete files)

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
Blocks: 518502
Version: unspecified → Trunk
Attached patch Search&Replace, 0 (deleted) — Splinter Review
Attachment #413878 - Flags: review?(roc)
Attachment #413878 - Flags: review?(roc) → review+
Keywords: checkin-needed
Assignee: nobody → lusian
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Sorry, rather badly bit-rotted and no longer comes close to applying.
Keywords: checkin-needed
Attached patch Same patch, using unix-style line-endings (obsolete) (deleted) — Splinter Review
Applies fine actually (no fuzz needed). It's got dos-style line-endings, though. Here's the patch passed through the "dos2unix" tool.
Attachment #417383 - Flags: review+
(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.
Keywords: checkin-needed
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.
Keywords: checkin-needed
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.
Attached patch Same patch, with build failure fixed (obsolete) (deleted) — Splinter Review
Here's the same patch, with the fix from comment 6. It builds fine on my linux machine. Carrying forward r=jwatt
Attachment #417383 - Attachment is obsolete: true
Attachment #418585 - Flags: review+
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.
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.
Attachment #418585 - Attachment is obsolete: true
Attachment #418605 - Flags: review+
tryserver agrees with you about that version
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: