Revert "Explicitly fail read/writePixels in invalid color space scenarios"

This reverts commit efcc41805b43347444b83c1705d3d60c8d0caa70.

Reason for revert: Possible culprit for Chromium failures.

Original change's description:
> Explicitly fail read/writePixels in invalid color space scenarios
>
> It's not well defined what to do when moving from a nullptr color space to
> a tagged destination (drawing, reading, writing, etc...). In these
> scenarios, at least, we can choose to disallow the operation (rather than
> produce an unexpected or inconsistent result).
>
> BUG=skia:
>
> Change-Id: I033b23c6f2bb00664efc8fdab1b3f52053d77695
> Reviewed-on: https://skia-review.googlesource.com/6600
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Matt Sarett <msarett@google.com>
>

TBR=mtklein@google.com,bsalomon@google.com,msarett@google.com,brianosman@google.com,reviews@skia.org
BUG=skia:
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Change-Id: I17791f9285089ede42b7921324e0dc264865be1d
Reviewed-on: https://skia-review.googlesource.com/6628
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>

diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp
index 310f455..30f596f 100644 (file)
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -278,11 +278,6 @@ bool GrContext::writeSurfacePixels(GrSurface* surface, SkColorSpace* dstColorSpa
         return false;
     }
 
-    // We don't allow writing to a color space tagged destination if the source isn't tagged
-    if (dstColorSpace && !srcColorSpace) {
-        return false;
-    }
-
     GrGpu::DrawPreference drawPreference = GrGpu::kNoDraw_DrawPreference;
     // Don't prefer to draw for the conversion (and thereby access a texture from the cache) when
     // we've already determined that there isn't a roundtrip preserving conversion processor pair.
@@ -432,11 +427,6 @@ bool GrContext::readSurfacePixels(GrSurface* src, SkColorSpace* srcColorSpace,
         return false;
     }
 
-    // We don't allow reading to a color space tagged destination if the source isn't tagged
-    if (dstColorSpace && !srcColorSpace) {
-        return false;
-    }
-
     GrGpu::DrawPreference drawPreference = GrGpu::kNoDraw_DrawPreference;
     // Don't prefer to draw for the conversion (and thereby access a texture from the cache) when
     // we've already determined that there isn't a roundtrip preserving conversion processor pair.

diff --git a/tests/ImageTest.cpp b/tests/ImageTest.cpp
index b2ac36d..161dff8 100644 (file)
--- a/tests/ImageTest.cpp
+++ b/tests/ImageTest.cpp
@@ -45,7 +45,8 @@ static void assert_equal(skiatest::Reporter* reporter, SkImage* a, const SkIRect
     // see https://bug.skia.org/3965
     //REPORTER_ASSERT(reporter, a->isOpaque() == b->isOpaque());
 
-    SkImageInfo info = SkImageInfo::MakeN32(widthA, heightA, a->alphaType());
+    // The codecs may have given us back F16, we can't read from F16 raster to N32, only S32.
+    SkImageInfo info = SkImageInfo::MakeS32(widthA, heightA, a->alphaType());
     SkAutoPixmapStorage pmapA, pmapB;
     pmapA.alloc(info);
     pmapB.alloc(info);

diff --git a/tests/ReadPixelsTest.cpp b/tests/ReadPixelsTest.cpp
index 487152c..71cd8f5 100644 (file)
--- a/tests/ReadPixelsTest.cpp
+++ b/tests/ReadPixelsTest.cpp
@@ -487,40 +487,3 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ReadPixels_Texture, reporter, ctxInfo) {
     }
 }
 #endif
-
-#if SK_SUPPORT_GPU
-DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ReadPixelsColorSpaceVariants_Gpu, reporter, ctxInfo) {
-    // Create surfaces with and without an attached color space
-    sk_sp<SkColorSpace> srgbColorSpace = SkColorSpace::MakeNamed(SkColorSpace::kSRGB_Named);
-    SkImageInfo srgbInfo = SkImageInfo::MakeS32(DEV_W, DEV_H, kPremul_SkAlphaType);
-    SkImageInfo legacyInfo = srgbInfo.makeColorSpace(nullptr);
-
-    sk_sp<SkSurface> srgbSurface = SkSurface::MakeRenderTarget(ctxInfo.grContext(), SkBudgeted::kNo,
-                                                               srgbInfo);
-    sk_sp<SkSurface> legacySurface = SkSurface::MakeRenderTarget(ctxInfo.grContext(),
-                                                                 SkBudgeted::kNo, legacyInfo);
-    SkCanvas* srgbCanvas = srgbSurface->getCanvas();
-    SkCanvas* legacyCanvas = legacySurface->getCanvas();
-
-    struct {
-        SkCanvas* fCanvas;
-        const SkImageInfo& fBmpInfo;
-        bool fExpectSuccess;
-    } kTestConfigs[] ={
-        // Both kinds of surface should be able to read into a legacy destination
-        { srgbCanvas, legacyInfo, true },
-        { legacyCanvas, legacyInfo, true },
-        // Tagged surface should be able to read into tagged destination
-        { srgbCanvas, srgbInfo, true },
-        // Legacy surface shouldn't read into tagged destination
-        { legacyCanvas, srgbInfo, false },
-    };
-
-    for (auto testConfig : kTestConfigs) {
-        SkBitmap bmp;
-        bmp.setInfo(testConfig.fBmpInfo);
-        bool result = testConfig.fCanvas->readPixels(&bmp, 0, 0);
-        REPORTER_ASSERT(reporter, result == testConfig.fExpectSuccess);
-    }
-}
-#endif