fix for uniform view matrix being uploaded but not used
As discussed, I'll follow this up by removing localmatrix/uniform view matrix from the base classe
BUG=skia:
Review URL: https://codereview.chromium.org/920933002
diff --git a/src/effects/SkColorCubeFilter.cpp b/src/effects/SkColorCubeFilter.cpp
index df9f5ea..31876aa 100644
--- a/src/effects/SkColorCubeFilter.cpp
+++ b/src/effects/SkColorCubeFilter.cpp
@@ -231,7 +231,6 @@
GrColorCubeEffect(GrTexture* colorCube);
- GrCoordTransform fColorCubeTransform;
GrTextureAccess fColorCubeAccess;
typedef GrFragmentProcessor INHERITED;
@@ -240,10 +239,8 @@
///////////////////////////////////////////////////////////////////////////////
GrColorCubeEffect::GrColorCubeEffect(GrTexture* colorCube)
- : fColorCubeTransform(kLocal_GrCoordSet, colorCube, GrTextureParams::kBilerp_FilterMode)
- , fColorCubeAccess(colorCube, "bgra", GrTextureParams::kBilerp_FilterMode) {
+ : fColorCubeAccess(colorCube, "bgra", GrTextureParams::kBilerp_FilterMode) {
this->initClassID<GrColorCubeEffect>();
- this->addCoordTransform(&fColorCubeTransform);
this->addTextureAccess(&fColorCubeAccess);
}
@@ -319,9 +316,9 @@
// Apply the cube.
fsBuilder->codeAppendf("%s = vec4(mix(", outputColor);
- fsBuilder->appendTextureLookup(samplers[0], cCoords1, coords[0].getType());
+ fsBuilder->appendTextureLookup(samplers[0], cCoords1);
fsBuilder->codeAppend(".rgb, ");
- fsBuilder->appendTextureLookup(samplers[0], cCoords2, coords[0].getType());
+ fsBuilder->appendTextureLookup(samplers[0], cCoords2);
// Premultiply color by alpha. Note that the input alpha is not modified by this shader.
fsBuilder->codeAppendf(".rgb, fract(%s.b)) * vec3(%s), %s.a);\n",
@@ -338,7 +335,6 @@
void GrColorCubeEffect::GLProcessor::GenKey(const GrProcessor& proc,
const GrGLCaps&, GrProcessorKeyBuilder* b) {
- b->add32(1); // Always same shader for now
}
GrFragmentProcessor* SkColorCubeFilter::asFragmentProcessor(GrContext* context) const {
diff --git a/src/gpu/GrAAConvexPathRenderer.cpp b/src/gpu/GrAAConvexPathRenderer.cpp
index f4a6266..6b825e7 100644
--- a/src/gpu/GrAAConvexPathRenderer.cpp
+++ b/src/gpu/GrAAConvexPathRenderer.cpp
@@ -542,12 +542,8 @@
this->setupColorPassThrough(pb, local.fInputColorType, args.fOutputColor, NULL,
&fColorUniform);
- // setup uniform viewMatrix
- this->addUniformViewMatrix(pb);
-
// Setup position
- SetupPosition(vsBuilder, gpArgs, qe.inPosition()->fName,
- qe.viewMatrix(), this->uViewM());
+ this->setupPosition(pb, gpArgs, qe.inPosition()->fName, qe.viewMatrix());
// emit transforms
this->emitTransforms(args.fPB, gpArgs->fPositionVar, qe.inPosition()->fName,
diff --git a/src/gpu/GrDefaultGeoProcFactory.cpp b/src/gpu/GrDefaultGeoProcFactory.cpp
index c0e8ea7..ebd685b 100644
--- a/src/gpu/GrDefaultGeoProcFactory.cpp
+++ b/src/gpu/GrDefaultGeoProcFactory.cpp
@@ -95,13 +95,8 @@
// Setup pass through color
this->setupColorPassThrough(pb, local.fInputColorType, args.fOutputColor, gp.inColor(),
&fColorUniform);
-
- // setup uniform viewMatrix
- this->addUniformViewMatrix(pb);
-
// Setup position
- SetupPosition(vsBuilder, gpArgs, gp.inPosition()->fName,
- gp.viewMatrix(), this->uViewM());
+ this->setupPosition(pb, gpArgs, gp.inPosition()->fName, gp.viewMatrix());
if (gp.inLocalCoords()) {
// emit transforms with explicit local coords
diff --git a/src/gpu/GrGeometryProcessor.cpp b/src/gpu/GrGeometryProcessor.cpp
index 23989cf..703c42e 100644
--- a/src/gpu/GrGeometryProcessor.cpp
+++ b/src/gpu/GrGeometryProcessor.cpp
@@ -25,5 +25,3 @@
void GrGeometryProcessor::getInvariantOutputCoverage(GrInitInvariantOutput* out) const {
this->onGetInvariantOutputCoverage(out);
}
-
-
diff --git a/src/gpu/GrOvalRenderer.cpp b/src/gpu/GrOvalRenderer.cpp
index 0a18b49..481501f 100644
--- a/src/gpu/GrOvalRenderer.cpp
+++ b/src/gpu/GrOvalRenderer.cpp
@@ -105,12 +105,8 @@
this->setupColorPassThrough(pb, local.fInputColorType, args.fOutputColor, NULL,
&fColorUniform);
- // setup uniform viewMatrix
- this->addUniformViewMatrix(pb);
-
// Setup position
- SetupPosition(vsBuilder, gpArgs, ce.inPosition()->fName,
- ce.viewMatrix(), this->uViewM());
+ this->setupPosition(pb, gpArgs, ce.inPosition()->fName, ce.viewMatrix());
// emit transforms
this->emitTransforms(args.fPB, gpArgs->fPositionVar, ce.inPosition()->fName,
@@ -288,12 +284,8 @@
this->setupColorPassThrough(pb, local.fInputColorType, args.fOutputColor, NULL,
&fColorUniform);
- // setup uniform viewMatrix
- this->addUniformViewMatrix(pb);
-
// Setup position
- SetupPosition(vsBuilder, gpArgs, ee.inPosition()->fName,
- ee.viewMatrix(), this->uViewM());
+ this->setupPosition(pb, gpArgs, ee.inPosition()->fName, ee.viewMatrix());
// emit transforms
this->emitTransforms(args.fPB, gpArgs->fPositionVar, ee.inPosition()->fName,
@@ -493,12 +485,8 @@
this->setupColorPassThrough(pb, local.fInputColorType, args.fOutputColor, NULL,
&fColorUniform);
- // setup uniform viewMatrix
- this->addUniformViewMatrix(pb);
-
// Setup position
- SetupPosition(vsBuilder, gpArgs, ee.inPosition()->fName,
- ee.viewMatrix(), this->uViewM());
+ this->setupPosition(pb, gpArgs, ee.inPosition()->fName, ee.viewMatrix());
// emit transforms
this->emitTransforms(args.fPB, gpArgs->fPositionVar, ee.inPosition()->fName,
diff --git a/src/gpu/effects/GrBezierEffect.cpp b/src/gpu/effects/GrBezierEffect.cpp
index 37e1dff..158fd5d 100644
--- a/src/gpu/effects/GrBezierEffect.cpp
+++ b/src/gpu/effects/GrBezierEffect.cpp
@@ -83,11 +83,8 @@
this->setupColorPassThrough(args.fPB, local.fInputColorType, args.fOutputColor, NULL,
&fColorUniform);
- // setup uniform viewMatrix
- this->addUniformViewMatrix(pb);
-
// Setup position
- SetupPosition(vsBuilder, gpArgs, gp.inPosition()->fName, gp.viewMatrix(), this->uViewM());
+ this->setupPosition(pb, gpArgs, gp.inPosition()->fName, gp.viewMatrix());
// emit transforms with position
this->emitTransforms(pb, gpArgs->fPositionVar, gp.inPosition()->fName, gp.localMatrix(),
@@ -153,10 +150,10 @@
if (0xff != local.fCoverageScale) {
const char* coverageScale;
fCoverageScaleUniform = pb->addUniform(GrGLProgramBuilder::kFragment_Visibility,
- kFloat_GrSLType,
- kDefault_GrSLPrecision,
- "Coverage",
- &coverageScale);
+ kFloat_GrSLType,
+ kDefault_GrSLPrecision,
+ "Coverage",
+ &coverageScale);
fsBuilder->codeAppendf("%s = vec4(%s * edgeAlpha);", args.fOutputCoverage, coverageScale);
} else {
fsBuilder->codeAppendf("%s = vec4(edgeAlpha);", args.fOutputCoverage);
@@ -321,11 +318,8 @@
this->setupColorPassThrough(args.fPB, local.fInputColorType, args.fOutputColor, NULL,
&fColorUniform);
- // setup uniform viewMatrix
- this->addUniformViewMatrix(pb);
-
// Setup position
- SetupPosition(vsBuilder, gpArgs, gp.inPosition()->fName, gp.viewMatrix(), this->uViewM());
+ this->setupPosition(pb, gpArgs, gp.inPosition()->fName, gp.viewMatrix());
// emit transforms with position
this->emitTransforms(pb, gpArgs->fPositionVar, gp.inPosition()->fName, gp.localMatrix(),
@@ -538,11 +532,8 @@
this->setupColorPassThrough(args.fPB, local.fInputColorType, args.fOutputColor, NULL,
&fColorUniform);
- // setup uniform viewMatrix
- this->addUniformViewMatrix(args.fPB);
-
// Setup position
- SetupPosition(vsBuilder, gpArgs, gp.inPosition()->fName, gp.viewMatrix(), this->uViewM());
+ this->setupPosition(args.fPB, gpArgs, gp.inPosition()->fName, gp.viewMatrix());
// emit transforms with position
this->emitTransforms(args.fPB, gpArgs->fPositionVar, gp.inPosition()->fName, gp.localMatrix(),
diff --git a/src/gpu/effects/GrBitmapTextGeoProc.cpp b/src/gpu/effects/GrBitmapTextGeoProc.cpp
index a699b04..2487b65 100644
--- a/src/gpu/effects/GrBitmapTextGeoProc.cpp
+++ b/src/gpu/effects/GrBitmapTextGeoProc.cpp
@@ -43,11 +43,8 @@
this->setupColorPassThrough(pb, local.fInputColorType, args.fOutputColor, cte.inColor(),
&fColorUniform);
- // setup uniform viewMatrix
- this->addUniformViewMatrix(pb);
-
// Setup position
- SetupPosition(vsBuilder, gpArgs, cte.inPosition()->fName, cte.viewMatrix(), this->uViewM());
+ this->setupPosition(pb, gpArgs, cte.inPosition()->fName, cte.viewMatrix());
// emit transforms
this->emitTransforms(args.fPB, gpArgs->fPositionVar, cte.inPosition()->fName,
diff --git a/src/gpu/effects/GrDashingEffect.cpp b/src/gpu/effects/GrDashingEffect.cpp
index a33d604..0b84c2c 100644
--- a/src/gpu/effects/GrDashingEffect.cpp
+++ b/src/gpu/effects/GrDashingEffect.cpp
@@ -645,11 +645,8 @@
// Setup pass through color
this->setupColorPassThrough(pb, local.fInputColorType, args.fOutputColor, NULL, &fColorUniform);
- // setup uniform viewMatrix
- this->addUniformViewMatrix(pb);
-
// Setup position
- SetupPosition(vsBuilder, gpArgs, dce.inPosition()->fName, dce.viewMatrix(), this->uViewM());
+ this->setupPosition(pb, gpArgs, dce.inPosition()->fName, dce.viewMatrix());
// emit transforms
this->emitTransforms(args.fPB, gpArgs->fPositionVar, dce.inPosition()->fName, dce.localMatrix(),
@@ -892,11 +889,8 @@
// Setup pass through color
this->setupColorPassThrough(pb, local.fInputColorType, args.fOutputColor, NULL, &fColorUniform);
- // setup uniform viewMatrix
- this->addUniformViewMatrix(pb);
-
// Setup position
- SetupPosition(vsBuilder, gpArgs, de.inPosition()->fName, de.viewMatrix(), this->uViewM());
+ this->setupPosition(pb, gpArgs, de.inPosition()->fName, de.viewMatrix());
// emit transforms
this->emitTransforms(args.fPB, gpArgs->fPositionVar, de.inPosition()->fName, de.localMatrix(),
diff --git a/src/gpu/effects/GrDistanceFieldTextureEffect.cpp b/src/gpu/effects/GrDistanceFieldTextureEffect.cpp
index dfc4ca1..1a8a07e 100755
--- a/src/gpu/effects/GrDistanceFieldTextureEffect.cpp
+++ b/src/gpu/effects/GrDistanceFieldTextureEffect.cpp
@@ -57,12 +57,8 @@
this->setupColorPassThrough(pb, local.fInputColorType, args.fOutputColor,
dfTexEffect.inColor(), &fColorUniform);
- // setup uniform viewMatrix
- this->addUniformViewMatrix(pb);
-
// Setup position
- SetupPosition(vsBuilder, gpArgs, dfTexEffect.inPosition()->fName,
- dfTexEffect.viewMatrix(), this->uViewM());
+ this->setupPosition(pb, gpArgs, dfTexEffect.inPosition()->fName, dfTexEffect.viewMatrix());
// emit transforms
this->emitTransforms(args.fPB, gpArgs->fPositionVar, dfTexEffect.inPosition()->fName,
@@ -357,12 +353,8 @@
vsBuilder->codeAppendf("%s = %s;", v.vsOut(), dfTexEffect.inTextureCoords()->fName);
- // setup uniform viewMatrix
- this->addUniformViewMatrix(pb);
-
// Setup position
- SetupPosition(vsBuilder, gpArgs, dfTexEffect.inPosition()->fName,
- dfTexEffect.viewMatrix(), this->uViewM());
+ this->setupPosition(pb, gpArgs, dfTexEffect.inPosition()->fName, dfTexEffect.viewMatrix());
// emit transforms
this->emitTransforms(args.fPB, gpArgs->fPositionVar, dfTexEffect.inPosition()->fName,
@@ -602,12 +594,8 @@
this->setupColorPassThrough(pb, local.fInputColorType, args.fOutputColor, NULL,
&fColorUniform);
- // setup uniform viewMatrix
- this->addUniformViewMatrix(pb);
-
// Setup position
- SetupPosition(vsBuilder, gpArgs, dfTexEffect.inPosition()->fName,
- dfTexEffect.viewMatrix(), this->uViewM());
+ this->setupPosition(pb, gpArgs, dfTexEffect.inPosition()->fName, dfTexEffect.viewMatrix());
// emit transforms
this->emitTransforms(args.fPB, gpArgs->fPositionVar, dfTexEffect.inPosition()->fName,
diff --git a/src/gpu/gl/GrGLGeometryProcessor.cpp b/src/gpu/gl/GrGLGeometryProcessor.cpp
index a8d020a..79bcb4c 100644
--- a/src/gpu/gl/GrGLGeometryProcessor.cpp
+++ b/src/gpu/gl/GrGLGeometryProcessor.cpp
@@ -106,24 +106,26 @@
}
}
-void GrGLGeometryProcessor::SetupPosition(GrGLVertexBuilder* vsBuilder,
+void GrGLGeometryProcessor::setupPosition(GrGLGPBuilder* pb,
GrGPArgs* gpArgs,
const char* posName,
- const SkMatrix& mat,
- const char* matName) {
+ const SkMatrix& mat) {
+ GrGLVertexBuilder* vsBuilder = pb->getVertexShaderBuilder();
if (mat.isIdentity()) {
gpArgs->fPositionVar.set(kVec2f_GrSLType, "pos2");
vsBuilder->codeAppendf("vec2 %s = %s;", gpArgs->fPositionVar.c_str(), posName);
} else if (!mat.hasPerspective()) {
+ this->addUniformViewMatrix(pb);
gpArgs->fPositionVar.set(kVec2f_GrSLType, "pos2");
vsBuilder->codeAppendf("vec2 %s = vec2(%s * vec3(%s, 1));",
- gpArgs->fPositionVar.c_str(), matName, posName);
+ gpArgs->fPositionVar.c_str(), this->uViewM(), posName);
} else {
+ this->addUniformViewMatrix(pb);
gpArgs->fPositionVar.set(kVec3f_GrSLType, "pos3");
vsBuilder->codeAppendf("vec3 %s = %s * vec3(%s, 1);",
- gpArgs->fPositionVar.c_str(), matName, posName);
+ gpArgs->fPositionVar.c_str(), this->uViewM(), posName);
}
}
diff --git a/src/gpu/gl/GrGLGeometryProcessor.h b/src/gpu/gl/GrGLGeometryProcessor.h
index e8b16af..2630bde 100644
--- a/src/gpu/gl/GrGLGeometryProcessor.h
+++ b/src/gpu/gl/GrGLGeometryProcessor.h
@@ -10,7 +10,7 @@
#include "GrGLPrimitiveProcessor.h"
-class GrGLVertexBuilder;
+class GrGLGPBuilder;
/**
* If a GL effect needs a GrGLFullShaderBuilder* object to emit vertex code, then it must inherit
@@ -51,11 +51,10 @@
};
// Create the correct type of position variable given the CTM
- static void SetupPosition(GrGLVertexBuilder* vsBuilder,
- GrGPArgs* gpArgs,
- const char* posName,
- const SkMatrix& mat,
- const char* matName);
+ void setupPosition(GrGLGPBuilder* pb,
+ GrGPArgs* gpArgs,
+ const char* posName,
+ const SkMatrix& mat);
static uint32_t ComputePosKey(const SkMatrix& mat) {
if (mat.isIdentity()) {
diff --git a/src/gpu/gl/GrGLPrimitiveProcessor.cpp b/src/gpu/gl/GrGLPrimitiveProcessor.cpp
index 335dc4f..2bb595a 100644
--- a/src/gpu/gl/GrGLPrimitiveProcessor.cpp
+++ b/src/gpu/gl/GrGLPrimitiveProcessor.cpp
@@ -64,7 +64,7 @@
void GrGLPrimitiveProcessor::setUniformViewMatrix(const GrGLProgramDataManager& pdman,
const SkMatrix& viewMatrix) {
- if (!fViewMatrix.cheapEqualTo(viewMatrix)) {
+ if (!viewMatrix.isIdentity() && !fViewMatrix.cheapEqualTo(viewMatrix)) {
SkASSERT(fViewMatrixUniform.isValid());
fViewMatrix = viewMatrix;
diff --git a/src/gpu/gl/GrGLProgramDataManager.cpp b/src/gpu/gl/GrGLProgramDataManager.cpp
index 67b7041..9cd8d57 100644
--- a/src/gpu/gl/GrGLProgramDataManager.cpp
+++ b/src/gpu/gl/GrGLProgramDataManager.cpp
@@ -185,8 +185,7 @@
const Uniform& uni = fUniforms[u.toProgramDataIndex()];
SkASSERT(uni.fType == kMat33f_GrSLType);
SkASSERT(GrGLShaderVar::kNonArray == uni.fArrayCount);
- // TODO: Re-enable this assert once texture matrices aren't forced on all effects
- // SkASSERT(kUnusedUniform != uni.fFSLocation || kUnusedUniform != uni.fVSLocation);
+ SkASSERT(kUnusedUniform != uni.fFSLocation || kUnusedUniform != uni.fVSLocation);
if (kUnusedUniform != uni.fFSLocation) {
GR_GL_CALL(fGpu->glInterface(), UniformMatrix3fv(uni.fFSLocation, 1, false, matrix));
}
diff --git a/src/gpu/gl/builders/GrGLShaderStringBuilder.cpp b/src/gpu/gl/builders/GrGLShaderStringBuilder.cpp
index 5265d20..1e75048 100644
--- a/src/gpu/gl/builders/GrGLShaderStringBuilder.cpp
+++ b/src/gpu/gl/builders/GrGLShaderStringBuilder.cpp
@@ -6,7 +6,7 @@
*/
#include "GrGLShaderStringBuilder.h"
-#include "../GrGLGpu.h"
+#include "gl/GrGLGpu.h"
#include "gl/GrGLSLPrettyPrint.h"
#include "SkRTConf.h"
#include "SkTraceEvent.h"