Clean up GrColorSpaceXformEffect to allow a null child FP.
Previously, GrColorSpaceXformEffect had separate Make methods; one took
a child FP, the other did not, and they had slightly different color-
transform inputs (the destination alpha type was assumed to be premul
when a child FP was provided). A null FP was considered invalid.
This has been updated to work more like other FPs. An childFP must
always be supplied, and nullptr is interpreted as "use the sk_InColor".
The destination alpha type is now always supplied. Existing call sites
were fixed up as needed.
Previously, the child-FP path would actually render the product of the
child FP and the sk_InColor. This extra multiplication step no longer
occurs; the sk_InColor is simply ignored when a child FP is used. In
practice, this did not affect any existing GM tests.
Change-Id: Id53449234948255cbdaab9b88b1c94201eeb0a0d
Bug: skia:10217
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/299576
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
diff --git a/src/core/SkColorFilter.cpp b/src/core/SkColorFilter.cpp
index a0800f3..31f58ba 100644
--- a/src/core/SkColorFilter.cpp
+++ b/src/core/SkColorFilter.cpp
@@ -255,10 +255,12 @@
SkAlphaType at = kPremul_SkAlphaType;
switch (fDir) {
case Direction::kLinearToSRGB:
- return GrColorSpaceXformEffect::Make(sk_srgb_linear_singleton(), at,
+ return GrColorSpaceXformEffect::Make(/*childFP=*/nullptr,
+ sk_srgb_linear_singleton(), at,
sk_srgb_singleton(), at);
case Direction::kSRGBToLinear:
- return GrColorSpaceXformEffect::Make(sk_srgb_singleton(), at,
+ return GrColorSpaceXformEffect::Make(/*childFP=*/nullptr,
+ sk_srgb_singleton(), at,
sk_srgb_linear_singleton(), at);
}
return nullptr;
diff --git a/src/effects/imagefilters/SkAlphaThresholdFilter.cpp b/src/effects/imagefilters/SkAlphaThresholdFilter.cpp
index 3f43373..0df8dc1 100644
--- a/src/effects/imagefilters/SkAlphaThresholdFilter.cpp
+++ b/src/effects/imagefilters/SkAlphaThresholdFilter.cpp
@@ -168,8 +168,9 @@
auto textureFP = GrTextureEffect::Make(
std::move(inputView), input->alphaType(),
SkMatrix::Translate(input->subset().x(), input->subset().y()));
- textureFP = GrColorSpaceXformEffect::Make(std::move(textureFP), input->getColorSpace(),
- input->alphaType(), ctx.colorSpace());
+ textureFP = GrColorSpaceXformEffect::Make(std::move(textureFP),
+ input->getColorSpace(), input->alphaType(),
+ ctx.colorSpace(), kPremul_SkAlphaType);
if (!textureFP) {
return nullptr;
}
diff --git a/src/effects/imagefilters/SkArithmeticImageFilter.cpp b/src/effects/imagefilters/SkArithmeticImageFilter.cpp
index ae7ff48..2d61cf0 100644
--- a/src/effects/imagefilters/SkArithmeticImageFilter.cpp
+++ b/src/effects/imagefilters/SkArithmeticImageFilter.cpp
@@ -358,9 +358,9 @@
SkIntToScalar(bgSubset.top() - backgroundOffset.fY));
bgFP = GrTextureEffect::MakeSubset(std::move(backgroundView), background->alphaType(),
backgroundMatrix, sampler, bgSubset, caps);
- bgFP = GrColorSpaceXformEffect::Make(std::move(bgFP), background->getColorSpace(),
- background->alphaType(),
- ctx.colorSpace());
+ bgFP = GrColorSpaceXformEffect::Make(std::move(bgFP),
+ background->getColorSpace(), background->alphaType(),
+ ctx.colorSpace(), kPremul_SkAlphaType);
} else {
bgFP = GrConstColorProcessor::Make(/*inputFP=*/nullptr, SK_PMColor4fTRANSPARENT,
GrConstColorProcessor::InputMode::kIgnore);
@@ -374,9 +374,8 @@
auto fgFP = GrTextureEffect::MakeSubset(std::move(foregroundView), foreground->alphaType(),
foregroundMatrix, sampler, fgSubset, caps);
fgFP = GrColorSpaceXformEffect::Make(std::move(fgFP),
- foreground->getColorSpace(),
- foreground->alphaType(),
- ctx.colorSpace());
+ foreground->getColorSpace(), foreground->alphaType(),
+ ctx.colorSpace(), kPremul_SkAlphaType);
paint.addColorFragmentProcessor(std::move(fgFP));
static auto effect = std::get<0>(SkRuntimeEffect::Make(SkString(SKSL_ARITHMETIC_SRC)));
diff --git a/src/effects/imagefilters/SkDisplacementMapEffect.cpp b/src/effects/imagefilters/SkDisplacementMapEffect.cpp
index febbf41..d50d2ad 100644
--- a/src/effects/imagefilters/SkDisplacementMapEffect.cpp
+++ b/src/effects/imagefilters/SkDisplacementMapEffect.cpp
@@ -346,8 +346,9 @@
std::move(colorView),
color->subset(),
*context->priv().caps());
- fp = GrColorSpaceXformEffect::Make(std::move(fp), color->getColorSpace(),
- color->alphaType(), ctx.colorSpace());
+ fp = GrColorSpaceXformEffect::Make(std::move(fp),
+ color->getColorSpace(), color->alphaType(),
+ ctx.colorSpace(), kPremul_SkAlphaType);
GrPaint paint;
paint.addColorFragmentProcessor(std::move(fp));
diff --git a/src/effects/imagefilters/SkMagnifierImageFilter.cpp b/src/effects/imagefilters/SkMagnifierImageFilter.cpp
index 4300a39..aaabc3b 100644
--- a/src/effects/imagefilters/SkMagnifierImageFilter.cpp
+++ b/src/effects/imagefilters/SkMagnifierImageFilter.cpp
@@ -144,8 +144,9 @@
invYZoom,
bounds.width() * invInset,
bounds.height() * invInset);
- fp = GrColorSpaceXformEffect::Make(std::move(fp), input->getColorSpace(),
- input->alphaType(), ctx.colorSpace());
+ fp = GrColorSpaceXformEffect::Make(std::move(fp),
+ input->getColorSpace(), input->alphaType(),
+ ctx.colorSpace(), kPremul_SkAlphaType);
if (!fp) {
return nullptr;
}
diff --git a/src/effects/imagefilters/SkXfermodeImageFilter.cpp b/src/effects/imagefilters/SkXfermodeImageFilter.cpp
index 7569e17..89fe7f2 100644
--- a/src/effects/imagefilters/SkXfermodeImageFilter.cpp
+++ b/src/effects/imagefilters/SkXfermodeImageFilter.cpp
@@ -268,9 +268,9 @@
SkIntToScalar(bgSubset.top() - backgroundOffset.fY));
bgFP = GrTextureEffect::MakeSubset(std::move(backgroundView), background->alphaType(),
bgMatrix, sampler, bgSubset, caps);
- bgFP = GrColorSpaceXformEffect::Make(std::move(bgFP), background->getColorSpace(),
- background->alphaType(),
- ctx.colorSpace());
+ bgFP = GrColorSpaceXformEffect::Make(std::move(bgFP),
+ background->getColorSpace(), background->alphaType(),
+ ctx.colorSpace(), kPremul_SkAlphaType);
} else {
bgFP = GrConstColorProcessor::Make(/*inputFP=*/nullptr, SK_PMColor4fTRANSPARENT,
GrConstColorProcessor::InputMode::kIgnore);
@@ -284,9 +284,8 @@
auto fgFP = GrTextureEffect::MakeSubset(std::move(foregroundView), foreground->alphaType(),
fgMatrix, sampler, fgSubset, caps);
fgFP = GrColorSpaceXformEffect::Make(std::move(fgFP),
- foreground->getColorSpace(),
- foreground->alphaType(),
- ctx.colorSpace());
+ foreground->getColorSpace(), foreground->alphaType(),
+ ctx.colorSpace(), kPremul_SkAlphaType);
paint.addColorFragmentProcessor(std::move(fgFP));
std::unique_ptr<GrFragmentProcessor> xferFP = this->makeFGFrag(std::move(bgFP));
diff --git a/src/gpu/GrColorSpaceXform.cpp b/src/gpu/GrColorSpaceXform.cpp
index b602b32..8b1d234 100644
--- a/src/gpu/GrColorSpaceXform.cpp
+++ b/src/gpu/GrColorSpaceXform.cpp
@@ -58,31 +58,26 @@
class GrGLColorSpaceXformEffect : public GrGLSLFragmentProcessor {
public:
void emitCode(EmitArgs& args) override {
- const GrColorSpaceXformEffect& csxe = args.fFp.cast<GrColorSpaceXformEffect>();
+ const GrColorSpaceXformEffect& proc = args.fFp.cast<GrColorSpaceXformEffect>();
GrGLSLFPFragmentBuilder* fragBuilder = args.fFragBuilder;
GrGLSLUniformHandler* uniformHandler = args.fUniformHandler;
- fColorSpaceHelper.emitCode(uniformHandler, csxe.colorXform());
+ fColorSpaceHelper.emitCode(uniformHandler, proc.colorXform());
- if (this->numChildProcessors()) {
- SkString childColor = this->invokeChild(0, args);
+ SkString childColor = this->numChildProcessors()
+ ? this->invokeChild(0, args.fInputColor, args)
+ : SkString(args.fInputColor);
- SkString xformedColor;
- fragBuilder->appendColorGamutXform(&xformedColor, childColor.c_str(), &fColorSpaceHelper);
- fragBuilder->codeAppendf("%s = %s * %s;", args.fOutputColor, xformedColor.c_str(),
- args.fInputColor);
- } else {
- SkString xformedColor;
- fragBuilder->appendColorGamutXform(&xformedColor, args.fInputColor, &fColorSpaceHelper);
- fragBuilder->codeAppendf("%s = %s;", args.fOutputColor, xformedColor.c_str());
- }
+ SkString xformedColor;
+ fragBuilder->appendColorGamutXform(&xformedColor, childColor.c_str(), &fColorSpaceHelper);
+ fragBuilder->codeAppendf("%s = %s;", args.fOutputColor, xformedColor.c_str());
}
private:
void onSetData(const GrGLSLProgramDataManager& pdman,
- const GrFragmentProcessor& processor) override {
- const GrColorSpaceXformEffect& csxe = processor.cast<GrColorSpaceXformEffect>();
- fColorSpaceHelper.setData(pdman, csxe.colorXform());
+ const GrFragmentProcessor& fp) override {
+ const GrColorSpaceXformEffect& proc = fp.cast<GrColorSpaceXformEffect>();
+ fColorSpaceHelper.setData(pdman, proc.colorXform());
}
GrGLSLColorSpaceXformHelper fColorSpaceHelper;
@@ -101,11 +96,14 @@
}
}
+GrColorSpaceXformEffect::GrColorSpaceXformEffect(const GrColorSpaceXformEffect& that)
+ : INHERITED(kGrColorSpaceXformEffect_ClassID, that.optimizationFlags())
+ , fColorXform(that.fColorXform) {
+ this->cloneAndRegisterAllChildProcessors(that);
+}
+
std::unique_ptr<GrFragmentProcessor> GrColorSpaceXformEffect::clone() const {
- std::unique_ptr<GrFragmentProcessor> child =
- this->numChildProcessors() ? this->childProcessor(0).clone() : nullptr;
- return std::unique_ptr<GrFragmentProcessor>(
- new GrColorSpaceXformEffect(std::move(child), fColorXform));
+ return std::unique_ptr<GrFragmentProcessor>(new GrColorSpaceXformEffect(*this));
}
bool GrColorSpaceXformEffect::onIsEqual(const GrFragmentProcessor& s) const {
@@ -124,23 +122,10 @@
GrFragmentProcessor::OptimizationFlags GrColorSpaceXformEffect::OptFlags(
const GrFragmentProcessor* child) {
- if (child) {
- OptimizationFlags flags = kNone_OptimizationFlags;
- if (child->compatibleWithCoverageAsAlpha()) {
- flags |= kCompatibleWithCoverageAsAlpha_OptimizationFlag;
- }
- if (child->preservesOpaqueInput()) {
- flags |= kPreservesOpaqueInput_OptimizationFlag;
- }
- if (child->hasConstantOutputForConstantInput()) {
- flags |= kConstantOutputForConstantInput_OptimizationFlag;
- }
- return flags;
- } else {
- return kCompatibleWithCoverageAsAlpha_OptimizationFlag |
+ return (child ? ProcessorOptimizationFlags(child) : kAll_OptimizationFlags) &
+ (kCompatibleWithCoverageAsAlpha_OptimizationFlag |
kPreservesOpaqueInput_OptimizationFlag |
- kConstantOutputForConstantInput_OptimizationFlag;
- }
+ kConstantOutputForConstantInput_OptimizationFlag);
}
SkPMColor4f GrColorSpaceXformEffect::constantOutputForConstantInput(
@@ -151,42 +136,16 @@
return this->fColorXform->apply(c0.unpremul()).premul();
}
-std::unique_ptr<GrFragmentProcessor> GrColorSpaceXformEffect::Make(SkColorSpace* src,
- SkAlphaType srcAT,
- SkColorSpace* dst,
- SkAlphaType dstAT) {
- auto xform = GrColorSpaceXform::Make(src, srcAT,
- dst, dstAT);
- if (!xform) {
- return nullptr;
- }
-
- return std::unique_ptr<GrFragmentProcessor>(new GrColorSpaceXformEffect(nullptr,
- std::move(xform)));
+std::unique_ptr<GrFragmentProcessor> GrColorSpaceXformEffect::Make(
+ std::unique_ptr<GrFragmentProcessor> child,
+ SkColorSpace* src, SkAlphaType srcAT,
+ SkColorSpace* dst, SkAlphaType dstAT) {
+ return Make(std::move(child), GrColorSpaceXform::Make(src, srcAT, dst, dstAT));
}
std::unique_ptr<GrFragmentProcessor> GrColorSpaceXformEffect::Make(
std::unique_ptr<GrFragmentProcessor> child,
- SkColorSpace* src, SkAlphaType srcAT, SkColorSpace* dst) {
- if (!child) {
- return nullptr;
- }
-
- auto xform = GrColorSpaceXform::Make(src, srcAT,
- dst, kPremul_SkAlphaType);
- if (!xform) {
- return child;
- }
-
- return std::unique_ptr<GrFragmentProcessor>(new GrColorSpaceXformEffect(std::move(child),
- std::move(xform)));
-}
-
-std::unique_ptr<GrFragmentProcessor> GrColorSpaceXformEffect::Make(
- std::unique_ptr<GrFragmentProcessor> child, sk_sp<GrColorSpaceXform> colorXform) {
- if (!child) {
- return nullptr;
- }
+ sk_sp<GrColorSpaceXform> colorXform) {
if (!colorXform) {
return child;
}
diff --git a/src/gpu/GrColorSpaceXform.h b/src/gpu/GrColorSpaceXform.h
index 9d540e4..63b5aa0 100644
--- a/src/gpu/GrColorSpaceXform.h
+++ b/src/gpu/GrColorSpaceXform.h
@@ -60,22 +60,17 @@
class GrColorSpaceXformEffect : public GrFragmentProcessor {
public:
/**
- * Returns a fragment processor that converts the input's color space from src to dst.
- */
- static std::unique_ptr<GrFragmentProcessor> Make(SkColorSpace* src, SkAlphaType srcAT,
- SkColorSpace* dst, SkAlphaType dstAT);
-
- /**
* Returns a fragment processor that calls the passed in fragment processor, and then converts
- * the color space of the output from src to dst.
+ * the color space of the output from src to dst. If the child is null, sk_InColor is used.
*/
static std::unique_ptr<GrFragmentProcessor> Make(std::unique_ptr<GrFragmentProcessor> child,
SkColorSpace* src, SkAlphaType srcAT,
- SkColorSpace* dst);
+ SkColorSpace* dst, SkAlphaType dstAT);
/**
* Returns a fragment processor that calls the passed in FP and then converts it with the given
- * color xform. Returns null if child is null, returns child if the xform is null (e.g. noop).
+ * color xform. If the child is null, sk_InColor is used. Returns child as-is if the xform is
+ * null (i.e. a no-op).
*/
static std::unique_ptr<GrFragmentProcessor> Make(std::unique_ptr<GrFragmentProcessor> child,
sk_sp<GrColorSpaceXform> colorXform);
@@ -89,6 +84,8 @@
GrColorSpaceXformEffect(std::unique_ptr<GrFragmentProcessor> child,
sk_sp<GrColorSpaceXform> colorXform);
+ GrColorSpaceXformEffect(const GrColorSpaceXformEffect& that);
+
static OptimizationFlags OptFlags(const GrFragmentProcessor* child);
SkPMColor4f constantOutputForConstantInput(const SkPMColor4f& input) const override;
diff --git a/src/gpu/GrImageTextureMaker.cpp b/src/gpu/GrImageTextureMaker.cpp
index 6ff41e3..ebd12c4 100644
--- a/src/gpu/GrImageTextureMaker.cpp
+++ b/src/gpu/GrImageTextureMaker.cpp
@@ -104,8 +104,9 @@
GrBicubicEffect::Direction::kXY);
}
if (fImage->fFromColorSpace) {
- fp = GrColorSpaceXformEffect::Make(std::move(fp), fImage->fFromColorSpace.get(),
- fImage->alphaType(), fImage->colorSpace());
+ fp = GrColorSpaceXformEffect::Make(std::move(fp),
+ fImage->fFromColorSpace.get(), fImage->alphaType(),
+ fImage->colorSpace(), kPremul_SkAlphaType);
}
return fp;
}
diff --git a/src/gpu/GrYUVProvider.cpp b/src/gpu/GrYUVProvider.cpp
index 3d5668c..2efe8ed 100644
--- a/src/gpu/GrYUVProvider.cpp
+++ b/src/gpu/GrYUVProvider.cpp
@@ -172,18 +172,16 @@
GrPaint paint;
const auto& caps = *ctx->priv().caps();
- auto yuvToRgbProcessor = GrYUVtoRGBEffect::Make(yuvViews, yuvaIndices, yuvColorSpace,
- GrSamplerState::Filter::kNearest, caps);
- paint.addColorFragmentProcessor(std::move(yuvToRgbProcessor));
+ std::unique_ptr<GrFragmentProcessor> yuvToRgbProcessor = GrYUVtoRGBEffect::Make(
+ yuvViews, yuvaIndices, yuvColorSpace, GrSamplerState::Filter::kNearest, caps);
// 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 =
- GrColorSpaceXformEffect::Make(srcColorSpace, kOpaque_SkAlphaType,
+ GrColorSpaceXformEffect::Make(std::move(yuvToRgbProcessor),
+ srcColorSpace, kOpaque_SkAlphaType,
dstColorSpace, kOpaque_SkAlphaType);
- if (colorConversionProcessor) {
- paint.addColorFragmentProcessor(std::move(colorConversionProcessor));
- }
+ paint.addColorFragmentProcessor(std::move(colorConversionProcessor));
paint.setPorterDuffXPFactory(SkBlendMode::kSrc);
const SkRect r = SkRect::MakeIWH(yuvSizeInfo.fSizes[0].fWidth,
diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp
index 585b530..700ef68 100644
--- a/src/gpu/SkGpuDevice.cpp
+++ b/src/gpu/SkGpuDevice.cpp
@@ -663,8 +663,10 @@
tmpUnfiltered.setImageFilter(nullptr);
auto fp = GrTextureEffect::Make(std::move(view), special->alphaType());
- fp = GrColorSpaceXformEffect::Make(std::move(fp), result->getColorSpace(), result->alphaType(),
- fRenderTargetContext->colorInfo().colorSpace());
+ fp = GrColorSpaceXformEffect::Make(
+ std::move(fp),
+ result->getColorSpace(), result->alphaType(),
+ fRenderTargetContext->colorInfo().colorSpace(), kPremul_SkAlphaType);
if (GrColorTypeIsAlphaOnly(SkColorTypeToGrColorType(result->colorType()))) {
fp = GrFragmentProcessor::MakeInputPremulAndMulByOutput(std::move(fp));
} else {
diff --git a/src/gpu/SkGpuDevice_drawTexture.cpp b/src/gpu/SkGpuDevice_drawTexture.cpp
index 4c4a2aa..f68abf3 100644
--- a/src/gpu/SkGpuDevice_drawTexture.cpp
+++ b/src/gpu/SkGpuDevice_drawTexture.cpp
@@ -462,8 +462,9 @@
}
auto fp = producer->createFragmentProcessor(textureMatrix, src, constraintMode,
coordsAllInsideSrcRect, wm, wm, filterMode);
- fp = GrColorSpaceXformEffect::Make(std::move(fp), producer->colorSpace(), producer->alphaType(),
- rtc->colorInfo().colorSpace());
+ fp = GrColorSpaceXformEffect::Make(std::move(fp),
+ producer->colorSpace(), producer->alphaType(),
+ rtc->colorInfo().colorSpace(), kPremul_SkAlphaType);
if (!fp) {
return;
}
diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp
index 914810d..1ce084f 100644
--- a/src/image/SkImage_Gpu.cpp
+++ b/src/image/SkImage_Gpu.cpp
@@ -87,7 +87,8 @@
return nullptr;
}
- auto xform = GrColorSpaceXformEffect::Make(this->colorSpace(), this->alphaType(),
+ auto xform = GrColorSpaceXformEffect::Make(/*childFP=*/nullptr,
+ this->colorSpace(), this->alphaType(),
targetCS.get(), this->alphaType());
SkASSERT(xform || targetCT != this->colorType());
diff --git a/src/shaders/SkImageShader.cpp b/src/shaders/SkImageShader.cpp
index f697b74..698f22a 100755
--- a/src/shaders/SkImageShader.cpp
+++ b/src/shaders/SkImageShader.cpp
@@ -246,7 +246,7 @@
inner = GrTextureEffect::Make(std::move(view), srcAlphaType, lmInverse, samplerState, caps);
}
inner = GrColorSpaceXformEffect::Make(std::move(inner), fImage->colorSpace(), srcAlphaType,
- args.fDstColorInfo->colorSpace());
+ args.fDstColorInfo->colorSpace(), kPremul_SkAlphaType);
bool isAlphaOnly = SkColorTypeIsAlphaOnly(fImage->colorType());
if (isAlphaOnly) {
