Allow batching ShaderEffects without atlas samplers

It is currently necessary to set supportsAtlasTextures to enable
batching of all ShaderEffect, even if they sample a
ShaderEffectSource or have no sampler uniform at all.

After fccbe57258 it is not necessary to
set the RequiresFullMatrix flag based on supportsAtlasTextures since
this now prevents batching altogether. Limiting batching should
however also first verify that there are any sampler of the atlas
used on that ShaderEffect in QQuickShaderEffectMaterial::compare,
since that check wouldn't have any effect in
QQuickShaderEffect::updatePaintNode through RequiresFullMatrix.

Also make sure that using texture()->normalizedTextureSubRect()
as the texture coordinate attribute is synchronized with calls to
removedFromAtlas for all samplers and make sure that this only
applies when the texture count == 1. This fixes a bug where
setting supportsAtlasTextures on a ShaderEffect with more that
one sampler would incorrectly use the texture coordinates of the
first atlas texture in the geometry.

This patch also removes:
- The unneeded check for the RequiresFullMatrix in compare; the same
  program can be used in batched geometries and the renderer itself
  will prevent merging in that case.
- The possibly heavy string comparison of m_source;
  QQuickShaderEffectMaterial::type() and QQuickShaderEffectMaterialKey
  are doing that for us more efficiently.

I updated the documentation to clarify the 1-texture rule and
explain the difference with qt_SubRect uniforms and when one or
the other should be used.

Change-Id: I85a86b8e7cb76f1536d5fcd369c7889f22537846
Reviewed-by: Michael Brasser <michael.brasser@live.com>
Reviewed-by: Gunnar Sletta <gunnar@sletta.org>

src/quick/items/qquickshadereffect.cpp

@@ -854,12 +854,17 @@ void QQuickShaderEffect::setCullMode(CullMode face)
 /*!
     \qmlproperty bool QtQuick::ShaderEffect::supportsAtlasTextures
 
-    Set this property true to indicate that the ShaderEffect is able to
-    use the default source texture without first removing it from an atlas.
-    In this case the range of qt_MultiTexCoord0 will based on the position of
-    the texture within the atlas, rather than (0,0) to (1,1).
+    Set this property true to confirm that your shader code doesn't rely on
+    qt_MultiTexCoord0 ranging from (0,0) to (1,1) relative to the mesh.
+    In this case the range of qt_MultiTexCoord0 will rather be based on the position
+    of the texture within the atlas. This property currently has no effect if there
+    is less, or more, than one sampler uniform used as input to your shader.
 
-    Setting this to true may enable some optimizations.
+    This differs from providing qt_SubRect_<name> uniforms in that the latter allows
+    drawing one or more textures from the atlas in a single ShaderEffect item, while
+    supportsAtlasTextures allows multiple instances of a ShaderEffect component using
+    a different source image from the atlas to be batched in a single draw.
+    Both prevent a texture from being copied out of the atlas when referenced by a ShaderEffect.
 
     The default value is false.
 
@@ -1029,24 +1034,23 @@ QSGNode *QQuickShaderEffect::updatePaintNode(QSGNode *oldNode, UpdatePaintNodeDa
         m_dirtyUniforms = m_dirtyUniformValues = m_dirtyTextureProviders = false;
     }
 
-    int textureCount = material->textureProviders.size();
-    bool preventBatching = m_customVertexShader || textureCount > 1 || (textureCount > 0 && !m_supportsAtlasTextures);
-
     QRectF srcRect(0, 0, 1, 1);
-    if (m_supportsAtlasTextures && textureCount != 0) {
-        if (QSGTextureProvider *provider = material->textureProviders.at(0)) {
-            if (provider->texture())
-                srcRect = provider->texture()->normalizedTextureSubRect();
+    bool geometryUsesTextureSubRect = false;
+    if (m_supportsAtlasTextures && material->textureProviders.size() == 1) {
+        QSGTextureProvider *provider = material->textureProviders.at(0);
+        if (provider->texture()) {
+            srcRect = provider->texture()->normalizedTextureSubRect();
+            geometryUsesTextureSubRect = true;
         }
     }
 
-    if (bool(material->flags() & QSGMaterial::RequiresFullMatrix) != preventBatching) {
-        material->setFlag(QSGMaterial::RequiresFullMatrix, preventBatching);
+    if (bool(material->flags() & QSGMaterial::RequiresFullMatrix) != m_customVertexShader) {
+        material->setFlag(QSGMaterial::RequiresFullMatrix, m_customVertexShader);
         node->markDirty(QSGNode::DirtyMaterial);
     }
 
-    if (material->supportsAtlasTextures != m_supportsAtlasTextures) {
-        material->supportsAtlasTextures = m_supportsAtlasTextures;
+    if (material->geometryUsesTextureSubRect != geometryUsesTextureSubRect) {
+        material->geometryUsesTextureSubRect = geometryUsesTextureSubRect;
         node->markDirty(QSGNode::DirtyMaterial);
     }
 

src/quick/items/qquickshadereffectnode.cpp

@@ -41,6 +41,16 @@
 
 QT_BEGIN_NAMESPACE
 
+static bool hasAtlasTexture(const QVector<QSGTextureProvider *> &textureProviders)
+{
+    for (int i = 0; i < textureProviders.size(); ++i) {
+        QSGTextureProvider *t = textureProviders.at(i);
+        if (t->texture() && t->texture()->isAtlasTexture())
+            return true;
+    }
+    return false;
+}
+
 class QQuickCustomMaterialShader : public QSGMaterialShader
 {
 public:
@@ -137,7 +147,7 @@ void QQuickCustomMaterialShader::updateState(const RenderState &state, QSGMateri
                         if (loc >= 0) {
                             QRectF r = texture->normalizedTextureSubRect();
                             program()->setUniformValue(loc, r.x(), r.y(), r.width(), r.height());
-                        } else if (texture->isAtlasTexture() && (idx != 0 || !material->supportsAtlasTextures)) {
+                        } else if (texture->isAtlasTexture() && !material->geometryUsesTextureSubRect) {
                             texture = texture->removedFromAtlas();
                         }
                         texture->bind();
@@ -342,7 +352,7 @@ QHash<QQuickShaderEffectMaterialKey, QSharedPointer<QSGMaterialType> > QQuickSha
 
 QQuickShaderEffectMaterial::QQuickShaderEffectMaterial(QQuickShaderEffectNode *node)
     : cullMode(NoCulling)
-    , supportsAtlasTextures(false)
+    , geometryUsesTextureSubRect(false)
     , m_node(node)
     , m_emittedLogChanged(false)
 {
@@ -380,14 +390,10 @@ bool QQuickShaderEffectMaterial::UniformData::operator == (const UniformData &ot
 int QQuickShaderEffectMaterial::compare(const QSGMaterial *o) const
 {
     const QQuickShaderEffectMaterial *other = static_cast<const QQuickShaderEffectMaterial *>(o);
-    if (!supportsAtlasTextures || !other->supportsAtlasTextures)
-        return 1;
-    if (bool(flags() & QSGMaterial::RequiresFullMatrix) || bool(other->flags() & QSGMaterial::RequiresFullMatrix))
+    if ((hasAtlasTexture(textureProviders) && !geometryUsesTextureSubRect) || (hasAtlasTexture(other->textureProviders) && !other->geometryUsesTextureSubRect))
         return 1;
     if (cullMode != other->cullMode)
         return 1;
-    if (m_source != other->m_source)
-        return 1;
     for (int shaderType = 0; shaderType < QQuickShaderEffectMaterialKey::ShaderTypeCount; ++shaderType) {
         if (uniforms[shaderType] != other->uniforms[shaderType])
             return 1;

src/quick/items/qquickshadereffectnode_p.h

@@ -95,7 +95,7 @@ public:
     QVector<UniformData> uniforms[QQuickShaderEffectMaterialKey::ShaderTypeCount];
     QVector<QSGTextureProvider *> textureProviders;
     CullMode cullMode;
-    bool supportsAtlasTextures;
+    bool geometryUsesTextureSubRect;
 
     void setProgramSource(const QQuickShaderEffectMaterialKey &source);
     void updateTextures() const;