From e6100d52fde6d90d0aece0d733b28f4288b29964 Mon Sep 17 00:00:00 2001
From: =?utf8?q?Gy=C3=B6rgy=20Straub?= <g.straub@partner.samsung.com>
Date: Tue, 18 Feb 2020 09:13:36 +0000
Subject: [PATCH] Improved correctness of Program::GetActiveSamplerUniforms().
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

Problems:
- The current implementation picks up samplers that are declared as other
  than uniforms (e.g. function arguments etc.), then, failing to find a
  location associated with the same name, issues warnings about them.
- Tabs are ignored when tokenizing the shader source;

Solution: find declarations starting with "uniform" and ending with
a semicolon, then tokenize on whitespace (only), looking for the name
following a sampler* type declaration.

This also eliminates the need to tokenize the whole shader source.

- Also made some global const c-strings const pointers too;

Change-Id: Ibaa026d2e7873c28142300c149e6295f3cdbf7c5
Signed-off-by: György Straub <g.straub@partner.samsung.com>
---
 dali/internal/render/shaders/program.cpp | 91 ++++++++++++++++++++++++--------
 1 file changed, 68 insertions(+), 23 deletions(-)

diff --git a/dali/internal/render/shaders/program.cpp b/dali/internal/render/shaders/program.cpp
index ec8ebe1..e8e95e2 100755
--- a/dali/internal/render/shaders/program.cpp
+++ b/dali/internal/render/shaders/program.cpp
@@ -77,13 +77,13 @@ namespace Internal
 namespace
 {
 
-const char* gStdAttribs[ Program::ATTRIB_TYPE_LAST ] =
+const char* const gStdAttribs[ Program::ATTRIB_TYPE_LAST ] =
 {
   "aPosition",    // ATTRIB_POSITION
   "aTexCoord",    // ATTRIB_TEXCOORD
 };
 
-const char* gStdUniforms[ Program::UNIFORM_TYPE_LAST ] =
+const char* const gStdUniforms[ Program::UNIFORM_TYPE_LAST ] =
 {
   "uMvpMatrix",           // UNIFORM_MVP_MATRIX
   "uModelView",           // UNIFORM_MODELVIEW_MATRIX
@@ -237,6 +237,41 @@ bool sortByPosition( LocationPosition a, LocationPosition b )
 {
   return a.position < b.position;
 }
+
+const char* const DELIMITERS = " \t\n";
+
+struct StringSize
+{
+  const char* const mString;
+  const uint32_t mLength;
+
+  template <uint32_t kLength>
+  constexpr StringSize(const char(&string)[kLength])
+  : mString(string),
+    mLength(kLength - 1) // remove terminating null; N.B. there should be no other null.
+  {}
+
+  operator const char*() const
+  {
+    return mString;
+  }
+};
+
+bool operator==(const StringSize& lhs, const char* rhs)
+{
+  return strncmp(lhs.mString, rhs, lhs.mLength) == 0;
+}
+
+constexpr StringSize UNIFORM{ "uniform" };
+constexpr StringSize SAMPLER_PREFIX{ "sampler" };
+constexpr StringSize SAMPLER_TYPES[] = {
+  "2D",
+  "Cube",
+  "ExternalOES"
+};
+
+constexpr auto END_SAMPLER_TYPES = SAMPLER_TYPES + std::extent<decltype(SAMPLER_TYPES)>::value;
+
 }
 
 void Program::GetActiveSamplerUniforms()
@@ -272,36 +307,46 @@ void Program::GetActiveSamplerUniforms()
 
   //Determine declaration order of each sampler
   char* fragShader = strdup( mProgramData->GetFragmentShader() );
-  char* nextPtr = NULL;
-  const char* token = strtok_r( fragShader, " ;\n", &nextPtr );
+  char* uniform = strstr( fragShader, UNIFORM );
   int samplerPosition = 0;
-  while( token )
+  while ( uniform )
   {
-    if( ( strncmp( token, "sampler2D", 9u )    == 0 ) ||
-        ( strncmp( token, "samplerCube", 11u ) == 0 ) ||
-        ( strncmp( token, "samplerExternalOES", 18u ) == 0 ) )
+    char* outerToken = strtok_r( uniform + UNIFORM.mLength, ";", &uniform );
+
+    char* nextPtr = NULL;
+    char* token = strtok_r( outerToken, DELIMITERS, &nextPtr );
+    while ( token )
     {
-      bool found( false );
-      token = strtok_r( NULL, " ;\n", &nextPtr );
-      for( uint32_t i=0; i < static_cast<uint32_t>( samplerUniformLocations.size() ); ++i )
+      if ( SAMPLER_PREFIX == token )
       {
-        if( samplerUniformLocations[i].position == -1 &&
-            strncmp( token, samplerNames[i].c_str(), samplerNames[i].size() ) == 0 )
+        token += SAMPLER_PREFIX.mLength;
+        if ( std::find(SAMPLER_TYPES, END_SAMPLER_TYPES, token) != END_SAMPLER_TYPES )
         {
-          samplerUniformLocations[i].position = samplerPosition++;
-          found = true;
+          bool found( false );
+          token = strtok_r( NULL, DELIMITERS, &nextPtr );
+          for (uint32_t i=0; i < static_cast<uint32_t>( 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 );
+          }
           break;
         }
       }
-      if( !found )
-      {
-        DALI_LOG_ERROR("Sampler uniform %s declared but not used in the shader\n", token );
-      }
-    }
-    else
-    {
-      token = strtok_r( NULL, " ;\n", &nextPtr );
+
+      token = strtok_r( NULL, DELIMITERS, &nextPtr );
     }
+
+    uniform = strstr( uniform, UNIFORM );
   }
 
   free( fragShader );
-- 
2.7.4