Revert "Revert "Allow linear 8888 surface contexts in GPU backend.""

This reverts commit 4ffdf3c67ff7f9ee66d56db02161d66fddd9fdf7.

Change-Id: I6c38328500ec9614b587121741e63c7b1a092c7d
Reviewed-on: https://skia-review.googlesource.com/107005
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>

1 files changed, 172 insertions, 85 deletions

diff --git a/tests/SRGBReadWritePixelsTest.cpp b/tests/SRGBReadWritePixelsTest.cpp
index a177f72ee5..c9fd99662a 100644
--- a/tests/SRGBReadWritePixelsTest.cpp
+++ b/tests/SRGBReadWritePixelsTest.cpp
@@ -112,6 +112,11 @@ static bool check_srgb_to_linear_to_srgb_conversion(uint32_t input, uint32_t out
     return check_double_conversion<srgb_to_linear, linear_to_srgb>(input, output, error);
 }
 
+static bool check_no_conversion(uint32_t input, uint32_t output, float error) {
+    // This is a bit of a hack to check identity transformations that may lose precision.
+    return check_srgb_to_linear_to_srgb_conversion(input, output, error);
+}
+
 typedef bool (*CheckFn) (uint32_t orig, uint32_t actual, float error);
 
 void read_and_check_pixels(skiatest::Reporter* reporter, GrSurfaceContext* context,
@@ -142,10 +147,43 @@ void read_and_check_pixels(skiatest::Reporter* reporter, GrSurfaceContext* conte
     }
 }
 
-// TODO: Add tests for copySurface between srgb/linear textures. Add tests for unpremul/premul
-// conversion during read/write along with srgb/linear conversions.
-DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SRGBReadWritePixels, reporter, ctxInfo) {
-    GrContext* context = ctxInfo.grContext();
+namespace {
+enum class Encoding {
+    kUntagged,
+    kLinear,
+    kSRGB,
+};
+}
+
+static sk_sp<SkColorSpace> encoding_as_color_space(Encoding encoding) {
+    switch (encoding) {
+        case Encoding::kUntagged: return nullptr;
+        case Encoding::kLinear:   return SkColorSpace::MakeSRGBLinear();
+        case Encoding::kSRGB:     return SkColorSpace::MakeSRGB();
+    }
+    return nullptr;
+}
+
+static GrPixelConfig encoding_as_pixel_config(Encoding encoding) {
+    switch (encoding) {
+        case Encoding::kUntagged: return kRGBA_8888_GrPixelConfig;
+        case Encoding::kLinear:   return kRGBA_8888_GrPixelConfig;
+        case Encoding::kSRGB:     return kSRGBA_8888_GrPixelConfig;
+    }
+    return kUnknown_GrPixelConfig;
+}
+
+static const char* encoding_as_str(Encoding encoding) {
+    switch (encoding) {
+        case Encoding::kUntagged: return "untagged";
+        case Encoding::kLinear:   return "linear";
+        case Encoding::kSRGB:     return "sRGB";
+    }
+    return nullptr;
+}
+
+static void do_test(Encoding contextEncoding, Encoding writeEncoding, Encoding readEncoding,
+                    float error, CheckFn check, GrContext* context, skiatest::Reporter* reporter) {
 #if defined(SK_BUILD_FOR_GOOGLE3)
     // Stack frame size is limited in SK_BUILD_FOR_GOOGLE3.
     static const int kW = 63;
@@ -161,97 +199,146 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SRGBReadWritePixels, reporter, ctxInfo) {
         }
     }
 
-    const SkImageInfo iiSRGBA = SkImageInfo::Make(kW, kH, kRGBA_8888_SkColorType,
-                                                  kPremul_SkAlphaType,
-                                                  SkColorSpace::MakeSRGB());
-    const SkImageInfo iiRGBA = SkImageInfo::Make(kW, kH, kRGBA_8888_SkColorType,
-                                                 kPremul_SkAlphaType);
     GrSurfaceDesc desc;
     desc.fFlags = kRenderTarget_GrSurfaceFlag;
     desc.fOrigin = kBottomLeft_GrSurfaceOrigin;
     desc.fWidth = kW;
     desc.fHeight = kH;
-    desc.fConfig = kSRGBA_8888_GrPixelConfig;
-    if (context->caps()->isConfigRenderable(desc.fConfig) &&
-        context->caps()->isConfigTexturable(desc.fConfig)) {
-        sk_sp<GrSurfaceContext> sContext = context->contextPriv().makeDeferredSurfaceContext(
-                desc, GrMipMapped::kNo, SkBackingFit::kExact, SkBudgeted::kNo,
-                SkColorSpace::MakeSRGB());
-        if (!sContext) {
-            ERRORF(reporter, "Could not create SRGBA surface context.");
-            return;
-        }
+    desc.fConfig = encoding_as_pixel_config(contextEncoding);
 
-        float error = context->caps()->shaderCaps()->halfIs32Bits() ? 0.5f : 1.2f;
-
-        // Write srgba data and read as srgba and then as rgba
-        if (sContext->writePixels(iiSRGBA, origData, 0, 0, 0)) {
-            // For the all-srgba case, we allow a small error only for devices that have
-            // precision variation because the srgba data gets converted to linear and back in
-            // the shader.
-            float smallError = context->caps()->shaderCaps()->halfIs32Bits() ? 0.0f : 1.f;
-            read_and_check_pixels(reporter, sContext.get(), origData, iiSRGBA,
-                                  check_srgb_to_linear_to_srgb_conversion, smallError,
-                                  "write/read srgba to srgba texture");
-            read_and_check_pixels(reporter, sContext.get(), origData, iiRGBA,
-                                  check_srgb_to_linear_conversion, error,
-                                  "write srgba/read rgba with srgba texture");
-        } else {
-            ERRORF(reporter, "Could not write srgba data to srgba texture.");
-        }
+    auto surfaceContext = context->contextPriv().makeDeferredSurfaceContext(
+            desc, GrMipMapped::kNo, SkBackingFit::kExact, SkBudgeted::kNo,
+            encoding_as_color_space(contextEncoding));
+    if (!surfaceContext) {
+        ERRORF(reporter, "Could not create %s surface context.", encoding_as_str(contextEncoding));
+        return;
+    }
+    auto writeII = SkImageInfo::Make(kW, kH, kRGBA_8888_SkColorType, kPremul_SkAlphaType,
+                                     encoding_as_color_space(writeEncoding));
 
-        // Now verify that we can write linear data
-        if (sContext->writePixels(iiRGBA, origData, 0, 0, 0)) {
-            // We allow more error on GPUs with lower precision shader variables.
-            read_and_check_pixels(reporter, sContext.get(), origData, iiSRGBA,
-                                  check_linear_to_srgb_conversion, error,
-                                  "write rgba/read srgba with srgba texture");
-            read_and_check_pixels(reporter, sContext.get(), origData, iiRGBA,
-                                  check_linear_to_srgb_to_linear_conversion, error,
-                                  "write/read rgba with srgba texture");
-        } else {
-            ERRORF(reporter, "Could not write rgba data to srgba texture.");
-        }
+    if (!surfaceContext->writePixels(writeII, origData, 0, 0, 0)) {
+        ERRORF(reporter, "Could not write %s to %s surface context.",
+               encoding_as_str(writeEncoding), encoding_as_str(contextEncoding));
+        return;
+    }
 
-        desc.fConfig = kRGBA_8888_GrPixelConfig;
-        sContext = context->contextPriv().makeDeferredSurfaceContext(desc,
-                                                                     GrMipMapped::kNo,
-                                                                     SkBackingFit::kExact,
-                                                                     SkBudgeted::kNo);
-        if (!sContext) {
-            ERRORF(reporter, "Could not create RGBA surface context.");
-            return;
-        }
+    auto readII = SkImageInfo::Make(kW, kH, kRGBA_8888_SkColorType, kPremul_SkAlphaType,
+                                    encoding_as_color_space(readEncoding));
+    SkString testName;
+    testName.printf("write %s data to a %s context and read as %s.", encoding_as_str(writeEncoding),
+                    encoding_as_str(contextEncoding), encoding_as_str(readEncoding));
+    read_and_check_pixels(reporter, surfaceContext.get(), origData, readII, check, error,
+                          testName.c_str());
+}
 
-        // Write srgba data to a rgba texture and read back as srgba and rgba
-        if (sContext->writePixels(iiSRGBA, origData, 0, 0, 0)) {
-#if 0
-            // We don't support this conversion (read from untagged source into tagged destination.
-            // If we decide there is a meaningful way to implement this, restore this test.
-            read_and_check_pixels(reporter, sContext.get(), origData, iiSRGBA,
-                                  check_srgb_to_linear_to_srgb_conversion, error,
-                                  "write/read srgba to rgba texture");
-#endif
-            // We expect the sRGB -> linear write to do no sRGB conversion (to match the behavior of
-            // drawing tagged sources). skbug.com/6547. So the data we read should still contain
-            // sRGB encoded values.
-            //
-            // srgb_to_linear_to_srgb is a proxy for the expected identity transform.
-            read_and_check_pixels(reporter, sContext.get(), origData, iiRGBA,
-                                  check_srgb_to_linear_to_srgb_conversion, error,
-                                  "write srgba/read rgba to rgba texture");
-        } else {
-            ERRORF(reporter, "Could not write srgba data to rgba texture.");
-        }
+// Test all combinations of writePixels/readPixels where the surface context/write source/read dst
+// are sRGB, linear, or untagged RGBA_8888.
+DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SRGBReadWritePixels, reporter, ctxInfo) {
+    GrContext* context = ctxInfo.grContext();
+    if (!context->caps()->isConfigRenderable(kSRGBA_8888_GrPixelConfig) &&
+        !context->caps()->isConfigTexturable(kSRGBA_8888_GrPixelConfig)) {
+        return;
+    }
+    // We allow more error on GPUs with lower precision shader variables.
+    float error = context->caps()->shaderCaps()->halfIs32Bits() ? 0.5f : 1.2f;
+    // For the all-sRGB case, we allow a small error only for devices that have
+    // precision variation because the sRGB data gets converted to linear and back in
+    // the shader.
+    float smallError = context->caps()->shaderCaps()->halfIs32Bits() ? 0.0f : 1.f;
 
-        // Write rgba data to a rgba texture and read back as srgba
-        if (sContext->writePixels(iiRGBA, origData, 0, 0, 0)) {
-            read_and_check_pixels(reporter, sContext.get(), origData, iiSRGBA,
-                                  check_linear_to_srgb_conversion, 1.2f,
-                                  "write rgba/read srgba to rgba texture");
-        } else {
-            ERRORF(reporter, "Could not write rgba data to rgba texture.");
-        }
+    ///////////////////////////////////////////////////////////////////////////////////////////////
+    // Write sRGB data to a sRGB context - no conversion on the write.
+
+    // back to sRGB no conversion
+    do_test(Encoding::kSRGB, Encoding::kSRGB, Encoding::kSRGB, smallError, check_no_conversion,
+            context, reporter);
+    // Untagged read from sRGB is treated as a conversion back to linear. TODO: Fail or don't
+    // convert?
+    do_test(Encoding::kSRGB, Encoding::kSRGB, Encoding::kUntagged, error,
+            check_srgb_to_linear_conversion, context, reporter);
+    // Converts back to linear
+    do_test(Encoding::kSRGB, Encoding::kSRGB, Encoding::kLinear, error,
+            check_srgb_to_linear_conversion, context, reporter);
+
+    ///////////////////////////////////////////////////////////////////////////////////////////////
+    // Write untagged data to a sRGB context - Currently this treats the untagged data as
+    // linear and converts to sRGB during the write. TODO: Fail or passthrough?
+
+    // read back to srgb, no additional conversion
+    do_test(Encoding::kSRGB, Encoding::kUntagged, Encoding::kSRGB, error,
+            check_linear_to_srgb_conversion, context, reporter);
+    // read back to untagged. Currently converts back to linear. TODO: Fail or don't convert?
+    do_test(Encoding::kSRGB, Encoding::kUntagged, Encoding::kUntagged, error,
+            check_linear_to_srgb_to_linear_conversion, context, reporter);
+    // Converts back to linear.
+    do_test(Encoding::kSRGB, Encoding::kUntagged, Encoding::kLinear, error,
+            check_linear_to_srgb_to_linear_conversion, context, reporter);
+
+    ///////////////////////////////////////////////////////////////////////////////////////////////
+    // Write linear data to a sRGB context. It gets converted to sRGB on write. The reads
+    // are all the same as the above cases where the original data was untagged.
+    do_test(Encoding::kSRGB, Encoding::kLinear, Encoding::kSRGB, error,
+            check_linear_to_srgb_conversion, context, reporter);
+    // TODO: Fail or don't convert?
+    do_test(Encoding::kSRGB, Encoding::kLinear, Encoding::kUntagged, error,
+            check_linear_to_srgb_to_linear_conversion, context, reporter);
+    do_test(Encoding::kSRGB, Encoding::kLinear, Encoding::kLinear, error,
+            check_linear_to_srgb_to_linear_conversion, context, reporter);
+
+    ///////////////////////////////////////////////////////////////////////////////////////////////
+    // Write data to an untagged context. The write does no conversion no matter what encoding the
+    // src data has.
+    for (auto writeEncoding : {Encoding::kSRGB, Encoding::kUntagged, Encoding::kLinear}) {
+        // Currently this converts to sRGB when we read. TODO: Should reading from an untagged
+        // context to sRGB fail or do no conversion?
+        do_test(Encoding::kUntagged, writeEncoding, Encoding::kSRGB, error,
+                check_linear_to_srgb_conversion, context, reporter);
+        // Reading untagged back as untagged should do no conversion.
+        do_test(Encoding::kUntagged, writeEncoding, Encoding::kUntagged, error, check_no_conversion,
+                context, reporter);
+        // Reading untagged back as linear does no conversion. TODO: Should it just fail?
+        do_test(Encoding::kUntagged, writeEncoding, Encoding::kLinear, error, check_no_conversion,
+                context, reporter);
     }
+
+    ///////////////////////////////////////////////////////////////////////////////////////////////
+    // Write sRGB data to a linear context - converts to sRGB on the write.
+
+    // converts back to sRGB on read.
+    do_test(Encoding::kLinear, Encoding::kSRGB, Encoding::kSRGB, error,
+            check_srgb_to_linear_to_srgb_conversion, context, reporter);
+    // Reading untagged data from linear currently does no conversion. TODO: Should it fail?
+    do_test(Encoding::kLinear, Encoding::kSRGB, Encoding::kUntagged, error,
+            check_srgb_to_linear_conversion, context, reporter);
+    // Stays linear when read.
+    do_test(Encoding::kLinear, Encoding::kSRGB, Encoding::kLinear, error,
+            check_srgb_to_linear_conversion, context, reporter);
+
+    ///////////////////////////////////////////////////////////////////////////////////////////////
+    // Write untagged data to a linear context. Currently does no conversion. TODO: Should this
+    // fail?
+
+    // Reading to sRGB does a conversion.
+    do_test(Encoding::kLinear, Encoding::kUntagged, Encoding::kSRGB, error,
+            check_linear_to_srgb_conversion, context, reporter);
+    // Reading to untagged does no conversion. TODO: Should it fail?
+    do_test(Encoding::kLinear, Encoding::kUntagged, Encoding::kUntagged, error, check_no_conversion,
+            context, reporter);
+    // Stays linear when read.
+    do_test(Encoding::kLinear, Encoding::kUntagged, Encoding::kLinear, error, check_no_conversion,
+            context, reporter);
+
+    ///////////////////////////////////////////////////////////////////////////////////////////////
+    // Write linear data to a linear context. Does no conversion.
+
+    // Reading to sRGB does a conversion.
+    do_test(Encoding::kLinear, Encoding::kLinear, Encoding::kSRGB, error,
+            check_linear_to_srgb_conversion, context, reporter);
+    // Reading to untagged does no conversion. TODO: Should it fail?
+    do_test(Encoding::kLinear, Encoding::kLinear, Encoding::kUntagged, error, check_no_conversion,
+            context, reporter);
+    // Stays linear when read.
+    do_test(Encoding::kLinear, Encoding::kLinear, Encoding::kLinear, error, check_no_conversion,
+            context, reporter);
 }
 #endif