Two changes:
1. Remove special premul handling from gamut xform code
Alpha is a constant, so the gamut transformation results remain unchanged
(it distributes across the linear matrix multiply).
2. Use SkMatrix44 rather than array of floats
Preserves semantic intention, and makes upcoming code (where we transform
colors on the CPU by that matrix) simpler.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2329553002
Review-Url: https://codereview.chromium.org/2329553002
diff --git a/src/gpu/GrColorSpaceXform.cpp b/src/gpu/GrColorSpaceXform.cpp
index c7be306..2d17610 100644
--- a/src/gpu/GrColorSpaceXform.cpp
+++ b/src/gpu/GrColorSpaceXform.cpp
@@ -35,13 +35,10 @@
sk_float_almost_equals(m.getFloat(3, 3), 1.0f, tol);
}
-GrColorSpaceXform::GrColorSpaceXform(const SkMatrix44& srcToDst, SkAlphaType srcAlphaType)
- : fSrcAlphaType(srcAlphaType) {
- srcToDst.asColMajorf(fSrcToDst);
-}
+GrColorSpaceXform::GrColorSpaceXform(const SkMatrix44& srcToDst)
+ : fSrcToDst(srcToDst) {}
-sk_sp<GrColorSpaceXform> GrColorSpaceXform::Make(SkColorSpace* src, SkColorSpace* dst,
- SkAlphaType srcAlphaType) {
+sk_sp<GrColorSpaceXform> GrColorSpaceXform::Make(SkColorSpace* src, SkColorSpace* dst) {
if (!src || !dst) {
// Invalid
return nullptr;
@@ -59,5 +56,5 @@
return nullptr;
}
- return sk_make_sp<GrColorSpaceXform>(srcToDst, srcAlphaType);
+ return sk_make_sp<GrColorSpaceXform>(srcToDst);
}
diff --git a/src/gpu/GrTextureParamsAdjuster.cpp b/src/gpu/GrTextureParamsAdjuster.cpp
index 1586d73..3ca90f5 100644
--- a/src/gpu/GrTextureParamsAdjuster.cpp
+++ b/src/gpu/GrTextureParamsAdjuster.cpp
@@ -424,8 +424,7 @@
(domain.fLeft <= domain.fRight && domain.fTop <= domain.fBottom));
textureMatrix.postIDiv(texture->width(), texture->height());
sk_sp<GrColorSpaceXform> colorSpaceXform = GrColorSpaceXform::Make(this->getColorSpace(),
- dstColorSpace,
- this->alphaType());
+ dstColorSpace);
return create_fp_for_domain_and_filter(texture, std::move(colorSpaceXform), textureMatrix,
domainMode, domain, filterOrNullForBicubic);
}
@@ -507,8 +506,7 @@
SkMatrix normalizedTextureMatrix = textureMatrix;
normalizedTextureMatrix.postIDiv(texture->width(), texture->height());
sk_sp<GrColorSpaceXform> colorSpaceXform = GrColorSpaceXform::Make(this->getColorSpace(),
- dstColorSpace,
- this->alphaType());
+ dstColorSpace);
return create_fp_for_domain_and_filter(texture, std::move(colorSpaceXform),
normalizedTextureMatrix, domainMode, domain,
filterOrNullForBicubic);
diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp
index 4b77431..1970711 100644
--- a/src/gpu/SkGpuDevice.cpp
+++ b/src/gpu/SkGpuDevice.cpp
@@ -1009,8 +1009,7 @@
return;
}
sk_sp<GrColorSpaceXform> colorSpaceXform =
- GrColorSpaceXform::Make(bitmap.colorSpace(), fDrawContext->getColorSpace(),
- bitmap.alphaType());
+ GrColorSpaceXform::Make(bitmap.colorSpace(), fDrawContext->getColorSpace());
SkScalar iw = 1.f / texture->width();
SkScalar ih = 1.f / texture->height();
@@ -1135,8 +1134,7 @@
tmpUnfiltered.setImageFilter(nullptr);
sk_sp<GrColorSpaceXform> colorSpaceXform =
- GrColorSpaceXform::Make(result->getColorSpace(), fDrawContext->getColorSpace(),
- result->alphaType());
+ GrColorSpaceXform::Make(result->getColorSpace(), fDrawContext->getColorSpace());
GrPaint grPaint;
sk_sp<GrFragmentProcessor> fp(GrSimpleTextureEffect::Make(texture.get(),
std::move(colorSpaceXform),
diff --git a/src/gpu/effects/GrBicubicEffect.cpp b/src/gpu/effects/GrBicubicEffect.cpp
index 69fe906..54c536a 100644
--- a/src/gpu/effects/GrBicubicEffect.cpp
+++ b/src/gpu/effects/GrBicubicEffect.cpp
@@ -134,7 +134,7 @@
pdman.setMatrix4f(fCoefficientsUni, bicubicEffect.coefficients());
fDomain.setData(pdman, bicubicEffect.domain(), texture.origin());
if (SkToBool(bicubicEffect.colorSpaceXform())) {
- pdman.setMatrix4f(fColorSpaceXformUni, bicubicEffect.colorSpaceXform()->srcToDst());
+ pdman.setSkMatrix44(fColorSpaceXformUni, bicubicEffect.colorSpaceXform()->srcToDst());
}
}
diff --git a/src/gpu/effects/GrSimpleTextureEffect.cpp b/src/gpu/effects/GrSimpleTextureEffect.cpp
index 8e45281..b819cf3 100644
--- a/src/gpu/effects/GrSimpleTextureEffect.cpp
+++ b/src/gpu/effects/GrSimpleTextureEffect.cpp
@@ -40,7 +40,7 @@
void onSetData(const GrGLSLProgramDataManager& pdman, const GrProcessor& processor) override {
const GrSimpleTextureEffect& textureEffect = processor.cast<GrSimpleTextureEffect>();
if (SkToBool(textureEffect.colorSpaceXform())) {
- pdman.setMatrix4f(fColorSpaceXformUni, textureEffect.colorSpaceXform()->srcToDst());
+ pdman.setSkMatrix44(fColorSpaceXformUni, textureEffect.colorSpaceXform()->srcToDst());
}
}
diff --git a/src/gpu/glsl/GrGLSLColorSpaceXformHelper.h b/src/gpu/glsl/GrGLSLColorSpaceXformHelper.h
index cd516e8..5e112f9 100644
--- a/src/gpu/glsl/GrGLSLColorSpaceXformHelper.h
+++ b/src/gpu/glsl/GrGLSLColorSpaceXformHelper.h
@@ -26,18 +26,15 @@
*handle = uniformHandler->addUniform(kFragment_GrShaderFlag, kMat44f_GrSLType,
kDefault_GrSLPrecision, "ColorXform",
&fXformMatrix);
- fAlphaType = colorSpaceXform->alphaType();
} else {
fXformMatrix = nullptr;
}
}
const char* getXformMatrix() const { return fXformMatrix; }
- SkAlphaType alphaType() const { return fAlphaType; }
private:
const char* fXformMatrix;
- SkAlphaType fAlphaType;
};
#endif
diff --git a/src/gpu/glsl/GrGLSLProgramDataManager.cpp b/src/gpu/glsl/GrGLSLProgramDataManager.cpp
index 0c98e08..0803f7a 100644
--- a/src/gpu/glsl/GrGLSLProgramDataManager.cpp
+++ b/src/gpu/glsl/GrGLSLProgramDataManager.cpp
@@ -8,6 +8,7 @@
#include "glsl/GrGLSLProgramDataManager.h"
#include "SkMatrix.h"
+#include "SkMatrix44.h"
void GrGLSLProgramDataManager::setSkMatrix(UniformHandle u, const SkMatrix& matrix) const {
float mt[] = {
@@ -23,3 +24,10 @@
};
this->setMatrix3f(u, mt);
}
+
+void GrGLSLProgramDataManager::setSkMatrix44(UniformHandle u, const SkMatrix44& matrix) const {
+ // TODO: We could skip this temporary buffer if we had direct access to the matrix storage
+ float m[16];
+ matrix.asColMajorf(m);
+ this->setMatrix4f(u, m);
+}
diff --git a/src/gpu/glsl/GrGLSLProgramDataManager.h b/src/gpu/glsl/GrGLSLProgramDataManager.h
index 442a1e5..8d58fc8 100644
--- a/src/gpu/glsl/GrGLSLProgramDataManager.h
+++ b/src/gpu/glsl/GrGLSLProgramDataManager.h
@@ -12,6 +12,7 @@
#include "SkTypes.h"
class SkMatrix;
+class SkMatrix44;
/** Manages the resources used by a shader program.
* The resources are objects the program uses to communicate with the
@@ -48,6 +49,9 @@
// convenience method for uploading a SkMatrix to a 3x3 matrix uniform
void setSkMatrix(UniformHandle, const SkMatrix&) const;
+ // convenience method for uploading a SkMatrix44 to a 4x4 matrix uniform
+ void setSkMatrix44(UniformHandle, const SkMatrix44&) const;
+
// for nvpr only
GR_DEFINE_RESOURCE_HANDLE_CLASS(VaryingHandle);
virtual void setPathFragmentInputTransform(VaryingHandle u, int components,
diff --git a/src/gpu/glsl/GrGLSLShaderBuilder.cpp b/src/gpu/glsl/GrGLSLShaderBuilder.cpp
index 26a7761..6d77bdb 100644
--- a/src/gpu/glsl/GrGLSLShaderBuilder.cpp
+++ b/src/gpu/glsl/GrGLSLShaderBuilder.cpp
@@ -133,27 +133,14 @@
GrGLSLColorSpaceXformHelper* colorXformHelper) {
// Our color is (r, g, b, a), but we want to multiply (r, g, b, 1) by our matrix, then
// re-insert the original alpha. The supplied srcColor is likely to be of the form
- // "texture(...)", and we don't want to evaluate that twice.
- //
- // Worse: We can't do the transformation on premultiplied colors, so if the source is premul,
- // we need to unpremul, transform, then multiply again. Anyways, we wrap all of the work in a
- // function.
+ // "texture(...)", and we don't want to evaluate that twice, so wrap everything in a function.
static const GrGLSLShaderVar gColorGamutXformArgs[] = {
GrGLSLShaderVar("color", kVec4f_GrSLType),
GrGLSLShaderVar("xform", kMat44f_GrSLType),
};
SkString functionBody;
- if (kPremul_SkAlphaType == colorXformHelper->alphaType()) {
- // Unpremultiply
- functionBody.append("\tfloat nonZeroAlpha = max(color.a, 0.00001);\n"
- "\tcolor.rgb = color.rgb / nonZeroAlpha;\n");
- }
// Gamut xform, clamp to destination gamut
functionBody.append("\tcolor.rgb = clamp((xform * vec4(color.rgb, 1.0)).rgb, 0.0, 1.0);\n");
- if (kPremul_SkAlphaType == colorXformHelper->alphaType()) {
- // Re-multiply by alpha
- functionBody.append("\tcolor.rgb = color.rgb * nonZeroAlpha;\n");
- }
functionBody.append("\treturn color;");
SkString colorGamutXformFuncName;
this->emitFunction(kVec4f_GrSLType,
diff --git a/src/image/SkImageShader.cpp b/src/image/SkImageShader.cpp
index 9afd207..e52deff 100644
--- a/src/image/SkImageShader.cpp
+++ b/src/image/SkImageShader.cpp
@@ -216,8 +216,7 @@
SkImageInfo info = as_IB(fImage)->onImageInfo();
sk_sp<GrColorSpaceXform> colorSpaceXform = GrColorSpaceXform::Make(info.colorSpace(),
- args.fDstColorSpace,
- info.alphaType());
+ args.fDstColorSpace);
sk_sp<GrFragmentProcessor> inner;
if (doBicubic) {
inner = GrBicubicEffect::Make(texture, std::move(colorSpaceXform), matrix, tm);