From 7d16f2925eb85931d9b11d2a3be1ee3c22fbb9cf Mon Sep 17 00:00:00 2001 From: Razakhel Date: Tue, 25 Jun 2024 17:59:43 +0200 Subject: [PATCH] [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() --- src/RaZ/Data/Image.cpp | 2 +- src/RaZ/Render/RenderGraph.cpp | 9 +++------ src/RaZ/Render/ShaderProgram.cpp | 17 +++++++++++------ tests/src/RaZ/Render/ShaderProgram.cpp | 6 +++++- tests/src/RaZ/Utils/Threading.cpp | 1 + 5 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/RaZ/Data/Image.cpp b/src/RaZ/Data/Image.cpp index c508f3bb..a81fa0db 100644 --- 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 index 315bcadb..492b407d 100644 --- 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 : m_nodes) { - if (!renderPass->isValid()) - return false; - } - - return true; + return std::all_of(m_nodes.cbegin(), m_nodes.cend(), [] (const std::unique_ptr& 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 index 2f63e667..08cad164 100644 --- 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 index 67fdfd39..7e2eaa33 100644 --- 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 index 083908e3..1833250d 100644 --- a/tests/src/RaZ/Utils/Threading.cpp +++ b/tests/src/RaZ/Utils/Threading.cpp @@ -2,6 +2,7 @@ #include +#include #include #ifdef RAZ_THREADS_AVAILABLE