From 65e841715ef4cfa1a181deb96260d3b60bf9f066 Mon Sep 17 00:00:00 2001
From: Mike Klein <mtklein@google.com>
Date: Wed, 13 Feb 2019 12:25:55 -0500
Subject: [PATCH] rewrite SkMasks::CreateMasks to work in bytesPerPixel

This ought to make it a bit more clear that undefined
shifts (like 1<<31, 1<<32, 1<<33) are not possible,
only 1<<8, 1<<16, 1<<24.

Change-Id: Ia358f9204e5956ba6c23603c5961af86a991b659
Reviewed-on: https://skia-review.googlesource.com/c/192030
Reviewed-by: Leon Scroggins <scroggo@google.com>
Reviewed-by: Greg Kaiser <gkaiser@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
---
 src/codec/SkBmpCodec.cpp       |  3 ++-
 src/codec/SkMasks.cpp          | 41 +++++++++++++++++-----------------
 src/codec/SkMasks.h            |  2 +-
 src/gpu/text/GrStrikeCache.cpp |  2 +-
 4 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/src/codec/SkBmpCodec.cpp b/src/codec/SkBmpCodec.cpp
index e13dccbd11..b09d39ea00 100644
--- a/src/codec/SkBmpCodec.cpp
+++ b/src/codec/SkBmpCodec.cpp
@@ -520,7 +520,8 @@ SkCodec::Result SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
 
             if (codecOut) {
                 // Check that input bit masks are valid and create the masks object
-                std::unique_ptr<SkMasks> masks(SkMasks::CreateMasks(inputMasks, bitsPerPixel));
+                SkASSERT(bitsPerPixel % 8 == 0);
+                std::unique_ptr<SkMasks> masks(SkMasks::CreateMasks(inputMasks, bitsPerPixel/8));
                 if (nullptr == masks) {
                     SkCodecPrintf("Error: invalid input masks.\n");
                     return kInvalidInput;
diff --git a/src/codec/SkMasks.cpp b/src/codec/SkMasks.cpp
index 79892ed922..2063ffae57 100644
--- a/src/codec/SkMasks.cpp
+++ b/src/codec/SkMasks.cpp
@@ -86,7 +86,7 @@ uint8_t SkMasks::getAlpha(uint32_t pixel) const {
  * Process an input mask to obtain the necessary information
  *
  */
-const SkMasks::MaskInfo process_mask(uint32_t mask, uint32_t bpp) {
+static const SkMasks::MaskInfo process_mask(uint32_t mask) {
     // Determine properties of the mask
     uint32_t tempMask = mask;
     uint32_t shift = 0;
@@ -116,9 +116,7 @@ const SkMasks::MaskInfo process_mask(uint32_t mask, uint32_t bpp) {
         }
     }
 
-    // Save the calculated values
-    const SkMasks::MaskInfo info = { mask, shift, size };
-    return info;
+    return { mask, shift, size };
 }
 
 /*
@@ -126,29 +124,32 @@ const SkMasks::MaskInfo process_mask(uint32_t mask, uint32_t bpp) {
  * Create the masks object
  *
  */
-SkMasks* SkMasks::CreateMasks(InputMasks masks, uint32_t bitsPerPixel) {
-    // Trim the input masks according to bitsPerPixel
-    if (bitsPerPixel < 32) {
-        masks.red &= (1 << bitsPerPixel) - 1;
+SkMasks* SkMasks::CreateMasks(InputMasks masks, int bytesPerPixel) {
+    SkASSERT(0 < bytesPerPixel && bytesPerPixel <= 4);
+
+    // Trim the input masks to match bytesPerPixel.
+    if (bytesPerPixel < 4) {
+        int bitsPerPixel = 8*bytesPerPixel;
+        masks.red   &= (1 << bitsPerPixel) - 1;
         masks.green &= (1 << bitsPerPixel) - 1;
-        masks.blue &= (1 << bitsPerPixel) - 1;
+        masks.blue  &= (1 << bitsPerPixel) - 1;
         masks.alpha &= (1 << bitsPerPixel) - 1;
     }
 
-    // Check that masks do not overlap
-    if (((masks.red & masks.green) | (masks.red & masks.blue) |
-            (masks.red & masks.alpha) | (masks.green & masks.blue) |
-            (masks.green & masks.alpha) | (masks.blue & masks.alpha)) != 0) {
+    // Check that masks do not overlap.
+    if (((masks.red   & masks.green) |
+         (masks.red   & masks.blue ) |
+         (masks.red   & masks.alpha) |
+         (masks.green & masks.blue ) |
+         (masks.green & masks.alpha) |
+         (masks.blue  & masks.alpha) ) != 0) {
         return nullptr;
     }
 
-    // Collect information about the masks
-    const MaskInfo red = process_mask(masks.red, bitsPerPixel);
-    const MaskInfo green = process_mask(masks.green, bitsPerPixel);
-    const MaskInfo blue = process_mask(masks.blue, bitsPerPixel);
-    const MaskInfo alpha = process_mask(masks.alpha, bitsPerPixel);
-
-    return new SkMasks(red, green, blue, alpha);
+    return new SkMasks(process_mask(masks.red  ),
+                       process_mask(masks.green),
+                       process_mask(masks.blue ),
+                       process_mask(masks.alpha));
 }
 
 
diff --git a/src/codec/SkMasks.h b/src/codec/SkMasks.h
index 9606cbfbeb..55abb11be6 100644
--- a/src/codec/SkMasks.h
+++ b/src/codec/SkMasks.h
@@ -45,7 +45,7 @@ public:
      * Create the masks object
      *
      */
-    static SkMasks* CreateMasks(InputMasks masks, uint32_t bpp);
+    static SkMasks* CreateMasks(InputMasks masks, int bytesPerPixel);
 
     /*
      *
diff --git a/src/gpu/text/GrStrikeCache.cpp b/src/gpu/text/GrStrikeCache.cpp
index 8edc977cbb..293ba29cd7 100644
--- a/src/gpu/text/GrStrikeCache.cpp
+++ b/src/gpu/text/GrStrikeCache.cpp
@@ -17,7 +17,7 @@
 GrStrikeCache::GrStrikeCache(const GrCaps* caps, size_t maxTextureBytes)
         : fPreserveStrike(nullptr)
         , f565Masks(SkMasks::CreateMasks({0xF800, 0x07E0, 0x001F, 0},
-                    GrMaskFormatBytesPerPixel(kA565_GrMaskFormat) * 8)) { }
+                    GrMaskFormatBytesPerPixel(kA565_GrMaskFormat))) { }
 
 GrStrikeCache::~GrStrikeCache() {
     StrikeHash::Iter iter(&fCache);