120690 - NSToCoordCeil() is buggy

Status: RESOLVED DUPLICATE of bug 118117

(Core :: Layout, defect)

Description

[Brian Ryner (not reading)]

Reproduced this on Linux and Windows:

Calling NSToCoordCeil(0.0) returns 1!  The reason seems to be that
CEIL_CONST_FLOAT at the top of nsUnitConversion.h has too many digits, and the
compiler rounds the number to 1.0.

Comment 1

[Doug Turner (:dougt)]

blames to peterl.  Peter can you look at this?

Comment 2

[Peter Lubczynski]

This isn't me, but the "peterl" before me. My CVS account is "peterlubczynski".

-->back to default owner

Comment 3

[Doug Turner (:dougt)]

Over to layout team.  This file should probably move there anyway.  

Comment 4

[karnaze (gone)]

The only active component in layout that uses this is mathml. -->mathml

Comment 5

[Brian Ryner (not reading)]

Uh, I'm going to put this back.  This function is used extensively by nsRect in
gfx, so it affects nearly everyone who uses nsRect's.

Comment 6

[Doug Turner (:dougt)]

This functionalty should be moved out of XPCOM.

Comment 7

[rbs]

It was in rev 3.3 that peterl replaced the standard C functions with these:
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/xpcom/ds/nsUnitConversion.h

I suspect the change may have been for speed reasons since reliable versions 
aren't that cheap. For example, JS uses the following one (s/-inf/+inf/, BTW):
http://lxr.mozilla.org/seamonkey/source/js/src/fdlibm/s_ceil.c#48

There could be similar problems with NSToCoordFloor which is used even more than 
NSToCoordCeil. Looks like a premature optimzation since the other side of the 
coin is that having such functions around for that long means people may have 
built on the behavior. 

Comment 8

[Boris Zbarsky [:bzbarsky]]

So can we switch to just using ceil() and floor() instead?

Comment 9

[rbs]

Possibly. Just need a good testing window (e.g. 1.5).

-> re-assigning to default, so that it gets triaged properly.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Ultimately I'd like to have only integer arithmetic in layout.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Isn't this a dup of bug 118117?

Comment 12

[Boris Zbarsky [:bzbarsky]]

*** This bug has been marked as a duplicate of 118117 ***