Bug 88259

Created attachment 145638 [details] Patch

Another downstream bug: http://code.google.com/p/chromium/issues/detail?id=130659

Comment on attachment 145638 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=145638&action=review > Source/WebCore/rendering/RenderBlockLineLayout.cpp:110 > - int m_left; > - int m_right; > + float m_left; > + float m_right; Yikes. Nice to fix this.

I plan on committing this (and the required rebaselines for mac and windows) later tonight when the tree is a bit more quiet.

In addition to the patch changes in comment #3, there seems to be some inconsistency in FractionalLayoutUnit.h for the various operators and the permutations for FractionalLayoutUnit, int, float, and double arguments for left and right hand sides. At initial glance, the following seem to be missing, although if they are not used, there may be no need for them: inline bool operator<=(const FractionalLayoutUnit& a, double b) { return a.toDouble() <= b; } inline bool operator<=(const double a, const FractionalLayoutUnit& b) { return a <= b.toDouble(); } inline bool operator>=(const FractionalLayoutUnit& a, float b) { return a.toFloat() >= b; } inline bool operator>=(const double a, const FractionalLayoutUnit& b) { return a >= b.toDouble(); } inline bool operator<(const double a, const FractionalLayoutUnit& b) { return a < b.toDouble(); } inline bool operator!=(const FractionalLayoutUnit& a, double b) { return a != FractionalLayoutUnit(b); } inline bool operator!=(const double a, const FractionalLayoutUnit& b) { return FractionalLayoutUnit(a) != b; } inline bool operator!=(const float a, const FractionalLayoutUnit& b) { return FractionalLayoutUnit(a) != b; } inline bool operator==(const FractionalLayoutUnit& a, double b) { return a.toDouble() == b; } inline FractionalLayoutUnit& operator+=(FractionalLayoutUnit& a, double b) { a = a + b; return a; } inline double& operator+=(double& a, const FractionalLayoutUnit& b) { a = a + b; return a; } inline int& operator+=(int& a, const FractionalLayoutUnit& b) { a = a + b; return a; } inline FractionalLayoutUnit& operator-=(FractionalLayoutUnit& a, float b) { a = a - b; return a; } inline FractionalLayoutUnit& operator-=(FractionalLayoutUnit& a, double b) { a = a - b; return a; } inline double& operator-=(double& a, const FractionalLayoutUnit& b) { a = a - b; return a; } inline int& operator-=(int& a, const FractionalLayoutUnit& b) { a = a - b; return a; } inline FractionalLayoutUnit& operator*=(FractionalLayoutUnit& a, const FractionalLayoutUnit& b) { a = a * b; return a; } inline FractionalLayoutUnit& operator*=(FractionalLayoutUnit& a, double b) { a = a * b; return a; } inline double& operator*=(double& a, const FractionalLayoutUnit& b) { a = a * b; return a; } inline float& operator*=(float& a, const FractionalLayoutUnit& b) { a = a * b; return a; } inline int& operator*=(int& a, const FractionalLayoutUnit& b) { a = a * b; return a; }

The current version of FractionalLayoutUnit::epsilon does not seem correct either as it would always evaluate to 0.0f rather than the float equivalent of 0.0166666667, which seems like it could cause problems in FractionalLayoutRect::enclosingFractionalLayoutRect(const FloatRect& rect). I'd recommend the following:     static float epsilon() { return 1.0f / kFixedPointDenominator; }

Committed r119456: <http://trac.webkit.org/changeset/119456>

(In reply to comment #5) > In addition to the patch changes in comment #3, there seems to be some inconsistency in FractionalLayoutUnit.h for the various operators and the permutations for FractionalLayoutUnit, int, float, and double arguments for left and right hand sides. > > At initial glance, the following seem to be missing, although if they are not used, there may be no need for them: Up until now we've only added the operators that we've needed. This bug, however, shows that this might not have been the best of ideas though. As such we are seriously considering adding the missing operators. The fact that epsilon is incorrect is a known problem and is on the list of things we need to fix. If you have any spare cycles and would like to help with any of this we'd _gladly_ welcome it! Either way, thank you for the comprehensive list of missing operators!

> The fact that epsilon is incorrect is a known problem and is on the list of things we need to fix. If you have any spare cycles and would like to help with any of this we'd _gladly_ welcome it! Either way, thank you for the comprehensive list of missing operators! Looks like Bug 86065 - "FractionalLayoutUnit minor math bugs" will address epsilon and some of the operator overloads. I may try to setup a WebKit build if I get a chance and post a more formal patch to that bug. Thanks for fixing this bug, it was driving me crazy in recent Chrome builds.