Add SkEncodedInfo to report properties of encoded image data

src/codec/SkBmpCodec.cpp

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkBmpCodec.h"
 #include "SkBmpMaskCodec.h"
 #include "SkBmpRLECodec.h"
@@ skipping 397 matching lines @@
         //                 Seems like we can just assume that the offset is zero and try to decode?
         //                 Maybe we don't want to try to decode corrupt images?
         SkCodecPrintf("Error: pixel data offset less than header size.\n");
         return false;
     }
 
 
 
     switch (inputFormat) {
         case kStandard_BmpInputFormat: {
-            // BMPs-in-ICOs often contain an alpha mask after the image, which
-            // means we cannot guarantee that an image is opaque, even if the
-            // embedded bmp is opaque.
-            // We use |isOpaque| to indicate if the BMP itself is opaque, but
-            // still need to recommend kUnpremul when it is contained in an ICO.
-            SkColorType colorType = kN32_SkColorType;
-            SkAlphaType alphaType = inIco ? kUnpremul_SkAlphaType : kOpaque_SkAlphaType;
+            // BMPs are generally opaque, however BMPs-in-ICOs may contain
+            // a transparency mask after the image.  Therefore, we mark the
+            // alpha as kBinary if the BMP is contained in an ICO.
+            // We use |isOpaque| to indicate if the BMP itself is opaque.
+            SkEncodedInfo::Alpha alpha = inIco ? SkEncodedInfo::kBinary_Alpha :
+                    SkEncodedInfo::kOpaque_Alpha;
             bool isOpaque = true;
+
+            uint8_t bitsPerComponent = SkTMin((uint16_t) 8, bitsPerPixel);

[scroggo, 2016/03/23 14:48:50]

It seems odd that you set bitsPerComponent to min(bitsPerPixel, 8). Might be
worth a comment? (e.g. <= 8 are palette; >8 are 8 bitsPerComponent)

[msarett, 2016/03/24 16:20:43]

Agreed.  I think maybe it's clearer to handle it in the switch statement.

+            SkEncodedInfo::Color color;
             switch (bitsPerPixel) {
                 // Palette formats
                 case 1:
                 case 2:
                 case 4:
                 case 8:
-                    // We cannot recommend a palette color type for ICOs because they
-                    // may contain a transparency mask.
-                    if (!inIco) {
-                        colorType = kIndex_8_SkColorType;
+                    // In the case of ICO, kBGRA is actually the closest match,
+                    // since we will need to apply a transparency mask.
+                    if (inIco) {
+                        color = SkEncodedInfo::kBGRA_Color;
+                        bitsPerComponent = 8;
+                    } else {
+                        color = SkEncodedInfo::kPalette_Color;
                     }
                     break;
                 case 24:
+                    color = SkEncodedInfo::kBGR_Color;
+                    break;
                 case 32:
                     // 32-bit BMP-in-ICOs actually use the alpha channel in place of a
                     // transparency mask.
                     if (inIco) {
                         isOpaque = false;
+                        alpha = SkEncodedInfo::kUnpremul_Alpha;
+                        color = SkEncodedInfo::kBGRA_Color;
+                    } else {
+                        color = SkEncodedInfo::kBGRX_Color;
                     }
                     break;
                 default:
                     SkCodecPrintf("Error: invalid input value for bits per pixel.\n");
                     return false;
             }
 
             if (codecOut) {
                 // We require streams to have a memory base for Bmp-in-Ico decodes.
                 SkASSERT(!inIco || nullptr != stream->getMemoryBase());
 
                 // Set the image info and create a codec.
-                const SkImageInfo imageInfo = SkImageInfo::Make(width, height, colorType,
-                        alphaType);
-                *codecOut = new SkBmpStandardCodec(imageInfo, stream, bitsPerPixel, numColors,
+                const SkEncodedInfo info = SkEncodedInfo::Make(width, height, color, alpha,
+                        bitsPerComponent);
+                *codecOut = new SkBmpStandardCodec(info, stream, bitsPerPixel, numColors,
                         bytesPerColor, offset - bytesRead, rowOrder, isOpaque, inIco);
 
             }
             return true;
         }
 
         case kBitMask_BmpInputFormat: {
             // Bmp-in-Ico must be standard mode
             if (inIco) {
                 SkCodecPrintf("Error: Icos may not use bit mask format.\n");
@@ skipping 19 matching lines @@
             }
 
             if (codecOut) {
                 // Check that input bit masks are valid and create the masks object
                 SkAutoTDelete<SkMasks> masks(SkMasks::CreateMasks(inputMasks, bitsPerPixel));
                 if (nullptr == masks) {
                     SkCodecPrintf("Error: invalid input masks.\n");
                     return false;
                 }
 
-                // Set the image info
-                SkAlphaType alphaType = masks->getAlphaMask() ? kUnpremul_SkAlphaType :
-                        kOpaque_SkAlphaType;
-                const SkImageInfo imageInfo = SkImageInfo::Make(width, height, kN32_SkColorType,
-                        alphaType);
-                *codecOut = new SkBmpMaskCodec(imageInfo, stream, bitsPerPixel, masks.release(),
+                // Masked bmps are not a great fit for SkEncodedInfo, since they have

[scroggo, 2016/03/23 14:48:50]

Should SkEncodedInfo adapt to support masked bmps? I thought that bmp had some
cases that weren't covered, but I couldn't remember them. I guess that whatever
info would be specific to mask bmp would not make a client decide to treat it
differently than if we just choose types that are already there?

[msarett, 2016/03/24 16:20:44]

I thought about 3 options:
(1) Create all of the modes we might see in masked bmps...

kGRB, kGBR, kRBG, kBRG (also other variations including A) with values of
bitsPerComponent for each component (since they don't have to be the same).

I think this is pretty clearly not worth it...it's a lot of extra complexity in
our struct without knowing if there are any clients who would use this info or
care.

Additionally, we would have to interpret the masks to figure all of this out.

(2) Add a kMask_Color to SkEncodedInfo

I almost did this...  I ended up not doing it because it's not a great fit with
the others.  Ex: we can't tell the number of components from the name, we still
have a problem with how to set bits per component.

Still, this might be worth it.  The client could recognize that this is a
"weird" bmp.  It could be useful to report the max/min bits per component.

(3) Include mask bmps with kBGR/kBGRA and handle them differently without
telling the client.

This seems easiest right now.  Especially because we don't really know what the
client wants or if they want anything.  I don't mind having bmp being a weird
special case.  We can always change this decision later.

[scroggo, 2016/03/24 20:51:03]

I'm also fine with Mask BMP being a weird special case.

+                // arbitrary component orderings and bits per component.  Here we choose
+                // somewhat reasonable values - it's ok that we don't match exactly
+                // because SkBmpMaskCodec has its own mask swizzler anyway.
+                SkEncodedInfo::Color color;
+                SkEncodedInfo::Alpha alpha;
+                if (masks->getAlphaMask()) {
+                    color = SkEncodedInfo::kBGRA_Color;
+                    alpha = SkEncodedInfo::kUnpremul_Alpha;
+                } else {
+                    color = SkEncodedInfo::kBGR_Color;
+                    alpha = SkEncodedInfo::kOpaque_Alpha;
+                }
+                const SkEncodedInfo info = SkEncodedInfo::Make(width, height, color, alpha, 8);
+                *codecOut = new SkBmpMaskCodec(info, stream, bitsPerPixel, masks.release(),
                         rowOrder);
             }
             return true;
         }
 
         case kRLE_BmpInputFormat: {
             // We should not reach this point without a valid value of bitsPerPixel.
             SkASSERT(4 == bitsPerPixel || 8 == bitsPerPixel || 24 == bitsPerPixel);
 
             // Check for a valid number of total bytes when in RLE mode
             if (totalBytes <= offset) {
                 SkCodecPrintf("Error: RLE requires valid input size.\n");
                 return false;
             }
             const size_t RLEBytes = totalBytes - offset;
 
             // Bmp-in-Ico must be standard mode
             // When inIco is true, this line cannot be reached, since we
             // require that RLE Bmps have a valid number of totalBytes, and
             // Icos skip the header that contains totalBytes.
             SkASSERT(!inIco);
 
             if (codecOut) {
                 // RLE inputs may skip pixels, leaving them as transparent.  This
                 // is uncommon, but we cannot be certain that an RLE bmp will be
-                // opaque.
-                const SkImageInfo imageInfo = SkImageInfo::Make(width, height, kN32_SkColorType,
-                        kUnpremul_SkAlphaType);
-                *codecOut = new SkBmpRLECodec(imageInfo, stream, bitsPerPixel, numColors,
+                // opaque or that we will be able to represent it with a palette.
+                // For that reason, we always indicate that we are kBGRA.
+                const SkEncodedInfo info = SkEncodedInfo::Make(width, height,
+                        SkEncodedInfo::kBGRA_Color, SkEncodedInfo::kBinary_Alpha, 8);
+                *codecOut = new SkBmpRLECodec(info, stream, bitsPerPixel, numColors,
                         bytesPerColor, offset - bytesRead, rowOrder, RLEBytes);
             }
             return true;
         }
         default:
             SkASSERT(false);
             return false;
     }
 }
 
 /*
  * Creates a bmp decoder
  * Reads enough of the stream to determine the image format
  */
 SkCodec* SkBmpCodec::NewFromStream(SkStream* stream, bool inIco) {
     SkAutoTDelete<SkStream> streamDeleter(stream);
     SkCodec* codec = nullptr;
     if (ReadHeader(stream, inIco, &codec)) {
         // codec has taken ownership of stream, so we do not need to
         // delete it.
         SkASSERT(codec);
         streamDeleter.release();
         return codec;
     }
     return nullptr;
 }
 
-SkBmpCodec::SkBmpCodec(const SkImageInfo& info, SkStream* stream,
+SkBmpCodec::SkBmpCodec(const SkEncodedInfo& info, SkStream* stream,
         uint16_t bitsPerPixel, SkCodec::SkScanlineOrder rowOrder)
     : INHERITED(info, stream)
     , fBitsPerPixel(bitsPerPixel)
     , fRowOrder(rowOrder)
     , fSrcRowBytes(SkAlign4(compute_row_bytes(info.width(), fBitsPerPixel)))
 {}
 
 bool SkBmpCodec::onRewind() {
     return SkBmpCodec::ReadHeader(this->stream(), this->inIco(), nullptr);
 }
@@ skipping 25 matching lines @@
 }
 
 bool SkBmpCodec::skipRows(int count) {
     const size_t bytesToSkip = count * fSrcRowBytes;
     return this->stream()->skip(bytesToSkip) == bytesToSkip;
 }
 
 bool SkBmpCodec::onSkipScanlines(int count) {
     return this->skipRows(count);
 }