Remove sRGB config checks based on color space
All of the restrictions/assumptions that led to this code are gone,
so we can always use appropriate color space.
For the YUV provider, if/when we re-introduce 8888 sRGB, the color
space will have a linear transfer function, so the color space
xform will automatically do what was happening here. That removes
the last usage of framebuffer sRGB control, so we can remove all
kinds of GrPaint and GrPipeline plumbing for that feature.
Change-Id: I24af1d498dbc75210f92f8c61b10aa31eec022f6
Reviewed-on: https://skia-review.googlesource.com/138986
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Brian Osman <brianosman@google.com>
diff --git a/src/core/SkGpuBlurUtils.cpp b/src/core/SkGpuBlurUtils.cpp
index 9ebed06..15d6fa5 100644
--- a/src/core/SkGpuBlurUtils.cpp
+++ b/src/core/SkGpuBlurUtils.cpp
@@ -91,17 +91,9 @@
SkRect::Make(dstRect), localMatrix);
}
-static GrPixelConfig get_blur_config(GrTextureProxy* proxy, SkColorSpace* cs) {
+static GrPixelConfig get_blur_config(GrTextureProxy* proxy) {
GrPixelConfig config = proxy->config();
- if (GrPixelConfigIsSRGB(config) && !cs) {
- // If the context doesn't have sRGB write control, and we make an sRGB RTC, we won't be
- // able to suppress the linear -> sRGB conversion out of the shader. Not all GL drivers
- // have that feature, and Vulkan is missing it entirely. To keep things simple, switch to
- // a non-sRGB destination, to ensure correct blurring behavior.
- config = kRGBA_8888_GrPixelConfig;
- }
-
SkASSERT(kBGRA_8888_GrPixelConfig == config || kRGBA_8888_GrPixelConfig == config ||
kRGB_888_GrPixelConfig == config || kRGBA_4444_GrPixelConfig == config ||
kRGB_565_GrPixelConfig == config || kSRGBA_8888_GrPixelConfig == config ||
@@ -123,7 +115,7 @@
const SkImageInfo& dstII,
SkBackingFit dstFit) {
- GrPixelConfig config = get_blur_config(proxy.get(), dstII.colorSpace());
+ GrPixelConfig config = get_blur_config(proxy.get());
sk_sp<GrRenderTargetContext> renderTargetContext;
renderTargetContext = context->contextPriv().makeDeferredRenderTargetContext(
@@ -163,7 +155,7 @@
SkBackingFit fit) {
SkASSERT(srcRect.width() <= dstII.width() && srcRect.height() <= dstII.height());
- GrPixelConfig config = get_blur_config(proxy.get(), dstII.colorSpace());
+ GrPixelConfig config = get_blur_config(proxy.get());
sk_sp<GrRenderTargetContext> dstRenderTargetContext;
dstRenderTargetContext = context->contextPriv().makeDeferredRenderTargetContext(
@@ -262,7 +254,7 @@
SkASSERT(SkIsPow2(scaleFactorX) && SkIsPow2(scaleFactorY));
SkASSERT(scaleFactorX > 1 || scaleFactorY > 1);
- GrPixelConfig config = get_blur_config(src.get(), dstII.colorSpace());
+ GrPixelConfig config = get_blur_config(src.get());
SkIRect srcRect;
if (GrTextureDomain::kIgnore_Mode == mode) {
@@ -381,7 +373,7 @@
srcRenderTargetContext = nullptr; // no longer needed
- GrPixelConfig config = get_blur_config(srcProxy.get(), dstII.colorSpace());
+ GrPixelConfig config = get_blur_config(srcProxy.get());
sk_sp<GrRenderTargetContext> dstRenderTargetContext =
context->contextPriv().makeDeferredRenderTargetContext(fit, dstII.width(), dstII.height(),
@@ -436,7 +428,7 @@
SkBackingFit fit) {
SkASSERT(context);
- const GrPixelConfig config = get_blur_config(srcProxy.get(), colorSpace.get());
+ const GrPixelConfig config = get_blur_config(srcProxy.get());
SkColorType ct;
if (!GrPixelConfigToColorType(config, &ct)) {
return nullptr;
diff --git a/src/core/SkSpecialImage.cpp b/src/core/SkSpecialImage.cpp
index 75946d0..0d229a1 100644
--- a/src/core/SkSpecialImage.cpp
+++ b/src/core/SkSpecialImage.cpp
@@ -407,12 +407,8 @@
if (!rec) {
return false;
}
- sk_sp<SkColorSpace> colorSpace;
- if (GrPixelConfigIsSRGB(fTextureProxy->config())) {
- colorSpace = SkColorSpace::MakeSRGB();
- }
sk_sp<GrSurfaceContext> sContext = fContext->contextPriv().makeWrappedSurfaceContext(
- fTextureProxy, std::move(colorSpace));
+ fTextureProxy, fColorSpace);
if (!sContext) {
return false;
}
diff --git a/src/gpu/GrPaint.cpp b/src/gpu/GrPaint.cpp
index bf198a8..deed0d4 100644
--- a/src/gpu/GrPaint.cpp
+++ b/src/gpu/GrPaint.cpp
@@ -15,7 +15,6 @@
: fXPFactory(that.fXPFactory)
, fColorFragmentProcessors(that.fColorFragmentProcessors.count())
, fCoverageFragmentProcessors(that.fCoverageFragmentProcessors.count())
- , fDisableOutputConversionToSRGB(that.fDisableOutputConversionToSRGB)
, fTrivial(that.fTrivial)
, fColor(that.fColor) {
for (int i = 0; i < that.fColorFragmentProcessors.count(); ++i) {
diff --git a/src/gpu/GrPaint.h b/src/gpu/GrPaint.h
index c7e52fa..5d87eb0 100644
--- a/src/gpu/GrPaint.h
+++ b/src/gpu/GrPaint.h
@@ -55,13 +55,6 @@
*/
GrColor getColor() const { return fColor.toGrColor(); }
- /**
- * Should shader output conversion from linear to sRGB be disabled.
- * Only relevant if the destination is sRGB. Defaults to false.
- */
- void setDisableOutputConversionToSRGB(bool srgb) { fDisableOutputConversionToSRGB = srgb; }
- bool getDisableOutputConversionToSRGB() const { return fDisableOutputConversionToSRGB; }
-
void setXPFactory(const GrXPFactory* xpFactory) {
fXPFactory = xpFactory;
fTrivial &= !SkToBool(xpFactory);
@@ -138,7 +131,6 @@
const GrXPFactory* fXPFactory = nullptr;
SkSTArray<4, std::unique_ptr<GrFragmentProcessor>> fColorFragmentProcessors;
SkSTArray<2, std::unique_ptr<GrFragmentProcessor>> fCoverageFragmentProcessors;
- bool fDisableOutputConversionToSRGB = false;
bool fTrivial = true;
GrColor4f fColor = GrColor4f::OpaqueWhite();
};
diff --git a/src/gpu/GrPipeline.h b/src/gpu/GrPipeline.h
index f60a6d4..3e5b643 100644
--- a/src/gpu/GrPipeline.h
+++ b/src/gpu/GrPipeline.h
@@ -52,18 +52,8 @@
* Modifies the vertex shader so that vertices will be positioned at pixel centers.
*/
kSnapVerticesToPixelCenters_Flag = 0x2,
- /** Disables conversion to sRGB from linear when writing to a sRGB destination. */
- kDisableOutputConversionToSRGB_Flag = 0x4,
};
- static uint32_t SRGBFlagsFromPaint(const GrPaint& paint) {
- uint32_t flags = 0;
- if (paint.getDisableOutputConversionToSRGB()) {
- flags |= kDisableOutputConversionToSRGB_Flag;
- }
- return flags;
- }
-
enum ScissorState : bool {
kEnabled = true,
kDisabled = false
@@ -189,9 +179,6 @@
bool snapVerticesToPixelCenters() const {
return SkToBool(fFlags & kSnapVerticesToPixelCenters_Flag);
}
- bool getDisableOutputConversionToSRGB() const {
- return SkToBool(fFlags & kDisableOutputConversionToSRGB_Flag);
- }
bool hasStencilClip() const {
return SkToBool(fFlags & kHasStencilClip_Flag);
}
@@ -211,9 +198,6 @@
if (flags & GrPipeline::kHWAntialias_Flag) {
result.append("HW Antialiasing enabled.\n");
}
- if (flags & GrPipeline::kDisableOutputConversionToSRGB_Flag) {
- result.append("Disable output conversion to sRGB.\n");
- }
return result;
}
return SkString("No pipeline flags\n");
diff --git a/src/gpu/GrYUVProvider.cpp b/src/gpu/GrYUVProvider.cpp
index 82e031c..8ffc491 100644
--- a/src/gpu/GrYUVProvider.cpp
+++ b/src/gpu/GrYUVProvider.cpp
@@ -18,7 +18,6 @@
#include "SkRefCnt.h"
#include "SkResourceCache.h"
#include "SkYUVPlanesCache.h"
-#include "effects/GrSRGBEffect.h"
#include "effects/GrYUVtoRGBEffect.h"
sk_sp<SkCachedData> init_provider(GrYUVProvider* provider, SkYUVPlanesCache::Info* yuvInfo,
@@ -128,21 +127,6 @@
yuvInfo.fSizeInfo.fSizes, yuvInfo.fColorSpace, false);
paint.addColorFragmentProcessor(std::move(yuvToRgbProcessor));
- // If we're decoding an sRGB image, the result of our linear math on the YUV planes is already
- // in sRGB. (The encoding is just math on bytes, with no concept of color spaces.) So, we need
- // to output the results of that math directly to the buffer that we will then consider sRGB.
- // If we have sRGB write control, we can just tell the HW not to do the Linear -> sRGB step.
- // Otherwise, we do our shader math to go from YUV -> sRGB, manually convert sRGB -> Linear,
- // then let the HW convert Linear -> sRGB.
- if (GrPixelConfigIsSRGB(desc.fConfig)) {
- if (ctx->contextPriv().caps()->srgbWriteControl()) {
- paint.setDisableOutputConversionToSRGB(true);
- } else {
- paint.addColorFragmentProcessor(GrSRGBEffect::Make(GrSRGBEffect::Mode::kSRGBToLinear,
- GrSRGBEffect::Alpha::kOpaque));
- }
- }
-
// If the caller expects the pixels in a different color space than the one from the image,
// apply a color conversion to do this.
std::unique_ptr<GrFragmentProcessor> colorConversionProcessor =
diff --git a/src/gpu/ccpr/GrCCDrawPathsOp.cpp b/src/gpu/ccpr/GrCCDrawPathsOp.cpp
index bbe3df2..3f70959 100644
--- a/src/gpu/ccpr/GrCCDrawPathsOp.cpp
+++ b/src/gpu/ccpr/GrCCDrawPathsOp.cpp
@@ -65,7 +65,6 @@
GrPaint&& paint)
: GrDrawOp(ClassID())
, fViewMatrixIfUsingLocalCoords(has_coord_transforms(paint) ? m : SkMatrix::I())
- , fSRGBFlags(GrPipeline::SRGBFlagsFromPaint(paint))
, fDraws(m, shape, shapeDevIBounds, maskDevIBounds, maskVisibility, paint.getColor())
, fProcessors(std::move(paint)) { // Paint must be moved after fetching its color above.
SkDEBUGCODE(fBaseInstance = -1);
@@ -125,7 +124,7 @@
SkASSERT(!that->fOwningPerOpListPaths || that->fOwningPerOpListPaths == fOwningPerOpListPaths);
SkASSERT(that->fNumDraws);
- if (fSRGBFlags != that->fSRGBFlags || fProcessors != that->fProcessors ||
+ if (fProcessors != that->fProcessors ||
fViewMatrixIfUsingLocalCoords != that->fViewMatrixIfUsingLocalCoords) {
return false;
}
@@ -332,7 +331,6 @@
}
GrPipeline::InitArgs initArgs;
- initArgs.fFlags = fSRGBFlags;
initArgs.fProxy = flushState->drawOpArgs().fProxy;
initArgs.fCaps = &flushState->caps();
initArgs.fResourceProvider = flushState->resourceProvider();
diff --git a/src/gpu/ccpr/GrCCDrawPathsOp.h b/src/gpu/ccpr/GrCCDrawPathsOp.h
index 8bfccc4..9509bb8 100644
--- a/src/gpu/ccpr/GrCCDrawPathsOp.h
+++ b/src/gpu/ccpr/GrCCDrawPathsOp.h
@@ -84,7 +84,6 @@
void recordInstance(const GrTextureProxy* atlasProxy, int instanceIdx);
const SkMatrix fViewMatrixIfUsingLocalCoords;
- const uint32_t fSRGBFlags;
struct SingleDraw {
SingleDraw(const SkMatrix&, const GrShape&, const SkIRect& shapeDevIBounds,
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index 40915a3..d5c3cc3 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -1733,7 +1733,7 @@
// This must come after textures are flushed because a texture may need
// to be msaa-resolved (which will modify bound FBO state).
- this->flushRenderTarget(glRT, pipeline.getDisableOutputConversionToSRGB());
+ this->flushRenderTarget(glRT);
return true;
}
@@ -2156,17 +2156,17 @@
}
void GrGLGpu::flushRenderTarget(GrGLRenderTarget* target, GrSurfaceOrigin origin,
- const SkIRect& bounds, bool disableSRGB) {
- this->flushRenderTargetNoColorWrites(target, disableSRGB);
+ const SkIRect& bounds) {
+ this->flushRenderTargetNoColorWrites(target);
this->didWriteToSurface(target, origin, &bounds);
}
-void GrGLGpu::flushRenderTarget(GrGLRenderTarget* target, bool disableSRGB) {
- this->flushRenderTargetNoColorWrites(target, disableSRGB);
+void GrGLGpu::flushRenderTarget(GrGLRenderTarget* target) {
+ this->flushRenderTargetNoColorWrites(target);
this->didWriteToSurface(target, kTopLeft_GrSurfaceOrigin, nullptr);
}
-void GrGLGpu::flushRenderTargetNoColorWrites(GrGLRenderTarget* target, bool disableSRGB) {
+void GrGLGpu::flushRenderTargetNoColorWrites(GrGLRenderTarget* target) {
SkASSERT(target);
GrGpuResource::UniqueID rtID = target->uniqueID();
if (fHWBoundRenderTargetUniqueID != rtID) {
@@ -2192,7 +2192,7 @@
}
if (this->glCaps().srgbWriteControl()) {
- this->flushFramebufferSRGB(GrPixelConfigIsSRGB(target->config()) && !disableSRGB);
+ this->flushFramebufferSRGB(GrPixelConfigIsSRGB(target->config()));
}
}
diff --git a/src/gpu/gl/GrGLGpu.h b/src/gpu/gl/GrGLGpu.h
index 765d5f6..5817b56 100644
--- a/src/gpu/gl/GrGLGpu.h
+++ b/src/gpu/gl/GrGLGpu.h
@@ -346,12 +346,11 @@
// The passed bounds contains the render target's color values that will subsequently be
// written.
- void flushRenderTarget(GrGLRenderTarget*, GrSurfaceOrigin, const SkIRect& bounds,
- bool disableSRGB = false);
+ void flushRenderTarget(GrGLRenderTarget*, GrSurfaceOrigin, const SkIRect& bounds);
// This version has an implicit bounds of the entire render target.
- void flushRenderTarget(GrGLRenderTarget*, bool disableSRGB = false);
+ void flushRenderTarget(GrGLRenderTarget*);
// This version can be used when the render target's colors will not be written.
- void flushRenderTargetNoColorWrites(GrGLRenderTarget*, bool disableSRGB = false);
+ void flushRenderTargetNoColorWrites(GrGLRenderTarget*);
// Need not be called if flushRenderTarget is used.
void flushViewport(const GrGLIRect&);
diff --git a/src/gpu/ops/GrAtlasTextOp.cpp b/src/gpu/ops/GrAtlasTextOp.cpp
index 29a5938..d956755 100644
--- a/src/gpu/ops/GrAtlasTextOp.cpp
+++ b/src/gpu/ops/GrAtlasTextOp.cpp
@@ -297,8 +297,9 @@
}
SkASSERT(proxies[0]);
- auto pipe =
- target->makePipeline(fSRGBFlags, std::move(fProcessors), target->detachAppliedClip());
+ static const uint32_t kPipelineFlags = 0;
+ auto pipe = target->makePipeline(kPipelineFlags, std::move(fProcessors),
+ target->detachAppliedClip());
FlushInfo flushInfo;
flushInfo.fPipeline = pipe.fPipeline;
diff --git a/src/gpu/ops/GrAtlasTextOp.h b/src/gpu/ops/GrAtlasTextOp.h
index a1f8f89..461c85c 100644
--- a/src/gpu/ops/GrAtlasTextOp.h
+++ b/src/gpu/ops/GrAtlasTextOp.h
@@ -101,7 +101,6 @@
GrAtlasTextOp(GrPaint&& paint)
: INHERITED(ClassID())
, fGeoDataAllocSize(kMinGeometryAllocated)
- , fSRGBFlags(GrPipeline::SRGBFlagsFromPaint(paint))
, fProcessors(std::move(paint)) {}
struct FlushInfo {
@@ -158,7 +157,6 @@
SkAutoSTMalloc<kMinGeometryAllocated, Geometry> fGeoData;
int fGeoDataAllocSize;
- uint32_t fSRGBFlags;
GrProcessorSet fProcessors;
struct {
uint32_t fUsesLocalCoords : 1;
diff --git a/src/gpu/ops/GrDashOp.cpp b/src/gpu/ops/GrDashOp.cpp
index d38bb9f..b5d7a47 100644
--- a/src/gpu/ops/GrDashOp.cpp
+++ b/src/gpu/ops/GrDashOp.cpp
@@ -335,7 +335,6 @@
bool fullDash, const GrUserStencilSettings* stencilSettings)
: INHERITED(ClassID())
, fColor(paint.getColor())
- , fDisableSRGBOutputConversion(paint.getDisableOutputConversionToSRGB())
, fFullDash(fullDash)
, fCap(cap)
, fAAMode(aaMode)
@@ -694,9 +693,6 @@
if (AAMode::kCoverageWithMSAA == fAAMode) {
pipelineFlags |= GrPipeline::kHWAntialias_Flag;
}
- if (fDisableSRGBOutputConversion) {
- pipelineFlags |= GrPipeline::kDisableOutputConversionToSRGB_Flag;
- }
auto pipe = target->makePipeline(pipelineFlags, std::move(fProcessorSet),
target->detachAppliedClip());
helper.recordDraw(target, gp.get(), pipe.fPipeline, pipe.fFixedDynamicState);
@@ -749,7 +745,6 @@
SkSTArray<1, LineData, true> fLines;
GrColor fColor;
- bool fDisableSRGBOutputConversion : 1;
bool fDisallowCombineOnTouchOrOverlap : 1;
bool fUsesLocalCoords : 1;
bool fFullDash : 1;
diff --git a/src/gpu/ops/GrDrawPathOp.cpp b/src/gpu/ops/GrDrawPathOp.cpp
index 98a4775..cc8a86d 100644
--- a/src/gpu/ops/GrDrawPathOp.cpp
+++ b/src/gpu/ops/GrDrawPathOp.cpp
@@ -19,7 +19,6 @@
, fInputColor(paint.getColor())
, fFillType(fill)
, fAAType(aaType)
- , fPipelineSRGBFlags(GrPipeline::SRGBFlagsFromPaint(paint))
, fProcessorSet(std::move(paint)) {}
SkString GrDrawPathOp::dumpInfo() const {
@@ -40,7 +39,6 @@
0xffff>()
};
GrPipeline::InitArgs args;
- args.fFlags = fPipelineSRGBFlags;
if (GrAATypeIsHW(fAAType)) {
args.fFlags |= GrPipeline::kHWAntialias_Flag;
}
diff --git a/src/gpu/ops/GrDrawPathOp.h b/src/gpu/ops/GrDrawPathOp.h
index 5ca9d22..02c0022 100644
--- a/src/gpu/ops/GrDrawPathOp.h
+++ b/src/gpu/ops/GrDrawPathOp.h
@@ -45,7 +45,6 @@
GrPathRendering::FillType fillType() const { return fFillType; }
const GrProcessorSet& processors() const { return fProcessorSet; }
GrProcessorSet detachProcessors() { return std::move(fProcessorSet); }
- uint32_t pipelineSRGBFlags() const { return fPipelineSRGBFlags; }
inline GrPipeline::InitArgs pipelineInitArgs(const GrOpFlushState&);
const GrProcessorSet::Analysis& doProcessorAnalysis(const GrCaps& caps,
const GrAppliedClip* clip,
@@ -68,7 +67,6 @@
GrProcessorSet::Analysis fAnalysis;
GrPathRendering::FillType fFillType;
GrAAType fAAType;
- uint32_t fPipelineSRGBFlags;
GrProcessorSet fProcessorSet;
typedef GrDrawOp INHERITED;
diff --git a/src/gpu/ops/GrSimpleMeshDrawOpHelper.cpp b/src/gpu/ops/GrSimpleMeshDrawOpHelper.cpp
index fc54475..d1565c1 100644
--- a/src/gpu/ops/GrSimpleMeshDrawOpHelper.cpp
+++ b/src/gpu/ops/GrSimpleMeshDrawOpHelper.cpp
@@ -14,7 +14,7 @@
GrSimpleMeshDrawOpHelper::GrSimpleMeshDrawOpHelper(const MakeArgs& args, GrAAType aaType,
Flags flags)
: fProcessors(args.fProcessorSet)
- , fPipelineFlags(args.fSRGBFlags)
+ , fPipelineFlags(0)
, fAAType((int)aaType)
, fRequiresDstTexture(false)
, fUsesLocalCoords(false)
diff --git a/src/gpu/ops/GrSimpleMeshDrawOpHelper.h b/src/gpu/ops/GrSimpleMeshDrawOpHelper.h
index cbf41a7..d2bfd36 100644
--- a/src/gpu/ops/GrSimpleMeshDrawOpHelper.h
+++ b/src/gpu/ops/GrSimpleMeshDrawOpHelper.h
@@ -99,7 +99,6 @@
MakeArgs() = default;
GrProcessorSet* fProcessorSet;
- uint32_t fSRGBFlags;
friend class GrSimpleMeshDrawOpHelper;
};
@@ -181,7 +180,6 @@
GrOpMemoryPool* pool = context->contextPriv().opMemoryPool();
MakeArgs makeArgs;
- makeArgs.fSRGBFlags = GrPipeline::SRGBFlagsFromPaint(paint);
GrColor color = paint.getColor();
if (paint.isTrivial()) {
diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp
index 8a7a789..1a89ba4 100644
--- a/src/image/SkImage_Gpu.cpp
+++ b/src/image/SkImage_Gpu.cpp
@@ -230,29 +230,8 @@
flags = GrContextPriv::kUnpremul_PixelOpsFlag;
}
- // This hack allows us to call makeNonTextureImage on images with arbitrary color spaces.
- // Otherwise, we'll be unable to create a render target context.
- // TODO: This shouldn't be necessary - we need more robust support for images (and surfaces)
- // 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.
- // 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())) {
- if (GrPixelConfigIsSRGB(fProxy->config())) {
- surfaceColorSpace = SkColorSpace::MakeSRGB();
- } else {
- surfaceColorSpace = nullptr;
- }
- }
- }
-
sk_sp<GrSurfaceContext> sContext = fContext->contextPriv().makeWrappedSurfaceContext(
- fProxy, surfaceColorSpace);
+ fProxy, fColorSpace);
if (!sContext) {
return false;
}