Allow numerical color spaces with legacy rendering

Bug: 720083
Change-Id: Ibd4dbf6ee95ac14857e8280a441f81976710e5e8
Reviewed-on: https://skia-review.googlesource.com/16700
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Matt Sarett <msarett@google.com>

diff --git a/src/core/SkImageInfoPriv.h b/src/core/SkImageInfoPriv.h
index 42e80ca..c9b4b6c 100644 (file)
--- a/src/core/SkImageInfoPriv.h
+++ b/src/core/SkImageInfoPriv.h
@@ -76,6 +76,19 @@ static inline bool SkImageInfoIsValidRenderingCS(const SkImageInfo& info) {
 }
 
 /**
+ *  Returns true if |info| contains a valid combination of width, height, colorType, alphaType,
+ *  colorSpace.  Uses |colorMode| to decide how to treat color spaces.
+ */
+static inline bool SkImageInfoIsValid(const SkImageInfo& info,
+                                      SkDestinationSurfaceColorMode colorMode) {
+    if (SkDestinationSurfaceColorMode::kGammaAndColorSpaceAware == colorMode) {
+        return SkImageInfoIsValidRenderingCS(info);
+    }
+
+    return SkImageInfoIsValidAllowNumericalCS(info);
+}
+
+/**
  *  Returns true if Skia has defined a pixel conversion from the |src| to the |dst|.
  *  Returns false otherwise.  Some discussion of false cases:
  *      We will not convert to kIndex8 unless it exactly matches the src, since color tables

diff --git a/src/gpu/GrBitmapTextureMaker.cpp b/src/gpu/GrBitmapTextureMaker.cpp
index 6c77167..bde8276 100644 (file)
--- a/src/gpu/GrBitmapTextureMaker.cpp
+++ b/src/gpu/GrBitmapTextureMaker.cpp
@@ -41,7 +41,8 @@ sk_sp<GrTextureProxy> GrBitmapTextureMaker::refOriginalTextureProxy(bool willBeM
         proxy = GrGenerateMipMapsAndUploadToTextureProxy(this->context(), fBitmap, dstColorSpace);
     }
     if (!proxy) {
-        proxy = GrUploadBitmapToTextureProxy(this->context()->resourceProvider(), fBitmap);
+        proxy = GrUploadBitmapToTextureProxy(this->context()->resourceProvider(), fBitmap,
+                                             dstColorSpace);
     }
     if (proxy && fOriginalKey.isValid()) {
         this->context()->resourceProvider()->assignUniqueKeyToProxy(fOriginalKey, proxy.get());

diff --git a/src/gpu/SkGr.cpp b/src/gpu/SkGr.cpp
index d331278..81fde35 100644 (file)
--- a/src/gpu/SkGr.cpp
+++ b/src/gpu/SkGr.cpp
@@ -62,7 +62,8 @@ void GrMakeKeyFromImageID(GrUniqueKey* key, uint32_t imageID, const SkIRect& ima
 
 //////////////////////////////////////////////////////////////////////////////
 sk_sp<GrTextureProxy> GrUploadBitmapToTextureProxy(GrResourceProvider* resourceProvider,
-                                                   const SkBitmap& bitmap) {
+                                                   const SkBitmap& bitmap,
+                                                   SkColorSpace* dstColorSpace) {
     if (!bitmap.readyToDraw()) {
         return nullptr;
     }
@@ -70,7 +71,7 @@ sk_sp<GrTextureProxy> GrUploadBitmapToTextureProxy(GrResourceProvider* resourceP
     if (!bitmap.peekPixels(&pixmap)) {
         return nullptr;
     }
-    return GrUploadPixmapToTextureProxy(resourceProvider, pixmap, SkBudgeted::kYes);
+    return GrUploadPixmapToTextureProxy(resourceProvider, pixmap, SkBudgeted::kYes, dstColorSpace);
 }
 
 static const SkPixmap* compute_desc(const GrCaps& caps, const SkPixmap& pixmap,
@@ -127,9 +128,18 @@ static const SkPixmap* compute_desc(const GrCaps& caps, const SkPixmap& pixmap,
     return pmap;
 }
 
-sk_sp<GrTextureProxy> GrUploadPixmapToTextureProxyNoCheck(GrResourceProvider* resourceProvider,
-                                                          const SkPixmap& pixmap,
-                                                          SkBudgeted budgeted) {
+sk_sp<GrTextureProxy> GrUploadPixmapToTextureProxy(GrResourceProvider* resourceProvider,
+                                                   const SkPixmap& pixmap,
+                                                   SkBudgeted budgeted,
+                                                   SkColorSpace* dstColorSpace) {
+    SkDestinationSurfaceColorMode colorMode = dstColorSpace
+        ? SkDestinationSurfaceColorMode::kGammaAndColorSpaceAware
+        : SkDestinationSurfaceColorMode::kLegacy;
+
+    if (!SkImageInfoIsValid(pixmap.info(), colorMode)) {
+        return nullptr;
+    }
+
     SkBitmap tmpBitmap;
     SkPixmap tmpPixmap;
     GrSurfaceDesc desc;
@@ -143,16 +153,6 @@ sk_sp<GrTextureProxy> GrUploadPixmapToTextureProxyNoCheck(GrResourceProvider* re
     return nullptr;
 }
 
-sk_sp<GrTextureProxy> GrUploadPixmapToTextureProxy(GrResourceProvider* resourceProvider,
-                                                   const SkPixmap& pixmap,
-                                                   SkBudgeted budgeted) {
-    if (!SkImageInfoIsValidRenderingCS(pixmap.info())) {
-        return nullptr;
-    }
-
-    return GrUploadPixmapToTextureProxyNoCheck(resourceProvider, pixmap, budgeted);
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 
 void GrInstallBitmapUniqueKeyInvalidator(const GrUniqueKey& key, SkPixelRef* pixelRef) {
@@ -175,7 +175,7 @@ sk_sp<GrTextureProxy> GrGenerateMipMapsAndUploadToTextureProxy(GrContext* ctx,
         ? SkDestinationSurfaceColorMode::kGammaAndColorSpaceAware
         : SkDestinationSurfaceColorMode::kLegacy;
 
-    if (!SkImageInfoIsValidRenderingCS(bitmap.info())) {
+    if (!SkImageInfoIsValid(bitmap.info(), colorMode)) {
         return nullptr;
     }
 
@@ -222,7 +222,7 @@ sk_sp<GrTextureProxy> GrUploadMipMapToTextureProxy(GrContext* ctx, const SkImage
                                                    const GrMipLevel* texels,
                                                    int mipLevelCount,
                                                    SkDestinationSurfaceColorMode colorMode) {
-    if (!SkImageInfoIsValidRenderingCS(info)) {
+    if (!SkImageInfoIsValid(info, colorMode)) {
         return nullptr;
     }
 
@@ -257,7 +257,10 @@ sk_sp<GrTextureProxy> GrMakeCachedBitmapProxy(GrResourceProvider* resourceProvid
         proxy = resourceProvider->findProxyByUniqueKey(originalKey);
     }
     if (!proxy) {
-        proxy = GrUploadBitmapToTextureProxy(resourceProvider, bitmap);
+        // Pass nullptr for |dstColorSpace|.  This is lenient - we allow a wider range of
+        // color spaces in legacy mode.  Unfortunately, we have to be lenient here, since
+        // we can't necessarily know the |dstColorSpace| at this time.
+        proxy = GrUploadBitmapToTextureProxy(resourceProvider, bitmap, nullptr);
         if (proxy && originalKey.isValid()) {
             resourceProvider->assignUniqueKeyToProxy(originalKey, proxy.get());
             // MDB TODO (caching): this has to play nice with the GrSurfaceProxy's caching

diff --git a/src/gpu/SkGr.h b/src/gpu/SkGr.h
index cefcc48..7784c53 100644 (file)
--- a/src/gpu/SkGr.h
+++ b/src/gpu/SkGr.h
@@ -211,7 +211,8 @@ sk_sp<GrTextureProxy> GrRefCachedBitmapTextureProxy(GrContext*,
  * The bitmap must have CPU-accessible pixels. Attempts to take advantage of faster paths for
  * compressed textures and yuv planes.
  */
-sk_sp<GrTextureProxy> GrUploadBitmapToTextureProxy(GrResourceProvider*, const SkBitmap&);
+sk_sp<GrTextureProxy> GrUploadBitmapToTextureProxy(GrResourceProvider*, const SkBitmap&,
+                                                   SkColorSpace* dstColorSpace);
 
 sk_sp<GrTextureProxy> GrGenerateMipMapsAndUploadToTextureProxy(GrContext*, const SkBitmap&,
                                                                SkColorSpace* dstColorSpace);
@@ -220,9 +221,7 @@ sk_sp<GrTextureProxy> GrGenerateMipMapsAndUploadToTextureProxy(GrContext*, const
  * Creates a new texture for the pixmap.
  */
 sk_sp<GrTextureProxy> GrUploadPixmapToTextureProxy(GrResourceProvider*,
-                                                   const SkPixmap&, SkBudgeted);
-sk_sp<GrTextureProxy> GrUploadPixmapToTextureProxyNoCheck(GrResourceProvider*,
-                                                          const SkPixmap&, SkBudgeted);
+                                                   const SkPixmap&, SkBudgeted, SkColorSpace*);
 
 /**
  * Creates a new texture populated with the mipmap levels.

diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp
index 4740543..501d95b 100644 (file)
--- a/src/image/SkImage_Gpu.cpp
+++ b/src/image/SkImage_Gpu.cpp
@@ -847,13 +847,11 @@ sk_sp<SkImage> SkImage::MakeFromDeferredTextureImageData(GrContext* context, con
         SkPixmap pixmap;
         pixmap.reset(info, dti->fMipMapLevelData[0].fPixelData, dti->fMipMapLevelData[0].fRowBytes);
 
-        // Use the NoCheck version because we have already validated the SkImage.  The |data|
-        // used to be an SkImage before calling getDeferredTextureImageData().  In legacy mode,
-        // getDeferredTextureImageData() will allow parametric transfer functions for images
-        // generated from codecs - which is slightly more lenient than typical SkImage
-        // constructors.
-        sk_sp<GrTextureProxy> proxy(GrUploadPixmapToTextureProxyNoCheck(
-                context->resourceProvider(), pixmap, budgeted));
+        // Pass nullptr for the |dstColorSpace|.  This opts in to more lenient color space
+        // verification.  This is ok because we've already verified the color space in
+        // getDeferredTextureImageData().
+        sk_sp<GrTextureProxy> proxy(GrUploadPixmapToTextureProxy(
+                context->resourceProvider(), pixmap, budgeted, nullptr));
         if (!proxy) {
             return nullptr;
         }

diff --git a/src/image/SkImage_Lazy.cpp b/src/image/SkImage_Lazy.cpp
index e94041b..7108a1a 100644 (file)
--- a/src/image/SkImage_Lazy.cpp
+++ b/src/image/SkImage_Lazy.cpp
@@ -743,7 +743,7 @@ sk_sp<GrTextureProxy> SkImage_Lazy::lockTextureProxy(GrContext* ctx,
             proxy = GrGenerateMipMapsAndUploadToTextureProxy(ctx, bitmap, dstColorSpace);
         }
         if (!proxy) {
-            proxy = GrUploadBitmapToTextureProxy(ctx->resourceProvider(), bitmap);
+            proxy = GrUploadBitmapToTextureProxy(ctx->resourceProvider(), bitmap, dstColorSpace);
         }
         if (proxy) {
             SK_HISTOGRAM_ENUMERATION("LockTexturePath", kRGBA_LockTexturePath,