Fill incomplete images in SkCodec parent class

src/codec/SkCodec_libico.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 "SkCodec_libico.h"
 #include "SkCodec_libpng.h"
@@ skipping 95 matching lines @@
     SkTQSort(directoryEntries.get(), directoryEntries.get() + numImages - 1,
             lessThan);
 
     // Now will construct a candidate codec for each of the embedded images
     uint32_t bytesRead = kIcoDirectoryBytes + numImages * kIcoDirEntryBytes;
     SkAutoTDelete<SkTArray<SkAutoTDelete<SkCodec>, true>> codecs(
             new (SkTArray<SkAutoTDelete<SkCodec>, true>)(numImages));
     for (uint32_t i = 0; i < numImages; i++) {
         uint32_t offset = directoryEntries.get()[i].offset;
         uint32_t size = directoryEntries.get()[i].size;
-        
+
         // Ensure that the offset is valid
         if (offset < bytesRead) {
             SkCodecPrintf("Warning: invalid ico offset.\n");
             continue;
         }
 
         // If we cannot skip, assume we have reached the end of the stream and
         // stop trying to make codecs
         if (inputStream.get()->skip(offset - bytesRead) != offset - bytesRead) {
             SkCodecPrintf("Warning: could not skip to ico offset.\n");
             break;
         }
         bytesRead = offset;
 
         // Create a new stream for the embedded codec
         SkAutoTUnref<SkData> data(
                 SkData::NewFromStream(inputStream.get(), size));
         if (nullptr == data.get()) {
             SkCodecPrintf("Warning: could not create embedded stream.\n");
+            // FIXME: This is a good solution if we have already created at
+            // least one valid codec.  Otherwise, we may want to try to
+            // partially decode this incomplete embedded image.

[scroggo, 2015/10/01 14:48:31]

Is there anything to partially decode if no embedded codecs were created?

[msarett, 2015/10/01 18:14:14]

Good point, this comment makes no sense.

             break;
         }
         SkAutoTDelete<SkMemoryStream> embeddedStream(new SkMemoryStream(data.get()));
         bytesRead += size;
 
         // Check if the embedded codec is bmp or png and create the codec
         const bool isPng = SkPngCodec::IsPng(embeddedStream);
         SkAssertResult(embeddedStream->rewind());
         SkCodec* codec = nullptr;
         if (isPng) {
@@ skipping 82 matching lines @@
     SkASSERT(minIndex >= 0);
 
     return fEmbeddedCodecs->operator[](minIndex)->getInfo().dimensions();
 }
 
 /*
  * Initiates the Ico decode
  */
 SkCodec::Result SkIcoCodec::onGetPixels(const SkImageInfo& dstInfo,
                                         void* dst, size_t dstRowBytes,
-                                        const Options& opts, SkPMColor* ct,
-                                        int* ptr) {
+                                        const Options& opts, SkPMColor* colorTable,
+                                        int* colorCount, int* rowsDecoded) {
     if (opts.fSubset) {
         // Subsets are not supported.
         return kUnimplemented;
     }
 
     if (!valid_alpha(dstInfo.alphaType(), this->getInfo().alphaType())) {
         return kInvalidConversion;
     }
 
     // We return invalid scale if there is no candidate image with matching
@@ skipping 15 matching lines @@
                     embeddedAlpha = dstInfo.alphaType();
                     break;
                 case kOpaque_SkAlphaType:
                     // If the embedded codec claims it is opaque, decode as if it is opaque.
                     break;
                 default:
                     SkASSERT(false);
                     break;
             }
             SkImageInfo info = dstInfo.makeAlphaType(embeddedAlpha);
-            result = embeddedCodec->getPixels(info, dst, dstRowBytes, &opts, ct, ptr);
+            result = embeddedCodec->getPixels(info, dst, dstRowBytes, &opts, colorTable,
+                    colorCount);
+            // The embedded codec will handle filling incomplete images, so we will indicate
+            // that all of the rows are initialized.
+            *rowsDecoded = info.height();

[msarett, 2015/10/01 12:44:52]

It's a bit strange when a call to getPixels() calls getPixels() or
getScanlines() on an owned codec.  We don't want to fill from this codec because
the embedded codec already handles filling.

This comes up in SkScaledCodec as well.

 
             // On a fatal error, keep trying to find an image to decode
             if (kInvalidConversion == result || kInvalidInput == result ||
                     kInvalidScale == result) {
                 SkCodecPrintf("Warning: Attempt to decode candidate ico failed.\n");
                 continue;
             }
 
             // On success or partial success, return the result
             return result;
         }
     }
 
     SkCodecPrintf("Error: No matching candidate image in ico.\n");
     return result;
 }