commit 65e841715ef4cfa1a181deb96260d3b60bf9f066
author Mike Klein <mtklein@google.com> Wed Feb 13 12:25:55 2019 -0500
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Wed Feb 13 18:48:09 2019 +0000
tree 233708ece575cedabf757cef8daddc12de48b6c0
parent e317664bbdce9881fca2bc8bbc49ae1df56a25fd

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/SkMasks.cpp

diff --git a/src/codec/SkMasks.cpp b/src/codec/SkMasks.cpp
index 79892ed..2063ffa 100644
--- a/src/codec/SkMasks.cpp
+++ b/src/codec/SkMasks.cpp
@@ -86,7 +86,7 @@
  * 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 @@
         }
     }
 
-    // Save the calculated values
-    const SkMasks::MaskInfo info = { mask, shift, size };
-    return info;
+    return { mask, shift, size };
 }
 
 /*
@@ -126,29 +124,32 @@
  * 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));
 }