Fix intel related OpenGL errors

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<Runtime::Texture>& 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;

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;

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<const char*>(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()

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;
 	};
 }}}

src/libs/vcl.graphics/vcl/graphics/runtime/opengl/state/inputlayout.cpp

@@ -26,6 +26,7 @@
 #include <vcl/graphics/runtime/opengl/state/inputlayout.h>
 
 // VCL
+#include <vcl/graphics/opengl/context.h>
 #include <vcl/graphics/opengl/gl.h>
 
 #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
 	}
 }}}}

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
 		{

src/tests/vcl.graphics.opengl/bindings.cpp

@@ -27,6 +27,12 @@
 #include <vcl/config/global.h>
 
 // C++ Standard Library
+#include <algorithm>
+#include <numeric>
+#include <random>
+
+// fmt
+#include <fmt/format.h>
 
 // Include the relevant parts from the library
 #include <vcl/graphics/runtime/opengl/resource/buffer.h>
@@ -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<int> 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<GLuint> 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());
 	}
 }

src/tests/vcl.graphics.opengl/engine.cpp

@@ -222,15 +222,15 @@ 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();
 	{
 		auto memory = engine.requestPerFrameConstantBuffer<ShaderConstants>();
 		{
 			auto memory2 = engine.requestPerFrameConstantBuffer<ShaderConstants>();
 		}
-		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<ShaderConstants>();
 		}
-		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();