Remove workarounds to support legacy coord transforms

Bug: skia:10416
Change-Id: I2f1b87521174d18afc59f12832441010cb94ea3f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/299294
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This commit is contained in:
Michael Ludwig 2020-06-26 12:53:03 -04:00 committed by Skia Commit-Bot
parent 9f2a333c01
commit e7e25ac904
7 changed files with 69 additions and 188 deletions

View File

@ -15,23 +15,17 @@
* the transform code is applied.
*/
enum SampleFlag {
kExplicitlySampled_Flag = 0b0000001, // GrFP::isSampledWithExplicitCoords()
kExplicitlySampled_Flag = 0b00001, // GrFP::isSampledWithExplicitCoords()
kLegacyCoordTransform_Flag = 0b0000010, // !GrFP::coordTransform(i)::isNoOp()
kLegacyCoordTransform_Flag = 0b00010, // !GrFP::coordTransform(i)::isNoOp()
kNone_SampleMatrix_Flag = 0b0000100, // GrFP::sampleMatrix()::isNoOp()
kConstUniform_SampleMatrix_Flag = 0b0001000, // GrFP::sampleMatrix()::isConstUniform()
kVariable_SampleMatrix_Flag = 0b0001100, // GrFP::sampleMatrix()::isVariable()
// Legacy coord transforms specialize on identity, S+T, no-perspective, and general matrix types
// FIXME these (and kLegacyCoordTransform) can be removed once all FPs no longer use them
kLCT_ScaleTranslate_Matrix_Flag = 0b0010000, // GrFP::coordTransform(i)::isScaleTranslate()
kLCT_NoPersp_Matrix_Flag = 0b0100000, // !GrFP::coordTransform(i)::hasPerspective()
kLCT_General_Matrix_Flag = 0b0110000, // any other matrix type
kNone_SampleMatrix_Flag = 0b00100, // GrFP::sampleMatrix()::isNoOp()
kConstUniform_SampleMatrix_Flag = 0b01000, // GrFP::sampleMatrix()::isConstUniform()
kVariable_SampleMatrix_Flag = 0b01100, // GrFP::sampleMatrix()::isVariable()
// Currently, sample(matrix) only specializes on no-perspective or general.
// FIXME add new flags as more matrix types are supported.
kPersp_Matrix_Flag = 0b1000000, // GrFP::sampleMatrix()::fHasPerspective
kPersp_Matrix_Flag = 0b10000, // GrFP::sampleMatrix()::fHasPerspective
};
GrPrimitiveProcessor::GrPrimitiveProcessor(ClassID classID) : GrProcessor(classID) {}
@ -42,31 +36,16 @@ const GrPrimitiveProcessor::TextureSampler& GrPrimitiveProcessor::textureSampler
}
uint32_t GrPrimitiveProcessor::computeCoordTransformsKey(const GrFragmentProcessor& fp) const {
// This is highly coupled with the code in GrGLSLGeometryProcessor::emitTransforms().
// At this point, all effects either don't use legacy coord transforms, or only use 1.
SkASSERT(fp.numCoordTransforms() <= 1);
// This is highly coupled with the code in GrGLSLGeometryProcessor::collectTransforms().
// At this point, all effects do not use really coord transforms; they may implicitly report
// a noop coord transform but this does not impact the key.
SkASSERT(fp.numCoordTransforms() == 0 ||
(fp.numCoordTransforms() == 1 && fp.coordTransform(0).isNoOp()));
uint32_t key = 0;
if (fp.isSampledWithExplicitCoords()) {
key |= kExplicitlySampled_Flag;
}
if (fp.numCoordTransforms() > 0) {
const GrCoordTransform& coordTransform = fp.coordTransform(0);
if (!coordTransform.isNoOp()) {
// A true identity matrix shouldn't result in a coord transform; proxy normalization
// and flipping will eventually present as a scale+translate matrix.
SkASSERT(!coordTransform.matrix().isIdentity() || coordTransform.normalize() ||
coordTransform.reverseY());
key |= kLegacyCoordTransform_Flag;
if (coordTransform.matrix().isScaleTranslate()) {
key |= kLCT_ScaleTranslate_Matrix_Flag;
} else if (!coordTransform.matrix().hasPerspective()) {
key |= kLCT_NoPersp_Matrix_Flag;
} else {
key |= kLCT_General_Matrix_Flag;
}
}
}
switch(fp.sampleMatrix().fKind) {
case SkSL::SampleMatrix::Kind::kNone:

View File

@ -14,8 +14,6 @@
#include "src/gpu/GrShaderVar.h"
#include "src/gpu/GrSwizzle.h"
class GrCoordTransform;
/*
* The GrPrimitiveProcessor represents some kind of geometric primitive. This includes the shape
* of the primitive and the inherent color of the primitive. The GrPrimitiveProcessor is

View File

@ -15,7 +15,6 @@
#include "src/core/SkWriteBuffer.h"
#include "src/gpu/GrAutoLocaleSetter.h"
#include "src/gpu/GrContextPriv.h"
#include "src/gpu/GrCoordTransform.h"
#include "src/gpu/GrPersistentCacheUtils.h"
#include "src/gpu/GrProgramDesc.h"
#include "src/gpu/GrShaderCaps.h"

View File

@ -5,7 +5,6 @@
* found in the LICENSE file.
*/
#include "src/gpu/GrCoordTransform.h"
#include "src/gpu/GrFragmentProcessor.h"
#include "src/gpu/GrProcessor.h"
#include "src/gpu/glsl/GrGLSLFragmentProcessor.h"

View File

@ -153,45 +153,13 @@ SkString GrGLSLFPFragmentBuilder::writeProcessorFunction(GrGLSLFragmentProcessor
// transforms), the value that would have been passed to _coords is lifted to the vertex shader
// and stored in a unique varying. In that case it uses that variable and does not have a
// second actual argument for _coords.
// FIXME: Once GrCoordTransforms are gone, and we can more easily associated this varying with
// the sample call site, then invokeChild() can pass the varying in, instead of requiring this
// dynamic signature.
int paramCount;
// FIXME: An alternative would be to have all FP functions have a float2 argument, and the
// parent FP invokes it with the varying reference when it's been lifted to the vertex shader.
int paramCount = 2;
GrShaderVar params[] = { GrShaderVar(args.fInputColor, kHalf4_GrSLType),
GrShaderVar(args.fSampleCoord, kFloat2_GrSLType) };
if (args.fFp.isSampledWithExplicitCoords()) {
// All invokeChild() that point to 'fp' will evaluate these expressions and pass the float2
// in, so we need the 2nd argument.
paramCount = 2;
// FIXME: This is only needed for the short term until FPs no longer put transformation
// data in a GrCoordTransform (and we can then mark the parameter as read-only)
if (args.fTransformedCoords.count() > 0) {
SkASSERT(args.fTransformedCoords.count() == 1);
const GrShaderVar& transform = args.fTransformedCoords[0].fTransform;
switch (transform.getType()) {
case kFloat4_GrSLType:
// This is a scale+translate, so there's no perspective division needed
this->codeAppendf("%s = %s * %s.xz + %s.yw;\n", args.fSampleCoord,
args.fSampleCoord,
transform.c_str(),
transform.c_str());
break;
case kFloat3x3_GrSLType:
this->codeAppend("{\n");
this->codeAppendf("float3 _coords3 = (%s * %s.xy1);\n",
transform.c_str(), args.fSampleCoord);
this->codeAppendf("%s = _coords3.xy / _coords3.z;\n", args.fSampleCoord);
this->codeAppend("}\n");
break;
default:
SkASSERT(transform.getType() == kVoid_GrSLType);
break;
}
}
} else {
if (!args.fFp.isSampledWithExplicitCoords()) {
// Sampled with a const/uniform matrix and/or a legacy coord transform. The actual
// transformation code is emitted in the vertex shader, so this only has to access it.
// Add a float2 _coords variable that maps to the associated varying and replaces the
@ -210,7 +178,7 @@ SkString GrGLSLFPFragmentBuilder::writeProcessorFunction(GrGLSLFragmentProcessor
// and since we won't actually have a function parameter for local coords, add
// it as a local variable.
this->codeAppendf("float2 %s = %s.xy / %s.z;\n", args.fSampleCoord,
varying.getName().c_str(), varying.getName().c_str());
varying.getName().c_str(), varying.getName().c_str());
break;
default:
SkDEBUGFAILF("Unexpected varying type for coord: %s %d\n",
@ -218,7 +186,7 @@ SkString GrGLSLFPFragmentBuilder::writeProcessorFunction(GrGLSLFragmentProcessor
break;
}
}
}
} // else the function keeps its two arguments
this->codeAppendf("half4 %s;\n", args.fOutputColor);
fp->emitCode(args);

View File

@ -20,12 +20,10 @@ void GrGLSLGeometryProcessor::emitCode(EmitArgs& args) {
GrGPArgs gpArgs;
this->onEmitCode(args, &gpArgs);
// FIXME This must always be called at the moment, even when fLocalCoordVar is uninitialized
// and void because collectTransforms registers the uniforms for legacy coord transforms, which
// still need to be added even if the FPs are sampled explicitly. When they are gone, we only
// need to call this if the local coord isn't void (plus verify that FPs really don't need it).
this->collectTransforms(args.fVertBuilder, args.fVaryingHandler, args.fUniformHandler,
gpArgs.fLocalCoordVar, args.fFPCoordTransformHandler);
if (gpArgs.fLocalCoordVar.getType() != kVoid_GrSLType) {
this->collectTransforms(args.fVertBuilder, args.fVaryingHandler, args.fUniformHandler,
gpArgs.fLocalCoordVar, args.fFPCoordTransformHandler);
}
if (args.fGP.willUseTessellationShaders()) {
// Tessellation shaders are temporarily responsible for integrating their own code strings
@ -75,8 +73,7 @@ void GrGLSLGeometryProcessor::collectTransforms(GrGLSLVertexBuilder* vb,
const GrShaderVar& localCoordsVar,
FPCoordTransformHandler* handler) {
SkASSERT(localCoordsVar.getType() == kFloat2_GrSLType ||
localCoordsVar.getType() == kFloat3_GrSLType ||
localCoordsVar.getType() == kVoid_GrSLType /* until coord transforms are gone */);
localCoordsVar.getType() == kFloat3_GrSLType);
// Cached varyings produced by parent FPs. If parent FPs introduce transformations, but all
// subsequent children are not transformed, they should share the same varying.
std::unordered_map<const GrFragmentProcessor*, GrShaderVar> localCoordsMap;
@ -98,43 +95,19 @@ void GrGLSLGeometryProcessor::collectTransforms(GrGLSLVertexBuilder* vb,
for (int i = 0; *handler; ++*handler, ++i) {
auto [coordTransform, fp] = handler->get();
// FIXME: GrCoordTransform is used solely as a vehicle for iterating over all FPs that
// require sample coordinates directly. We should make this iteration lighter weight.
SkASSERT(coordTransform.isNoOp());
// FPs that use the legacy coord transform system will need a uniform registered for them
// to hold the coord transform's matrix.
GrShaderVar transformVar;
// FPs that use local coordinates need a varying to convey the coordinate. This may be the
// base GP's local coord if transforms have to be computed in the FS, or it may be a unique
// varying that computes the equivalent transformation hierarchy in the VS.
GrShaderVar varyingVar;
// If this is true, the FP's signature takes a float2 local coordinate. Otherwise, it
// doesn't use local coordinates, or it can be lifted to a varying and referenced directly.
bool localCoordComputedInFS = fp.isSampledWithExplicitCoords();
if (!coordTransform.isNoOp()) {
// Legacy coord transform that actually is doing something. This matrix is the last
// transformation to affect the local coordinate.
SkString strUniName;
strUniName.printf("CoordTransformMatrix_%d", i);
auto flag = localCoordComputedInFS ? kFragment_GrShaderFlag
: kVertex_GrShaderFlag;
auto& uni = fInstalledTransforms.push_back();
if (fp.isSampledWithExplicitCoords() && coordTransform.matrix().isScaleTranslate()) {
uni.fType = kFloat4_GrSLType;
} else {
uni.fType = kFloat3x3_GrSLType;
}
uni.fHandle =
uniformHandler->addUniform(&fp, flag, uni.fType, strUniName.c_str());
transformVar = uniformHandler->getUniformVariable(uni.fHandle);
} else {
// Must stay parallel with calls to handler
fInstalledTransforms.push_back();
}
// If the FP references local coords, we need to make sure the vertex shader sets up the
// right transforms or pass-through variables for the FP to evaluate in the fragment shader
if (fp.referencesSampleCoords()) {
if (localCoordComputedInFS) {
if (fp.isSampledWithExplicitCoords()) {
// If the FP local coords are evaluated in the fragment shader, we only need to
// produce the original local coordinate to pass into the root; any other situation,
// the FP will have a 2nd parameter to its function and the caller sends the coords
@ -168,19 +141,11 @@ void GrGLSLGeometryProcessor::collectTransforms(GrGLSLVertexBuilder* vb,
node = node->parent();
}
// Legacy coord transform workaround (if the transform hierarchy appears identity
// but we have GrCoordTransform that does something, we still need to record a
// varying for it).
if (!coordOwner && !coordTransform.isNoOp()) {
coordOwner = &fp;
}
if (coordOwner) {
// The FP will use coordOwner's varying; add varying if this was the first use
if (transformedLocalCoord.getType() == kVoid_GrSLType) {
GrGLSLVarying v(kFloat2_GrSLType);
if (coordTransform.matrix().hasPerspective() ||
GrSLTypeVecLength(localCoordsVar.getType()) == 3 ||
if (GrSLTypeVecLength(localCoordsVar.getType()) == 3 ||
coordOwner->hasPerspectiveTransform()) {
v = GrGLSLVarying(kFloat3_GrSLType);
}
@ -189,17 +154,11 @@ void GrGLSLGeometryProcessor::collectTransforms(GrGLSLVertexBuilder* vb,
varyingHandler->addVarying(strVaryingName.c_str(), &v);
fTransformInfos.push_back({GrShaderVar(v.vsOut(), v.type()),
transformVar.getName(),
localCoordsVar,
coordOwner});
transformedLocalCoord = GrShaderVar(SkString(v.fsIn()), v.type(),
GrShaderVar::TypeModifier::In);
if (coordOwner->numCoordTransforms() < 1 ||
coordOwner->coordTransform(0).isNoOp()) {
// As long as a legacy coord transform doesn't get in the way, we can
// reuse this expression for children (see comment in emitTransformCode)
localCoordsMap[coordOwner] = transformedLocalCoord;
}
localCoordsMap[coordOwner] = transformedLocalCoord;
}
varyingVar = transformedLocalCoord;
@ -210,11 +169,13 @@ void GrGLSLGeometryProcessor::collectTransforms(GrGLSLVertexBuilder* vb,
}
}
if (varyingVar.getType() != kVoid_GrSLType || transformVar.getType() != kVoid_GrSLType) {
handler->specifyCoordsForCurrCoordTransform(transformVar, varyingVar);
if (varyingVar.getType() != kVoid_GrSLType) {
handler->specifyCoordsForCurrCoordTransform(GrShaderVar(), varyingVar);
} else {
handler->omitCoordsForCurrCoordTransform();
}
// Must stay parallel with calls to handler
fInstalledTransforms.push_back();
}
}
@ -222,63 +183,53 @@ void GrGLSLGeometryProcessor::emitTransformCode(GrGLSLVertexBuilder* vb,
GrGLSLUniformHandler* uniformHandler) {
std::unordered_map<const GrFragmentProcessor*, GrShaderVar> localCoordsMap;
for (const auto& tr : fTransformInfos) {
// If we recorded a transform info, its sample matrix must be const/uniform, or we have a
// legacy coord transform that actually does something.
SkASSERT(tr.fFP->sampleMatrix().isConstUniform() ||
(tr.fFP->sampleMatrix().isNoOp() && !tr.fMatrix.isEmpty()));
// If we recorded a transform info, its sample matrix must be const/uniform
SkASSERT(tr.fFP->sampleMatrix().isConstUniform());
SkString localCoords;
// Build a concatenated matrix expression that we apply to the root local coord.
// If we have an expression cached from an early FP in the hierarchy chain, we can stop
// there instead of going all the way to the GP.
SkString transformExpression;
if (!tr.fMatrix.isEmpty()) {
// We have both a const/uniform sample matrix and a legacy coord transform
transformExpression.printf("%s", tr.fMatrix.c_str());
}
// If the sample matrix is kNone, then the current transform expression of just the
// coord transform matrix is sufficient.
if (tr.fFP->sampleMatrix().isConstUniform()) {
const auto* base = tr.fFP;
while(base) {
GrShaderVar cachedBaseCoord = localCoordsMap[base];
if (cachedBaseCoord.getType() != kVoid_GrSLType) {
// Can stop here, as this varying already holds all transforms from higher FPs
if (cachedBaseCoord.getType() == kFloat3_GrSLType) {
localCoords = cachedBaseCoord.getName();
} else {
localCoords = SkStringPrintf("%s.xy1", cachedBaseCoord.getName().c_str());
}
break;
} else if (base->sampleMatrix().isConstUniform()) {
// The FP knows the matrix expression it's sampled with, but its parent defined
// the uniform (when the expression is not a constant).
GrShaderVar uniform = uniformHandler->liftUniformToVertexShader(
*base->parent(), SkString(base->sampleMatrix().fExpression));
// Accumulate the base matrix expression as a preConcat
SkString matrix;
if (uniform.getType() != kVoid_GrSLType) {
SkASSERT(uniform.getType() == kFloat3x3_GrSLType);
matrix = uniform.getName();
} else {
// No uniform found, so presumably this is a constant
matrix = SkString(base->sampleMatrix().fExpression);
}
if (!transformExpression.isEmpty()) {
transformExpression.append(" * ");
}
transformExpression.appendf("(%s)", matrix.c_str());
const auto* base = tr.fFP;
while(base) {
GrShaderVar cachedBaseCoord = localCoordsMap[base];
if (cachedBaseCoord.getType() != kVoid_GrSLType) {
// Can stop here, as this varying already holds all transforms from higher FPs
if (cachedBaseCoord.getType() == kFloat3_GrSLType) {
localCoords = cachedBaseCoord.getName();
} else {
// This intermediate FP is just a pass through and doesn't need to be built
// in to the expression, but must visit its parents in case they add transforms
SkASSERT(base->sampleMatrix().isNoOp());
localCoords = SkStringPrintf("%s.xy1", cachedBaseCoord.getName().c_str());
}
break;
} else if (base->sampleMatrix().isConstUniform()) {
// The FP knows the matrix expression it's sampled with, but its parent defined
// the uniform (when the expression is not a constant).
GrShaderVar uniform = uniformHandler->liftUniformToVertexShader(
*base->parent(), SkString(base->sampleMatrix().fExpression));
// Accumulate the base matrix expression as a preConcat
SkString matrix;
if (uniform.getType() != kVoid_GrSLType) {
SkASSERT(uniform.getType() == kFloat3x3_GrSLType);
matrix = uniform.getName();
} else {
// No uniform found, so presumably this is a constant
matrix = SkString(base->sampleMatrix().fExpression);
}
base = base->parent();
if (!transformExpression.isEmpty()) {
transformExpression.append(" * ");
}
transformExpression.appendf("(%s)", matrix.c_str());
} else {
// This intermediate FP is just a pass through and doesn't need to be built
// in to the expression, but must visit its parents in case they add transforms
SkASSERT(base->sampleMatrix().isNoOp());
}
base = base->parent();
}
if (localCoords.isEmpty()) {
@ -304,18 +255,7 @@ void GrGLSLGeometryProcessor::emitTransformCode(GrGLSLVertexBuilder* vb,
vb->codeAppend(";\n");
vb->codeAppend("}\n");
if (tr.fMatrix.isEmpty()) {
// Subtle work around: only cache the intermediate varying when there's no extra
// coord transform. If the FP uses a coord transform for a legacy effect, but also
// delegates to a child FP, we want the coordinates pre-GrCoordTransform to be sent
// to the child FP, but have the FP use the post-coordtransform legacy values
// (e.g. sampling a texture and relying on the GrCoordTransform for normalization
// and mixing with a child FP that should not be normalized).
// FIXME: It's not really possible to apply this logic cleanly when transforms
// have been moved to the FS; in practice this doesn't seem to occur in our tests and
// the issue will go away once legacy coord transforms only have no-op matrices.
localCoordsMap.insert({ tr.fFP, tr.fOutputCoords });
}
localCoordsMap.insert({ tr.fFP, tr.fOutputCoords });
}
}

View File

@ -124,8 +124,6 @@ private:
struct TransformInfo {
// The vertex-shader output variable to assign the transformed coordinates to
GrShaderVar fOutputCoords;
// The name of a coord transform uniform to apply
SkString fMatrix;
// The coordinate to be transformed
GrShaderVar fLocalCoords;
// The leaf FP of a transform hierarchy to be evaluated in the vertex shader;