Dest color space no longer impacts mipmaps or texture sampling
PS5: Removes SkDestinationSurfaceColorMode, tracking of mipmap
mode on GrTexture, sRGB decode state per-texture. Because we
were often choosing sRGB configs for RGB color types, legacy
rendering would then be incorrect (too dark). So...
PS7: Stops ever using sRGB pixel configs when translating
image info or color type. Also removes a bunch of GrCaps bits
and a GrContextOption that are no longer relevant.
PS9: Adjusts surface creation unit test expectations, and
changes the raster rules accordingly.
At this point, sRGB configs are (obviously) going to be broken.
Locally, I ran 8888, gl, and the gbr- versions of both. Across
all GMs x configs, there are 13 diffs. 12 are GMs that create
surfaces with a color-space attached (and thus, the offscreen
is no longer getting sRGB pixel config). The only remainder
constructs an SkPictureImageGenerator, (with an attached color
space) and renders it to the gbr-gl canvas, which triggers a
a tagged surface inside the generator.
Bug: skia:
Change-Id: Ie5edfa157dd799f3121e8173fc4f97f6c8ed6789
Reviewed-on: https://skia-review.googlesource.com/131282
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
diff --git a/tests/GLProgramsTest.cpp b/tests/GLProgramsTest.cpp
index 6910ab5..81fb21f 100644
--- a/tests/GLProgramsTest.cpp
+++ b/tests/GLProgramsTest.cpp
@@ -252,9 +252,6 @@
if (random->nextBool()) {
paint->setDisableOutputConversionToSRGB(true);
}
- if (random->nextBool()) {
- paint->setAllowSRGBInputs(true);
- }
}
#endif
diff --git a/tests/GrContextFactoryTest.cpp b/tests/GrContextFactoryTest.cpp
index 92801eb..b05f2e2 100644
--- a/tests/GrContextFactoryTest.cpp
+++ b/tests/GrContextFactoryTest.cpp
@@ -47,28 +47,6 @@
}
}
-DEF_GPUTEST(GrContextFactory_RequiredSRGBSupport, reporter, options) {
- // Test that if sRGB support is requested, the context always has that capability
- // or the context creation fails. Also test that if the creation fails, a context
- // created without that flag would not have had sRGB support.
- for (int i = 0; i < GrContextFactory::kContextTypeCnt; ++i) {
- GrContextFactory testFactory(options);
- // Test that if sRGB is requested, caps are in sync.
- GrContextFactory::ContextType ctxType = static_cast<GrContextFactory::ContextType>(i);
- GrContext* context =
- testFactory.get(ctxType, GrContextFactory::ContextOverrides::kRequireSRGBSupport);
-
- if (context) {
- REPORTER_ASSERT(reporter, context->contextPriv().caps()->srgbSupport());
- } else {
- context = testFactory.get(ctxType);
- if (context) {
- REPORTER_ASSERT(reporter, !context->contextPriv().caps()->srgbSupport());
- }
- }
- }
-}
-
DEF_GPUTEST(GrContextFactory_abandon, reporter, options) {
for (int i = 0; i < GrContextFactory::kContextTypeCnt; ++i) {
GrContextFactory testFactory(options);
diff --git a/tests/ImageTest.cpp b/tests/ImageTest.cpp
index 9d03948..930d7c0 100644
--- a/tests/ImageTest.cpp
+++ b/tests/ImageTest.cpp
@@ -485,7 +485,7 @@
bool can = ctxInfo.grContext()->colorTypeSupportedAsImage(colorType);
auto* gpu = ctxInfo.grContext()->contextPriv().getGpu();
GrBackendTexture backendTex = gpu->createTestingOnlyBackendTexture(
- nullptr, kSize, kSize, colorType, nullptr, false, GrMipMapped::kNo);
+ nullptr, kSize, kSize, colorType, false, GrMipMapped::kNo);
auto img =
SkImage::MakeFromTexture(ctxInfo.grContext(), backendTex, kTopLeft_GrSurfaceOrigin,
colorType, kOpaque_SkAlphaType, nullptr);
diff --git a/tests/MipMapTest.cpp b/tests/MipMapTest.cpp
index 7e0a653..938e7234 100644
--- a/tests/MipMapTest.cpp
+++ b/tests/MipMapTest.cpp
@@ -23,7 +23,7 @@
int width = 1 + rand.nextU() % 1000;
int height = 1 + rand.nextU() % 1000;
make_bitmap(&bm, width, height);
- sk_sp<SkMipMap> mm(SkMipMap::Build(bm, SkDestinationSurfaceColorMode::kLegacy, nullptr));
+ sk_sp<SkMipMap> mm(SkMipMap::Build(bm, nullptr));
REPORTER_ASSERT(reporter, mm->countLevels() == SkMipMap::ComputeLevelCount(width, height));
REPORTER_ASSERT(reporter, !mm->extractLevel(SkSize::Make(SK_Scalar1, SK_Scalar1),
@@ -60,7 +60,7 @@
SkBitmap bm;
bm.allocN32Pixels(width, height);
bm.eraseColor(SK_ColorWHITE);
- sk_sp<SkMipMap> mm(SkMipMap::Build(bm, SkDestinationSurfaceColorMode::kLegacy, nullptr));
+ sk_sp<SkMipMap> mm(SkMipMap::Build(bm, nullptr));
const int mipLevelCount = mm->countLevels();
REPORTER_ASSERT(reporter, mipLevelCount == expectedMipLevelCount);
@@ -90,7 +90,7 @@
SkBitmap bm;
bm.allocN32Pixels(1, 1);
bm.eraseColor(SK_ColorWHITE);
- sk_sp<SkMipMap> mm(SkMipMap::Build(bm, SkDestinationSurfaceColorMode::kLegacy, nullptr));
+ sk_sp<SkMipMap> mm(SkMipMap::Build(bm, nullptr));
REPORTER_ASSERT(reporter, mm == nullptr);
}
diff --git a/tests/OnFlushCallbackTest.cpp b/tests/OnFlushCallbackTest.cpp
index e980e99..463e44f 100644
--- a/tests/OnFlushCallbackTest.cpp
+++ b/tests/OnFlushCallbackTest.cpp
@@ -477,7 +477,7 @@
save_bm(bm, "atlas-fake.png");
#endif
- GrSurfaceDesc desc = GrImageInfoToSurfaceDesc(bm.info(), *context->caps());
+ GrSurfaceDesc desc = GrImageInfoToSurfaceDesc(bm.info());
desc.fFlags |= kRenderTarget_GrSurfaceFlag;
sk_sp<GrSurfaceProxy> tmp = GrSurfaceProxy::MakeDeferred(*context->caps(),
diff --git a/tests/ProxyTest.cpp b/tests/ProxyTest.cpp
index 19e0598..a1f8b8b 100644
--- a/tests/ProxyTest.cpp
+++ b/tests/ProxyTest.cpp
@@ -223,7 +223,7 @@
}
for (auto numSamples : {1, 4}) {
- GrPixelConfig config = SkImageInfo2GrPixelConfig(colorType, nullptr, caps);
+ GrPixelConfig config = SkColorType2GrPixelConfig(colorType);
SkASSERT(kUnknown_GrPixelConfig != config);
int supportedNumSamples = caps.getRenderTargetSampleCount(numSamples, config);
@@ -253,7 +253,7 @@
{
GrBackendTexture backendTex =
gpu->createTestingOnlyBackendTexture(nullptr, kWidthHeight,
- kWidthHeight, colorType, nullptr,
+ kWidthHeight, colorType,
true, GrMipMapped::kNo);
sk_sp<GrSurfaceProxy> sProxy = proxyProvider->wrapBackendTextureAsRenderTarget(
backendTex, origin, supportedNumSamples);
@@ -277,7 +277,7 @@
{
GrBackendTexture backendTex =
gpu->createTestingOnlyBackendTexture(nullptr, kWidthHeight,
- kWidthHeight, colorType, nullptr,
+ kWidthHeight, colorType,
true, GrMipMapped::kNo);
sk_sp<GrSurfaceProxy> sProxy = proxyProvider->wrapRenderableBackendTexture(
@@ -303,7 +303,7 @@
// Internal offscreen texture
GrBackendTexture backendTex =
gpu->createTestingOnlyBackendTexture(nullptr, kWidthHeight,
- kWidthHeight, colorType, nullptr,
+ kWidthHeight, colorType,
false, GrMipMapped::kNo);
sk_sp<GrSurfaceProxy> sProxy = proxyProvider->wrapBackendTexture(
diff --git a/tests/SRGBMipMapTest.cpp b/tests/SRGBMipMapTest.cpp
deleted file mode 100644
index ac7b5cc..0000000
--- a/tests/SRGBMipMapTest.cpp
+++ /dev/null
@@ -1,186 +0,0 @@
-/*
- * Copyright 2016 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#include "Test.h"
-#include "GrCaps.h"
-#include "GrClip.h"
-#include "GrContext.h"
-#include "GrContextPriv.h"
-#include "GrProxyProvider.h"
-#include "GrRenderTargetContext.h"
-#include "SkCanvas.h"
-#include "SkGr.h"
-#include "SkSurface.h"
-#include "gl/GrGLGpu.h"
-
-// using anonymous namespace because these functions are used as template params.
-namespace {
-/** convert 0..1 srgb value to 0..1 linear */
-float srgb_to_linear(float srgb) {
- if (srgb <= 0.04045f) {
- return srgb / 12.92f;
- } else {
- return powf((srgb + 0.055f) / 1.055f, 2.4f);
- }
-}
-
-/** convert 0..1 linear value to 0..1 srgb */
-float linear_to_srgb(float linear) {
- if (linear <= 0.0031308) {
- return linear * 12.92f;
- } else {
- return 1.055f * powf(linear, 1.f / 2.4f) - 0.055f;
- }
-}
-}
-
-static bool check_value(U8CPU value, U8CPU expected, U8CPU error) {
- if (value >= expected) {
- return (value - expected) <= error;
- } else {
- return (expected - value) <= error;
- }
-}
-
-void read_and_check_pixels(skiatest::Reporter* reporter, GrSurfaceContext* context,
- U8CPU expected, const SkImageInfo& dstInfo,
- U8CPU error, const char* subtestName) {
- int w = dstInfo.width();
- int h = dstInfo.height();
- SkAutoTMalloc<uint32_t> readData(w * h);
- memset(readData.get(), 0, sizeof(uint32_t) * w * h);
-
- if (!context->readPixels(dstInfo, readData.get(), 0, 0, 0)) {
- ERRORF(reporter, "Could not read pixels for %s.", subtestName);
- return;
- }
-
- for (int j = 0; j < h; ++j) {
- for (int i = 0; i < w; ++i) {
- uint32_t read = readData[j * w + i];
-
- bool success =
- check_value(read & 0xff, expected, error) &&
- check_value((read >> 8) & 0xff, expected, error) &&
- check_value((read >> 16) & 0xff, expected, error);
-
- if (!success) {
- ERRORF(reporter, "Expected 0xff%02x%02x%02x, read back as 0x%08x in %s at %d, %d.",
- expected, expected, expected, read, subtestName, i, j);
- return;
- }
- }
- }
-}
-
-DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(SRGBMipMaps, reporter, ctxInfo) {
- GrContext* context = ctxInfo.grContext();
- if (!context->contextPriv().caps()->srgbSupport()) {
- return;
- }
-
- const int rtS = 16;
- const int texS = rtS * 2;
-
- // Fill texture with a dither of black and 60% sRGB (~ 32.5% linear) gray. Although there is
- // only one likely failure mode (doing a direct downsample of the sRGB values), this pattern
- // maximizes the minimum error across all three conceivable failure modes:
- // 1) Likely incorrect:
- // (A + B) / 2
- // 2) No input decode, decode output:
- // linear_to_srgb((A + B) / 2)
- // 3) Decode input, no output encode:
- // (srgb_to_linear(A) + srgb_to_linear(B)) / 2
-
- const U8CPU srgb60 = sk_float_round2int(0.6f * 255.0f);
- static const SkPMColor colors[2] = {
- SkPackARGB32(0xFF, srgb60, srgb60, srgb60),
- SkPackARGB32(0xFF, 0x00, 0x00, 0x00)
- };
- uint32_t texData[texS * texS];
- for (int y = 0; y < texS; ++y) {
- for (int x = 0; x < texS; ++x) {
- texData[y * texS + x] = colors[(x + y) % 2];
- }
- }
-
- // We can be pretty generous with the error detection, thanks to the choice of input.
- // The closest likely failure mode is off by > 0.1, so anything that encodes within
- // 10/255 of optimal is more than good enough for this test.
- const U8CPU expectedSRGB = sk_float_round2int(
- linear_to_srgb(srgb_to_linear(srgb60 / 255.0f) / 2.0f) * 255.0f);
- const U8CPU expectedLinear = srgb60 / 2;
- const U8CPU error = 10;
-
- const SkImageInfo iiSRGBA = SkImageInfo::Make(rtS, rtS, kRGBA_8888_SkColorType,
- kPremul_SkAlphaType,
- SkColorSpace::MakeSRGB());
- const SkImageInfo iiRGBA = SkImageInfo::Make(rtS, rtS, kRGBA_8888_SkColorType,
- kPremul_SkAlphaType);
-
- // Create our test texture
- GrSurfaceDesc desc;
- desc.fFlags = kNone_GrSurfaceFlags;
- desc.fWidth = texS;
- desc.fHeight = texS;
- desc.fConfig = kSRGBA_8888_GrPixelConfig;
-
- GrProxyProvider* proxyProvider = context->contextPriv().proxyProvider();
- sk_sp<GrTextureProxy> proxy =
- proxyProvider->createTextureProxy(desc, SkBudgeted::kNo, texData, 0);
-
- // Create two render target contexts (L32 and S32)
- sk_sp<SkColorSpace> srgbColorSpace = SkColorSpace::MakeSRGB();
- sk_sp<GrRenderTargetContext> l32RenderTargetContext =
- context->contextPriv().makeDeferredRenderTargetContext(
- SkBackingFit::kExact, rtS, rtS,
- kRGBA_8888_GrPixelConfig, nullptr);
- sk_sp<GrRenderTargetContext> s32RenderTargetContext =
- context->contextPriv().makeDeferredRenderTargetContext(
- SkBackingFit::kExact, rtS, rtS,
- kSRGBA_8888_GrPixelConfig, std::move(srgbColorSpace));
-
- SkRect rect = SkRect::MakeWH(SkIntToScalar(rtS), SkIntToScalar(rtS));
- GrNoClip noClip;
- GrPaint paint;
- paint.setPorterDuffXPFactory(SkBlendMode::kSrc);
- GrSamplerState mipMapSamplerState(GrSamplerState::WrapMode::kRepeat,
- GrSamplerState::Filter::kMipMap);
- paint.addColorTextureProcessor(std::move(proxy), SkMatrix::MakeScale(rtS), mipMapSamplerState);
-
- // 1) Draw texture to S32 surface (should generate/use sRGB mips)
- paint.setGammaCorrect(true);
- s32RenderTargetContext->drawRect(noClip, GrPaint::Clone(paint), GrAA::kNo, SkMatrix::I(), rect);
- read_and_check_pixels(reporter, s32RenderTargetContext.get(), expectedSRGB, iiSRGBA, error,
- "first render of sRGB");
-
- // 2) Draw texture to L32 surface (should generate/use linear mips)
- paint.setGammaCorrect(false);
- l32RenderTargetContext->drawRect(noClip, GrPaint::Clone(paint), GrAA::kNo, SkMatrix::I(), rect);
-
- // Right now, this test only runs on GL (because Vulkan doesn't support legacy mip-mapping
- // skbug.com/5048). On GL, we may not have sRGB decode support. In that case, rendering sRGB
- // textures to a legacy surface produces nonsense, so this part of the test is meaningless.
- //
- // We also skip this part of the test on command buffer (via srgbDecodeDisableAffectsMipmaps),
- // because that implementation of the extension doesn't ensure that mips respect the setting.
- //
- // TODO: Once Vulkan supports legacy mip-mapping, we can promote this to GrCaps. Right now,
- // Vulkan has most of the functionality, but not the mip-mapping part that's being tested here.
- GrGLGpu* glGpu = static_cast<GrGLGpu*>(context->contextPriv().getGpu());
- if (glGpu->glCaps().srgbDecodeDisableSupport() &&
- glGpu->glCaps().srgbDecodeDisableAffectsMipmaps()) {
- read_and_check_pixels(reporter, l32RenderTargetContext.get(), expectedLinear, iiRGBA,
- error, "re-render as linear");
- }
-
- // 3) Go back to sRGB
- paint.setGammaCorrect(true);
- s32RenderTargetContext->drawRect(noClip, std::move(paint), GrAA::kNo, SkMatrix::I(), rect);
- read_and_check_pixels(reporter, s32RenderTargetContext.get(), expectedSRGB, iiSRGBA, error,
- "re-render as sRGB");
-}
diff --git a/tests/SkResourceCacheTest.cpp b/tests/SkResourceCacheTest.cpp
index 02b75dd..341eb07 100644
--- a/tests/SkResourceCacheTest.cpp
+++ b/tests/SkResourceCacheTest.cpp
@@ -44,18 +44,14 @@
src.allocN32Pixels(5, 5);
src.setImmutable();
- const SkDestinationSurfaceColorMode colorMode = SkDestinationSurfaceColorMode::kLegacy;
-
- const SkMipMap* mipmap = SkMipMapCache::FindAndRef(SkBitmapCacheDesc::Make(src), colorMode,
- cache);
+ const SkMipMap* mipmap = SkMipMapCache::FindAndRef(SkBitmapCacheDesc::Make(src), cache);
REPORTER_ASSERT(reporter, nullptr == mipmap);
- mipmap = SkMipMapCache::AddAndRef(src, colorMode, cache);
+ mipmap = SkMipMapCache::AddAndRef(src, cache);
REPORTER_ASSERT(reporter, mipmap);
{
- const SkMipMap* mm = SkMipMapCache::FindAndRef(SkBitmapCacheDesc::Make(src), colorMode,
- cache);
+ const SkMipMap* mm = SkMipMapCache::FindAndRef(SkBitmapCacheDesc::Make(src), cache);
REPORTER_ASSERT(reporter, mm);
REPORTER_ASSERT(reporter, mm == mipmap);
mm->unref();
@@ -69,7 +65,7 @@
check_data(reporter, mipmap, 1, kInCache, kNotLocked);
// find us again
- mipmap = SkMipMapCache::FindAndRef(SkBitmapCacheDesc::Make(src), colorMode, cache);
+ mipmap = SkMipMapCache::FindAndRef(SkBitmapCacheDesc::Make(src), cache);
check_data(reporter, mipmap, 2, kInCache, kLocked);
cache->purgeAll();
@@ -79,19 +75,18 @@
}
static void test_mipmap_notify(skiatest::Reporter* reporter, SkResourceCache* cache) {
- const SkDestinationSurfaceColorMode colorMode = SkDestinationSurfaceColorMode::kLegacy;
const int N = 3;
SkBitmap src[N];
for (int i = 0; i < N; ++i) {
src[i].allocN32Pixels(5, 5);
src[i].setImmutable();
- SkMipMapCache::AddAndRef(src[i], colorMode, cache)->unref();
+ SkMipMapCache::AddAndRef(src[i], cache)->unref();
}
for (int i = 0; i < N; ++i) {
const auto desc = SkBitmapCacheDesc::Make(src[i]);
- const SkMipMap* mipmap = SkMipMapCache::FindAndRef(desc, colorMode, cache);
+ const SkMipMap* mipmap = SkMipMapCache::FindAndRef(desc, cache);
if (cache) {
// if cache is null, we're working on the global cache, and other threads might purge
// it, making this check fragile.
@@ -101,7 +96,7 @@
src[i].reset(); // delete the underlying pixelref, which *should* remove us from the cache
- mipmap = SkMipMapCache::FindAndRef(desc, colorMode, cache);
+ mipmap = SkMipMapCache::FindAndRef(desc, cache);
REPORTER_ASSERT(reporter, !mipmap);
}
}
diff --git a/tests/SpecialImageTest.cpp b/tests/SpecialImageTest.cpp
index b2f8743..d2549c4 100644
--- a/tests/SpecialImageTest.cpp
+++ b/tests/SpecialImageTest.cpp
@@ -231,8 +231,7 @@
{
// gpu
- const GrSurfaceDesc desc =
- GrImageInfoToSurfaceDesc(bm.info(), *context->contextPriv().caps());
+ const GrSurfaceDesc desc = GrImageInfoToSurfaceDesc(bm.info());
sk_sp<GrTextureProxy> proxy = proxyProvider->createTextureProxy(
desc, SkBudgeted::kNo, bm.getPixels(), bm.rowBytes());
@@ -265,7 +264,7 @@
GrProxyProvider* proxyProvider = context->contextPriv().proxyProvider();
SkBitmap bm = create_bm();
- const GrSurfaceDesc desc = GrImageInfoToSurfaceDesc(bm.info(), *context->contextPriv().caps());
+ const GrSurfaceDesc desc = GrImageInfoToSurfaceDesc(bm.info());
sk_sp<GrTextureProxy> proxy =
proxyProvider->createTextureProxy(desc, SkBudgeted::kNo, bm.getPixels(), bm.rowBytes());
diff --git a/tests/SurfaceTest.cpp b/tests/SurfaceTest.cpp
index bb5291c..9a3a3aa 100644
--- a/tests/SurfaceTest.cpp
+++ b/tests/SurfaceTest.cpp
@@ -101,7 +101,7 @@
auto* gpu = ctxInfo.grContext()->contextPriv().getGpu();
GrBackendTexture backendTex = gpu->createTestingOnlyBackendTexture(
- nullptr, kSize, kSize, colorType, nullptr, true, GrMipMapped::kNo);
+ nullptr, kSize, kSize, colorType, true, GrMipMapped::kNo);
surf = SkSurface::MakeFromBackendTexture(ctxInfo.grContext(), backendTex,
kTopLeft_GrSurfaceOrigin, 0, colorType, nullptr,
nullptr);
@@ -128,8 +128,8 @@
REPORTER_ASSERT(reporter, can == SkToBool(surf), "ct: %d, can: %d, surf: %d",
colorType, can, SkToBool(surf));
- backendTex = gpu->createTestingOnlyBackendTexture(nullptr, kSize, kSize, colorType, nullptr,
- true, GrMipMapped::kNo);
+ backendTex = gpu->createTestingOnlyBackendTexture(nullptr, kSize, kSize, colorType, true,
+ GrMipMapped::kNo);
surf = SkSurface::MakeFromBackendTexture(ctxInfo.grContext(), backendTex,
kTopLeft_GrSurfaceOrigin, kSampleCnt, colorType,
nullptr, nullptr);
@@ -182,7 +182,7 @@
}
auto* gpu = ctxInfo.grContext()->contextPriv().getGpu();
GrBackendTexture backendTex = gpu->createTestingOnlyBackendTexture(
- nullptr, kSize, kSize, colorType, nullptr, true, GrMipMapped::kNo);
+ nullptr, kSize, kSize, colorType, true, GrMipMapped::kNo);
if (!backendTex.isValid()) {
continue;
}
@@ -915,9 +915,9 @@
const char* fDescription;
} testConfigs[] = {
{ kN32_SkColorType, nullptr, true, "N32-nullptr" },
- { kN32_SkColorType, linearColorSpace, false, "N32-linear" },
+ { kN32_SkColorType, linearColorSpace, true, "N32-linear" },
{ kN32_SkColorType, srgbColorSpace, true, "N32-srgb" },
- { kN32_SkColorType, oddColorSpace, false, "N32-odd" },
+ { kN32_SkColorType, oddColorSpace, true, "N32-odd" },
{ kRGBA_F16_SkColorType, nullptr, true, "F16-nullptr" },
{ kRGBA_F16_SkColorType, linearColorSpace, true, "F16-linear" },
{ kRGBA_F16_SkColorType, srgbColorSpace, true, "F16-srgb" },
@@ -978,7 +978,7 @@
GrGpu* gpu = context->contextPriv().getGpu();
static const int kSize = 10;
- GrPixelConfig config = SkImageInfo2GrPixelConfig(info, *context->contextPriv().caps());
+ GrPixelConfig config = SkImageInfo2GrPixelConfig(info);
SkASSERT(kUnknown_GrPixelConfig != config);
GrBackendTexture backendTex = gpu->createTestingOnlyBackendTexture(