577357 - BC_BORDER_*_HALF_COORD macros use floating point unnecessarily

Status: RESOLVED FIXED

Description

[Jeff Muizelaar [:jrmuizel]]

Created attachment 456328 [details] [diff] [review]
Avoid floating point arithmetic and convert to inline functions

twips per pixel is an integer so these macros don't need to use floating point.

Comment 1

[Jeff Muizelaar [:jrmuizel]]

For the record this was showing up in profiles of http://www.fifa.com/worldcup/statistics/players/passelength.html

Comment 2

[Jeff Muizelaar [:jrmuizel]]

http://hg.mozilla.org/mozilla-central/rev/72a7db3de71b

Comment 3

[Alfred Kayser]

Watch out for unwanted rounding due to use of integer's.
With px = 9 and p2t = 10:
(9 / 2) * 10 = 40 which is different than (9 * 10) / 2 = 45.
But possibly this is correct, as the rounding is to happen at 'pixel' level, and not at 'twips'?

Comment 4

[Jeff Muizelaar [:jrmuizel]]

I'm not sure what's correct in this case, but the behaviour should not have changed with this patch.

Comment 5

[David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-8 (if account gets disabled due to email bounces, ask a bugzilla admin to reenable it)]

I think the intent here was to round to pixels (like we do for borders in general), so that border widths do not vary based on subpixel position.

Comment 6

[Bernd]

The result of the the macro should be:

1. BC_BORDER_TOP_HALF_COORD + BC_BORDER_BOTTOM_HALF_COORD should be exactly the number of px* twips wide, so that on the screen a real pixel is covered.

2. The rounding  should be that the top is wider than the bottom part lets say for 5px wide border the top half should be 3px and the bottom 2px wide but together the should be 5px as rule 1 states.