Add dm target to BUILD and refactor BUILD file. Fix blaze compilation errors.

src/codec/SkJpegCodec.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 "SkCodec.h"
 #include "SkJpegCodec.h"
 #include "SkJpegDecoderMgr.h"
@@ skipping 133 matching lines @@
         JpegDecoderMgr* decoderMgr)
     : INHERITED(srcInfo, stream)
     , fDecoderMgr(decoderMgr)
     , fReadyState(decoderMgr->dinfo()->global_state)
 {}
 
 /*
  * Return the row bytes of a particular image type and width
  */
 static int get_row_bytes(const j_decompress_ptr dinfo) {
+#if defined (GOOGLE3)

[scroggo, 2015/10/27 13:51:23]

Is this because GOOGLE3 is expected to build against a particular version of
libjpeg? Or perhaps just not libjpeg-turbo? Or it should build against a variety
of libjpegs?

This define just seems too vague (and yet too specific - maybe other builders
outside of GOOGLE3 need these special cases). I think we should take this one of
two ways:
1. Require we build with a particular version of libjpeg-turbo.
This means we will need to update them in sync with us, which may be more work
in some ways.
2. Use flags which correspond to specific features.
We already do this with TURBO_HAS_SKIP. It seems like libpng has every
individual feature behind some kind of flag; I do not remember whether libjpeg
does as well, but I recall SkImageDecoder_libjpeg having something like
ANDROID_RGB_565 for some features that were not in libjpeg. Even if the library
does not, we could have our own flags, which would be a lot clearer than
checking for GOOGLE3. This is potentially more work in its own way - bugs can
arise for flags that we're not actively testing (or we just test more
combinations...). I know that Blink decided to remove many of its ifdefs and
just assume particular versions of libraries are being used. (They're in a
different situation, since Blink is not a library that is expected to be used
outside of Chromium.)

[mtklein, 2015/10/27 14:27:59]

Why don't we just use libjpeg's own defines as the guards, e.g. 
#if defined(JCS_RGB565)
...
#endif

#if defined(JCS_EXT_RGBA) && defined(JSC_EXT_BGRA)
...
#endif.

[msarett, 2015/10/27 14:31:36]

sgtm

They are actually enums.  Will that still work?

[dogben, 2015/10/27 15:07:22]

Sorry, I should have left a comment or TODO. I forwarded you a thread about
updating the version of libjpeg_turbo in Google3.

+    int colorBytes = dinfo->out_color_components;
+#else
     int colorBytes = (dinfo->out_color_space == JCS_RGB565) ? 2 : dinfo->out_color_components;
+#endif
     return dinfo->output_width * colorBytes;
 
 }
 
 /*
  *  Calculate output dimensions based on the provided factors.
  *
  *  Not to be used on the actual jpeg_decompress_struct used for decoding, since it will
  *  incorrectly modify num_components.
  */
@@ skipping 75 matching lines @@
     bool isCMYK = JCS_CMYK == colorSpace || JCS_YCCK == colorSpace;
 
     // Check for valid color types and set the output color space
     switch (dst.colorType()) {
         case kN32_SkColorType:
             if (isCMYK) {
                 fDecoderMgr->dinfo()->out_color_space = JCS_CMYK;
             } else {
                 // Check the byte ordering of the RGBA color space for the
                 // current platform
-#if defined(SK_PMCOLOR_IS_RGBA)
+#if defined(GOOGLE3)
+                return false;

[scroggo, 2015/10/27 13:51:23]

It appears that in GOOGLE3, SkJpegCodec will not work for most images. Why
bother building it there at all?

[dogben, 2015/10/27 15:07:22]

I didn't realize that not supporting JCS_EXT_* would mean it wouldn't work for
most images. I can send a CL to conditionally remove Jpeg from SkCodec.cpp.

+#else
+    #if defined(SK_PMCOLOR_IS_RGBA)
                 fDecoderMgr->dinfo()->out_color_space = JCS_EXT_RGBA;
-#else
+    #else
                 fDecoderMgr->dinfo()->out_color_space = JCS_EXT_BGRA;
+    #endif
 #endif
             }
             return true;
         case kRGB_565_SkColorType:
             if (isCMYK) {
                 // FIXME (msarett): We need to support 565 here.  It's not hard to do, considering
                 // we already convert CMYK to RGBA, I just need to do it.  I think it might be
                 // best to do this in SkSwizzler and also move convert_CMYK_to_RGBA into SkSwizzler.
                 return false;
             } else {
+#if defined(GOOGLE3)
+                return false;
+#else
                 fDecoderMgr->dinfo()->dither_mode = JDITHER_NONE;
                 fDecoderMgr->dinfo()->out_color_space = JCS_RGB565;
+#endif
             }
             return true;
         case kGray_8_SkColorType:
             if (isCMYK) {
                 return false;
             } else {
                 // We will enable decodes to gray even if the image is color because this is
                 // much faster than decoding to color and then converting
                 fDecoderMgr->dinfo()->out_color_space = JCS_GRAYSCALE;
             }
@@ skipping 114 matching lines @@
             srcConfig = SkSwizzler::kRGBX;
             break;
         case kBGRA_8888_SkColorType:
             srcConfig = SkSwizzler::kBGRX;
             break;
         case kRGB_565_SkColorType:
             srcConfig = SkSwizzler::kRGB_565;
             break;
         default:
             // This function should only be called if the colorType is supported by jpeg
+#if defined(GOOGLE3)
+            SK_CRASH();

[scroggo, 2015/10/27 13:51:23]

Why this difference? I assume that SkASSERT works in GOOGLE3, or our code would
be littered with this conditional (or we'd just fix it in SkASSERT/SkPreConfig,
so we wouldn't need this here).

[dogben, 2015/10/27 15:07:22]

Sorry, I added this before https://codereview.chromium.org/1400343005 and didn't
realize it wasn't needed anymore.

+#else
             SkASSERT(false);
+#endif
     }
 
     fSwizzler.reset(SkSwizzler::CreateSwizzler(srcConfig, nullptr, dstInfo, options));
     fStorage.reset(get_row_bytes(fDecoderMgr->dinfo()));
     fSrcRow = static_cast<uint8_t*>(fStorage.get());
 }
 
 SkSampler* SkJpegCodec::getSampler(bool createIfNecessary) {
     if (!createIfNecessary || fSwizzler) {
         SkASSERT(!fSwizzler || (fSrcRow && static_cast<uint8_t*>(fStorage.get()) == fSrcRow));
@@ skipping 70 matching lines @@
             dst = SkTAddOffset<JSAMPLE>(dst, rowBytes);
         } else {
             dstRow = SkTAddOffset<JSAMPLE>(dstRow, rowBytes);
         }
     }
     return count;
 }
 
 #ifndef TURBO_HAS_SKIP
 // TODO (msarett): Avoid reallocating the memory buffer on each call to skip.
-static uint32_t jpeg_skip_scanlines(dinfo, count) {
+static uint32_t jpeg_skip_scanlines(jpeg_decompress_struct* dinfo, int count) {

[msarett, 2015/10/15 15:02:33]

+1

     SkAutoMalloc storage(get_row_bytes(dinfo));
     uint8_t* storagePtr = static_cast<uint8_t*>(storage.get());
     for (int y = 0; y < count; y++) {
         if (1 != jpeg_read_scanlines(dinfo, &storagePtr, 1)) {
             return y;
         }
     }
     return count;
 }
 #endif
 
 bool SkJpegCodec::onSkipScanlines(int count) {
     // Set the jump location for libjpeg errors
     if (setjmp(fDecoderMgr->getJmpBuf())) {
         return fDecoderMgr->returnFalse("setjmp");
     }
 
     return (uint32_t) count == jpeg_skip_scanlines(fDecoderMgr->dinfo(), count);
 }