Revert "Allow linear 8888 surface contexts in GPU backend."
This reverts commit 6a46fb210b810553b2a03a5f316109ccd9710ebc.
Reason for revert: breaks gms
Original change's description:
> Allow linear 8888 surface contexts in GPU backend.
>
> Make SRGBReadWritePixels test test all combinations of sRGB, untagged, and linear.
>
> Bug: skia:
> Change-Id: I0c75fa27b1bf60c6e7ce3b666ff79e1ad1c91b94
> Reviewed-on: https://skia-review.googlesource.com/106922
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
TBR=bsalomon@google.com,brianosman@google.com
Change-Id: Ifb28fdde315ccc0b6fc1c9dd944ea319bafba754
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:
Reviewed-on: https://skia-review.googlesource.com/107004
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
diff --git a/src/image/SkSurface_Gpu.cpp b/src/image/SkSurface_Gpu.cpp
index 03780c0..04333ae 100644
--- a/src/image/SkSurface_Gpu.cpp
+++ b/src/image/SkSurface_Gpu.cpp
@@ -261,11 +261,10 @@
return context->caps()->srgbSupport() && colorSpace && colorSpace->gammaCloseToSRGB();
case kRGBA_8888_GrPixelConfig:
case kBGRA_8888_GrPixelConfig:
- // We may get here with either a linear-gamma color space or with a sRGB-gamma color
- // space when we lack GPU sRGB support.
+ // If we don't have sRGB support, we may get here with a color space. It still needs
+ // to be sRGB-like (so that the application will work correctly on sRGB devices.)
return !colorSpace ||
- (colorSpace->gammaCloseToSRGB() && !context->caps()->srgbSupport()) ||
- colorSpace->gammaIsLinear();
+ (colorSpace->gammaCloseToSRGB() && !context->caps()->srgbSupport());
default:
return !colorSpace;
}
diff --git a/tests/SRGBReadWritePixelsTest.cpp b/tests/SRGBReadWritePixelsTest.cpp
index c9fd996..a177f72 100644
--- a/tests/SRGBReadWritePixelsTest.cpp
+++ b/tests/SRGBReadWritePixelsTest.cpp
@@ -112,11 +112,6 @@
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,
@@ -147,43 +142,10 @@
}
}
-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) {
+// 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();
#if defined(SK_BUILD_FOR_GOOGLE3)
// Stack frame size is limited in SK_BUILD_FOR_GOOGLE3.
static const int kW = 63;
@@ -199,146 +161,97 @@
}
}
+ 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 = encoding_as_pixel_config(contextEncoding);
+ 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;
+ }
- 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;
+ 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.");
+ }
+
+ // 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.");
+ }
+
+ 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;
+ }
+
+ // 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.");
+ }
+
+ // 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.");
+ }
}
- auto writeII = SkImageInfo::Make(kW, kH, kRGBA_8888_SkColorType, kPremul_SkAlphaType,
- encoding_as_color_space(writeEncoding));
-
- 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;
- }
-
- 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());
-}
-
-// 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 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