Make it so that GrSurfaceContext with a sRGB GrPixelConfig must have a

color space with a sRGB-like gamma.

Change-Id: I99b80a9846caacd6848b0f9f55ed0f7f23e69b90
Reviewed-on: https://skia-review.googlesource.com/106640
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This commit is contained in:
Brian Salomon 2018-02-13 09:25:22 -05:00 committed by Skia Commit-Bot
parent a3cc32c945
commit 366093f212
15 changed files with 109 additions and 50 deletions

View File

@ -409,9 +409,12 @@ public:
if (!rec) {
return false;
}
sk_sp<SkColorSpace> colorSpace;
if (GrPixelConfigIsSRGB(fTextureProxy->config())) {
colorSpace = SkColorSpace::MakeSRGB();
}
sk_sp<GrSurfaceContext> sContext = fContext->contextPriv().makeWrappedSurfaceContext(
fTextureProxy);
fTextureProxy, std::move(colorSpace));
if (!sContext) {
return false;
}

View File

@ -184,10 +184,14 @@ sk_sp<GrTextureProxy> GrBackendTextureImageGenerator::onGenerateTexture(
// because Vulkan will want to do the copy as a draw. All other copies would require a
// layout change in Vulkan and we do not change the layout of borrowed images.
GrMipMapped mipMapped = willNeedMipMaps ? GrMipMapped::kYes : GrMipMapped::kNo;
sk_sp<SkColorSpace> colorSpace;
if (GrPixelConfigIsSRGB(desc.fConfig)) {
colorSpace = SkColorSpace::MakeSRGB();
}
sk_sp<GrRenderTargetContext> rtContext(context->makeDeferredRenderTargetContext(
SkBackingFit::kExact, info.width(), info.height(), proxy->config(), nullptr, 1,
mipMapped, proxy->origin(), nullptr, SkBudgeted::kYes));
SkBackingFit::kExact, info.width(), info.height(), proxy->config(),
std::move(colorSpace), 1, mipMapped, proxy->origin(), nullptr, SkBudgeted::kYes));
if (!rtContext) {
return nullptr;

View File

@ -707,16 +707,22 @@ bool GrContextPriv::readSurfacePixels(GrSurfaceContext* src,
}
// TODO: Need to decide the semantics of this function for color spaces. Do we support
// conversion to a passed-in color space? For now, specifying nullptr means that this
// path will do no conversion, so it will match the behavior of the non-draw path.
sk_sp<GrRenderTargetContext> tempRTC = fContext->makeDeferredRenderTargetContext(
tempDrawInfo.fTempSurfaceFit,
tempDrawInfo.fTempSurfaceDesc.fWidth,
tempDrawInfo.fTempSurfaceDesc.fHeight,
tempDrawInfo.fTempSurfaceDesc.fConfig,
nullptr,
tempDrawInfo.fTempSurfaceDesc.fSampleCnt,
GrMipMapped::kNo,
tempDrawInfo.fTempSurfaceDesc.fOrigin);
// path will do no conversion, so it will match the behavior of the non-draw path. For
// now we simply infer an sRGB color space if the config is sRGB in order to avoid an
// illegal combination.
sk_sp<SkColorSpace> colorSpace;
if (GrPixelConfigIsSRGB(tempDrawInfo.fTempSurfaceDesc.fConfig)) {
colorSpace = SkColorSpace::MakeSRGB();
}
sk_sp<GrRenderTargetContext> tempRTC =
fContext->makeDeferredRenderTargetContext(tempDrawInfo.fTempSurfaceFit,
tempDrawInfo.fTempSurfaceDesc.fWidth,
tempDrawInfo.fTempSurfaceDesc.fHeight,
tempDrawInfo.fTempSurfaceDesc.fConfig,
std::move(colorSpace),
tempDrawInfo.fTempSurfaceDesc.fSampleCnt,
GrMipMapped::kNo,
tempDrawInfo.fTempSurfaceDesc.fOrigin);
if (tempRTC) {
SkMatrix textureMatrix = SkMatrix::MakeTrans(SkIntToScalar(left), SkIntToScalar(top));
sk_sp<GrTextureProxy> proxy = src->asTextureProxyRef();
@ -821,6 +827,12 @@ sk_sp<GrSurfaceContext> GrContextPriv::makeWrappedSurfaceContext(sk_sp<GrSurface
const SkSurfaceProps* props) {
ASSERT_SINGLE_OWNER_PRIV
// sRGB pixel configs may only be used with near-sRGB gamma color spaces.
if (GrPixelConfigIsSRGB(proxy->config())) {
if (!colorSpace || !colorSpace->gammaCloseToSRGB()) {
return nullptr;
}
}
if (proxy->asRenderTargetProxy()) {
return this->drawingManager()->makeRenderTargetContext(std::move(proxy),
std::move(colorSpace), props);
@ -834,8 +846,9 @@ sk_sp<GrSurfaceContext> GrContextPriv::makeWrappedSurfaceContext(sk_sp<GrSurface
sk_sp<GrSurfaceContext> GrContextPriv::makeDeferredSurfaceContext(const GrSurfaceDesc& dstDesc,
GrMipMapped mipMapped,
SkBackingFit fit,
SkBudgeted isDstBudgeted) {
SkBudgeted isDstBudgeted,
sk_sp<SkColorSpace> colorSpace,
const SkSurfaceProps* props) {
sk_sp<GrTextureProxy> proxy;
if (GrMipMapped::kNo == mipMapped) {
proxy = this->proxyProvider()->createProxy(dstDesc, fit, isDstBudgeted);
@ -847,7 +860,7 @@ sk_sp<GrSurfaceContext> GrContextPriv::makeDeferredSurfaceContext(const GrSurfac
return nullptr;
}
return this->makeWrappedSurfaceContext(std::move(proxy));
return this->makeWrappedSurfaceContext(std::move(proxy), std::move(colorSpace), props);
}
sk_sp<GrTextureContext> GrContextPriv::makeBackendTextureContext(const GrBackendTexture& tex,

View File

@ -38,7 +38,9 @@ public:
sk_sp<GrSurfaceContext> makeDeferredSurfaceContext(const GrSurfaceDesc&,
GrMipMapped,
SkBackingFit,
SkBudgeted);
SkBudgeted,
sk_sp<SkColorSpace> colorSpace = nullptr,
const SkSurfaceProps* = nullptr);
sk_sp<GrTextureContext> makeBackendTextureContext(const GrBackendTexture& tex,
GrSurfaceOrigin origin,

View File

@ -474,9 +474,8 @@ sk_sp<GrRenderTargetContext> GrDrawingManager::makeRenderTargetContext(
}
// SkSurface catches bad color space usage at creation. This check handles anything that slips
// by, including internal usage. We allow a null color space here, for read/write pixels and
// other special code paths. If a color space is provided, though, enforce all other rules.
if (colorSpace && !SkSurface_Gpu::Valid(fContext, sProxy->config(), colorSpace.get())) {
// by, including internal usage.
if (!SkSurface_Gpu::Valid(fContext, sProxy->config(), colorSpace.get())) {
SkDEBUGFAIL("Invalid config and colorspace combination");
return nullptr;
}
@ -497,9 +496,8 @@ sk_sp<GrTextureContext> GrDrawingManager::makeTextureContext(sk_sp<GrSurfaceProx
}
// SkSurface catches bad color space usage at creation. This check handles anything that slips
// by, including internal usage. We allow a null color space here, for read/write pixels and
// other special code paths. If a color space is provided, though, enforce all other rules.
if (colorSpace && !SkSurface_Gpu::Valid(fContext, sProxy->config(), colorSpace.get())) {
// by, including internal usage.
if (!SkSurface_Gpu::Valid(fContext, sProxy->config(), colorSpace.get())) {
SkDEBUGFAIL("Invalid config and colorspace combination");
return nullptr;
}

View File

@ -135,8 +135,17 @@ sk_sp<GrTexture> GrResourceProvider::createTexture(const GrSurfaceDesc& desc,
fit,
budgeted);
if (proxy) {
// We use an ephemeral surface context to do the write pixels. Here it isn't clear what
// color space to tag it with. That's ok because GrSurfaceContext::writePixels doesn't
// do any color space conversions. Though, that is likely to change. However, if the
// pixel config is sRGB then the passed color space here must have sRGB gamma or
// GrSurfaceContext creation fails.
sk_sp<SkColorSpace> colorSpace;
if (GrPixelConfigIsSRGB(desc.fConfig)) {
colorSpace = SkColorSpace::MakeSRGB();
}
sk_sp<GrSurfaceContext> sContext = context->contextPriv().makeWrappedSurfaceContext(
std::move(proxy));
std::move(proxy), std::move(colorSpace));
if (sContext) {
if (sContext->writePixels(srcInfo, mipLevel.fPixels, mipLevel.fRowBytes, 0, 0)) {
return sk_ref_sp(sContext->asTextureProxy()->priv().peekTexture());

View File

@ -36,6 +36,9 @@ GrSurfaceContext::GrSurfaceContext(GrContext* context,
, fSingleOwner(singleOwner)
#endif
{
// We never should have a sRGB pixel config with a non-SRGB gamma color space.
SkASSERT(!GrPixelConfigIsSRGB(config) ||
(fColorSpaceInfo.colorSpace() && fColorSpaceInfo.colorSpace()->gammaCloseToSRGB()));
}
bool GrSurfaceContext::readPixels(const SkImageInfo& dstInfo, void* dstBuffer,

View File

@ -275,11 +275,17 @@ sk_sp<GrTextureProxy> GrSurfaceProxy::Copy(GrContext* context,
dstDesc.fHeight = srcRect.height();
dstDesc.fConfig = src->config();
// We use an ephemeral surface context to make the copy. Here it isn't clear what color space
// to tag it with. That's ok because GrSurfaceContext::copy doesn't do any color space
// conversions. However, if the pixel config is sRGB then the passed color space here must
// have sRGB gamma or GrSurfaceContext creation fails. See skbug.com/7611 about making this
// with the correct color space information and returning the context to the caller.
sk_sp<SkColorSpace> colorSpace;
if (GrPixelConfigIsSRGB(dstDesc.fConfig)) {
colorSpace = SkColorSpace::MakeSRGB();
}
sk_sp<GrSurfaceContext> dstContext(context->contextPriv().makeDeferredSurfaceContext(
dstDesc,
mipMapped,
SkBackingFit::kExact,
budgeted));
dstDesc, mipMapped, SkBackingFit::kExact, budgeted, std::move(colorSpace)));
if (!dstContext) {
return nullptr;
}

View File

@ -24,9 +24,13 @@ sk_sp<GrTextureProxy> GrTextureProducer::CopyOnGpu(GrContext* context,
const SkRect dstRect = SkRect::MakeIWH(copyParams.fWidth, copyParams.fHeight);
GrMipMapped mipMapped = dstWillRequireMipMaps ? GrMipMapped::kYes : GrMipMapped::kNo;
sk_sp<SkColorSpace> colorSpace;
if (GrPixelConfigIsSRGB(inputProxy->config())) {
colorSpace = SkColorSpace::MakeSRGB();
}
sk_sp<GrRenderTargetContext> copyRTC = context->makeDeferredRenderTargetContextWithFallback(
SkBackingFit::kExact, dstRect.width(), dstRect.height(), inputProxy->config(), nullptr,
1, mipMapped, inputProxy->origin());
SkBackingFit::kExact, dstRect.width(), dstRect.height(), inputProxy->config(),
std::move(colorSpace), 1, mipMapped, inputProxy->origin());
if (!copyRTC) {
return nullptr;
}

View File

@ -109,15 +109,16 @@ sk_sp<GrTextureProxy> GrYUVProvider::refAsTextureProxy(GrContext* ctx, const GrS
1, SkBudgeted::kYes, fit);
}
// We never want to perform color-space conversion during the decode
// We never want to perform color-space conversion during the decode. However, if the proxy
// config is sRGB then we must use a sRGB color space.
sk_sp<SkColorSpace> colorSpace;
if (GrPixelConfigIsSRGB(desc.fConfig)) {
colorSpace = SkColorSpace::MakeSRGB();
}
// TODO: investigate preallocating mip maps here
sk_sp<GrRenderTargetContext> renderTargetContext(ctx->makeDeferredRenderTargetContext(
SkBackingFit::kExact,
desc.fWidth, desc.fHeight,
desc.fConfig, nullptr,
desc.fSampleCnt,
GrMipMapped::kNo,
kTopLeft_GrSurfaceOrigin));
SkBackingFit::kExact, desc.fWidth, desc.fHeight, desc.fConfig, std::move(colorSpace),
desc.fSampleCnt, GrMipMapped::kNo, kTopLeft_GrSurfaceOrigin));
if (!renderTargetContext) {
return nullptr;
}

View File

@ -131,7 +131,12 @@ sk_sp<GrTextureProxy> GrCopyBaseMipMapToTextureProxy(GrContext* ctx, GrTexturePr
}
// Copy the base layer to our proxy
sk_sp<GrSurfaceContext> sContext = ctx->contextPriv().makeWrappedSurfaceContext(proxy);
sk_sp<SkColorSpace> colorSpace;
if (GrPixelConfigIsSRGB(proxy->config())) {
colorSpace = SkColorSpace::MakeSRGB();
}
sk_sp<GrSurfaceContext> sContext =
ctx->contextPriv().makeWrappedSurfaceContext(proxy, std::move(colorSpace));
SkASSERT(sContext);
SkAssertResult(sContext->copy(baseProxy));

View File

@ -230,12 +230,18 @@ bool SkImage_Gpu::onReadPixels(const SkImageInfo& dstInfo, void* dstPixels, size
// with arbitrary color spaces. Unfortunately, this is one spot where we go from image to
// surface (rather than the opposite), and our lenient image rules break our (currently) more
// strict surface rules.
// We treat null-dst color space as always equal to fColorSpace for this kind of read-back.
// GrSurfaceContext::readPixels does not make use of the context's color space. However, we
// don't allow creating a surface context for a sRGB GrPixelConfig unless the color space has
// sRGB gamma. So we choose null for non-SRGB GrPixelConfigs and sRGB for sRGB GrPixelConfigs.
sk_sp<SkColorSpace> surfaceColorSpace = fColorSpace;
if (!flags) {
if (!dstInfo.colorSpace() ||
SkColorSpace::Equals(fColorSpace.get(), dstInfo.colorSpace())) {
surfaceColorSpace = nullptr;
SkColorSpace::Equals(fColorSpace.get(), dstInfo.colorSpace())) {
if (GrPixelConfigIsSRGB(fProxy->config())) {
surfaceColorSpace = SkColorSpace::MakeSRGB();
} else {
surfaceColorSpace = nullptr;
}
}
}

View File

@ -154,6 +154,10 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(InitialTextureClear, reporter, context_info)
for (int c = 0; c <= kLast_GrPixelConfig; ++c) {
desc.fConfig = static_cast<GrPixelConfig>(c);
sk_sp<SkColorSpace> colorSpace;
if (GrPixelConfigIsSRGB(desc.fConfig)) {
colorSpace = SkColorSpace::MakeSRGB();
}
if (!context_info.grContext()->caps()->isConfigTexturable(desc.fConfig)) {
continue;
}
@ -175,9 +179,8 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(InitialTextureClear, reporter, context_info)
if (!proxy) {
continue;
}
auto texCtx = context->contextPriv().makeWrappedSurfaceContext(
std::move(proxy));
std::move(proxy), colorSpace);
SkImageInfo info = SkImageInfo::Make(
kSize, kSize, kRGBA_8888_SkColorType, kPremul_SkAlphaType);
memset(data.get(), 0xAB, kSize * kSize * sizeof(uint32_t));
@ -202,7 +205,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(InitialTextureClear, reporter, context_info)
// Try creating the texture as a deferred proxy.
for (int i = 0; i < 2; ++i) {
auto surfCtx = context->contextPriv().makeDeferredSurfaceContext(
desc, GrMipMapped::kNo, fit, SkBudgeted::kYes);
desc, GrMipMapped::kNo, fit, SkBudgeted::kYes, colorSpace);
if (!surfCtx) {
continue;
}

View File

@ -178,8 +178,12 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ReadWriteAlpha, reporter, ctxInfo) {
continue;
}
sk_sp<SkColorSpace> colorSpace;
if (GrPixelConfigIsSRGB(proxy->config())) {
colorSpace = SkColorSpace::MakeSRGB();
}
sk_sp<GrSurfaceContext> sContext = context->contextPriv().makeWrappedSurfaceContext(
std::move(proxy));
std::move(proxy), std::move(colorSpace));
for (auto rowBytes : kRowBytes) {
size_t nonZeroRowBytes = rowBytes ? rowBytes : X_SIZE;

View File

@ -174,11 +174,9 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SRGBReadWritePixels, reporter, ctxInfo) {
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);
desc, GrMipMapped::kNo, SkBackingFit::kExact, SkBudgeted::kNo,
SkColorSpace::MakeSRGB());
if (!sContext) {
ERRORF(reporter, "Could not create SRGBA surface context.");
return;