Cleaned up unused code in Program 50/300650/6
authorDavid Steele <david.steele@samsung.com>
Mon, 30 Oct 2023 16:49:34 +0000 (16:49 +0000)
committerDavid Steele <david.steele@samsung.com>
Tue, 7 Nov 2023 17:16:40 +0000 (17:16 +0000)
Fixed WARNING debug to only have 1 function location string
Added debug to show shader code when missing attributes found:
  run with:
  export LOG_PIPELINE_CACHE=5

Change-Id: I852f703ae86c6e877b8c50380f7f24c1a7e21e77

16 files changed:
automated-tests/src/dali/dali-test-suite-utils/test-graphics-controller.cpp
automated-tests/src/dali/dali-test-suite-utils/test-graphics-controller.h
automated-tests/src/dali/dali-test-suite-utils/test-graphics-program.cpp
automated-tests/src/dali/dali-test-suite-utils/test-graphics-program.h
automated-tests/src/dali/dali-test-suite-utils/test-graphics-reflection.cpp
automated-tests/src/dali/dali-test-suite-utils/test-graphics-reflection.h
automated-tests/src/dali/utc-Dali-Renderer.cpp
dali/integration-api/debug.h
dali/internal/render/common/render-manager.cpp
dali/internal/render/common/render-manager.h
dali/internal/render/renderers/pipeline-cache.cpp
dali/internal/render/shaders/program-controller.cpp
dali/internal/render/shaders/program-controller.h
dali/internal/render/shaders/program.cpp
dali/internal/render/shaders/program.h
dali/internal/render/shaders/render-shader.h

index 527b9f4..24bb51e 100644 (file)
@@ -1289,7 +1289,7 @@ Graphics::UniquePtr<Graphics::Program> TestGraphicsController::CreateProgram(con
   }
 
   mProgramCache.emplace_back();
-  mProgramCache.back().programImpl = new TestGraphicsProgramImpl(mGl, programCreateInfo, mVertexFormats, mCustomUniforms, mCustomUniformBlocks);
+  mProgramCache.back().programImpl = new TestGraphicsProgramImpl(*this, mGl, programCreateInfo, mVertexFormats, mCustomUniforms, mCustomUniformBlocks);
 
   for(auto& shader : *(programCreateInfo.shaderState))
   {
index 9540c3e..e14499b 100644 (file)
@@ -424,6 +424,15 @@ public: // ResourceId relative API.
   Graphics::UniquePtr<Graphics::Texture> ReleaseTextureFromResourceId(uint32_t resourceId) override;
 
 public: // Test Functions
+  void SetAutoAttrCreation(bool v)
+  {
+    mAutoAttrCreation = v;
+  }
+  bool AutoAttrCreation()
+  {
+    return mAutoAttrCreation;
+  }
+
   void SetVertexFormats(Property::Array& vfs)
   {
     mVertexFormats = vfs;
@@ -458,9 +467,9 @@ public:
   TestGraphicsSyncImplementation mGraphicsSyncImpl;
   TestGlContextHelperAbstraction mGlContextHelperAbstraction;
 
-  bool isDiscardQueueEmptyResult{true};
-  bool isDrawOnResumeRequiredResult{true};
-
+  bool            isDiscardQueueEmptyResult{true};
+  bool            isDrawOnResumeRequiredResult{true};
+  bool            mAutoAttrCreation{true};
   Property::Array mVertexFormats;
 
   struct ProgramCache
index 2706305..3c8418e 100644 (file)
 
 namespace Dali
 {
-TestGraphicsProgramImpl::TestGraphicsProgramImpl(TestGlAbstraction& gl, const Graphics::ProgramCreateInfo& createInfo, Property::Array& vertexFormats, std::vector<UniformData>& customUniforms, std::vector<TestGraphicsReflection::TestUniformBlockInfo>& customUniformBlocks)
-: mGl(gl),
+TestGraphicsProgramImpl::TestGraphicsProgramImpl(TestGraphicsController& controller, TestGlAbstraction& gl, const Graphics::ProgramCreateInfo& createInfo, Property::Array& vertexFormats, std::vector<UniformData>& customUniforms, std::vector<TestGraphicsReflection::TestUniformBlockInfo>& customUniformBlocks)
+: mController(controller),
+  mGl(gl),
   mId(gl.CreateProgram()),
   mCreateInfo(createInfo),
-  mReflection(gl, mId, vertexFormats, createInfo, customUniforms, customUniformBlocks)
+  mReflection(controller, gl, mId, vertexFormats, createInfo, customUniforms, customUniformBlocks)
 {
   // Ensure active sampler uniforms are set
   mGl.SetCustomUniforms(customUniforms);
index 8641800..b81185a 100644 (file)
 
 namespace Dali
 {
+class TestGraphicsController;
+
 class TestGraphicsProgramImpl
 {
 public:
-  TestGraphicsProgramImpl(TestGlAbstraction& gl, const Graphics::ProgramCreateInfo& createInfo, Property::Array& vertexFormats, std::vector<UniformData>& customUniforms, std::vector<TestGraphicsReflection::TestUniformBlockInfo>& customUniformBlocks);
+  TestGraphicsProgramImpl(TestGraphicsController& controller, TestGlAbstraction& gl, const Graphics::ProgramCreateInfo& createInfo, Property::Array& vertexFormats, std::vector<UniformData>& customUniforms, std::vector<TestGraphicsReflection::TestUniformBlockInfo>& customUniformBlocks);
 
   // For API
   const TestGraphicsReflection& GetReflection() const
@@ -45,6 +47,7 @@ public:
   bool GetParameter(uint32_t parameterId, void* outData);
 
 public:
+  TestGraphicsController&     mController;
   TestGlAbstraction&          mGl;
   uint32_t                    mId;
   Graphics::ProgramCreateInfo mCreateInfo;
index 2822c0a..7b3e6c8 100644 (file)
@@ -15,6 +15,7 @@
  */
 
 #include "test-graphics-reflection.h"
+#include "test-graphics-controller.h"
 #include "test-graphics-shader.h"
 
 #include <dali/public-api/object/property-map.h>
@@ -110,8 +111,9 @@ constexpr int GetSizeForType(Property::Type type)
 
 } // namespace
 
-TestGraphicsReflection::TestGraphicsReflection(TestGlAbstraction& gl, uint32_t programId, Property::Array& vfs, const Graphics::ProgramCreateInfo& createInfo, std::vector<UniformData>& customUniforms, std::vector<TestGraphicsReflection::TestUniformBlockInfo>& customUniformBlocks)
-: mGl(gl),
+TestGraphicsReflection::TestGraphicsReflection(TestGraphicsController& controller, TestGlAbstraction& gl, uint32_t programId, Property::Array& vfs, const Graphics::ProgramCreateInfo& createInfo, std::vector<UniformData>& customUniforms, std::vector<TestGraphicsReflection::TestUniformBlockInfo>& customUniformBlocks)
+: mController(controller),
+  mGl(gl),
   mCustomUniforms(customUniforms)
 {
   for(Property::Array::SizeType i = 0; i < vfs.Count(); ++i)
@@ -241,19 +243,19 @@ TestGraphicsReflection::TestGraphicsReflection(TestGlAbstraction& gl, uint32_t p
 
 uint32_t TestGraphicsReflection::GetVertexAttributeLocation(const std::string& name) const
 {
-  // Automatically assign locations to named attributes when requested
   auto iter = std::find(mAttributes.begin(), mAttributes.end(), name);
   if(iter != mAttributes.end())
   {
     return iter - mAttributes.begin();
   }
-  else
+  else if(mController.AutoAttrCreation())
   {
     uint32_t location = mAttributes.size();
     mAttributes.push_back(name);
     return location;
   }
-  return 0u;
+
+  return -1;
 }
 
 Dali::Graphics::VertexInputAttributeFormat TestGraphicsReflection::GetVertexAttributeFormat(uint32_t location) const
index e701e17..b598435 100644 (file)
 
 namespace Dali
 {
+class TestGraphicsController;
+
 class TestGraphicsReflection : public Graphics::Reflection
 {
 public:
   class TestUniformBlockInfo;
 
-  TestGraphicsReflection(TestGlAbstraction& gl, uint32_t program_id, Property::Array& vertexFormats, const Graphics::ProgramCreateInfo& createInfo, std::vector<UniformData>& customUniforms, std::vector<TestUniformBlockInfo>& customUniformBlocks);
+  TestGraphicsReflection(TestGraphicsController& controller, TestGlAbstraction& gl, uint32_t program_id, Property::Array& vertexFormats, const Graphics::ProgramCreateInfo& createInfo, std::vector<UniformData>& customUniforms, std::vector<TestUniformBlockInfo>& customUniformBlocks);
 
   uint32_t                                        GetVertexAttributeLocation(const std::string& name) const override;
   Dali::Graphics::VertexInputAttributeFormat      GetVertexAttributeFormat(uint32_t location) const override;
@@ -84,6 +86,7 @@ public: // Test methods
     return mUniformBlocks[index];
   }
 
+  TestGraphicsController&          mController;
   TestGlAbstraction&               mGl;
   mutable std::vector<std::string> mAttributes;
   std::vector<UniformData>         mCustomUniforms;
index 0ccc460..1d9d6a6 100644 (file)
@@ -15,6 +15,8 @@
  *
  */
 
+#define DEBUG_ENABLED 1
+
 // EXTERNAL INCLUDES
 #include <dali/devel-api/actors/actor-devel.h>
 #include <dali/devel-api/common/capabilities.h>
@@ -3927,6 +3929,59 @@ int UtcDaliRendererPreparePipeline(void)
   END_TEST;
 }
 
+int UtcDaliRendererPreparePipelineMissingAttrs(void)
+{
+  TestApplication application;
+
+  tet_infoline("Test that rendering an actor tries to bind the attributes locs from the reflection, but fails");
+  Debug::Filter::SetGlobalLogLevel(Debug::Verbose);
+
+  Property::Map modelVF;
+  modelVF["aPosition"] = Property::VECTOR3;
+  modelVF["aNormal"]   = Property::VECTOR3;
+  Property::Array vfs;
+  vfs.PushBack(modelVF);
+
+  TestGraphicsController& graphics = application.GetGraphicsController();
+  graphics.SetAutoAttrCreation(false);
+  graphics.SetVertexFormats(vfs);
+
+  Property::Map vf            = CreateModelVertexFormat();
+  Geometry      modelGeometry = CreateModelGeometry(vf);
+  Shader        shader        = Shader::New("vertexSrc", "fragmentSrc");
+  Renderer      renderer      = Renderer::New(modelGeometry, shader);
+  Actor         actor         = Actor::New();
+
+  actor.AddRenderer(renderer);
+  actor.SetProperty(Actor::Property::SIZE, Vector2(400.0f, 400.0f));
+  actor.SetProperty(Actor::Property::COLOR, Color::WHITE);
+  application.GetScene().Add(actor);
+
+  TraceCallStack& cmdBufCallstack   = graphics.mCommandBufferCallStack;
+  TraceCallStack& graphicsCallstack = graphics.mCallStack;
+  cmdBufCallstack.Enable(true);
+  graphicsCallstack.Enable(true);
+
+  application.SendNotification();
+  application.Render();
+
+  DALI_TEST_CHECK(graphicsCallstack.FindMethod("SubmitCommandBuffers"));
+  std::vector<Graphics::SubmitInfo>& submissions = graphics.mSubmitStack;
+  DALI_TEST_CHECK(submissions.size() > 0);
+
+  TestGraphicsCommandBuffer* cmdBuf = static_cast<TestGraphicsCommandBuffer*>((submissions.back().cmdBuffer[0]));
+
+  auto result   = cmdBuf->GetChildCommandsByType(0 | CommandType::BIND_PIPELINE);
+  auto pipeline = result[0]->data.bindPipeline.pipeline;
+
+  if(pipeline)
+  {
+    DALI_TEST_EQUALS(pipeline->vertexInputState.attributes.size(), 2, TEST_LOCATION);
+  }
+
+  END_TEST;
+}
+
 int UtcDaliRendererUniformArrayOfStruct(void)
 {
   TestApplication application;
index 1a19291..38a0ff0 100644 (file)
@@ -178,7 +178,7 @@ DALI_CORE_API void UninstallLogFunction();
 /**
  * Provides unfiltered logging for global warning level messages
  */
-#define DALI_LOG_WARNING(format, ...) Dali::Integration::Log::LogMessageWithFunctionLine(Dali::Integration::Log::WARNING, "%s " format, ASSERT_LOCATION, ##__VA_ARGS__)
+#define DALI_LOG_WARNING(format, ...) Dali::Integration::Log::LogMessageWithFunctionLine(Dali::Integration::Log::WARNING, format, ##__VA_ARGS__)
 
 #else // DEBUG_ENABLED
 
index 1bc9be3..6d42e84 100644 (file)
@@ -995,10 +995,6 @@ void RenderManager::RenderScene(Integration::RenderStatus& status, Integration::
 
     targetstoPresent.emplace_back(currentRenderTarget);
 
-    // reset the program matrices for all programs once per frame
-    // this ensures we will set view and projection matrix once per program per camera
-    mImpl->programController.ResetProgramMatrices();
-
     if(!instruction.mIgnoreRenderToFbo && (instruction.mFrameBuffer != nullptr))
     {
       // Offscreen buffer rendering
@@ -1104,7 +1100,6 @@ void RenderManager::RenderScene(Integration::RenderStatus& status, Integration::
 
   // Flush UBOs
   mImpl->uniformBufferManager->Flush(sceneObject, renderToFbo);
-
   mImpl->renderAlgorithms.SubmitCommandBuffer();
   mImpl->commandBufferSubmitted = true;
 
index 9793f8a..4d355cb 100644 (file)
@@ -41,7 +41,6 @@ struct Vector4;
 
 namespace Internal
 {
-class ProgramCache;
 class ShaderSaver;
 
 namespace Render
index c2545c0..e78b7f7 100644 (file)
@@ -29,6 +29,10 @@ namespace Dali::Internal::Render
 {
 namespace
 {
+#if defined(DEBUG_ENABLED)
+Debug::Filter* gLogFilter = Debug::Filter::New(Debug::NoLogging, false, "LOG_PIPELINE_CACHE");
+#endif
+
 constexpr uint32_t CACHE_CLEAN_FRAME_COUNT = 600; // 60fps * 10sec
 
 // Helper to get the vertex input format
@@ -207,6 +211,7 @@ PipelineCacheL0Ptr PipelineCache::GetPipelineCacheL0(Program* program, Render::G
     Graphics::VertexInputState vertexInputState{};
     uint32_t                   base = 0;
 
+    bool attrNotFound = false;
     for(auto&& vertexBuffer : geometry->GetVertexBuffers())
     {
       const VertexBuffer::Format& vertexFormat = *vertexBuffer->GetFormat();
@@ -237,6 +242,7 @@ PipelineCacheL0Ptr PipelineCache::GetPipelineCacheL0(Program* program, Render::G
         }
         else
         {
+          attrNotFound = true;
           DALI_LOG_WARNING("Attribute not found in the shader: %s\n", attributeName.GetCString());
           // Don't bind unused attributes.
         }
@@ -250,6 +256,15 @@ PipelineCacheL0Ptr PipelineCache::GetPipelineCacheL0(Program* program, Render::G
     level0.inputState = vertexInputState;
 
     it = level0nodes.insert(level0nodes.end(), std::move(level0));
+
+    if(attrNotFound)
+    {
+      DALI_LOG_INFO(gLogFilter, Debug::General,
+                    "!!!!!!!  Attributes not found. !!!!!!!!\n"
+                    "Shader src: VERT:\n%s\nFRAGMENT:\n%s\n",
+                    program->GetShaderData()->GetVertexShader(),
+                    program->GetShaderData()->GetFragmentShader());
+    }
   }
 
   return it;
index b15715e..133dc43 100644 (file)
@@ -34,17 +34,6 @@ ProgramController::ProgramController(Graphics::Controller& graphicsController)
 
 ProgramController::~ProgramController() = default;
 
-void ProgramController::ResetProgramMatrices()
-{
-  const ProgramIterator end = mProgramCache.End();
-  for(ProgramIterator iter = mProgramCache.Begin(); iter != end; ++iter)
-  {
-    Program* program = (*iter)->GetProgram();
-    program->SetProjectionMatrix(nullptr);
-    program->SetViewMatrix(nullptr);
-  }
-}
-
 void ProgramController::ResetReferenceCount()
 {
   for(auto&& item : mProgramCache)
@@ -87,7 +76,7 @@ Program* ProgramController::GetProgram(size_t shaderHash)
 
 void ProgramController::AddProgram(size_t shaderHash, Program* program)
 {
-  // we expect unique hash values so it is event thread side's job to guarantee that
+  // we expect unique hash values so it is caller's job to guarantee that
   // AddProgram is only called after program checks that GetProgram returns NULL
   mProgramCache.PushBack(new ProgramPair(program, shaderHash));
 }
index 062a96f..57e97ca 100644 (file)
@@ -32,7 +32,7 @@ namespace Internal
 class ShaderSaver;
 
 /**
- * This class is the owner of shader programs
+ * This class ensures that only unique programs are created.
  */
 class ProgramController : public ProgramCache
 {
@@ -121,11 +121,6 @@ public:
 
 public: // API
   /**
-   * Resets the program matrices. Must be called at the beginning of every frame
-   */
-  void ResetProgramMatrices();
-
-  /**
    * @brief Reset all program reference count as 0.
    */
   void ResetReferenceCount();
index bdc7053..02e1b18 100644 (file)
@@ -84,7 +84,8 @@ Program* Program::New(ProgramCache& cache, const Internal::ShaderDataPtr& shader
   {
     // program not found so create it
     program = new Program(cache, shaderData, gfxController);
-    DALI_LOG_RELEASE_INFO("Program::New() created a unique program\n");
+
+    DALI_LOG_INFO(Debug::Filter::gShader, Debug::Verbose, "Program::New() created a unique program:\n  VertexShader:\n%s\n\n  FragShader:\n%s\n", shaderData->GetVertexShader(), shaderData->GetFragmentShader());
     cache.AddProgram(shaderHash, program);
   }
 
@@ -93,8 +94,6 @@ Program* Program::New(ProgramCache& cache, const Internal::ShaderDataPtr& shader
 
 Program::Program(ProgramCache& cache, Internal::ShaderDataPtr shaderData, Graphics::Controller& controller)
 : mCache(cache),
-  mProjectionMatrix(nullptr),
-  mViewMatrix(nullptr),
   mGfxProgram(nullptr),
   mGfxController(controller),
   mProgramData(std::move(shaderData))
index 8130f93..f657159 100644 (file)
@@ -86,40 +86,9 @@ public:
    */
   static Program* New(ProgramCache& cache, const Internal::ShaderDataPtr& shaderData, Graphics::Controller& gfxController);
 
-  /**
-   * Set the projection matrix that has currently been sent
-   * @param matrix to set
-   */
-  void SetProjectionMatrix(const Matrix* matrix)
-  {
-    mProjectionMatrix = matrix;
-  }
-
-  /**
-   * Get the projection matrix that has currently been sent
-   * @return the matrix that is set
-   */
-  const Matrix* GetProjectionMatrix()
-  {
-    return mProjectionMatrix;
-  }
-
-  /**
-   * Set the projection matrix that has currently been sent
-   * @param matrix to set
-   */
-  void SetViewMatrix(const Matrix* matrix)
+  Internal::ShaderDataPtr GetShaderData()
   {
-    mViewMatrix = matrix;
-  }
-
-  /**
-   * Get the projection matrix that has currently been sent
-   * @return the matrix that is set
-   */
-  const Matrix* GetViewMatrix()
-  {
-    return mViewMatrix;
+    return mProgramData;
   }
 
   [[nodiscard]] Graphics::Program& GetGraphicsProgram() const
@@ -220,11 +189,8 @@ public:
     return mUniformBlockMemoryRequirements;
   }
 
-private:                           // Data
-  ProgramCache& mCache;            ///< The program cache
-  const Matrix* mProjectionMatrix; ///< currently set projection matrix
-  const Matrix* mViewMatrix;       ///< currently set view matrix
-
+private:                                                 // Data
+  ProgramCache&                          mCache;         ///< The program cache
   Graphics::UniquePtr<Graphics::Program> mGfxProgram;    ///< Gfx program
   Graphics::Controller&                  mGfxController; /// < Gfx controller
   Internal::ShaderDataPtr                mProgramData;   ///< Shader program source and binary (when compiled & linked or loaded)
index 9be08d6..a291b81 100644 (file)
@@ -28,7 +28,6 @@ namespace Dali
 namespace Internal
 {
 class Program;
-class ProgramCache;
 
 namespace SceneGraph
 {