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 795 matching lines @@
       // RGB data.  Decode pixels one at a time, left to right.
       while (m_coord.x() < endX) {
         const uint32_t pixel = readCurrentPixel(bytesPerPixel);
 
         // 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 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;
           alpha = 255;
         } else {
           m_seenNonZeroAlphaPixel = true;
           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);
         }
 
         setRGBA(getComponent(pixel, 0), getComponent(pixel, 1),
                 getComponent(pixel, 2), alpha);
       }
     }
 
     // Success, keep going.
     m_decodedOffset += paddedNumBytes;
     if (inRLE)
       return Success;
     moveBufferToNextRow();
   }
 
   // Finished decoding whole image.
   return Success;
 }
 
 void BMPImageReader::moveBufferToNextRow() {
   m_coord.move(-m_coord.x(), m_isTopDown ? 1 : -1);
 }
 
 }  // namespace blink