From: Ferran Sole <ferran.sole@samsung.com>
Date: Fri, 4 Nov 2016 08:53:54 +0000 (+0000)
Subject: Added warning message for inactive samplers in the shader
X-Git-Tag: dali_1.2.14~1
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=refs%2Fchanges%2F03%2F95703%2F8;p=platform%2Fcore%2Fuifw%2Fdali-core.git

Added warning message for inactive samplers in the shader

* Added warning message when a sampler uniform is declared but not used in the shader
* Added warning message when TextureSet contains a different number of textures
  than the number of active samplers in the shader
* Changed renderer behaviour to render even if the number of textures is
  greater than the number of active samplers

Change-Id: Idbd4bebee8ddb6cd48aa84cce5607ae24f38a964
---

diff --git a/automated-tests/src/dali/utc-Dali-Renderer.cpp b/automated-tests/src/dali/utc-Dali-Renderer.cpp
index 6591dd9..8d1bc31 100644
--- a/automated-tests/src/dali/utc-Dali-Renderer.cpp
+++ b/automated-tests/src/dali/utc-Dali-Renderer.cpp
@@ -2561,3 +2561,53 @@ int UtcDaliRendererSetStencilMask(void)
 
   END_TEST;
 }
+
+int UtcDaliRendererWrongNumberOfTextures(void)
+{
+  TestApplication application;
+  tet_infoline("Test renderer does render even if number of textures is different than active samplers in the shader");
+
+  //Create a TextureSet with 4 textures (One more texture in the texture set than active samplers)
+  //@note Shaders in the test suit have 3 active samplers. See TestGlAbstraction::GetActiveUniform()
+  Texture texture = Texture::New( TextureType::TEXTURE_2D, Pixel::RGBA8888, 64u, 64u );
+  TextureSet textureSet = CreateTextureSet();
+  textureSet.SetTexture(0, texture );
+  textureSet.SetTexture(1, texture );
+  textureSet.SetTexture(2, texture );
+  textureSet.SetTexture(3, texture );
+  Shader shader = Shader::New("VertexSource", "FragmentSource");
+  Geometry geometry = CreateQuadGeometry();
+  Renderer renderer = Renderer::New( geometry, shader );
+  renderer.SetTextures( textureSet );
+
+  Actor actor= Actor::New();
+  actor.AddRenderer(renderer);
+  actor.SetPosition(0.0f,0.0f);
+  actor.SetSize(100, 100);
+  Stage::GetCurrent().Add(actor);
+
+  TestGlAbstraction& gl = application.GetGlAbstraction();
+  TraceCallStack& drawTrace = gl.GetDrawTrace();
+  drawTrace.Reset();
+  drawTrace.Enable(true);
+
+  application.SendNotification();
+  application.Render(0);
+
+  //Test we do the drawcall when TextureSet has more textures than there are active samplers in the shader
+  DALI_TEST_EQUALS( drawTrace.CountMethod("DrawElements"), 1, TEST_LOCATION );
+
+  //Create a TextureSet with 1 texture (two more active samplers than texture in the texture set)
+  //@note Shaders in the test suit have 3 active samplers. See TestGlAbstraction::GetActiveUniform()
+  textureSet = CreateTextureSet();
+  renderer.SetTextures( textureSet );
+  textureSet.SetTexture(0, texture );
+  drawTrace.Reset();
+  application.SendNotification();
+  application.Render(0);
+
+  //Test we do the drawcall when TextureSet has less textures than there are active samplers in the shader.
+  DALI_TEST_EQUALS( drawTrace.CountMethod("DrawElements"), 1, TEST_LOCATION );
+
+  END_TEST;
+}
diff --git a/dali/internal/render/renderers/render-renderer.cpp b/dali/internal/render/renderers/render-renderer.cpp
index b7db6ac..2a15906 100644
--- a/dali/internal/render/renderers/render-renderer.cpp
+++ b/dali/internal/render/renderers/render-renderer.cpp
@@ -398,14 +398,12 @@ bool Renderer::BindTextures( Context& context, SceneGraph::TextureCache& texture
   }
 
   std::vector<Render::NewTexture*>& newTextures( mRenderDataProvider->GetNewTextures() );
-  for( size_t i(0); result && i<newTextures.size(); ++i )
+  for( size_t i(0); i<newTextures.size() && result; ++i )
   {
     if( newTextures[i] )
     {
-      result = program.GetSamplerUniformLocation( i, uniformLocation ) &&
-               newTextures[i]->Bind(context, textureUnit, samplers[i] );
-
-      if( result )
+      result = newTextures[i]->Bind(context, textureUnit, samplers[i] );
+      if( result && program.GetSamplerUniformLocation( i, uniformLocation ) )
       {
         program.SetUniform1i( uniformLocation, textureUnit );
         ++textureUnit;
diff --git a/dali/internal/render/shaders/program.cpp b/dali/internal/render/shaders/program.cpp
index e59308d..a41c694 100644
--- a/dali/internal/render/shaders/program.cpp
+++ b/dali/internal/render/shaders/program.cpp
@@ -20,6 +20,7 @@
 
 // EXTERNAL INCLUDES
 #include <iomanip>
+#include <cstring>
 
 // INTERNAL INCLUDES
 #include <dali/public-api/common/dali-common.h>
@@ -231,16 +232,16 @@ namespace
 struct LocationPosition
 {
   GLint uniformLocation; ///< The location of the uniform (used as an identifier)
-  int characterPosition; ///< the position of the uniform declaration
-  LocationPosition( GLint uniformLocation, int characterPosition )
-  : uniformLocation(uniformLocation), characterPosition(characterPosition)
+  int position;          ///< the position of the uniform declaration
+  LocationPosition( GLint uniformLocation, int position )
+  : uniformLocation(uniformLocation), position(position)
   {
   }
 };
 
 bool sortByPosition( LocationPosition a, LocationPosition b )
 {
-  return a.characterPosition < b.characterPosition;
+  return a.position < b.position;
 }
 }
 
@@ -270,29 +271,55 @@ void Program::GetActiveSamplerUniforms()
       {
         GLuint location = mGlAbstraction.GetUniformLocation( mProgramId, name );
         samplerNames.push_back(name);
-        samplerUniformLocations.push_back(LocationPosition(location, 0u));
+        samplerUniformLocations.push_back(LocationPosition(location, -1));
       }
     }
   }
 
-  if( samplerUniformLocations.size() > 1 )
+  //Determine declaration order of each sampler
+  char* fragShader = strdup( mProgramData->GetFragmentShader() );
+  const char* token = strtok( fragShader, " ;\n");
+  int samplerPosition = 0;
+  while( token )
   {
-    // Now, re-order according to declaration order in the fragment source.
-    std::string fragShader( mProgramData->GetFragmentShader() );
-    for( unsigned int i=0; i<samplerUniformLocations.size(); ++i )
+    if( ( strncmp( token, "sampler2D", 9u ) == 0 ) || ( strncmp( token, "samplerCube", 11u ) == 0 ) )
     {
-      // Better to write own search algorithm that searches for all of
-      // the sampler names simultaneously, ensuring only one iteration
-      // over fragShader.
-      size_t characterPosition = fragShader.find( samplerNames[i] );
-      samplerUniformLocations[i].characterPosition = characterPosition;
+      bool found( false );
+      token = strtok( NULL, " ;\n");
+      for( size_t i=0; i<samplerUniformLocations.size(); ++i )
+      {
+        if( samplerUniformLocations[i].position == -1 &&
+            strncmp( token, samplerNames[i].c_str(), samplerNames[i].size() ) == 0 )
+        {
+          samplerUniformLocations[i].position = samplerPosition++;
+          found = true;
+          break;
+        }
+      }
+      if( !found )
+      {
+        DALI_LOG_ERROR("Sampler uniform %s declared but not used in the shader\n", token );
+      }
+    }
+    else
+    {
+      token = strtok( NULL, " ;\n");
     }
+  }
+
+  free( fragShader );
+
+  // Re-order according to declaration order in the fragment source.
+  size_t samplerUniformCount = samplerUniformLocations.size();
+  if( samplerUniformCount > 1 )
+  {
     std::sort( samplerUniformLocations.begin(), samplerUniformLocations.end(), sortByPosition);
   }
 
-  for( unsigned int i=0; i<samplerUniformLocations.size(); ++i )
+  mSamplerUniformLocations.resize( samplerUniformCount );
+  for( size_t i=0; i<samplerUniformCount; ++i )
   {
-    mSamplerUniformLocations.push_back( samplerUniformLocations[i].uniformLocation );
+    mSamplerUniformLocations[i] = samplerUniformLocations[i].uniformLocation;
   }
 }
 
@@ -307,6 +334,11 @@ bool Program::GetSamplerUniformLocation( unsigned int index, GLint& location  )
   return result;
 }
 
+size_t Program::GetActiveSamplerCount() const
+{
+  return mSamplerUniformLocations.size();
+}
+
 void Program::SetUniform1i( GLint location, GLint value0 )
 {
   DALI_ASSERT_DEBUG( IsUsed() ); // should not call this if this program is not used
diff --git a/dali/internal/render/shaders/program.h b/dali/internal/render/shaders/program.h
index 0ed9ea2..b818508 100644
--- a/dali/internal/render/shaders/program.h
+++ b/dali/internal/render/shaders/program.h
@@ -170,6 +170,12 @@ public:
   bool GetSamplerUniformLocation( unsigned int index, GLint& location );
 
   /**
+   * Get the number of active samplers present in the shader
+   * @return The number of active samplers
+   */
+  size_t GetActiveSamplerCount() const;
+
+  /**
    * Sets the uniform value
    * @param [in] location of uniform
    * @param [in] value0 as int
diff --git a/dali/internal/update/rendering/scene-graph-renderer.cpp b/dali/internal/update/rendering/scene-graph-renderer.cpp
index e560f43..331a93d 100644
--- a/dali/internal/update/rendering/scene-graph-renderer.cpp
+++ b/dali/internal/update/rendering/scene-graph-renderer.cpp
@@ -18,17 +18,17 @@
 #include "scene-graph-renderer.h"
 
 // INTERNAL INCLUDES
-#include <dali/internal/update/controllers/scene-controller.h>
-#include <dali/internal/render/renderers/render-geometry.h>
+#include <dali/internal/common/internal-constants.h>
+#include <dali/internal/common/memory-pool-object-allocator.h>
 #include <dali/internal/update/controllers/render-message-dispatcher.h>
+#include <dali/internal/update/controllers/scene-controller.h>
+#include <dali/internal/update/nodes/node.h>
 #include <dali/internal/update/rendering/scene-graph-texture-set.h>
-#include <dali/internal/render/shaders/scene-graph-shader.h>
 #include <dali/internal/render/data-providers/node-data-provider.h>
-#include <dali/internal/update/nodes/node.h>
 #include <dali/internal/render/queue/render-queue.h>
-#include <dali/internal/common/internal-constants.h>
-#include <dali/internal/common/memory-pool-object-allocator.h>
-
+#include <dali/internal/render/renderers/render-geometry.h>
+#include <dali/internal/render/shaders/program.h>
+#include <dali/internal/render/shaders/scene-graph-shader.h>
 
 namespace // unnamed namespace
 {
@@ -585,7 +585,17 @@ RenderDataProvider* Renderer::NewRenderDataProvider()
 
   if( mTextureSet )
   {
-    size_t textureCount( mTextureSet->GetTextureCount() );
+    size_t textureCount = mTextureSet->GetTextureCount();
+    size_t newTextureCount = mTextureSet->GetNewTextureCount();
+
+    Program* program = mShader->GetProgram();
+    if( program && program->GetActiveSamplerCount() != textureCount + newTextureCount )
+    {
+      DALI_LOG_ERROR("The number of active samplers in the shader(%lu) does not match the number of textures in the TextureSet(%lu)\n",
+                       program->GetActiveSamplerCount(),
+                       textureCount + newTextureCount );
+    }
+
     dataProvider->mTextures.resize( textureCount );
     dataProvider->mSamplers.resize( textureCount );
     for( unsigned int i(0); i<textureCount; ++i )
@@ -594,10 +604,9 @@ RenderDataProvider* Renderer::NewRenderDataProvider()
       dataProvider->mSamplers[i] = mTextureSet->GetTextureSampler(i);
     }
 
-    textureCount = mTextureSet->GetNewTextureCount();
-    dataProvider->mNewTextures.resize( textureCount );
-    dataProvider->mSamplers.resize( textureCount );
-    for( unsigned int i(0); i<textureCount; ++i )
+    dataProvider->mNewTextures.resize( newTextureCount );
+    dataProvider->mSamplers.resize( newTextureCount );
+    for( unsigned int i(0); i<newTextureCount; ++i )
     {
       dataProvider->mNewTextures[i] = mTextureSet->GetNewTexture(i);
       dataProvider->mSamplers[i] = mTextureSet->GetTextureSampler(i);