Address MSVC warnings about possible value truncation.

include/core/SkColorPriv.h

Index: include/core/SkColorPriv.h
diff --git a/include/core/SkColorPriv.h b/include/core/SkColorPriv.h
index c8d71a5a8c21c22c5cdb81caca94b2be44208a0b..3647335edabc4afb05abb26905856012200692a3 100644
--- a/include/core/SkColorPriv.h
+++ b/include/core/SkColorPriv.h
@@ -306,7 +306,8 @@ static inline void SkBlendRGB16(const uint16_t src[], uint16_t dst[],
     do {
         uint32_t src32 = SkExpand_rgb_16(*src++);
         uint32_t dst32 = SkExpand_rgb_16(*dst);
-        *dst++ = SkCompact_rgb_16(dst32 + ((src32 - dst32) * srcScale >> 5));
+        *dst++ = SkToU16(
+            SkCompact_rgb_16(dst32 + ((src32 - dst32) * srcScale >> 5)));
     } while (--count > 0);
 }
 
@@ -785,7 +786,7 @@ static inline SkPMColor16 SkPackARGB4444(unsigned a, unsigned r,
                          (g << SK_G4444_SHIFT) | (b << SK_B4444_SHIFT));
 }
 
-static inline U16CPU SkAlphaMulQ4(U16CPU c, unsigned scale) {
+static inline SkPMColor16 SkAlphaMulQ4(SkPMColor16 c, int scale) {
     SkASSERT(scale <= 16);
 
     const unsigned mask = 0xF0F;    //gMask_0F0F;
@@ -795,14 +796,14 @@ static inline U16CPU SkAlphaMulQ4(U16CPU c, unsigned scale) {
     unsigned ag = ((c >> 4) & mask) * scale;
     return (rb & mask) | (ag & ~mask);
 #else
-    c = (c & mask) | ((c & (mask << 4)) << 12);
-    c = c * scale >> 4;
-    return (c & mask) | ((c >> 12) & (mask << 4));
+    unsigned expanded_c = (c & mask) | ((c & (mask << 4)) << 12);
+    unsigned scaled_c = (expanded_c * scale) >> 4;
+    return (scaled_c & mask) | ((scaled_c >> 12) & (mask << 4));

[reed1, 2014/08/29 12:48:42]

since you changed the return type to be explicitly 16bits, will this return
expression generate a warning? Will it generate extraneous shifts/masks to
"ensure" that we're only returning 16bits?

[Peter Kasting, 2014/08/29 20:50:35]

No.  MSVC is smart enough to realize that the result of this expression will
always fit in 16 bits.
I don't know, but if it does, then the existing code must be generating those in
order to perform the U16CPU -> SkPMColor16 truncation on the caller side
(remember that this function is only used in one caller below, which is storing
the result in an SkPMColor16), so at worst, we're moving those shifts/masks from
the caller side to here.

But I suspect that, since the compile can figure out that the value here won't
be truncated, it's not generating any masks either.  So at best, we're actually
removing any existing caller-side shift/mask code (since the caller has less
visibility into the construction of the returned value), thus improving perf.

[reed1, 2014/09/02 14:10:07]

Ack - you're right. The caller is SkSrcOver4444To16() which takes SkPMColor16.
That is a shame, as it could also have taken U16CPU to be safe.

I'm fine with this either way for now. 

 #endif
 }
 
 /** Expand the SkPMColor16 color into a 32bit value that can be scaled all at
-    once by a value up to 16. Used in conjunction with SkCompact_4444.
+    once by a value up to 16.
 */
 static inline uint32_t SkExpand_4444(U16CPU c) {
     SkASSERT(c == (uint16_t)c);
@@ -811,18 +812,6 @@ static inline uint32_t SkExpand_4444(U16CPU c) {
     return (c & mask) | ((c & ~mask) << 12);
 }
 
-/** Compress an expanded value (from SkExpand_4444) back down to a SkPMColor16.
-    NOTE: this explicitly does not clean the top 16 bits (which may be garbage).
-    It does this for speed, since if it is being written directly to 16bits of
-    memory, the top 16bits will be ignored. Casting the result to uint16_t here
-    would add 2 more instructions, slow us down. It is up to the caller to
-    perform the cast if needed.
-*/
-static inline U16CPU SkCompact_4444(uint32_t c) {
-    const unsigned mask = 0xF0F;    //gMask_0F0F;
-    return (c & mask) | ((c >> 12) & ~mask);
-}
-
 static inline uint16_t SkSrcOver4444To16(SkPMColor16 s, uint16_t d) {
     unsigned sa = SkGetPackedA4444(s);
     unsigned sr = SkR4444ToR565(SkGetPackedR4444(s));
@@ -854,22 +843,6 @@ static inline uint16_t SkBlend4444To16(SkPMColor16 src, uint16_t dst, int scale1
     return SkSrcOver4444To16(SkAlphaMulQ4(src, scale16), dst);
 }
 
-static inline uint16_t SkBlend4444(SkPMColor16 src, SkPMColor16 dst, int scale16) {
-    SkASSERT((unsigned)scale16 <= 16);
-
-    uint32_t src32 = SkExpand_4444(src) * scale16;
-    // the scaled srcAlpha is the bottom byte
-#ifdef SK_DEBUG
-    {
-        unsigned srcA = SkGetPackedA4444(src) * scale16;
-        SkASSERT(srcA == (src32 & 0xFF));
-    }
-#endif
-    unsigned dstScale = SkAlpha255To256(255 - (src32 & 0xFF)) >> 4;
-    uint32_t dst32 = SkExpand_4444(dst) * dstScale;
-    return SkCompact_4444((src32 + dst32) >> 4);
-}
-
 static inline SkPMColor SkPixel4444ToPixel32(U16CPU c) {
     uint32_t d = (SkGetPackedA4444(c) << SK_A32_SHIFT) |
                  (SkGetPackedR4444(c) << SK_R32_SHIFT) |