Disabled YUV decoding for subsets

src/gpu/SkGr.cpp

 /*
  * Copyright 2010 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkGr.h"
 #include "SkColorFilter.h"
 #include "SkConfig8888.h"
@@ skipping 202 matching lines @@
     } else {
         return NULL;
     }
 
     return sk_gr_allocate_texture(ctx, cache, params, bm, desc, bytes, 0);
 }
 #endif   // SK_IGNORE_ETC1_SUPPORT
 
 static GrTexture *load_yuv_texture(GrContext* ctx, bool cache, const GrTextureParams* params,
                                    const SkBitmap& bm, const GrTextureDesc& desc) {
-    SkPixelRef* pixelRef = bm.pixelRef();
-    SkISize yuvSizes[3];
-    if ((NULL == pixelRef) || !pixelRef->getYUV8Planes(yuvSizes, NULL, NULL, NULL)) {
+    // Subsets are not supported, the whole pixelRef is loaded when using YUV decoding
+    if ((bm.pixelRef()->info().width()  != bm.info().width()) ||
+        (bm.pixelRef()->info().height() != bm.info().height())) {
         return NULL;
     }
 
+    SkPixelRef* pixelRef = bm.pixelRef();
+    SkISize yuvSizes[3];
+    const int maxTextureSize = ctx->getMaxTextureSize();
+    if ((NULL == pixelRef) || !pixelRef->getYUV8Planes(yuvSizes, NULL, NULL, NULL) ||

[bsalomon, 2014/10/15 15:34:42]

Should the failure happen in getYUV8Planes rather than here? It seems like it is
kind of broken as is. It will "succeed" but give the wrong result for subset
bitmaps.

[sugoi1, 2014/10/15 18:13:28]

getYUV8Planes() doesn't know anything about the bitmap. It is perfectly capable
of decoding that pixelRef properly and yielding the YUV planes, no matter the
size (that part doesn't use the GPU). It doesn't know that we are planning on
only using a subset of the YUV planes from the pixelRef itself. I don't think
there's any difference between the pixelRef of a complete bitmap or of a bitmap
subset (in fact, I think they can share the same pixelRef).

In fact, the jpeg images causing the issue I'm trying to fix could be decoded
properly using YUV decoding, if it wasn't for the fact that shouldTileBitmap()
returns true when the size is large as compared to the GrResourceCache size.

[bsalomon, 2014/10/15 18:31:54]

Acknowledged.

+        (yuvSizes[0].fWidth > maxTextureSize) || (yuvSizes[0].fHeight > maxTextureSize) ||
+        (yuvSizes[1].fWidth > maxTextureSize) || (yuvSizes[1].fHeight > maxTextureSize) ||

[bsalomon, 2014/10/15 15:34:42]

Are these checks needed?

[sugoi1, 2014/10/15 18:13:28]

Well, it's not impossible that these would be required, but it's unlikely. Let
me explain: yuvSizes are the sizes that we need to allocate and these sizes are
expressed in (number of blocks) * (size of a block), so the size of a JPEG image
is rounded up to the nearest block to produce the yuvSizes. So even if a
bitmap's dimensions fit into maxTextureSize at the moment where we call
shouldTileBitmap(), yuvSizes' dimensions may exceed that number and no longer
fit into maxTextureSize.

That's all theoretical, though, as in practice, block sizes are usually 8, 16 or
32, and as long as maxTextureSize is an exact multiple of 8, 16 or 32 (which is
most likely always the case), than these checks can probably be safely removed.

[bsalomon, 2014/10/15 18:31:54]

In any case, wouldn't we just fail down below in refScratchTexture() and fail
gracefully?

[sugoi1, 2014/10/15 19:08:56]

Ah, well, if that's the case I'll just remove these. Thanks.

+        (yuvSizes[2].fWidth > maxTextureSize) || (yuvSizes[2].fHeight > maxTextureSize)) {
+        return NULL;
+    }
+
     // Allocate the memory for YUV
     size_t totalSize(0);
     size_t sizes[3], rowBytes[3];
     for (int i = 0; i < 3; ++i) {
         rowBytes[i] = yuvSizes[i].fWidth;
         totalSize  += sizes[i] = rowBytes[i] * yuvSizes[i].fHeight;
     }
     SkAutoMalloc storage(totalSize);
     void* planes[3];
     planes[0] = storage.get();
@@ skipping 330 matching lines @@
         if (shader->asFragmentProcessor(context, skPaint, NULL, &paintColor, &fp) && fp) {
             grPaint->addColorProcessor(fp)->unref();
             constantColor = false;
         }
     }
 
     // The grcolor is automatically set when calling asFragmentProcessor.
     // If the shader can be seen as an effect it returns true and adds its effect to the grpaint.
     SkPaint2GrPaintNoShader(context, skPaint, paintColor, constantColor, grPaint);
 }