[Render/ShaderProgram] Prevented sRGB(A) textures to be image textures

- imageLoad/Store() cannot be used with such textures - Replaced RenderGraph::isValid()'s loop by std::all_of()
Razakhel · Jun 25, 2024 · 7d16f29 · 7d16f29
1 parent f5ce32b
commit 7d16f29
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 14 deletions.
diff --git a/src/RaZ/Data/Image.cpp b/src/RaZ/Data/Image.cpp
@@ -27,7 +27,7 @@ bool ImageDataF::operator==(const ImageData& imgData) const {
 }
 
 Image::Image(ImageColorspace colorspace, ImageDataType dataType) : m_colorspace{ colorspace }, m_dataType{ dataType } {
-  assert("Error: An sRGB[A] image must have a byte data type."
+  assert("Error: An sRGB(A) image must have a byte data type."
       && (m_colorspace != ImageColorspace::SRGB || m_colorspace != ImageColorspace::SRGBA || m_dataType == ImageDataType::BYTE));
 
   switch (colorspace) {

diff --git a/src/RaZ/Render/RenderGraph.cpp b/src/RaZ/Render/RenderGraph.cpp
@@ -11,12 +11,9 @@
 namespace Raz {
 
 bool RenderGraph::isValid() const {
-  for (const std::unique_ptr<RenderPass>& renderPass : m_nodes) {
-    if (!renderPass->isValid())
-      return false;
-  }
-
-  return true;
+  return std::all_of(m_nodes.cbegin(), m_nodes.cend(), [] (const std::unique_ptr<RenderPass>& renderPass) {
+    return renderPass->isValid();
+  });
 }
 
 void RenderGraph::resizeViewport(unsigned int width, unsigned int height) {

diff --git a/src/RaZ/Render/ShaderProgram.cpp b/src/RaZ/Render/ShaderProgram.cpp
@@ -49,7 +49,7 @@ ImageInternalFormat recoverImageTextureFormat(const Texture& texture) {
       break;
   }
 
-  throw std::invalid_argument("Error: The given image texture is not supported");
+  throw std::invalid_argument("[ShaderProgram] The given image texture is not supported");
 }
 
 } // namespace
@@ -79,7 +79,7 @@ const Texture& ShaderProgram::getTexture(const std::string& uniformName) const {
   });
 
   if (textureIt == m_textures.cend())
-    throw std::invalid_argument("Error: The given attribute uniform name does not exist");
+    throw std::invalid_argument("[ShaderProgram] The given attribute uniform name does not exist");
 
   return *textureIt->first;
 }
@@ -103,7 +103,7 @@ const Texture& ShaderProgram::getImageTexture(const std::string& uniformName) co
   });
 
   if (textureIt == m_imageTextures.cend())
-    throw std::invalid_argument("Error: The given attribute uniform name does not exist");
+    throw std::invalid_argument("[ShaderProgram] The given attribute uniform name does not exist");
 
   return *textureIt->first;
 }
@@ -129,11 +129,16 @@ void ShaderProgram::setImageTexture(TexturePtr texture, const std::string& unifo
     !Renderer::checkVersion(3, 1)
 #endif
     ) {
-    throw std::runtime_error("Error: Using image textures requires OpenGL 4.2+ or OpenGL ES 3.1+");
+    throw std::runtime_error("[ShaderProgram] Using image textures requires OpenGL 4.2+ or OpenGL ES 3.1+");
   }
 
   if (texture->getColorspace() == TextureColorspace::INVALID || texture->getColorspace() == TextureColorspace::DEPTH)
-    throw std::invalid_argument("Error: The given image texture's colorspace is invalid");
+    throw std::invalid_argument("[ShaderProgram] The given image texture's colorspace is invalid");
+
+  if (texture->getColorspace() == TextureColorspace::SRGB || texture->getColorspace() == TextureColorspace::SRGBA) {
+    // See: https://www.khronos.org/opengl/wiki/Image_Load_Store#Format_compatibility
+    throw std::invalid_argument("[ShaderProgram] Textures with an sRGB(A) colorspace cannot be used as image textures");
+  }
 
   auto imgTextureIt = std::find_if(m_imageTextures.begin(), m_imageTextures.end(), [&uniformName] (const auto& element) {
     return (element.second.uniformName == uniformName);
@@ -213,7 +218,7 @@ void ShaderProgram::removeAttribute(const std::string& uniformName) {
   const auto attribIt = m_attributes.find(uniformName);
 
   if (attribIt == m_attributes.end())
-    throw std::invalid_argument("Error: The given attribute uniform name does not exist");
+    throw std::invalid_argument("[ShaderProgram] The given attribute uniform name does not exist");
 
   m_attributes.erase(attribIt);
 }

diff --git a/tests/src/RaZ/Render/ShaderProgram.cpp b/tests/src/RaZ/Render/ShaderProgram.cpp
@@ -293,7 +293,11 @@ TEST_CASE("ShaderProgram image textures", "[render]") {
   CHECK(program.getImageTextureCount() == 0);
 
   // Adding an image texture without a valid colorspace throws an exception
-  CHECK_THROWS(program.setImageTexture(Raz::Texture2D::create(), "noColorspace"));
+  CHECK_THROWS(program.setImageTexture(Raz::Texture2D::create(), {}));
+  CHECK_THROWS(program.setImageTexture(Raz::Texture2D::create(Raz::TextureColorspace::DEPTH), {}));
+  // Textures with an sRGB(A) colorspace aren't valid as image textures
+  CHECK_THROWS(program.setImageTexture(Raz::Texture2D::create(Raz::TextureColorspace::SRGB), {}));
+  CHECK_THROWS(program.setImageTexture(Raz::Texture3D::create(Raz::TextureColorspace::SRGBA), {}));
 
   const auto tex2D = Raz::Texture2D::create(Raz::TextureColorspace::RGB, Raz::TextureDataType::BYTE);
   const auto tex3D = Raz::Texture3D::create(Raz::TextureColorspace::GRAY, Raz::TextureDataType::FLOAT16);

diff --git a/tests/src/RaZ/Utils/Threading.cpp b/tests/src/RaZ/Utils/Threading.cpp
@@ -2,6 +2,7 @@
 
 #include <catch2/catch_test_macros.hpp>
 
+#include <algorithm>
 #include <array>
 
 #ifdef RAZ_THREADS_AVAILABLE