From daabcfd6fc520ff84cd488a03b76c4cdf4a2de2a Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Wed, 29 Oct 2014 19:50:39 -0500 Subject: [PATCH 1/3] Removes Qualcomm's rotated framebuffer bug from DriverDetails. Due to changes in how we render to the final framebuffer we no longer encounter this bug. With the change to post processing being enabled at all times and no longer using glBlitFramebuffer, Qualcomm no longer has the chance to rotate our framebuffer underneath of us. --- Source/Core/VideoBackends/OGL/Render.cpp | 11 ++--------- Source/Core/VideoCommon/DriverDetails.cpp | 1 - Source/Core/VideoCommon/DriverDetails.h | 9 --------- 3 files changed, 2 insertions(+), 19 deletions(-) diff --git a/Source/Core/VideoBackends/OGL/Render.cpp b/Source/Core/VideoBackends/OGL/Render.cpp index 69dbd4052f..1479107cc6 100644 --- a/Source/Core/VideoBackends/OGL/Render.cpp +++ b/Source/Core/VideoBackends/OGL/Render.cpp @@ -1395,15 +1395,8 @@ void Renderer::SwapImpl(u32 xfbAddr, u32 fbWidth, u32 fbStride, u32 fbHeight, co UpdateDrawRectangle(s_backbuffer_width, s_backbuffer_height); TargetRectangle flipped_trc = GetTargetRectangle(); - if (DriverDetails::HasBug(DriverDetails::BUG_ROTATEDFRAMEBUFFER)) - { - std::swap(flipped_trc.left, flipped_trc.right); - } - else - { - // Flip top and bottom for some reason; TODO: Fix the code to suck less? - std::swap(flipped_trc.top, flipped_trc.bottom); - } + // Flip top and bottom for some reason; TODO: Fix the code to suck less? + std::swap(flipped_trc.top, flipped_trc.bottom); // Copy the framebuffer to screen. const XFBSource* xfbSource = nullptr; diff --git a/Source/Core/VideoCommon/DriverDetails.cpp b/Source/Core/VideoCommon/DriverDetails.cpp index 4d0d03d0d0..260a2ca63f 100644 --- a/Source/Core/VideoCommon/DriverDetails.cpp +++ b/Source/Core/VideoCommon/DriverDetails.cpp @@ -47,7 +47,6 @@ namespace DriverDetails {OS_ALL, VENDOR_QUALCOMM, DRIVER_QUALCOMM_3XX, -1, BUG_BROKENSWAP, -1.0, 46.0, true}, {OS_ALL, VENDOR_QUALCOMM, DRIVER_QUALCOMM_3XX, -1, BUG_BROKENBUFFERSTREAM, -1.0, -1.0, true}, {OS_ALL, VENDOR_QUALCOMM, DRIVER_QUALCOMM_3XX, -1, BUG_BROKENTEXTURESIZE, -1.0, 65.0, true}, - {OS_ALL, VENDOR_QUALCOMM, DRIVER_QUALCOMM_3XX, -1, BUG_ROTATEDFRAMEBUFFER, 53.0, 65.0, true}, {OS_ALL, VENDOR_ARM, DRIVER_ARM_MIDGARD, -1, BUG_BROKENBUFFERSTREAM, -1.0, -1.0, true}, {OS_ALL, VENDOR_MESA, DRIVER_NOUVEAU, -1, BUG_BROKENUBO, 900, 916, true}, {OS_ALL, VENDOR_MESA, DRIVER_R600, -1, BUG_BROKENUBO, 900, 913, true}, diff --git a/Source/Core/VideoCommon/DriverDetails.h b/Source/Core/VideoCommon/DriverDetails.h index 3c65258c8c..2571c2f1a8 100644 --- a/Source/Core/VideoCommon/DriverDetails.h +++ b/Source/Core/VideoCommon/DriverDetails.h @@ -162,15 +162,6 @@ namespace DriverDetails // TODO: some windows AMD driver/gpu combination seems also affected // but as they all support pinned memory, it doesn't matter BUG_BROKENUNSYNCMAPPING, - // Bug: Adreno now rotates the framebuffer on blit a full 180 degrees - // Affected devices: Adreno - // Started Version: v53 (dev drivers) - // Ended Version: v66 (07-14-2014 dev drivers) - // Qualcomm is a super pro company that has recently updated their development drivers - // These drivers are available to the Nexus 5 and report as v53 - // Qualcomm in their infinite wisdom thought it was a good idea to rotate the framebuffer 180 degrees on glBlit - // This bug allows us to work around that rotation by rotating it the right way around again. - BUG_ROTATEDFRAMEBUFFER, // Bug: Intel's Window driver broke buffer_storage with GL_ELEMENT_ARRAY_BUFFER // Affected devices: Intel (Windows) // Started Version: 15.36.3.64.3907 (10.18.10.3907) From 9da7e6ae79ea51afe7470beb8407080d54f8fd89 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Wed, 29 Oct 2014 19:58:18 -0500 Subject: [PATCH 2/3] Adds a DriverDetails bug to track Qualcomm attributeless rendering. This particular issue was fixed in the v66 (07-08-2014) development drivers from Qualcomm. To make sure we cover all drivers that may or may not have the issue fixed, make sure to mandate v95 minimum to work around the issue. The next commit is the actual work around for post processing for this. --- Source/Core/VideoCommon/DriverDetails.cpp | 1 + Source/Core/VideoCommon/DriverDetails.h | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/Source/Core/VideoCommon/DriverDetails.cpp b/Source/Core/VideoCommon/DriverDetails.cpp index 260a2ca63f..4664f17e16 100644 --- a/Source/Core/VideoCommon/DriverDetails.cpp +++ b/Source/Core/VideoCommon/DriverDetails.cpp @@ -47,6 +47,7 @@ namespace DriverDetails {OS_ALL, VENDOR_QUALCOMM, DRIVER_QUALCOMM_3XX, -1, BUG_BROKENSWAP, -1.0, 46.0, true}, {OS_ALL, VENDOR_QUALCOMM, DRIVER_QUALCOMM_3XX, -1, BUG_BROKENBUFFERSTREAM, -1.0, -1.0, true}, {OS_ALL, VENDOR_QUALCOMM, DRIVER_QUALCOMM_3XX, -1, BUG_BROKENTEXTURESIZE, -1.0, 65.0, true}, + {OS_ALL, VENDOR_QUALCOMM, DRIVER_QUALCOMM_3XX, -1, BUG_BROKENATTRIBUTELESS, -1.0, 94.0, true}, {OS_ALL, VENDOR_ARM, DRIVER_ARM_MIDGARD, -1, BUG_BROKENBUFFERSTREAM, -1.0, -1.0, true}, {OS_ALL, VENDOR_MESA, DRIVER_NOUVEAU, -1, BUG_BROKENUBO, 900, 916, true}, {OS_ALL, VENDOR_MESA, DRIVER_R600, -1, BUG_BROKENUBO, 900, 913, true}, diff --git a/Source/Core/VideoCommon/DriverDetails.h b/Source/Core/VideoCommon/DriverDetails.h index 2571c2f1a8..02b56e5d08 100644 --- a/Source/Core/VideoCommon/DriverDetails.h +++ b/Source/Core/VideoCommon/DriverDetails.h @@ -170,6 +170,14 @@ namespace DriverDetails // It works for all the buffer types we use except GL_ELEMENT_ARRAY_BUFFER. // Causes complete blackscreen issues. BUG_INTELBROKENBUFFERSTORAGE, + // Bug: Qualcomm has broken attributeless rendering + // Affected devices: Adreno + // Started Version: -1 + // Ended Version: v66 (07-09-2014 dev version), v95 shipping + // Qualcomm has had attributeless rendering broken forever + // This was fixed in a v66 development version, the first shipping driver version with the release was v95. + // To be safe, make v95 the minimum version to work around this issue + BUG_BROKENATTRIBUTELESS, }; // Initializes our internal vendor, device family, and driver version From 181ff6750ea4113378497beebf49400ba83a8e07 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Wed, 29 Oct 2014 20:06:45 -0500 Subject: [PATCH 3/3] Implements PP shader system using attribute workaround. This is pretty much a step backwards in our code. We used to use attributes in our PP shader system a long time ago but we changed it to attributeless for code simplicity and cleanliness. This reimplements the attribute code path as an optional path to take in the case your system doesn't work with attributeless rendering. In this case the only shipping drivers that we can know for sure supports attributeless rendering is the Nexus 5's v95 driver that is included in the Android 5.0 image. I hadn't planned on implementing a work around to get post processing working in these cases, but due to us force enabling the PP shader system at all times it sort of went up on the priority list. We can't be having a supported platform black screening at all times can we? --- .../Core/VideoBackends/OGL/PostProcessing.cpp | 53 ++++++++++++++++++- .../Core/VideoBackends/OGL/PostProcessing.h | 5 ++ 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/Source/Core/VideoBackends/OGL/PostProcessing.cpp b/Source/Core/VideoBackends/OGL/PostProcessing.cpp index 519406cb0f..b364954bd7 100644 --- a/Source/Core/VideoBackends/OGL/PostProcessing.cpp +++ b/Source/Core/VideoBackends/OGL/PostProcessing.cpp @@ -11,12 +11,22 @@ #include "VideoBackends/OGL/PostProcessing.h" #include "VideoBackends/OGL/ProgramShaderCache.h" +#include "VideoCommon/DriverDetails.h" #include "VideoCommon/VideoCommon.h" #include "VideoCommon/VideoConfig.h" namespace OGL { +static char s_vertex_workaround_shader[] = + "in vec4 rawpos;\n" + "out vec2 uv0;\n" + "uniform vec4 src_rect;\n" + "void main(void) {\n" + " gl_Position = vec4(rawpos.xy, 0.0, 1.0);\n" + " uv0 = rawpos.zw * src_rect.zw + src_rect.xy;\n" + "}\n"; + static char s_vertex_shader[] = "out vec2 uv0;\n" "uniform vec4 src_rect;\n" @@ -29,12 +39,26 @@ static char s_vertex_shader[] = OpenGLPostProcessing::OpenGLPostProcessing() { CreateHeader(); + + m_attribute_workaround = DriverDetails::HasBug(DriverDetails::BUG_BROKENATTRIBUTELESS); + if (m_attribute_workaround) + { + glGenBuffers(1, &m_attribute_vbo); + glGenVertexArrays(1, &m_attribute_vao); + } + m_initialized = false; } OpenGLPostProcessing::~OpenGLPostProcessing() { m_shader.Destroy(); + + if (m_attribute_workaround) + { + glDeleteBuffers(1, &m_attribute_vbo); + glDeleteVertexArrays(1, &m_attribute_vao); + } } void OpenGLPostProcessing::BlitFromTexture(TargetRectangle src, TargetRectangle dst, @@ -46,6 +70,9 @@ void OpenGLPostProcessing::BlitFromTexture(TargetRectangle src, TargetRectangle glViewport(dst.left, dst.bottom, dst.GetWidth(), dst.GetHeight()); + if (m_attribute_workaround) + glBindVertexArray(m_attribute_vao); + m_shader.Bind(); glUniform4f(m_uniform_resolution, (float)src_width, (float)src_height, 1.0f / (float)src_width, 1.0f / (float)src_height); @@ -148,13 +175,18 @@ void OpenGLPostProcessing::ApplyShader() code = LoadShaderOptions(code); + const char* vertex_shader = s_vertex_shader; + + if (m_attribute_workaround) + vertex_shader = s_vertex_workaround_shader; + // and compile it - if (!ProgramShaderCache::CompileShader(m_shader, s_vertex_shader, code.c_str())) + if (!ProgramShaderCache::CompileShader(m_shader, vertex_shader, code.c_str())) { ERROR_LOG(VIDEO, "Failed to compile post-processing shader %s", m_config.GetShader().c_str()); code = LoadShaderOptions(default_shader); - ProgramShaderCache::CompileShader(m_shader, s_vertex_shader, code.c_str()); + ProgramShaderCache::CompileShader(m_shader, vertex_shader, code.c_str()); } // read uniform locations @@ -162,6 +194,23 @@ void OpenGLPostProcessing::ApplyShader() m_uniform_time = glGetUniformLocation(m_shader.glprogid, "time"); m_uniform_src_rect = glGetUniformLocation(m_shader.glprogid, "src_rect"); + if (m_attribute_workaround) + { + GLfloat vertices[] = { + -1.f, -1.f, 0.f, 0.f, + 1.f, -1.f, 1.f, 0.f, + -1.f, 1.f, 0.f, 1.f, + 1.f, 1.f, 1.f, 1.f, + }; + + glBindBuffer(GL_ARRAY_BUFFER, m_attribute_vbo); + glBufferData(GL_ARRAY_BUFFER, sizeof(vertices), vertices, GL_STATIC_DRAW); + + glBindVertexArray(m_attribute_vao); + glEnableVertexAttribArray(SHADER_POSITION_ATTRIB); + glVertexAttribPointer(SHADER_POSITION_ATTRIB, 4, GL_FLOAT, 0, 0, nullptr); + } + for (const auto& it : m_config.GetOptions()) { std::string glsl_name = "option_" + it.first; diff --git a/Source/Core/VideoBackends/OGL/PostProcessing.h b/Source/Core/VideoBackends/OGL/PostProcessing.h index 3e53c229b0..9f4f6ba5bc 100644 --- a/Source/Core/VideoBackends/OGL/PostProcessing.h +++ b/Source/Core/VideoBackends/OGL/PostProcessing.h @@ -34,6 +34,11 @@ private: GLuint m_uniform_time; std::string m_glsl_header; + // These are only used when working around Qualcomm's broken attributeless rendering + GLuint m_attribute_vao; + GLuint m_attribute_vbo; + bool m_attribute_workaround = false; + std::unordered_map m_uniform_bindings; void CreateHeader();