Test and fix SkPMFloat rounding.

SSE rounds for free (that was a happy accident: they also have a truncating version).
NEON does not, nor obviously the portable code, so they add 0.5 before truncating.

NOPRESUBMIT=true

BUG=skia:

Review URL: https://codereview.chromium.org/974643002

5 files changed, 18 insertions, 9 deletions

diff --git a/src/core/SkPMFloat.h b/src/core/SkPMFloat.h
index b0cb3146c7..783fc0d40e 100644
--- a/src/core/SkPMFloat.h
+++ b/src/core/SkPMFloat.h
@@ -29,6 +29,7 @@ public:
 
     void set(SkPMColor);
 
+    // get() and clamped() round component values to the nearest integer.
     SkPMColor     get() const;  // May SkASSERT(this->isValid()).  Some implementations may clamp.
     SkPMColor clamped() const;  // Will clamp all values to [0, 255].  Then may assert isValid().
 
diff --git a/src/opts/SkPMFloat_SSE2.h b/src/opts/SkPMFloat_SSE2.h
index f28608079f..908b8baf87 100644
--- a/src/opts/SkPMFloat_SSE2.h
+++ b/src/opts/SkPMFloat_SSE2.h
@@ -23,7 +23,7 @@ inline SkPMColor SkPMFloat::get() const {
 }
 
 inline SkPMColor SkPMFloat::clamped() const {
-    __m128i fix8_32 = _mm_cvtps_epi32(_mm_load_ps(fColor)),
+    __m128i fix8_32 = _mm_cvtps_epi32(_mm_load_ps(fColor)),  // _mm_cvtps_epi32 rounds for us!
             fix8_16 = _mm_packus_epi16(fix8_32, fix8_32),
             fix8    = _mm_packus_epi16(fix8_16, fix8_16);
     SkPMColor c = _mm_cvtsi128_si32(fix8);
diff --git a/src/opts/SkPMFloat_neon.h b/src/opts/SkPMFloat_neon.h
index 15ba3a58e4..179c674550 100644
--- a/src/opts/SkPMFloat_neon.h
+++ b/src/opts/SkPMFloat_neon.h
@@ -19,18 +19,20 @@ inline void SkPMFloat::set(SkPMColor c) {
 
 inline SkPMColor SkPMFloat::get() const {
     SkASSERT(this->isValid());
-    uint32x4_t  fix8_32 = vcvtq_u32_f32(vld1q_f32(fColor));
-    uint16x4_t  fix8_16 = vmovn_u32(fix8_32);
-    uint8x8_t   fix8    = vmovn_u16(vcombine_u16(fix8_16, vdup_n_u16(0)));
+    float32x4_t add_half = vaddq_f32(vld1q_f32(fColor), vdupq_n_f32(0.5f));
+    uint32x4_t  fix8_32  = vcvtq_u32_f32(add_half);  // vcvtq_u32_f32 truncates, so round manually
+    uint16x4_t  fix8_16  = vmovn_u32(fix8_32);
+    uint8x8_t   fix8     = vmovn_u16(vcombine_u16(fix8_16, vdup_n_u16(0)));
     SkPMColor c = vget_lane_u32((uint32x2_t)fix8, 0);
     SkPMColorAssert(c);
     return c;
 }
 
 inline SkPMColor SkPMFloat::clamped() const {
-    uint32x4_t  fix8_32 = vcvtq_u32_f32(vld1q_f32(fColor));
-    uint16x4_t  fix8_16 = vqmovn_u32(fix8_32);
-    uint8x8_t   fix8    = vqmovn_u16(vcombine_u16(fix8_16, vdup_n_u16(0)));
+    float32x4_t add_half = vaddq_f32(vld1q_f32(fColor), vdupq_n_f32(0.5f));
+    uint32x4_t  fix8_32  = vcvtq_u32_f32(add_half);  // vcvtq_u32_f32 truncates, so round manually
+    uint16x4_t  fix8_16  = vqmovn_u32(fix8_32);
+    uint8x8_t   fix8     = vqmovn_u16(vcombine_u16(fix8_16, vdup_n_u16(0)));
     SkPMColor c = vget_lane_u32((uint32x2_t)fix8, 0);
     SkPMColorAssert(c);
     return c;
diff --git a/src/opts/SkPMFloat_none.h b/src/opts/SkPMFloat_none.h
index 448b509d40..b67c8d7420 100644
--- a/src/opts/SkPMFloat_none.h
+++ b/src/opts/SkPMFloat_none.h
@@ -10,7 +10,7 @@ inline void SkPMFloat::set(SkPMColor c) {
 
 inline SkPMColor SkPMFloat::get() const {
     SkASSERT(this->isValid());
-    return SkPackARGB32(this->a(), this->r(), this->g(), this->b());
+    return SkPackARGB32(this->a()+0.5f, this->r()+0.5f, this->g()+0.5f, this->b()+0.5f);
 }
 
 inline SkPMColor SkPMFloat::clamped() const {
@@ -22,5 +22,5 @@ inline SkPMColor SkPMFloat::clamped() const {
     r = r < 0 ? 0 : (r > 255 ? 255 : r);
     g = g < 0 ? 0 : (g > 255 ? 255 : g);
     b = b < 0 ? 0 : (b > 255 ? 255 : b);
-    return SkPackARGB32(a, r, g, b);
+    return SkPackARGB32(a+0.5f, r+0.5f, g+0.5f, b+0.5f);
 }
diff --git a/tests/PMFloatTest.cpp b/tests/PMFloatTest.cpp
index fceeb47fa1..fbd8626cc8 100644
--- a/tests/PMFloatTest.cpp
+++ b/tests/PMFloatTest.cpp
@@ -11,7 +11,13 @@ DEF_TEST(SkPMFloat, r) {
     REPORTER_ASSERT(r, SkScalarNearlyEqual(204.0f, pmf.r()));
     REPORTER_ASSERT(r, SkScalarNearlyEqual(153.0f, pmf.g()));
     REPORTER_ASSERT(r, SkScalarNearlyEqual( 51.0f, pmf.b()));
+    REPORTER_ASSERT(r, c == pmf.get());
 
+    // Test rounding.  (Don't bother testing .5... we don't care which way it goes.)
+    pmf.setA(254.6f);
+    pmf.setR(204.3f);
+    pmf.setG(153.1f);
+    pmf.setB( 50.8f);
     REPORTER_ASSERT(r, c == pmf.get());
 
     // Test clamping.