Enable correct GL for integer attribute 00/291800/7
authorDavid Steele <david.steele@samsung.com>
Fri, 21 Apr 2023 09:08:34 +0000 (10:08 +0100)
committerDavid Steele <david.steele@samsung.com>
Wed, 31 May 2023 11:11:30 +0000 (12:11 +0100)
Was using VertexAttribPointer for defining all attributes, which meant
that integers got wierd due to normalization / float conversion.

Switched to using VertexAttribIPointer for integer attributes, now they
can be used properly in shaders, and the driver can optimize shader
code that uses them (e.g. for sampler array lookup)

Change-Id: I9d70a98ce0174370ab95e17e3e8132993ca9360e

automated-tests/src/dali-adaptor/dali-test-suite-utils/test-actor-utils.cpp
automated-tests/src/dali-adaptor/dali-test-suite-utils/test-actor-utils.h
automated-tests/src/dali-adaptor/dali-test-suite-utils/test-gl-abstraction.cpp
automated-tests/src/dali-adaptor/dali-test-suite-utils/test-gl-abstraction.h
automated-tests/src/dali-graphics/CMakeLists.txt
automated-tests/src/dali-graphics/utc-Dali-GraphicsDraw.cpp [new file with mode: 0644]
automated-tests/src/dali-graphics/utc-Dali-GraphicsTexture.cpp
dali/internal/graphics/gles-impl/gles-context.cpp

index bf62bb8..40388b3 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2023 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -131,4 +131,11 @@ Texture CreateTexture(TextureType::Type type, Pixel::Format format, int width, i
   return texture;
 }
 
+TextureSet CreateTextureSet(Pixel::Format format, int width, int height)
+{
+  TextureSet textureSet = TextureSet::New();
+  textureSet.SetTexture(0u, CreateTexture(TextureType::TEXTURE_2D, format, width, height));
+  return textureSet;
+}
+
 } // namespace Dali
index 0dbe07e..94f1493 100644 (file)
@@ -2,7 +2,7 @@
 #define DALI_TEST_ACTOR_UTILS_H
 
 /*
- * Copyright (c) 2021 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2023 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -60,7 +60,8 @@ Actor CreateRenderableActor(Texture texture, const std::string& vertexShader, co
  */
 Actor CreateRenderableActor2(TextureSet textures, const std::string& vertexShader, const std::string& fragmentShader);
 
-Texture CreateTexture(TextureType::Type type, Pixel::Format format, int width, int height);
+Texture    CreateTexture(TextureType::Type type, Pixel::Format format, int width, int height);
+TextureSet CreateTextureSet(Pixel::Format format, int width, int height);
 
 } // namespace Dali
 
index 6021026..1d20a6a 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2023 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -101,9 +101,9 @@ void TestGlAbstraction::Initialize()
   mProgramUniforms3f.clear();
   mProgramUniforms4f.clear();
 
-  mAttribLocs.clear();
-  mAttribLocs.push_back("aPosition");
-  mAttribLocs.push_back("aTexCoord");
+  mAttribLocs  = {"aPosition", "aTexCoord"};
+  mAttribTypes = {GL_FLOAT, GL_FLOAT};
+
   mCullFaceTrace.Reset();
   mDepthFunctionTrace.Reset();
   mEnableDisableTrace.Reset();
index 834ae6f..cc27d9e 100644 (file)
@@ -796,6 +796,8 @@ public:
 
   inline void GetActiveAttrib(GLuint program, GLuint index, GLsizei bufsize, GLsizei* length, GLint* size, GLenum* type, char* name) override
   {
+    strncpy(name, mAttribLocs[index].c_str(), 99);
+    *type = mAttribTypes[index];
   }
 
   inline void SetActiveUniforms(const std::vector<ActiveUniform>& uniforms)
@@ -1590,6 +1592,18 @@ public:
     mBufferTrace.PushCall("VertexAttribPointer", namedParams.str(), namedParams);
   }
 
+  inline void VertexAttribIPointer(GLuint index, GLint size, GLenum type, GLsizei stride, const GLvoid* pointer) override
+  {
+    TraceCallStack::NamedParams namedParams;
+    namedParams["index"] << index;
+    namedParams["size"] << size;
+    namedParams["type"] << std::hex << type;
+    namedParams["stride"] << stride;
+    namedParams["offset"] << std::to_string(reinterpret_cast<unsigned long>(pointer));
+
+    mBufferTrace.PushCall("VertexAttribIPointer", namedParams.str(), namedParams);
+  }
+
   inline void Viewport(GLint x, GLint y, GLsizei width, GLsizei height) override
   {
     std::string commaString(", ");
@@ -1772,10 +1786,6 @@ public:
   {
   }
 
-  inline void VertexAttribIPointer(GLuint index, GLint size, GLenum type, GLsizei stride, const GLvoid* pointer) override
-  {
-  }
-
   inline void GetVertexAttribIiv(GLuint index, GLenum pname, GLint* params) override
   {
   }
@@ -2099,10 +2109,14 @@ public: // TEST FUNCTIONS
   {
     mLinkStatus = value;
   }
-  inline void SetAttribLocations(std::vector<std::string> locs)
+  inline void SetAttribLocations(std::vector<std::string>& locs)
   {
     mAttribLocs = locs;
   }
+  inline void SetAttribTypes(std::vector<GLenum>& types)
+  {
+    mAttribTypes = types;
+  }
   inline void SetGetErrorResult(GLenum result)
   {
     mGetErrorResult = result;
@@ -2573,7 +2587,8 @@ public:
   bool                                  mGetProgramBinaryCalled;
   typedef std::map<GLuint, std::string> ShaderSourceMap;
   ShaderSourceMap                       mShaderSources;
-  std::vector<std::string>              mAttribLocs; // should be bound to shader
+  std::vector<std::string>              mAttribLocs;  // should be bound to shader
+  std::vector<GLenum>                   mAttribTypes; // should be bound to shader
   GLuint                                mLastShaderCompiled;
   GLbitfield                            mLastClearBitMask;
   Vector4                               mLastClearColor;
index 262c656..45ca6cb 100644 (file)
@@ -8,6 +8,7 @@ SET(TC_SOURCES
     utc-Dali-GlImplementation.cpp
     utc-Dali-GlesImplementation.cpp
     utc-Dali-GraphicsBuffer.cpp
+    utc-Dali-GraphicsDraw.cpp
     utc-Dali-GraphicsFramebuffer.cpp
     utc-Dali-GraphicsGeometry.cpp
     utc-Dali-GraphicsNativeImage.cpp
diff --git a/automated-tests/src/dali-graphics/utc-Dali-GraphicsDraw.cpp b/automated-tests/src/dali-graphics/utc-Dali-GraphicsDraw.cpp
new file mode 100644 (file)
index 0000000..41e991d
--- /dev/null
@@ -0,0 +1,107 @@
+/*
+ * Copyright (c) 2023 Samsung Electronics Co., Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <dali-test-suite-utils.h>
+#include <dali/dali.h>
+
+#include <dali/internal/graphics/gles-impl/egl-graphics-controller.h>
+#include <test-actor-utils.h>
+#include <test-graphics-application.h>
+#include <test-graphics-framebuffer.h>
+
+using namespace Dali;
+
+namespace
+{
+const char* VERTEX_SHADER = DALI_COMPOSE_SHADER(
+  INPUT mediump vec2       aPos;\n
+    INPUT mediump int      aCount;\n
+      uniform mediump mat4 uMvpMatrix;\n
+        OUTPUT flat int vCount;\n void main()\n {
+          \n
+            mediump vec4 vertexPosition(aPos, 0.0, 1.0);
+          \n
+            gl_Position = uMvpMatrix * vertexPosition;
+          \n
+        }\n);
+
+const char* FRAGMENT_SHADER = DALI_COMPOSE_SHADER(
+  uniform lowp vec4 uColor;\n
+    INPUT flat int vCount;\n void main()\n {
+      \n
+        mediump float g = (128.0 + vCount * 16) / 255.0;
+      \n
+        gl_FragColor = uColor * g;
+      \n
+    }\n);
+
+} // namespace
+
+void utc_dali_texture_startup(void)
+{
+  test_return_value = TET_UNDEF;
+}
+void utc_dali_texture_cleanup(void)
+{
+  test_return_value = TET_PASS;
+}
+
+int UtcDaliGraphicsDrawIntegerVertexAttribs(void)
+{
+  TestGraphicsApplication app;
+  tet_infoline("UtcDaliGraphicsDrawIntegerVertexAttribs - Test that integer vertex attribs use correct GL call");
+
+  auto& gl          = app.GetGlAbstraction();
+  auto& bufferTrace = gl.GetBufferTrace();
+  bufferTrace.EnableLogging(true);
+  bufferTrace.Enable(true);
+
+  // Initalize GL shader reflection
+  std::vector<std::string> aLocs  = {"aPos", "aCount"};
+  std::vector<GLenum>      aTypes = {GL_FLOAT, GL_INT};
+  gl.SetAttribLocations(aLocs);
+  gl.SetAttribTypes(aTypes);
+
+  TextureSet    textureSet = CreateTextureSet(Pixel::RGBA8888, 200, 200);
+  Property::Map vertexFormat{{"aPos", Property::VECTOR2}, {"aCount", Property::INTEGER}};
+  VertexBuffer  vertexBuffer = VertexBuffer::New(vertexFormat);
+
+  struct VertexFormat
+  {
+    Vector2 aPos;
+    int     aCount;
+  };
+  std::vector<VertexFormat> vertexData = {{Vector2{10, 20}, 1}, {Vector2{10, 20}, 2}, {Vector2{10, 20}, 3}, {Vector2{10, 20}, 4}};
+  vertexBuffer.SetData(&vertexData[0], sizeof(vertexData) / sizeof(VertexFormat));
+  Geometry geometry = Geometry::New();
+  geometry.AddVertexBuffer(vertexBuffer);
+  Shader   shader   = Shader::New(VERTEX_SHADER, FRAGMENT_SHADER);
+  Renderer renderer = Renderer::New(geometry, shader);
+  renderer.SetTextures(textureSet);
+  Actor dummyActor                  = Actor::New();
+  dummyActor[Actor::Property::SIZE] = Vector2(200, 200);
+  dummyActor.AddRenderer(renderer);
+  app.GetScene().Add(dummyActor);
+
+  app.SendNotification();
+  app.Render(16);
+
+  tet_infoline("Test that we have both VertexAttribPointer and VertexAttribIPointer called");
+  DALI_TEST_CHECK(bufferTrace.FindMethod("VertexAttribPointer"));
+  DALI_TEST_CHECK(bufferTrace.FindMethod("VertexAttribIPointer"));
+
+  END_TEST;
+}
index 62864ac..5e2c0f2 100644 (file)
@@ -28,11 +28,11 @@ namespace
 {
 } // namespace
 
-void utc_dali_texture_startup(void)
+void utc_dali_graphics_draw_startup(void)
 {
   test_return_value = TET_UNDEF;
 }
-void utc_dali_texture_cleanup(void)
+void utc_dali_graphics_draw_cleanup(void)
 {
   test_return_value = TET_PASS;
 }
index 8a3602a..787f4ce 100644 (file)
@@ -348,7 +348,6 @@ void Context::Flush(bool reset, const GLES::DrawCallDescriptor& drawCall, GLES::
   }
 
   // for each attribute bind vertices
-
   const auto& pipelineState    = mImpl->mNewPipeline ? mImpl->mNewPipeline->GetCreateInfo() : mImpl->mCurrentPipeline->GetCreateInfo();
   const auto& vertexInputState = pipelineState.vertexInputState;
 
@@ -373,12 +372,26 @@ void Context::Flush(bool reset, const GLES::DrawCallDescriptor& drawCall, GLES::
     // Bind buffer
     BindBuffer(GL_ARRAY_BUFFER, glesBuffer);
 
-    gl.VertexAttribPointer(attr.location,
-                           GLVertexFormat(attr.format).size,
-                           GLVertexFormat(attr.format).format,
-                           GL_FALSE,
-                           bufferBinding.stride,
-                           reinterpret_cast<void*>(attr.offset));
+    if(attr.format == VertexInputFormat::FLOAT ||
+       attr.format == VertexInputFormat::FVECTOR2 ||
+       attr.format == VertexInputFormat::FVECTOR3 ||
+       attr.format == VertexInputFormat::FVECTOR4)
+    {
+      gl.VertexAttribPointer(attr.location,
+                             GLVertexFormat(attr.format).size,
+                             GLVertexFormat(attr.format).format,
+                             GL_FALSE,
+                             bufferBinding.stride,
+                             reinterpret_cast<void*>(attr.offset));
+    }
+    else
+    {
+      gl.VertexAttribIPointer(attr.location,
+                              GLVertexFormat(attr.format).size,
+                              GLVertexFormat(attr.format).format,
+                              bufferBinding.stride,
+                              reinterpret_cast<void*>(attr.offset));
+    }
 
     switch(bufferBinding.inputRate)
     {