Changeset 185916 in webkit

Timestamp:

Jun 24, 2015, 10:53:44 AM (10 years ago)

Author:

Alan Bujtas

Message:

Subpixel rendering: roundToDevicePixel() snaps to wrong value.
​https://bugs.webkit.org/show_bug.cgi?id=146273
rdar://problem/18509840

Reviewed by Simon Fraser.

Due to the floating point approximate representation, we can't always produce
the correct snap value. This patch addresses the issue by removing redundant kFixedPointDenominator multiplication
and by changing the rounding in roundToDevicePixel() from float to double.

Source/WebCore:

API test is added.

• platform/LayoutUnit.h:

(WebCore::roundToDevicePixel):

Tools:

• TestWebKitAPI/Tests/WebCore/LayoutUnit.cpp:

(TestWebKitAPI::TEST):

Location:

trunk

Files:

• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/LayoutUnit.h (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebCore/LayoutUnit.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-06-24  Zalan Bujtas  <zalan@apple.com>
+
+        Subpixel rendering: roundToDevicePixel() snaps to wrong value.
+        https://bugs.webkit.org/show_bug.cgi?id=146273
+        rdar://problem/18509840
+
+        Reviewed by Simon Fraser.
+
+        Due to the floating point approximate representation, we can't always produce
+        the correct snap value. This patch addresses the issue by removing redundant kFixedPointDenominator multiplication
+        and by changing the rounding in roundToDevicePixel() from float to double.
+
+        API test is added.
+
+        * platform/LayoutUnit.h:
+        (WebCore::roundToDevicePixel):
+
 2015-06-23  Matt Rajca  <mrajca@apple.com>
 

trunk/Source/WebCore/platform/LayoutUnit.h

@@ -875 +875 @@
 inline float roundToDevicePixel(LayoutUnit value, const float pixelSnappingFactor, bool needsDirectionalRounding = false)
 {
-    auto roundInternal = [&] (float valueToRound) { return roundf((valueToRound * pixelSnappingFactor) / kFixedPointDenominator) / pixelSnappingFactor; };
-
-    float adjustedValue = value.rawValue() - (needsDirectionalRounding ? LayoutUnit::epsilon() / 2.0f : 0);
-    if (adjustedValue >= 0)
-        return roundInternal(adjustedValue);
+    double valueToRound = value.toDouble();
+    if (needsDirectionalRounding)
+        valueToRound -= LayoutUnit::epsilon() / (2 * kFixedPointDenominator);
+
+    if (valueToRound >= 0)
+        return round(valueToRound * pixelSnappingFactor) / pixelSnappingFactor;
 
     // This adjusts directional rounding on negative halfway values. It produces the same direction for both negative and positive values.
+    // Instead of rounding negative halfway cases away from zero, we translate them to positive values before rounding.
     // It helps snapping relative negative coordinates to the same position as if they were positive absolute coordinates.
-    float translateOrigin = fabsf(adjustedValue - LayoutUnit::fromPixel(1));
-    return roundInternal(adjustedValue + (translateOrigin * kFixedPointDenominator)) - translateOrigin;
+    unsigned translateOrigin = -value.rawValue();
+    return (round((valueToRound + translateOrigin) * pixelSnappingFactor) / pixelSnappingFactor) - translateOrigin;
 }
 

trunk/Tools/ChangeLog

@@ +1 @@
+2015-06-24  Zalan Bujtas  <zalan@apple.com>
+
+        Subpixel rendering: roundToDevicePixel() snaps to wrong value.
+        https://bugs.webkit.org/show_bug.cgi?id=146273
+        rdar://problem/18509840
+
+        Reviewed by Simon Fraser.
+
+        Due to the floating point approximate representation, we can't always produce
+        the correct snap value. This patch addresses the issue by removing redundant kFixedPointDenominator multiplication
+        and by changing the rounding in roundToDevicePixel() from float to double.
+
+        * TestWebKitAPI/Tests/WebCore/LayoutUnit.cpp:
+        (TestWebKitAPI::TEST):
+
 2015-06-24  Brady Eidson  <beidson@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebCore/LayoutUnit.cpp

@@ -232 +232 @@
 }
 
+TEST(WebCoreLayoutUnit, LayoutUnitPixelSnapping)
+{
+    for (int i = -100000; i <= 100000; ++i) {
+        ASSERT_EQ(roundToDevicePixel(LayoutUnit(i), 1), i);
+        ASSERT_EQ(roundToDevicePixel(LayoutUnit(i), 2), i);
+        ASSERT_EQ(roundToDevicePixel(LayoutUnit(i), 3), i);
+    }
+
+    for (float i = -10000; i < 0; i = i + 0.5)
+        ASSERT_FLOAT_EQ(roundToDevicePixel(LayoutUnit(i), 2), i);
+
+    for (float i = -10000.25; i < 0; i = i + 0.5)
+        ASSERT_FLOAT_EQ(roundToDevicePixel(LayoutUnit(i), 2), i + 0.25);
+}
 
 } // namespace TestWebKitAPI