Diff
Modified: trunk/Source/WTF/ChangeLog (175795 => 175796)
--- trunk/Source/WTF/ChangeLog 2014-11-09 19:46:24 UTC (rev 175795)
+++ trunk/Source/WTF/ChangeLog 2014-11-10 00:21:56 UTC (rev 175796)
@@ -1,3 +1,32 @@
+2014-11-09 Chris Dumez <[email protected]>
+
+ Add a more correct way to compare floating point numbers and use it
+ https://bugs.webkit.org/show_bug.cgi?id=138527
+
+ Reviewed by Darin Adler.
+
+ To compare floating point numbers in the code base, we would often rely
+ on the following check:
+ std::abs(a - b) <= std::numeric_limits<T>::epsilon()
+
+ However, this is not correct for arbitrary floating point values, and
+ will fail for values that are not close to zero.
+
+ This patch introduces a WTF::areEssentiallyEqual() templated function
+ that can only be called with floating point types and relies on the
+ following formula from [1][2] that defines u as being "essentially
+ equal" to v if: | u - v | / |u| <= e and | u - v | / |v| <= e
+
+ [1] Knuth, D. E. "Accuracy of Floating Point Arithmetic." The Art of
+ Computer Programming. 3rd ed. Vol. 2. Boston: Addison-Wesley, 1998.
+ 229-45.
+ [2] http://www.boost.org/doc/libs/1_34_0/libs/test/doc/components/test_tools/floating_point_comparison.html
+
+ * wtf/MathExtras.h:
+ (WTF::safeFPDivision):
+ (WTF::areEssentiallyEqual):
+ (WTF::withinEpsilon): Deleted.
+
2014-11-08 Darin Adler <[email protected]>
Replace FileThread class with a single function
Modified: trunk/Source/WTF/wtf/MathExtras.h (175795 => 175796)
--- trunk/Source/WTF/wtf/MathExtras.h 2014-11-09 19:46:24 UTC (rev 175795)
+++ trunk/Source/WTF/wtf/MathExtras.h 2014-11-10 00:21:56 UTC (rev 175796)
@@ -393,11 +393,32 @@
}
template <typename T>
-inline bool withinEpsilon(T a, T b)
+inline typename std::enable_if<std::is_floating_point<T>::value, T>::type safeFPDivision(T u, T v)
{
- return std::abs(a - b) <= std::numeric_limits<T>::epsilon();
+ // Protect against overflow / underflow.
+ if (v < 1 && u > v * std::numeric_limits<T>::max())
+ return std::numeric_limits<T>::max();
+ if (v > 1 && u < v * std::numeric_limits<T>::min())
+ return 0;
+ return u / v;
}
+// Floating point numbers comparison:
+// u is "essentially equal" [1][2] to v if: | u - v | / |u| <= e and | u - v | / |v| <= e
+//
+// [1] Knuth, D. E. "Accuracy of Floating Point Arithmetic." The Art of Computer Programming. 3rd ed. Vol. 2.
+// Boston: Addison-Wesley, 1998. 229-45.
+// [2] http://www.boost.org/doc/libs/1_34_0/libs/test/doc/components/test_tools/floating_point_comparison.html
+template <typename T>
+inline typename std::enable_if<std::is_floating_point<T>::value, bool>::type areEssentiallyEqual(T u, T v, T epsilon = std::numeric_limits<T>::epsilon())
+{
+ if (u == v)
+ return true;
+
+ const T delta = std::abs(u - v);
+ return safeFPDivision(delta, std::abs(u)) <= epsilon && safeFPDivision(delta, std::abs(v)) <= epsilon;
+}
+
} // namespace WTF
#endif // #ifndef WTF_MathExtras_h
Modified: trunk/Source/WebCore/ChangeLog (175795 => 175796)
--- trunk/Source/WebCore/ChangeLog 2014-11-09 19:46:24 UTC (rev 175795)
+++ trunk/Source/WebCore/ChangeLog 2014-11-10 00:21:56 UTC (rev 175796)
@@ -1,3 +1,25 @@
+2014-11-09 Chris Dumez <[email protected]>
+
+ Add a more correct way to compare floating point numbers and use it
+ https://bugs.webkit.org/show_bug.cgi?id=138527
+
+ Reviewed by Darin Adler.
+
+ Use the new WTF::areEssentuallyEqual() utility function from MathExtras.h
+ to compare floating-point numbers.
+
+ No new tests, no behavior change.
+
+ * page/DOMTimer.cpp:
+ (WebCore::DOMTimer::updateTimerIntervalIfNecessary):
+ * platform/graphics/FloatQuad.cpp:
+ (WebCore::FloatQuad::isRectilinear):
+ * platform/graphics/FloatRoundedRect.cpp:
+ (WebCore::FloatRoundedRect::Radii::isUniformCornerRadius):
+ * platform/graphics/FloatSize.h:
+ (WebCore::areEssentiallyEqual):
+ (WebCore::withinEpsilon): Deleted.
+
2014-11-08 Simon Fraser <[email protected]>
Implement round-rect clipping on video elements
Modified: trunk/Source/WebCore/page/DOMTimer.cpp (175795 => 175796)
--- trunk/Source/WebCore/page/DOMTimer.cpp 2014-11-09 19:46:24 UTC (rev 175795)
+++ trunk/Source/WebCore/page/DOMTimer.cpp 2014-11-10 00:21:56 UTC (rev 175796)
@@ -270,11 +270,11 @@
double previousInterval = m_currentTimerInterval;
m_currentTimerInterval = intervalClampedToMinimum();
- if (WTF::withinEpsilon(previousInterval, m_currentTimerInterval))
+ if (WTF::areEssentiallyEqual(previousInterval, m_currentTimerInterval))
return;
if (repeatInterval()) {
- ASSERT(WTF::withinEpsilon(repeatInterval(), previousInterval));
+ ASSERT(WTF::areEssentiallyEqual(repeatInterval(), previousInterval));
augmentRepeatInterval(m_currentTimerInterval - previousInterval);
} else
augmentFireInterval(m_currentTimerInterval - previousInterval);
Modified: trunk/Source/WebCore/platform/graphics/FloatQuad.cpp (175795 => 175796)
--- trunk/Source/WebCore/platform/graphics/FloatQuad.cpp 2014-11-09 19:46:24 UTC (rev 175795)
+++ trunk/Source/WebCore/platform/graphics/FloatQuad.cpp 2014-11-10 00:21:56 UTC (rev 175796)
@@ -93,8 +93,8 @@
bool FloatQuad::isRectilinear() const
{
- return (WTF::withinEpsilon(m_p1.x(), m_p2.x()) && WTF::withinEpsilon(m_p2.y(), m_p3.y()) && WTF::withinEpsilon(m_p3.x(), m_p4.x()) && WTF::withinEpsilon(m_p4.y(), m_p1.y()))
- || (WTF::withinEpsilon(m_p1.y(), m_p2.y()) && WTF::withinEpsilon(m_p2.x(), m_p3.x()) && WTF::withinEpsilon(m_p3.y(), m_p4.y()) && WTF::withinEpsilon(m_p4.x(), m_p1.x()));
+ return (WTF::areEssentiallyEqual(m_p1.x(), m_p2.x()) && WTF::areEssentiallyEqual(m_p2.y(), m_p3.y()) && WTF::areEssentiallyEqual(m_p3.x(), m_p4.x()) && WTF::areEssentiallyEqual(m_p4.y(), m_p1.y()))
+ || (WTF::areEssentiallyEqual(m_p1.y(), m_p2.y()) && WTF::areEssentiallyEqual(m_p2.x(), m_p3.x()) && WTF::areEssentiallyEqual(m_p3.y(), m_p4.y()) && WTF::areEssentiallyEqual(m_p4.x(), m_p1.x()));
}
bool FloatQuad::containsPoint(const FloatPoint& p) const
Modified: trunk/Source/WebCore/platform/graphics/FloatRoundedRect.cpp (175795 => 175796)
--- trunk/Source/WebCore/platform/graphics/FloatRoundedRect.cpp 2014-11-09 19:46:24 UTC (rev 175795)
+++ trunk/Source/WebCore/platform/graphics/FloatRoundedRect.cpp 2014-11-10 00:21:56 UTC (rev 175796)
@@ -64,10 +64,10 @@
bool FloatRoundedRect::Radii::isUniformCornerRadius() const
{
- return WTF::withinEpsilon(m_topLeft.width(), m_topLeft.height())
- && withinEpsilon(m_topLeft, m_topRight)
- && withinEpsilon(m_topLeft, m_bottomLeft)
- && withinEpsilon(m_topLeft, m_bottomRight);
+ return WTF::areEssentiallyEqual(m_topLeft.width(), m_topLeft.height())
+ && areEssentiallyEqual(m_topLeft, m_topRight)
+ && areEssentiallyEqual(m_topLeft, m_bottomLeft)
+ && areEssentiallyEqual(m_topLeft, m_bottomRight);
}
void FloatRoundedRect::Radii::scale(float factor)
Modified: trunk/Source/WebCore/platform/graphics/FloatSize.h (175795 => 175796)
--- trunk/Source/WebCore/platform/graphics/FloatSize.h 2014-11-09 19:46:24 UTC (rev 175795)
+++ trunk/Source/WebCore/platform/graphics/FloatSize.h 2014-11-10 00:21:56 UTC (rev 175796)
@@ -168,9 +168,9 @@
return a.width() == b.width() && a.height() == b.height();
}
-inline bool withinEpsilon(const FloatSize& a, const FloatSize& b)
+inline bool areEssentiallyEqual(const FloatSize& a, const FloatSize& b)
{
- return WTF::withinEpsilon(a.width(), b.width()) && WTF::withinEpsilon(a.height(), b.height());
+ return WTF::areEssentiallyEqual(a.width(), b.width()) && WTF::areEssentiallyEqual(a.height(), b.height());
}
inline bool operator!=(const FloatSize& a, const FloatSize& b)
Modified: trunk/Source/WebKit/mac/ChangeLog (175795 => 175796)
--- trunk/Source/WebKit/mac/ChangeLog 2014-11-09 19:46:24 UTC (rev 175795)
+++ trunk/Source/WebKit/mac/ChangeLog 2014-11-10 00:21:56 UTC (rev 175796)
@@ -1,3 +1,16 @@
+2014-11-09 Chris Dumez <[email protected]>
+
+ Add a more correct way to compare floating point numbers and use it
+ https://bugs.webkit.org/show_bug.cgi?id=138527
+
+ Reviewed by Darin Adler.
+
+ Use the new WTF::areEssentuallyEqual() utility function from MathExtras.h
+ to compare floating-point numbers.
+
+ * WebView/WebHTMLView.mm:
+ (-[WebHTMLView _adjustedBottomOfPageWithTop:bottom:limit:]):
+
2014-11-08 Alexey Proskuryakov <[email protected]>
Delete cookies between tests
Modified: trunk/Source/WebKit/mac/WebView/WebHTMLView.mm (175795 => 175796)
--- trunk/Source/WebKit/mac/WebView/WebHTMLView.mm 2014-11-09 19:46:24 UTC (rev 175795)
+++ trunk/Source/WebKit/mac/WebView/WebHTMLView.mm 2014-11-10 00:21:56 UTC (rev 175796)
@@ -126,6 +126,7 @@
#import <limits>
#import <runtime/InitializeThreading.h>
#import <wtf/MainThread.h>
+#import <wtf/MathExtras.h>
#import <wtf/ObjcRuntimeExtras.h>
#import <wtf/RunLoop.h>
@@ -2412,7 +2413,7 @@
#ifdef __LP64__
// If the new bottom is equal to the old bottom (when both are treated as floats), we just return the original
// bottom. This prevents rounding errors that can occur when converting newBottom to a double.
- if (fabs(static_cast<float>(bottom) - newBottom) <= std::numeric_limits<float>::epsilon())
+ if (WTF::areEssentiallyEqual(static_cast<float>(bottom), newBottom))
return bottom;
else
#endif
Modified: trunk/Source/WebKit2/ChangeLog (175795 => 175796)
--- trunk/Source/WebKit2/ChangeLog 2014-11-09 19:46:24 UTC (rev 175795)
+++ trunk/Source/WebKit2/ChangeLog 2014-11-10 00:21:56 UTC (rev 175796)
@@ -1,3 +1,35 @@
+2014-11-09 Chris Dumez <[email protected]>
+
+ Add a more correct way to compare floating point numbers and use it
+ https://bugs.webkit.org/show_bug.cgi?id=138527
+
+ Reviewed by Darin Adler.
+
+ Use the new WTF::areEssentuallyEqual() utility function from MathExtras.h
+ to compare floating-point numbers.
+
+ * UIProcess/API/Cocoa/WKWebView.mm:
+ (withinEpsilon):
+ * UIProcess/CoordinatedGraphics/PageViewportController.cpp:
+ (WebKit::PageViewportController::updateMinimumScaleToFit):
+ (WebKit::fuzzyCompare): Deleted.
+ * UIProcess/CoordinatedGraphics/PageViewportController.h:
+ * WebProcess/WebPage/ios/WebPageIOS.mm:
+ (WebKit::withinEpsilon):
+
+ * UIProcess/API/Cocoa/WKWebView.mm:
+ (areEssentiallyEqualAsFloat):
+ (-[WKWebView _didCommitLayerTree:]):
+ (withinEpsilon): Deleted.
+ * UIProcess/CoordinatedGraphics/PageViewportController.cpp:
+ (WebKit::PageViewportController::updateMinimumScaleToFit):
+ (WebKit::fuzzyCompare): Deleted.
+ * UIProcess/CoordinatedGraphics/PageViewportController.h:
+ * WebProcess/WebPage/ios/WebPageIOS.mm:
+ (WebKit::areEssentialEqualAsFloat):
+ (WebKit::WebPage::dynamicViewportSizeUpdate):
+ (WebKit::withinEpsilon): Deleted.
+
2014-11-08 Simon Fraser <[email protected]>
Implement round-rect clipping on video elements
Modified: trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm (175795 => 175796)
--- trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm 2014-11-09 19:46:24 UTC (rev 175795)
+++ trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm 2014-11-10 00:21:56 UTC (rev 175796)
@@ -75,6 +75,7 @@
#import <_javascript_Core/JSContext.h>
#import <_javascript_Core/JSValue.h>
#import <wtf/HashMap.h>
+#import <wtf/MathExtras.h>
#import <wtf/NeverDestroyed.h>
#import <wtf/RetainPtr.h>
@@ -829,10 +830,9 @@
// WebCore stores the page scale factor as float instead of double. When we get a scale from WebCore,
// we need to ignore differences that are within a small rounding error on floats.
-template <typename TypeA, typename TypeB>
-static inline bool withinEpsilon(TypeA a, TypeB b)
+static inline bool areEssentiallyEqualAsFloat(float a, float b)
{
- return std::abs(a - b) < std::numeric_limits<float>::epsilon();
+ return WTF::areEssentiallyEqual(a, b);
}
- (void)_didCommitLayerTree:(const WebKit::RemoteLayerTreeTransaction&)layerTreeTransaction
@@ -882,7 +882,7 @@
if (_needsToRestoreExposedRect && layerTreeTransaction.transactionID() >= _firstTransactionIDAfterPageRestore) {
_needsToRestoreExposedRect = NO;
- if (withinEpsilon(contentZoomScale(self), _scaleToRestore)) {
+ if (areEssentiallyEqualAsFloat(contentZoomScale(self), _scaleToRestore)) {
WebCore::FloatPoint exposedPosition = _exposedRectToRestore.location();
exposedPosition.scale(_scaleToRestore, _scaleToRestore);
@@ -894,7 +894,7 @@
if (_needsToRestoreUnobscuredCenter && layerTreeTransaction.transactionID() >= _firstTransactionIDAfterPageRestore) {
_needsToRestoreUnobscuredCenter = NO;
- if (withinEpsilon(contentZoomScale(self), _scaleToRestore)) {
+ if (areEssentiallyEqualAsFloat(contentZoomScale(self), _scaleToRestore)) {
CGRect unobscuredRect = UIEdgeInsetsInsetRect(self.bounds, _obscuredInsets);
WebCore::FloatSize unobscuredContentSizeAtNewScale(unobscuredRect.size.width / _scaleToRestore, unobscuredRect.size.height / _scaleToRestore);
WebCore::FloatPoint topLeftInDocumentCoordinate(_unobscuredCenterToRestore.x() - unobscuredContentSizeAtNewScale.width() / 2, _unobscuredCenterToRestore.y() - unobscuredContentSizeAtNewScale.height() / 2);
Modified: trunk/Source/WebKit2/UIProcess/CoordinatedGraphics/PageViewportController.cpp (175795 => 175796)
--- trunk/Source/WebKit2/UIProcess/CoordinatedGraphics/PageViewportController.cpp 2014-11-09 19:46:24 UTC (rev 175795)
+++ trunk/Source/WebKit2/UIProcess/CoordinatedGraphics/PageViewportController.cpp 2014-11-10 00:21:56 UTC (rev 175796)
@@ -33,11 +33,6 @@
namespace WebKit {
-bool fuzzyCompare(float a, float b, float epsilon)
-{
- return std::abs(a - b) < epsilon;
-}
-
PageViewportController::PageViewportController(WebKit::WebPageProxy* proxy, PageViewportControllerClient& client)
: m_webPageProxy(proxy)
, m_client(client)
@@ -327,14 +322,16 @@
if (m_viewportSize.isEmpty() || m_contentsSize.isEmpty())
return false;
- bool currentlyScaledToFit = fuzzyCompare(m_pageScaleFactor, m_minimumScaleToFit, 0.0001);
+ // FIXME: Why this arbitrary precision? We likely want to omit the third argument so that
+ // std::numeric_limits<float>::epsilon() is used instead, similarly to Mac / iOS.
+ bool currentlyScaledToFit = WTF::areEssentiallyEqual(m_pageScaleFactor, m_minimumScaleToFit, 0.0001f);
float minimumScale = WebCore::computeMinimumScaleFactorForContentContained(m_rawAttributes, WebCore::roundedIntSize(m_viewportSize), WebCore::roundedIntSize(m_contentsSize));
if (minimumScale <= 0)
return false;
- if (!fuzzyCompare(minimumScale, m_minimumScaleToFit, 0.0001)) {
+ if (!WTF::areEssentiallyEqual(minimumScale, m_minimumScaleToFit, 0.0001f)) {
m_minimumScaleToFit = minimumScale;
if (!m_webPageProxy->areActiveDOMObjectsAndAnimationsSuspended()) {
@@ -343,7 +340,7 @@
else {
// Ensure the effective scale stays within bounds.
float boundedScale = innerBoundedViewportScale(m_pageScaleFactor);
- if (!fuzzyCompare(boundedScale, m_pageScaleFactor, 0.0001))
+ if (!WTF::areEssentiallyEqual(boundedScale, m_pageScaleFactor, 0.0001f))
applyScaleAfterRenderingContents(boundedScale);
}
}
Modified: trunk/Source/WebKit2/UIProcess/CoordinatedGraphics/PageViewportController.h (175795 => 175796)
--- trunk/Source/WebKit2/UIProcess/CoordinatedGraphics/PageViewportController.h 2014-11-09 19:46:24 UTC (rev 175795)
+++ trunk/Source/WebKit2/UIProcess/CoordinatedGraphics/PageViewportController.h 2014-11-10 00:21:56 UTC (rev 175796)
@@ -108,8 +108,6 @@
WebCore::FloatRect m_lastFrameCoveredRect;
};
-bool fuzzyCompare(float, float, float epsilon);
-
} // namespace WebKit
#endif // PageViewportController_h
Modified: trunk/Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm (175795 => 175796)
--- trunk/Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm 2014-11-09 19:46:24 UTC (rev 175795)
+++ trunk/Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm 2014-11-10 00:21:56 UTC (rev 175796)
@@ -87,6 +87,7 @@
#import <WebCore/VisibleUnits.h>
#import <WebCore/WKContentObservation.h>
#import <WebCore/WebEvent.h>
+#import <wtf/MathExtras.h>
#import <wtf/TemporaryChange.h>
using namespace WebCore;
@@ -2211,9 +2212,11 @@
m_page->mainFrame().orientationChanged();
}
-static inline bool withinEpsilon(float a, float b)
+// WebCore stores the page scale factor as float instead of double. When we get a scale from WebCore,
+// we need to ignore differences that are within a small rounding error on floats.
+static inline bool areEssentialEqualAsFloat(float a, float b)
{
- return fabs(a - b) < std::numeric_limits<float>::epsilon();
+ return WTF::areEssentiallyEqual(a, b);
}
void WebPage::resetTextAutosizingBeforeLayoutIfNeeded(const FloatSize& oldSize, const FloatSize& newSize)
@@ -2282,7 +2285,7 @@
FloatRect newUnobscuredContentRect = targetUnobscuredRect;
FloatRect newExposedContentRect = targetExposedContentRect;
- bool scaleChanged = !withinEpsilon(scale, targetScale);
+ bool scaleChanged = !areEssentialEqualAsFloat(scale, targetScale);
if (scaleChanged) {
// The target scale the UI is using cannot be reached by the content. We need to compute new targets based
// on the viewport constraint and report everything back to the UIProcess.
@@ -2347,7 +2350,7 @@
newExposedContentRect.setY(0);
}
- bool likelyResponsiveDesignViewport = newLayoutSize.width() == minimumLayoutSize.width() && withinEpsilon(scale, 1);
+ bool likelyResponsiveDesignViewport = newLayoutSize.width() == minimumLayoutSize.width() && areEssentialEqualAsFloat(scale, 1);
bool contentBleedsOutsideLayoutWidth = newContentSize.width() > newLayoutSize.width();
bool originalScrollPositionWasOnTheLeftEdge = targetUnobscuredRect.x() <= 0;
if (likelyResponsiveDesignViewport && contentBleedsOutsideLayoutWidth && originalScrollPositionWasOnTheLeftEdge) {