Revert "Revert "New read pixels implementation that is simpler but does all conversions on CPU.""
This reverts commit be5947c2f38a79b7c709accfb1047d8fd06a0227.
Bug: skia:
Change-Id: I06dc15b31042d7827511d0ac2a7f4262c3f09622
Reviewed-on: https://skia-review.googlesource.com/115079
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
diff --git a/tests/PackedConfigsTextureTest.cpp b/tests/PackedConfigsTextureTest.cpp
index b1fbc77..5f18ef3 100644
--- a/tests/PackedConfigsTextureTest.cpp
+++ b/tests/PackedConfigsTextureTest.cpp
@@ -97,7 +97,7 @@
}
static void run_test(skiatest::Reporter* reporter, GrContext* context, int arraySize,
- GrColorType colorType) {
+ SkColorType colorType) {
SkTDArray<uint16_t> controlPixelData;
// We will read back into an 8888 buffer since 565/4444 read backs aren't supported
SkTDArray<GrColor> readBuffer;
@@ -113,19 +113,24 @@
kRGBA_8888_SkColorType, kOpaque_SkAlphaType);
for (auto origin : { kTopLeft_GrSurfaceOrigin, kBottomLeft_GrSurfaceOrigin }) {
- auto proxy = sk_gpu_test::MakeTextureProxyFromData(context, false, DEV_W, DEV_H, colorType,
+ auto proxy = sk_gpu_test::MakeTextureProxyFromData(context, false, DEV_W, DEV_H,
+ SkColorTypeToGrColorType(colorType),
origin, controlPixelData.begin(), 0);
SkASSERT(proxy);
sk_sp<GrSurfaceContext> sContext = context->contextPriv().makeWrappedSurfaceContext(
std::move(proxy));
- SkAssertResult(sContext->readPixels(dstInfo, readBuffer.begin(), 0, 0, 0));
+ if (!sContext->readPixels(dstInfo, readBuffer.begin(), 0, 0, 0)) {
+ // We only require this to succeed if the format is renderable.
+ REPORTER_ASSERT(reporter, !context->colorTypeSupportedAsSurface(colorType));
+ return;
+ }
- if (GrColorType::kABGR_4444 == colorType) {
+ if (kARGB_4444_SkColorType == colorType) {
check_4444(reporter, controlPixelData, readBuffer);
} else {
- SkASSERT(GrColorType::kRGB_565 == colorType);
+ SkASSERT(kRGB_565_SkColorType == colorType);
check_565(reporter, controlPixelData, readBuffer);
}
}
@@ -134,11 +139,11 @@
static const int CONTROL_ARRAY_SIZE = DEV_W * DEV_H;
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(RGBA4444TextureTest, reporter, ctxInfo) {
- run_test(reporter, ctxInfo.grContext(), CONTROL_ARRAY_SIZE, GrColorType::kABGR_4444);
+ run_test(reporter, ctxInfo.grContext(), CONTROL_ARRAY_SIZE, kARGB_4444_SkColorType);
}
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(RGB565TextureTest, reporter, ctxInfo) {
- run_test(reporter, ctxInfo.grContext(), CONTROL_ARRAY_SIZE, GrColorType::kRGB_565);
+ run_test(reporter, ctxInfo.grContext(), CONTROL_ARRAY_SIZE, kRGB_565_SkColorType);
}
#endif
diff --git a/tests/ReadWriteAlphaTest.cpp b/tests/ReadWriteAlphaTest.cpp
index eaad411..b82c48f 100644
--- a/tests/ReadWriteAlphaTest.cpp
+++ b/tests/ReadWriteAlphaTest.cpp
@@ -99,6 +99,11 @@
// read the texture back
result = sContext->readPixels(ii, readback.get(), rowBytes, 0, 0);
+ // We don't require reading from kAlpha_8 to be supported. TODO: At least make this work
+ // when kAlpha_8 is renderable.
+ if (!result) {
+ continue;
+ }
REPORTER_ASSERT(reporter, result, "Initial A8 readPixels failed");
// make sure the original & read back versions match
diff --git a/tests/SRGBReadWritePixelsTest.cpp b/tests/SRGBReadWritePixelsTest.cpp
index 18ed738..569490c 100644
--- a/tests/SRGBReadWritePixelsTest.cpp
+++ b/tests/SRGBReadWritePixelsTest.cpp
@@ -139,8 +139,8 @@
uint32_t read = readData[j * w + i];
if (!checker(orig, read, error)) {
- ERRORF(reporter, "Expected 0x%08x, read back as 0x%08x in %s at %d, %d).",
- orig, read, subtestName, i, j);
+ ERRORF(reporter, "Original 0x%08x, read back as 0x%08x in %s at %d, %d).", orig,
+ read, subtestName, i, j);
return;
}
}
@@ -272,13 +272,20 @@
///////////////////////////////////////////////////////////////////////////////////////////////
// Write sRGB data to a sRGB context - no conversion on the write.
- // back to sRGB no conversion
+ // back to sRGB - no conversion.
test_write_read(Encoding::kSRGB, Encoding::kSRGB, Encoding::kSRGB, smallError,
check_no_conversion, context, reporter);
+#ifdef SK_LEGACY_GPU_PIXEL_OPS
// Untagged read from sRGB is treated as a conversion back to linear. TODO: Fail or don't
// convert?
test_write_read(Encoding::kSRGB, Encoding::kSRGB, Encoding::kUntagged, error,
check_srgb_to_linear_conversion, context, reporter);
+#else
+ // Reading back to untagged should be a pass through with no conversion.
+ test_write_read(Encoding::kSRGB, Encoding::kSRGB, Encoding::kUntagged, error,
+ check_no_conversion, context, reporter);
+#endif
+
// Converts back to linear
test_write_read(Encoding::kSRGB, Encoding::kSRGB, Encoding::kLinear, error,
check_srgb_to_linear_conversion, context, reporter);
@@ -307,9 +314,15 @@
// are all the same as the above cases where the original data was untagged.
test_write_read(Encoding::kSRGB, Encoding::kLinear, Encoding::kSRGB, error,
check_linear_to_srgb_conversion, context, reporter);
+#ifdef SK_LEGACY_GPU_PIXEL_OPS
// TODO: Fail or don't convert?
test_write_read(Encoding::kSRGB, Encoding::kLinear, Encoding::kUntagged, error,
check_linear_to_srgb_to_linear_conversion, context, reporter);
+#else
+ // When the dst buffer is untagged there should be no conversion on the read.
+ test_write_read(Encoding::kSRGB, Encoding::kLinear, Encoding::kUntagged, error,
+ check_linear_to_srgb_conversion, context, reporter);
+#endif
test_write_read(Encoding::kSRGB, Encoding::kLinear, Encoding::kLinear, error,
check_linear_to_srgb_to_linear_conversion, context, reporter);
@@ -317,13 +330,13 @@
// 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}) {
- // The read from untagged to sRGB also does no conversion. TODO: Should it just fail?
+ // The read from untagged to sRGB also does no conversion.
test_write_read(Encoding::kUntagged, writeEncoding, Encoding::kSRGB, error,
check_no_conversion, context, reporter);
// Reading untagged back as untagged should do no conversion.
test_write_read(Encoding::kUntagged, writeEncoding, Encoding::kUntagged, error,
check_no_conversion, context, reporter);
- // Reading untagged back as linear does no conversion. TODO: Should it just fail?
+ // Reading untagged back as linear does no conversion.
test_write_read(Encoding::kUntagged, writeEncoding, Encoding::kLinear, error,
check_no_conversion, context, reporter);
}
@@ -334,18 +347,18 @@
// converts back to sRGB on read.
test_write_read(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?
+ // Reading untagged data from linear currently does no conversion.
test_write_read(Encoding::kLinear, Encoding::kSRGB, Encoding::kUntagged, error,
check_srgb_to_linear_conversion, context, reporter);
// Stays linear when read.
test_write_read(Encoding::kLinear, Encoding::kSRGB, Encoding::kLinear, error,
check_srgb_to_linear_conversion, context, reporter);
+#ifdef SK_LEGACY_GPU_PIXEL_OPS
///////////////////////////////////////////////////////////////////////////////////////////////
// Write untagged data to a linear context. Currently does no conversion. TODO: Should this
// fail?
-#ifdef SK_LEGACY_GPU_PIXEL_OPS
// Reading to sRGB does a conversion.
test_write_read(Encoding::kLinear, Encoding::kUntagged, Encoding::kSRGB, error,
check_linear_to_srgb_conversion, context, reporter);
@@ -366,7 +379,7 @@
// Reading to sRGB does a conversion.
test_write_read(Encoding::kLinear, Encoding::kLinear, Encoding::kSRGB, error,
check_linear_to_srgb_conversion, context, reporter);
- // Reading to untagged does no conversion. TODO: Should it fail?
+ // Reading to untagged does no conversion.
test_write_read(Encoding::kLinear, Encoding::kLinear, Encoding::kUntagged, error,
check_no_conversion, context, reporter);
// Stays linear when read.