Remove opaque alpha channel special case

third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp

 /*
  * Copyright (c) 2008, 2009, Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 128 matching lines @@
   // Decode the data.
   if (!decoding_and_mask_ && !PastEndOfImage(0) &&
       !DecodePixelData((info_header_.bi_compression != RLE4) &&
                        (info_header_.bi_compression != RLE8) &&
                        (info_header_.bi_compression != RLE24)))
     return false;
 
   // If the image has an AND mask and there was no alpha data, process the
   // mask.
   if (is_in_ico_ && !decoding_and_mask_ &&
-      ((info_header_.bi_bit_count < 16) || !bit_masks_[3] ||
+      ((info_header_.bi_bit_count < 16) || IsAlphaSupported() ||

[scroggo_chromium, 2017/05/30 19:45:17]

Now that you flipped IsAlphaSupported() back, don't you want !IsAlphaSupported()
here?

[cblume, 2017/05/30 23:45:14]

Done.

        !seen_non_zero_alpha_pixel_)) {
     // Reset decoding coordinates to start of image.
     coord_.SetX(0);
     coord_.SetY(is_top_down_ ? 0 : (parent_->Size().Height() - 1));
 
     // The AND mask is stored as 1-bit data.
     info_header_.bi_bit_count = 1;
 
     decoding_and_mask_ = true;
   }
@@ skipping 650 matching lines @@
       // RGB data.  Decode pixels one at a time, left to right.
       while (coord_.X() < end_x) {
         const uint32_t pixel = ReadCurrentPixel(bytes_per_pixel);
 
         // Some BMPs specify an alpha channel but don't actually use it
         // (it contains all 0s).  To avoid displaying these images as
         // fully-transparent, decode as if images are fully opaque
         // until we actually see a non-zero alpha value; at that point,
         // reset any previously-decoded pixels to fully transparent and
         // continue decoding based on the real alpha channel values.
-        // As an optimization, avoid calling SetHasAlpha(true) for
-        // images where all alpha values are 255; opaque images are
-        // faster to draw.
-        int alpha = GetAlpha(pixel);
-        if (!seen_non_zero_alpha_pixel_ && !alpha) {
-          seen_zero_alpha_pixel_ = true;
-          alpha = 255;
-        } else {
-          seen_non_zero_alpha_pixel_ = true;
-          if (seen_zero_alpha_pixel_) {
-            buffer_->ZeroFillPixelData();
-            seen_zero_alpha_pixel_ = false;
-          } else if (alpha != 255)
-            buffer_->SetHasAlpha(true);
+        int alpha = 255;
+        seen_zero_alpha_pixel_ = true;

[scroggo_chromium, 2017/05/30 19:45:17]

Now this always gets set to true before we read it. I don't see how this could
be the intended behavior.

(I already brought this up in
https://codereview.chromium.org/2756463003/diff/160001/third_party/WebKit/Sou...)

[cblume, 2017/05/30 23:37:34]

Done.

+        if (IsAlphaSupported()) {
+          alpha = GetAlpha(pixel);
+          if (!seen_non_zero_alpha_pixel_ && !alpha) {
+            alpha = 255;
+          } else {
+            if (seen_zero_alpha_pixel_) {

[scroggo_chromium, 2017/05/30 19:45:17]

This will always be true, since we always set it to true on line 821.

[cblume, 2017/05/30 23:37:34]

Done.

+              buffer_->ZeroFillPixelData();
+              seen_zero_alpha_pixel_ = false;

[scroggo_chromium, 2017/05/30 19:45:17]

No one ever reads this value, since we set it to true again on line 821 before
we ever read it.

[cblume, 2017/05/30 23:37:34]

Done.

+            } else {

[scroggo_chromium, 2017/05/30 19:45:17]

We will never hit this branch.

[cblume, 2017/05/30 23:37:34]

Done.

+              buffer_->SetHasAlpha(true);
+            }
+          }
         }
 
         SetRGBA(GetComponent(pixel, 0), GetComponent(pixel, 1),
                 GetComponent(pixel, 2), alpha);
       }
     }
 
     // Success, keep going.
     decoded_offset_ += padded_num_bytes;
     if (in_rle)
       return kSuccess;
     MoveBufferToNextRow();
   }
 
   // Finished decoding whole image.
   return kSuccess;
 }
 
 void BMPImageReader::MoveBufferToNextRow() {
   coord_.Move(-coord_.X(), is_top_down_ ? 1 : -1);
 }
 
 }  // namespace blink