diff --git a/src/libs/vcl.graphics/vcl/graphics/imageprocessing/link.cpp b/src/libs/vcl.graphics/vcl/graphics/imageprocessing/link.cpp index 755cb656..ba09d8be 100644 --- a/src/libs/vcl.graphics/vcl/graphics/imageprocessing/link.cpp +++ b/src/libs/vcl.graphics/vcl/graphics/imageprocessing/link.cpp @@ -161,7 +161,7 @@ namespace Vcl { namespace Graphics { namespace ImageProcessing { void OutputSlot::setResource(const std::shared_ptr& res, unsigned int width, unsigned int height) { VclRequire(implies(res, width <= (unsigned int)res->width()), "'width' is in range."); - VclRequire(implies(res, height <= (unsigned int)res->height()), "'width' is in range."); + VclRequire(implies(res, height <= (unsigned int)res->height()), "'height' is in range."); _resource = res; _width = width; diff --git a/src/libs/vcl.graphics/vcl/graphics/imageprocessing/opengl/gaussian.cpp b/src/libs/vcl.graphics/vcl/graphics/imageprocessing/opengl/gaussian.cpp index 1fad423f..4a7fad5e 100644 --- a/src/libs/vcl.graphics/vcl/graphics/imageprocessing/opengl/gaussian.cpp +++ b/src/libs/vcl.graphics/vcl/graphics/imageprocessing/opengl/gaussian.cpp @@ -115,7 +115,7 @@ namespace Vcl { namespace Graphics { namespace ImageProcessing { namespace OpenG uniform ivec4 inputRange0; // Kernel output - layout(rgba16f, binding = 1) restrict writeonly uniform image2D output0; + layout(binding = 1) restrict writeonly uniform image2D output0; // Output ranges uniform ivec4 outputRange0; diff --git a/src/libs/vcl.graphics/vcl/graphics/opengl/context.cpp b/src/libs/vcl.graphics/vcl/graphics/opengl/context.cpp index 366d44b4..50e3c473 100644 --- a/src/libs/vcl.graphics/vcl/graphics/opengl/context.cpp +++ b/src/libs/vcl.graphics/vcl/graphics/opengl/context.cpp @@ -126,6 +126,9 @@ namespace { } namespace Vcl { namespace Graphics { namespace OpenGL { + + bool Context::_vertex_array_attrib_iformat_bug = false; + const char* Context::profileType() { GLint profile = GL::getInteger(GL_CONTEXT_PROFILE_MASK); @@ -397,12 +400,23 @@ namespace Vcl { namespace Graphics { namespace OpenGL { std::terminate(); } + const auto vendor = glGetString(GL_VENDOR); std::cout << "Status: Using OpenGL: " << glGetString(GL_VERSION) << std::endl; - std::cout << "Status: Vendor: " << glGetString(GL_VENDOR) << std::endl; + std::cout << "Status: Vendor: " << vendor << std::endl; std::cout << "Status: Renderer: " << glGetString(GL_RENDERER) << std::endl; std::cout << "Status: Profile: " << profileType() << std::endl; std::cout << "Status: Shading: " << glGetString(GL_SHADING_LANGUAGE_VERSION) << std::endl; std::cout << "Status: Using GLEW: " << glewGetString(GLEW_VERSION) << std::endl; + +#if defined VCL_ABI_WINAPI + // Helpful list of existing OpenGL driver bugs: + // https://doc.magnum.graphics/magnum/opengl-workarounds.html + if (std::string(reinterpret_cast(vendor)).find("Intel") != std::string::npos) + { + std::cout << "Status: Intel driver detected. Workaround for Intel driver bug: glVertexArrayAttribIFormat." << std::endl; + _vertex_array_attrib_iformat_bug = true; + } +#endif } void Context::setupDebugMessaging() diff --git a/src/libs/vcl.graphics/vcl/graphics/opengl/context.h b/src/libs/vcl.graphics/vcl/graphics/opengl/context.h index f8cb28a0..f6761807 100644 --- a/src/libs/vcl.graphics/vcl/graphics/opengl/context.h +++ b/src/libs/vcl.graphics/vcl/graphics/opengl/context.h @@ -63,6 +63,13 @@ namespace Vcl { namespace Graphics { namespace OpenGL { //! Enable the OpenGL debug message extension static void setupDebugMessaging(); + //! Check state of glVertexArrayAttribIFormat driver bug + //! \returns True, if the driver is affected by the bug + static bool affectedByGLVertexArrayAttribIFormat() + { + return _vertex_array_attrib_iformat_bug; + } + public: Context(const ContextDesc& desc = {}); #if defined VCL_EGL_SUPPORT @@ -107,5 +114,8 @@ namespace Vcl { namespace Graphics { namespace OpenGL { //! Allocated surface bool _allocated_surface{ false }; + + //! Affected by glVertexArrayAttribIFormat bug + static bool _vertex_array_attrib_iformat_bug; }; }}} diff --git a/src/libs/vcl.graphics/vcl/graphics/runtime/opengl/state/inputlayout.cpp b/src/libs/vcl.graphics/vcl/graphics/runtime/opengl/state/inputlayout.cpp index ae991882..9b32086b 100644 --- a/src/libs/vcl.graphics/vcl/graphics/runtime/opengl/state/inputlayout.cpp +++ b/src/libs/vcl.graphics/vcl/graphics/runtime/opengl/state/inputlayout.cpp @@ -26,6 +26,7 @@ #include // VCL +#include #include #if defined(VCL_GL_ARB_direct_state_access) @@ -77,6 +78,11 @@ namespace Vcl { namespace Graphics { namespace Runtime { namespace OpenGL { glCreateVertexArraysVCL(1, &_vaoID); #if !(defined(VCL_GL_ARB_direct_state_access) || defined(VCL_GL_EXT_direct_state_access)) glBindVertexArray(_vaoID); +#else + if (Vcl::Graphics::OpenGL::Context::affectedByGLVertexArrayAttribIFormat()) + { + glBindVertexArray(_vaoID); + } #endif int idx = 0; @@ -123,6 +129,11 @@ namespace Vcl { namespace Graphics { namespace Runtime { namespace OpenGL { } #if !(defined(VCL_GL_ARB_direct_state_access) || defined(VCL_GL_EXT_direct_state_access)) glBindVertexArray(GL_NONE); +#else + if (Vcl::Graphics::OpenGL::Context::affectedByGLVertexArrayAttribIFormat()) + { + glBindVertexArray(GL_NONE); + } #endif } }}}} diff --git a/src/libs/vcl.graphics/vcl/graphics/runtime/opengl/state/shaderprogram.cpp b/src/libs/vcl.graphics/vcl/graphics/runtime/opengl/state/shaderprogram.cpp index 20272511..f066676c 100644 --- a/src/libs/vcl.graphics/vcl/graphics/runtime/opengl/state/shaderprogram.cpp +++ b/src/libs/vcl.graphics/vcl/graphics/runtime/opengl/state/shaderprogram.cpp @@ -189,6 +189,9 @@ namespace Vcl { namespace Graphics { namespace Runtime { namespace OpenGL { glGetProgramResourceName(program, GL_UNIFORM_BLOCK, u, (GLsizei)name.size(), nullptr, name.data()); GLint loc = glGetProgramResourceIndex(program, GL_UNIFORM_BLOCK, name.data()); + if (loc == GL_INVALID_INDEX) + continue; + GLint res_loc = -1; glGetActiveUniformBlockiv(program, loc, GL_UNIFORM_BLOCK_BINDING, &res_loc); @@ -698,6 +701,8 @@ namespace Vcl { namespace Graphics { namespace Runtime { namespace OpenGL { // The interface may contain more data than the shader can consume. // The interface must provide at least the used by the shader. ProgramAttributes attribs{ _glId }; + + bool relink = false; for (const auto& attrib : attribs) { const auto& name = attrib.Name; @@ -713,6 +718,7 @@ namespace Vcl { namespace Graphics { namespace Runtime { namespace OpenGL { int loc = layout.location(idx); glBindAttribLocation(_glId, loc, name.c_str()); + relink = true; } else if (name.find("gl_") != 0) { // Append to error output @@ -730,7 +736,8 @@ namespace Vcl { namespace Graphics { namespace Runtime { namespace OpenGL { //} // Relink the program to enable the changed binding configuration - glLinkProgram(id()); + if (relink) + glLinkProgram(id()); VclAssertBlock { diff --git a/src/tests/vcl.graphics.opengl/bindings.cpp b/src/tests/vcl.graphics.opengl/bindings.cpp index 669ec3b0..f5f3ed8b 100644 --- a/src/tests/vcl.graphics.opengl/bindings.cpp +++ b/src/tests/vcl.graphics.opengl/bindings.cpp @@ -27,6 +27,12 @@ #include // C++ Standard Library +#include +#include +#include + +// fmt +#include // Include the relevant parts from the library #include @@ -37,58 +43,58 @@ // Tests the shader compilation const char* MaxBindingsVS = - R"( + R"glsl( #version 440 core layout(location = 0) in vec2 Position; layout(location = 0) out PerVertexData -{ +{{ vec3 Colour; -} Out; +}} Out; -layout(binding = 0) uniform U00 { float u00; }; +layout(binding = 0) uniform U00 {{ float u00; }}; -layout(std430, binding = 0) buffer B00 { float b00; }; -layout(std430, binding = 1) buffer B01 { float b01; }; -layout(std430, binding = 2) buffer B02 { float b02; }; -layout(std430, binding = 3) buffer B03 { float b03; }; -layout(std430, binding = 4) buffer B04 { float b04; }; -layout(std430, binding = 5) buffer B05 { float b05; }; -layout(std430, binding = 6) buffer B06 { float b06; }; -layout(std430, binding = 7) buffer B07 { float b07; }; +layout(std430, binding = {}) buffer B00 {{ float b00; }}; +layout(std430, binding = {}) buffer B01 {{ float b01; }}; +layout(std430, binding = {}) buffer B02 {{ float b02; }}; +layout(std430, binding = {}) buffer B03 {{ float b03; }}; +layout(std430, binding = {}) buffer B04 {{ float b04; }}; +layout(std430, binding = {}) buffer B05 {{ float b05; }}; +layout(std430, binding = {}) buffer B06 {{ float b06; }}; +layout(std430, binding = {}) buffer B07 {{ float b07; }}; void main() -{ +{{ float b = u00 + b00 + b01 + b02 + b03 + b04 + b05 + b06 + b07; gl_Position = vec4(Position, b, 1); -} -)"; +}} +)glsl"; const char* MaxBindingsFS = - R"( + R"glsl( #version 440 core layout(location = 0) in PerVertexData -{ +{{ vec3 Colour; -} In; +}} In; layout(location = 0) out vec4 Colour; -layout(std430, binding = 8) buffer B10 { float b10; }; -layout(std430, binding = 31) buffer B11 { float b11; }; -layout(std430, binding = 32) buffer B12 { float b12; }; -layout(std430, binding = 33) buffer B13 { float b13; }; -layout(std430, binding = 34) buffer B14 { float b14; }; -layout(std430, binding = 35) buffer B15 { float b15; }; -layout(std430, binding = 36) buffer B16 { float b16; }; -layout(std430, binding = 37) buffer B17 { float b17; }; +layout(std430, binding = {}) buffer B10 {{ float b10; }}; +layout(std430, binding = {}) buffer B11 {{ float b11; }}; +layout(std430, binding = {}) buffer B12 {{ float b12; }}; +layout(std430, binding = {}) buffer B13 {{ float b13; }}; +layout(std430, binding = {}) buffer B14 {{ float b14; }}; +layout(std430, binding = {}) buffer B15 {{ float b15; }}; +layout(std430, binding = {}) buffer B16 {{ float b16; }}; +layout(std430, binding = {}) buffer B17 {{ float b17; }}; void main() -{ +{{ float b = b10 + b11 + b12 + b13 + b14 + b15 + b16 + b17; Colour = vec4(b); -} -)"; +}} +)glsl"; TEST(OpenGL, MaxBindingPoints) { @@ -104,10 +110,29 @@ TEST(OpenGL, MaxBindingPoints) glGetIntegerv(GL_MAX_COMBINED_SHADER_STORAGE_BLOCKS, &max_comb_bindings); glGetIntegerv(GL_MAX_SHADER_STORAGE_BUFFER_BINDINGS, &max_bindings); + // Minimum vertex and fragment shader storage blocks should be 8. + // According to the spcification the number can be lower, but it doesn't make sense to have less than 8. + // The gpuinfo database reports only one with less than 8: + // https://opengl.gpuinfo.org/displaycapability.php?name=GL_MAX_VERTEX_SHADER_STORAGE_BLOCKS + EXPECT_GE(max_vert_bindings, 8); + EXPECT_GE(max_frag_bindings, 8); + + // Minimum across all shader stages should be 16 + EXPECT_GE(max_bindings, 16); + + std::vector bindings(max_bindings); + std::iota(std::begin(bindings), std::end(bindings), 0); + + std::minstd_rand rnd; + std::shuffle(std::begin(bindings), std::end(bindings), rnd); + + const auto vs_code = fmt::format(MaxBindingsVS, bindings[0], bindings[1], bindings[2], bindings[3], bindings[4], bindings[5], bindings[6], bindings[7]); + const auto fs_code = fmt::format(MaxBindingsFS, bindings[8], bindings[9], bindings[10], bindings[11], bindings[12], bindings[13], bindings[14], bindings[15]); + // Compile the shader stages - auto vs = Runtime::OpenGL::makeShader(ShaderType::VertexShader, 0, MaxBindingsVS); + auto vs = Runtime::OpenGL::makeShader(ShaderType::VertexShader, 0, vs_code.c_str()); EXPECT_TRUE(vs) << vs.get_unexpected().value(); - auto fs = Runtime::OpenGL::makeShader(ShaderType::FragmentShader, 0, MaxBindingsFS); + auto fs = Runtime::OpenGL::makeShader(ShaderType::FragmentShader, 0, fs_code.c_str()); EXPECT_TRUE(fs) << fs.get_unexpected().value(); // Create the input definition @@ -137,20 +162,21 @@ TEST(OpenGL, MaxBindingPoints) {} }; Runtime::OpenGL::Buffer buf(buf_desc); - std::vector ids(max_bindings, buf.id()); - glBindBuffersBase(GL_SHADER_STORAGE_BUFFER, 0, 8, ids.data()); - glBindBuffersBase(GL_SHADER_STORAGE_BUFFER, 31, 8, ids.data()); + for (const auto binding : bindings) + { + glBindBufferBase(GL_SHADER_STORAGE_BUFFER, binding, buf.id()); + } for (int i = 0; i < 8; i++) { GLint id = 0; - glGetIntegeri_v(GL_SHADER_STORAGE_BUFFER_BINDING, i, &id); + glGetIntegeri_v(GL_SHADER_STORAGE_BUFFER_BINDING, bindings[i], &id); EXPECT_EQ(id, buf.id()); } for (int i = 0; i < 8; i++) { GLint id = 0; - glGetIntegeri_v(GL_SHADER_STORAGE_BUFFER_BINDING, i + 31, &id); + glGetIntegeri_v(GL_SHADER_STORAGE_BUFFER_BINDING, bindings[i + 8], &id); EXPECT_EQ(id, buf.id()); } } diff --git a/src/tests/vcl.graphics.opengl/engine.cpp b/src/tests/vcl.graphics.opengl/engine.cpp index ae49b19f..e49b20b5 100644 --- a/src/tests/vcl.graphics.opengl/engine.cpp +++ b/src/tests/vcl.graphics.opengl/engine.cpp @@ -222,7 +222,7 @@ TEST(OpenGL, ConstantBufferUsage) }; // Base address of constants. Must be the same at the start of the next cycle - void* base_address{ nullptr }; + const Runtime::Buffer* owner_zero{ nullptr }; engine.beginFrame(); { @@ -230,7 +230,7 @@ TEST(OpenGL, ConstantBufferUsage) { auto memory2 = engine.requestPerFrameConstantBuffer(); } - base_address = memory.data(); + owner_zero = &memory.owner(); engine.setConstantBuffer(0, std::move(memory)); } @@ -265,11 +265,12 @@ TEST(OpenGL, ConstantBufferUsage) { auto memory2 = engine.requestPerFrameConstantBuffer(); } - const void* new_base_address = memory.data(); + const Runtime::Buffer* owner_four = &memory.owner(); engine.setConstantBuffer(0, std::move(memory)); - EXPECT_EQ(base_address, new_base_address); + // Silly test as the memory buffers are remapped every frame potentially changing the pointer + EXPECT_EQ(owner_zero, owner_four); } engine.endFrame(); glFinish(); diff --git a/src/tests/vcl.graphics.opengl/inputlayout.cpp b/src/tests/vcl.graphics.opengl/inputlayout.cpp index a64aafb8..f52f9424 100644 --- a/src/tests/vcl.graphics.opengl/inputlayout.cpp +++ b/src/tests/vcl.graphics.opengl/inputlayout.cpp @@ -36,42 +36,44 @@ #include namespace { - GLint vertexAttribiv(GLuint i, GLenum param) +#if defined(VCL_GL_ARB_direct_state_access) + GLint vertexAttribiv(GLuint vao, GLuint i, GLenum param) { GLint value = 0; - glGetVertexAttribiv(i, param, &value); + glGetVertexArrayIndexediv(vao, i, param, &value); return value; } - - GLuint vertexAttribIuiv(GLuint i, GLenum param) +#else + GLint vertexAttribiv(GLuint vao, GLuint i, GLenum param) { - GLuint value = 0; - glGetVertexAttribIuiv(i, param, &value); + GLint value = 0; + glGetVertexAttribiv(i, param, &value); return value; } +#endif - void checkEnabled(GLuint lower, GLuint upper) + void checkEnabled(GLuint vao, GLint lower, GLint upper) { for (int i = 0; i < Vcl::Graphics::OpenGL::GL::getInteger(GL_MAX_VERTEX_ATTRIBS); i++) { - GLuint enabled = vertexAttribIuiv(i, GL_VERTEX_ATTRIB_ARRAY_ENABLED); + GLint enabled = vertexAttribiv(vao, i, GL_VERTEX_ATTRIB_ARRAY_ENABLED); EXPECT_EQ(i >= lower && i <= upper, enabled == 1); } } - void checkSettings(GLuint i, GLuint ref_size, GLuint ref_stride, GLuint ref_type, GLuint ref_norm, GLuint ref_integral, GLuint ref_div) + void checkSettings(GLuint vao, GLuint i, GLint ref_size, GLint ref_stride, GLint ref_type, GLint ref_norm, GLint ref_integral, GLint ref_div) { - GLuint size = vertexAttribIuiv(i, GL_VERTEX_ATTRIB_ARRAY_SIZE); + GLint size = vertexAttribiv(vao, i, GL_VERTEX_ATTRIB_ARRAY_SIZE); EXPECT_EQ(size, ref_size); - GLuint stride = vertexAttribIuiv(i, GL_VERTEX_ATTRIB_ARRAY_STRIDE); + GLint stride = vertexAttribiv(vao, i, GL_VERTEX_ATTRIB_ARRAY_STRIDE); EXPECT_EQ(stride, ref_stride); - GLuint type = vertexAttribIuiv(i, GL_VERTEX_ATTRIB_ARRAY_TYPE); + GLint type = vertexAttribiv(vao, i, GL_VERTEX_ATTRIB_ARRAY_TYPE); EXPECT_EQ(type, ref_type); - GLuint norm = vertexAttribIuiv(i, GL_VERTEX_ATTRIB_ARRAY_NORMALIZED); + GLint norm = vertexAttribiv(vao, i, GL_VERTEX_ATTRIB_ARRAY_NORMALIZED); EXPECT_EQ(norm, ref_norm); - GLuint integral = vertexAttribIuiv(i, GL_VERTEX_ATTRIB_ARRAY_INTEGER); + GLint integral = vertexAttribiv(vao, i, GL_VERTEX_ATTRIB_ARRAY_INTEGER); EXPECT_EQ(integral, ref_integral); - GLuint divisor = vertexAttribIuiv(i, GL_VERTEX_ATTRIB_ARRAY_DIVISOR); + GLint divisor = vertexAttribiv(vao, i, GL_VERTEX_ATTRIB_ARRAY_DIVISOR); EXPECT_EQ(divisor, ref_div); } } @@ -102,8 +104,8 @@ TEST(OpenGL, Float4Layout) layout.bind(); - checkEnabled(0, 0); - checkSettings(0, 4, 0, GL_FLOAT, GL_FALSE, GL_FALSE, 0); + checkEnabled(layout.id(), 0, 0); + checkSettings(layout.id(), 0, 4, 0, GL_FLOAT, GL_FALSE, GL_FALSE, 0); } TEST(OpenGL, NormalizedSignedShort2Layout) @@ -121,8 +123,27 @@ TEST(OpenGL, NormalizedSignedShort2Layout) layout.bind(); - checkEnabled(0, 0); - checkSettings(0, 2, 0, GL_SHORT, GL_TRUE, GL_FALSE, 1); + checkEnabled(layout.id(), 0, 0); + checkSettings(layout.id(), 0, 2, 0, GL_SHORT, GL_TRUE, GL_FALSE, 1); +} + +TEST(OpenGL, NormalizedUnsignedShort4Layout) +{ + InputLayoutDescription in{ + { + { 0, 4 * sizeof(unsigned short), VertexDataClassification::VertexDataPerInstance }, + }, + { + { "Position", SurfaceFormat::R16G16B16A16_UNORM, 0, 0, 0 }, + } + }; + InputLayout layout{ in }; + EXPECT_NE(0, layout.id()); + + layout.bind(); + + checkEnabled(layout.id(), 0, 0); + checkSettings(layout.id(), 0, 4, 0, GL_UNSIGNED_SHORT, GL_TRUE, GL_FALSE, 1); } TEST(OpenGL, SignedByte1Layout) @@ -140,6 +161,6 @@ TEST(OpenGL, SignedByte1Layout) layout.bind(); - checkEnabled(0, 0); - checkSettings(0, 1, 0, GL_BYTE, GL_FALSE, GL_TRUE, 0); + checkEnabled(layout.id(), 0, 0); + checkSettings(layout.id(), 0, 1, 0, GL_BYTE, GL_FALSE, GL_TRUE, 0); }