Disabled YUV decoding for subsets

src/gpu/SkGr.cpp

Index: src/gpu/SkGr.cpp
diff --git a/src/gpu/SkGr.cpp b/src/gpu/SkGr.cpp
index fdd4a79e48b22a8dffc74198bf764315f1059666..8d80c6ea507dd2bde11fa54feaa042e35ba972fa 100644
--- a/src/gpu/SkGr.cpp
+++ b/src/gpu/SkGr.cpp
@@ -220,9 +220,19 @@ static GrTexture *load_etc1_texture(GrContext* ctx, bool cache,
 
 static GrTexture *load_yuv_texture(GrContext* ctx, bool cache, const GrTextureParams* params,
                                    const SkBitmap& bm, const GrTextureDesc& desc) {
+    // 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];
-    if ((NULL == pixelRef) || !pixelRef->getYUV8Planes(yuvSizes, NULL, NULL, NULL)) {
+    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;
     }