Remove opaque alpha channel special case

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

Index: third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp
diff --git a/third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp b/third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp
index 734a7b2527bed95be6dc11f960bf2a1024f337ac..e12e9c0867a490651b18b222dd300ce9471cfb4d 100644
--- a/third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp
+++ b/third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp
@@ -813,9 +813,6 @@ BMPImageReader::ProcessingResult BMPImageReader::processNonRLEData(
         // 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 setting "hasAlpha" to true for
-        // images where all alpha values are 255; opaque images are
-        // faster to draw.
         int alpha = getAlpha(pixel);
         if (!m_seenNonZeroAlphaPixel && !alpha) {
           m_seenZeroAlphaPixel = true;
@@ -825,7 +822,7 @@ BMPImageReader::ProcessingResult BMPImageReader::processNonRLEData(
           if (m_seenZeroAlphaPixel) {
             m_buffer->zeroFillPixelData();
             m_seenZeroAlphaPixel = false;
-          } else if (alpha != 255)
+          } else

[Peter Kasting, 2017/03/16 19:43:27]

I'm still confused.  If you really want to do this, why not mark all BMPs as
having alpha all the time, i.e. rip out this conditional and rip out the
setHasAlpha(false) call earlier in the file?

More generally, why expose setHasAlpha() at all?  If there's no benefit, then no
callers should ever call it, and the API shouldn't exist.  This is the same
comment I had for the Skia code.

I don't really want to go halfway on this one.  If we believe this isn't a win,
let's rip the entire API out as far down as we can.  Otherwise let's leave
everything.

[cblume, 2017/03/16 20:39:45]

I think those are different.
An image may or may not have alpha. That is on the concept level.
If the code paths for painting are the same speed, that's on the implementation
level.

Just because the implementation has no speed difference doesn't mean we should
get rid of the concept.


The image decoders had this as an optimization. And that optimization may no
longer be valid. It doesn't mean the concept of alpha is bad. It means the times
have changed, hardware has changed, code has changed, and this optimization has
finished serving its purpose.

             m_buffer->setHasAlpha(true);
         }