BC_BORDER_*_HALF_COORD macros use floating point unnecessarily




9 years ago
9 years ago


(Reporter: jrmuizel, Assigned: jrmuizel)



Firefox Tracking Flags

(Not tracked)



(1 attachment)



9 years ago
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.
Attachment #456328 - Flags: review?(bzbarsky)

Comment 1

9 years ago
For the record this was showing up in profiles of http://www.fifa.com/worldcup/statistics/players/passelength.html
Attachment #456328 - Flags: review?(bzbarsky) → review+
Component: Layout → Layout: Tables
OS: Mac OS X → All
QA Contact: layout → layout.tables
Hardware: x86 → All
Assignee: nobody → jmuizelaar
Summary: BC_BORDER_*_HALF_COORD macros use floating point unncessarily → BC_BORDER_*_HALF_COORD macros use floating point unnecessarily

Comment 2

9 years ago
Last Resolved: 9 years ago
Resolution: --- → FIXED

Comment 3

9 years ago
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

9 years ago
I'm not sure what's correct in this case, but the behaviour should not have changed with this patch.
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

9 years ago
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.
You need to log in before you can comment on or make changes to this bug.