Use an attribute value for the PMV matrix rather than a uniform

This has several advantages: First, updating an attribute value
seems to be cheaper than updating a uniform. Second, vertex atribute
values are independent of shader program, which means they persist
across changing of the shader program. This makes code simpler and
reduces GL state changes. Note: Credit goes to Samuel for finding
this little gem. :-)

For the 25920 solid QGraphicsRectItem test case, this gives 10%
improvement on desktop and 27% on the SGX.

Reviewed-By: Kim

5 files changed, 42 insertions, 31 deletions

diff --git a/src/opengl/gl2paintengineex/qglengineshadermanager.cpp b/src/opengl/gl2paintengineex/qglengineshadermanager.cpp
index 556b888..9fd9e18 100644
--- a/src/opengl/gl2paintengineex/qglengineshadermanager.cpp
+++ b/src/opengl/gl2paintengineex/qglengineshadermanager.cpp
@@ -324,6 +324,11 @@ QGLEngineShaderProg *QGLEngineSharedShaders::findProgramInCache(const QGLEngineS
             newProg->program->bindAttributeLocation("textureCoordArray", QT_TEXTURE_COORDS_ATTR);
         if (newProg->useOpacityAttribute)
             newProg->program->bindAttributeLocation("opacityArray", QT_OPACITY_ATTR);
+        if (newProg->usePmvMatrix) {
+            newProg->program->bindAttributeLocation("pmvMatrix1", QT_PMV_MATRIX_1_ATTR);
+            newProg->program->bindAttributeLocation("pmvMatrix2", QT_PMV_MATRIX_2_ATTR);
+            newProg->program->bindAttributeLocation("pmvMatrix3", QT_PMV_MATRIX_3_ATTR);
+        }
 
         newProg->program->link();
         if (!newProg->program->isLinked()) {
@@ -424,7 +429,6 @@ GLuint QGLEngineShaderManager::getUniformLocation(Uniform id)
         "patternColor",
         "globalOpacity",
         "depth",
-        "pmvMatrix",
         "maskTexture",
         "fragmentColor",
         "linearData",
@@ -743,6 +747,7 @@ bool QGLEngineShaderManager::useCorrectShaderProg()
     }
     requiredProgram.useTextureCoords = texCoords;
     requiredProgram.useOpacityAttribute = (opacityMode == AttributeOpacity);
+    requiredProgram.usePmvMatrix = true;
 
     // At this point, requiredProgram is fully populated so try to find the program in the cache
     currentShaderProg = sharedShaders->findProgramInCache(requiredProgram);
diff --git a/src/opengl/gl2paintengineex/qglengineshadermanager_p.h b/src/opengl/gl2paintengineex/qglengineshadermanager_p.h
index c52e5c0..3ab4ebe 100644
--- a/src/opengl/gl2paintengineex/qglengineshadermanager_p.h
+++ b/src/opengl/gl2paintengineex/qglengineshadermanager_p.h
@@ -253,6 +253,9 @@ struct QGLEngineCachedShaderProg
 static const GLuint QT_VERTEX_COORDS_ATTR  = 0;
 static const GLuint QT_TEXTURE_COORDS_ATTR = 1;
 static const GLuint QT_OPACITY_ATTR = 2;
+static const GLuint QT_PMV_MATRIX_1_ATTR = 3;
+static const GLuint QT_PMV_MATRIX_2_ATTR = 4;
+static const GLuint QT_PMV_MATRIX_3_ATTR = 5;
 
 class QGLEngineShaderProg;
 
@@ -397,6 +400,7 @@ public:
 
     bool                useTextureCoords;
     bool                useOpacityAttribute;
+    bool                usePmvMatrix;
 
     bool operator==(const QGLEngineShaderProg& other) {
         // We don't care about the program
@@ -431,7 +435,6 @@ public:
         PatternColor,
         GlobalOpacity,
         Depth,
-        PmvMatrix,
         MaskTexture,
         FragmentColor,
         LinearData,
diff --git a/src/opengl/gl2paintengineex/qglengineshadersource_p.h b/src/opengl/gl2paintengineex/qglengineshadersource_p.h
index 6e98b02..b471b81 100644
--- a/src/opengl/gl2paintengineex/qglengineshadersource_p.h
+++ b/src/opengl/gl2paintengineex/qglengineshadersource_p.h
@@ -97,9 +97,12 @@ static const char* const qglslMainWithTexCoordsAndOpacityVertexShader = "\
 //       shader are also perspective corrected.
 static const char* const qglslPositionOnlyVertexShader = "\
     attribute highp vec2    vertexCoordsArray;\
-    uniform   highp mat3    pmvMatrix;\
+    attribute highp vec3    pmvMatrix1; \
+    attribute highp vec3    pmvMatrix2; \
+    attribute highp vec3    pmvMatrix3; \
     void setPosition(void)\
     {\
+        highp mat3 pmvMatrix = mat3(pmvMatrix1, pmvMatrix2, pmvMatrix3); \
         vec3 transformedPos = pmvMatrix * vec3(vertexCoordsArray.xy, 1.0); \
         gl_Position = vec4(transformedPos.xy, 0.0, transformedPos.z); \
     }";
@@ -114,12 +117,15 @@ static const char* const qglslUntransformedPositionVertexShader = "\
 // Pattern Brush - This assumes the texture size is 8x8 and thus, the inverted size is 0.125
 static const char* const qglslPositionWithPatternBrushVertexShader = "\
     attribute highp   vec2  vertexCoordsArray; \
-    uniform   highp   mat3  pmvMatrix; \
+    attribute highp   vec3  pmvMatrix1; \
+    attribute highp   vec3  pmvMatrix2; \
+    attribute highp   vec3  pmvMatrix3; \
     uniform   mediump vec2  halfViewportSize; \
     uniform   highp   vec2  invertedTextureSize; \
     uniform   highp   mat3  brushTransform; \
     varying   highp   vec2  patternTexCoords; \
     void setPosition(void) { \
+            highp mat3 pmvMatrix = mat3(pmvMatrix1, pmvMatrix2, pmvMatrix3); \
             vec3 transformedPos = pmvMatrix * vec3(vertexCoordsArray.xy, 1.0); \
             gl_Position.xy = transformedPos.xy / transformedPos.z; \
             mediump vec2 viewportCoords = (gl_Position.xy + 1.0) * halfViewportSize; \
@@ -144,12 +150,15 @@ static const char* const qglslPatternBrushSrcFragmentShader = "\
 // Linear Gradient Brush
 static const char* const qglslPositionWithLinearGradientBrushVertexShader = "\
     attribute highp   vec2  vertexCoordsArray; \
-    uniform   highp   mat3  pmvMatrix; \
+    attribute highp   vec3  pmvMatrix1; \
+    attribute highp   vec3  pmvMatrix2; \
+    attribute highp   vec3  pmvMatrix3; \
     uniform   mediump vec2  halfViewportSize; \
     uniform   highp   vec3  linearData; \
     uniform   highp   mat3  brushTransform; \
     varying   mediump float index; \
     void setPosition() { \
+        highp mat3 pmvMatrix = mat3(pmvMatrix1, pmvMatrix2, pmvMatrix3); \
         vec3 transformedPos = pmvMatrix * vec3(vertexCoordsArray.xy, 1.0); \
         gl_Position.xy = transformedPos.xy / transformedPos.z; \
         mediump vec2 viewportCoords = (gl_Position.xy + 1.0) * halfViewportSize; \
@@ -174,12 +183,15 @@ static const char* const qglslLinearGradientBrushSrcFragmentShader = "\
 // Conical Gradient Brush
 static const char* const qglslPositionWithConicalGradientBrushVertexShader = "\
     attribute highp   vec2  vertexCoordsArray;\
-    uniform   highp   mat3  pmvMatrix;\
+    attribute highp   vec3  pmvMatrix1; \
+    attribute highp   vec3  pmvMatrix2; \
+    attribute highp   vec3  pmvMatrix3; \
     uniform   mediump vec2  halfViewportSize; \
     uniform   highp   mat3  brushTransform; \
     varying   highp   vec2  A; \
-    void setPosition(void)\
-    {\
+    void setPosition(void) \
+    { \
+        highp mat3 pmvMatrix = mat3(pmvMatrix1, pmvMatrix2, pmvMatrix3); \
         vec3 transformedPos = pmvMatrix * vec3(vertexCoordsArray.xy, 1.0); \
         gl_Position.xy = transformedPos.xy / transformedPos.z; \
         mediump vec2  viewportCoords = (gl_Position.xy + 1.0) * halfViewportSize; \
@@ -210,7 +222,9 @@ static const char* const qglslConicalGradientBrushSrcFragmentShader = "\n\
 // Radial Gradient Brush
 static const char* const qglslPositionWithRadialGradientBrushVertexShader = "\
     attribute highp   vec2 vertexCoordsArray;\
-    uniform   highp   mat3 pmvMatrix;\
+    attribute highp   vec3  pmvMatrix1; \
+    attribute highp   vec3  pmvMatrix2; \
+    attribute highp   vec3  pmvMatrix3; \
     uniform   mediump vec2 halfViewportSize; \
     uniform   highp   mat3 brushTransform; \
     uniform   highp   vec2 fmp; \
@@ -218,6 +232,7 @@ static const char* const qglslPositionWithRadialGradientBrushVertexShader = "\
     varying   highp   vec2  A; \
     void setPosition(void) \
     {\
+        highp mat3 pmvMatrix = mat3(pmvMatrix1, pmvMatrix2, pmvMatrix3); \
         vec3 transformedPos = pmvMatrix * vec3(vertexCoordsArray.xy, 1.0); \
         gl_Position.xy = transformedPos.xy / transformedPos.z; \
         mediump vec2 viewportCoords = (gl_Position.xy + 1.0) * halfViewportSize; \
@@ -247,12 +262,15 @@ static const char* const qglslRadialGradientBrushSrcFragmentShader = "\
 // Texture Brush
 static const char* const qglslPositionWithTextureBrushVertexShader = "\
     attribute highp   vec2 vertexCoordsArray; \
-    uniform   highp   mat3  pmvMatrix; \
+    attribute highp   vec3  pmvMatrix1; \
+    attribute highp   vec3  pmvMatrix2; \
+    attribute highp   vec3  pmvMatrix3; \
     uniform   mediump vec2  halfViewportSize; \
     uniform   highp   vec2  invertedTextureSize; \
     uniform   highp   mat3  brushTransform; \
     varying   highp   vec2  textureCoords; \
     void setPosition(void) { \
+            highp mat3 pmvMatrix = mat3(pmvMatrix1, pmvMatrix2, pmvMatrix3); \
             vec3 transformedPos = pmvMatrix * vec3(vertexCoordsArray.xy, 1.0); \
             gl_Position.xy = transformedPos.xy / transformedPos.z; \
             mediump vec2 viewportCoords = (gl_Position.xy + 1.0) * halfViewportSize; \
diff --git a/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp b/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp
index caa679b..b282676 100644
--- a/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp
+++ b/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp
@@ -168,12 +168,6 @@ void QGL2PaintEngineExPrivate::useSimpleShader()
 
     if (matrixDirty)
         updateMatrix();
-
-    if (simpleShaderMatrixUniformDirty) {
-        const GLuint location = shaderManager->simpleProgram()->uniformLocation("pmvMatrix");
-        glUniformMatrix3fv(location, 1, GL_FALSE, (GLfloat*)pmvMatrix);
-        simpleShaderMatrixUniformDirty = false;
-    }
 }
 
 void QGL2PaintEngineExPrivate::updateBrushTexture()
@@ -392,9 +386,11 @@ void QGL2PaintEngineExPrivate::updateMatrix()
 
     matrixDirty = false;
 
-    // The actual data has been updated so both shader program's uniforms need updating
-    simpleShaderMatrixUniformDirty = true;
-    shaderMatrixUniformDirty = true;
+    // Set the PMV matrix attribute. As we use an attributes rather than uniforms, we only
+    // need to do this once for every matrix change and persists across all shader programs.
+    glVertexAttrib3fv(QT_PMV_MATRIX_1_ATTR, pmvMatrix[0]);
+    glVertexAttrib3fv(QT_PMV_MATRIX_2_ATTR, pmvMatrix[1]);
+    glVertexAttrib3fv(QT_PMV_MATRIX_3_ATTR, pmvMatrix[2]);
 
     dasher.setInvScale(inverseScale);
     stroker.setInvScale(inverseScale);
@@ -932,18 +928,12 @@ bool QGL2PaintEngineExPrivate::prepareForDraw(bool srcPixelsAreOpaque)
     if (changed) {
         // The shader program has changed so mark all uniforms as dirty:
         brushUniformsDirty = true;
-        shaderMatrixUniformDirty = true;
         opacityUniformDirty = true;
     }
 
     if (brushUniformsDirty && mode != ImageDrawingMode && mode != ImageArrayDrawingMode)
         updateBrushUniforms();
 
-    if (shaderMatrixUniformDirty) {
-        glUniformMatrix3fv(location(QGLEngineShaderManager::PmvMatrix), 1, GL_FALSE, (GLfloat*)pmvMatrix);
-        shaderMatrixUniformDirty = false;
-    }
-
     if (opacityMode == QGLEngineShaderManager::UniformOpacity && opacityUniformDirty) {
         shaderManager->currentProgram()->setUniformValue(location(QGLEngineShaderManager::GlobalOpacity), (GLfloat)q->state()->opacity);
         opacityUniformDirty = false;
@@ -1955,11 +1945,8 @@ void QGL2PaintEngineEx::setState(QPainterState *new_state)
     if (old_state == s || old_state->renderHintsChanged)
         renderHintsChanged();
 
-    if (old_state == s || old_state->matrixChanged) {
+    if (old_state == s || old_state->matrixChanged)
         d->matrixDirty = true;
-        d->simpleShaderMatrixUniformDirty = true;
-        d->shaderMatrixUniformDirty = true;
-    }
 
     if (old_state == s || old_state->compositionModeChanged)
         d->compositionModeDirty = true;
diff --git a/src/opengl/gl2paintengineex/qpaintengineex_opengl2_p.h b/src/opengl/gl2paintengineex/qpaintengineex_opengl2_p.h
index ce1b538..8fa0eff 100644
--- a/src/opengl/gl2paintengineex/qpaintengineex_opengl2_p.h
+++ b/src/opengl/gl2paintengineex/qpaintengineex_opengl2_p.h
@@ -252,8 +252,6 @@ public:
     bool compositionModeDirty;
     bool brushTextureDirty;
     bool brushUniformsDirty;
-    bool simpleShaderMatrixUniformDirty;
-    bool shaderMatrixUniformDirty;
     bool opacityUniformDirty;
 
     bool stencilClean; // Has the stencil not been used for clipping so far?