From ce99921c20ef63ea7ed1361a6453bc6384ab0748 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Fri, 27 Dec 2013 10:56:03 -0600 Subject: [PATCH 1/5] [buffer_storage] Implement ARB_buffer_storage. Disable it for GL_ARRAY_BUFFER due to a bug in Nvidia's drivers that causes black screen with it. --- Source/Core/VideoBackends/OGL/Src/Render.cpp | 2 + Source/Core/VideoBackends/OGL/Src/Render.h | 1 + .../VideoBackends/OGL/Src/StreamBuffer.cpp | 52 +++++++++++++++---- .../Core/VideoBackends/OGL/Src/StreamBuffer.h | 5 +- Source/Core/VideoCommon/Src/DriverDetails.cpp | 1 + Source/Core/VideoCommon/Src/DriverDetails.h | 8 +++ 6 files changed, 56 insertions(+), 13 deletions(-) diff --git a/Source/Core/VideoBackends/OGL/Src/Render.cpp b/Source/Core/VideoBackends/OGL/Src/Render.cpp index 4d9980fc65..694301b6f8 100644 --- a/Source/Core/VideoBackends/OGL/Src/Render.cpp +++ b/Source/Core/VideoBackends/OGL/Src/Render.cpp @@ -478,6 +478,7 @@ Renderer::Renderer() g_ogl_config.bSupportsGLSync = TO_BOOL(GLEW_ARB_sync); g_ogl_config.bSupportsGLBaseVertex = TO_BOOL(GLEW_ARB_draw_elements_base_vertex) && !DriverDetails::HasBug(DriverDetails::BUG_BROKENPINNEDMEMORY); + g_ogl_config.bSupportsGLBufferStorage = TO_BOOL(GLEW_ARB_buffer_storage); g_ogl_config.bSupportCoverageMSAA = TO_BOOL(GLEW_NV_framebuffer_multisample_coverage); g_ogl_config.bSupportSampleShading = TO_BOOL(GLEW_ARB_sample_shading); g_ogl_config.bSupportOGL31 = TO_BOOL(GLEW_VERSION_3_1); @@ -554,6 +555,7 @@ Renderer::Renderer() g_ogl_config.bSupportsGLPinnedMemory ? "" : "PinnedMemory ", g_ogl_config.bSupportsGLSLCache ? "" : "ShaderCache ", g_ogl_config.bSupportsGLBaseVertex ? "" : "BaseVertex ", + g_ogl_config.bSupportsGLBufferStorage ? "" : "BufferStorage ", g_ogl_config.bSupportsGLSync ? "" : "Sync ", g_ogl_config.bSupportCoverageMSAA ? "" : "CSAA ", g_ogl_config.bSupportSampleShading ? "" : "SSAA " diff --git a/Source/Core/VideoBackends/OGL/Src/Render.h b/Source/Core/VideoBackends/OGL/Src/Render.h index 9489d5c7ac..58ba086d18 100644 --- a/Source/Core/VideoBackends/OGL/Src/Render.h +++ b/Source/Core/VideoBackends/OGL/Src/Render.h @@ -22,6 +22,7 @@ extern struct VideoConfig { bool bSupportsGLPinnedMemory; bool bSupportsGLSync; bool bSupportsGLBaseVertex; + bool bSupportsGLBufferStorage; bool bSupportCoverageMSAA; bool bSupportSampleShading; GLSL_VERSION eSupportedGLSLVersion; diff --git a/Source/Core/VideoBackends/OGL/Src/StreamBuffer.cpp b/Source/Core/VideoBackends/OGL/Src/StreamBuffer.cpp index 8ec9c3c1cb..5e3d4dfd38 100644 --- a/Source/Core/VideoBackends/OGL/Src/StreamBuffer.cpp +++ b/Source/Core/VideoBackends/OGL/Src/StreamBuffer.cpp @@ -33,7 +33,11 @@ StreamBuffer::StreamBuffer(u32 type, size_t size, StreamType uploadType) g_Config.bHackedBufferUpload = false; } - if(!g_ogl_config.bSupportsGLBaseVertex && (m_uploadtype & BUFFERSUBDATA) + if (g_ogl_config.bSupportsGLBufferStorage && + !(DriverDetails::HasBug(DriverDetails::BUG_BROKENBUFFERSTORAGE) && type == GL_ARRAY_BUFFER) && + (m_uploadtype & BUFFERSTORAGE)) + m_uploadtype = BUFFERSTORAGE; + else if(!g_ogl_config.bSupportsGLBaseVertex && (m_uploadtype & BUFFERSUBDATA) && !DriverDetails::HasBug(DriverDetails::BUG_BROKENBUFFERSTREAM)) m_uploadtype = BUFFERSUBDATA; else if(!g_ogl_config.bSupportsGLBaseVertex && (m_uploadtype & BUFFERDATA)) @@ -79,9 +83,9 @@ void StreamBuffer::Alloc ( size_t size, u32 stride ) break; case MAP_AND_SYNC: case PINNED_MEMORY: - + case BUFFERSTORAGE: // insert waiting slots for used memory - for(size_t i=SLOT(m_used_iterator); i= m_size) { + if (iter_end >= m_size) { // insert waiting slots in unused space at the end of the buffer for (size_t i = SLOT(m_used_iterator); i < SYNC_POINTS; i++) @@ -109,7 +113,7 @@ void StreamBuffer::Alloc ( size_t size, u32 stride ) iter_end = size; // wait for space at the start - for(u32 i=0; i<=SLOT(iter_end); i++) + for (u32 i = 0; i <= SLOT(iter_end); i++) { glClientWaitSync(fences[i], GL_SYNC_FLUSH_COMMANDS_BIT, GL_TIMEOUT_IGNORED); glDeleteSync(fences[i]); @@ -128,7 +132,7 @@ void StreamBuffer::Alloc ( size_t size, u32 stride ) m_iterator_aligned = 0; break; case STREAM_DETECT: - case DETECT_MASK: // Just to shutup warnings + case DETECT_MASK: // To shutup compiler warnings break; } m_iterator = m_iterator_aligned; @@ -149,8 +153,9 @@ size_t StreamBuffer::Upload ( u8* data, size_t size ) break; case PINNED_MEMORY: case MAP_AND_RISK: - if(pointer) - memcpy(pointer+m_iterator, data, size); + case BUFFERSTORAGE: + if (pointer) + memcpy(pointer + m_iterator, data, size); break; case BUFFERSUBDATA: glBufferSubData(m_buffertype, m_iterator, size, data); @@ -159,7 +164,7 @@ size_t StreamBuffer::Upload ( u8* data, size_t size ) glBufferData(m_buffertype, size, data, GL_STREAM_DRAW); break; case STREAM_DETECT: - case DETECT_MASK: // Just to shutup warnings + case DETECT_MASK: // To shutup compiler warnings break; } size_t ret = m_iterator; @@ -204,6 +209,25 @@ void StreamBuffer::Init() Init(); } break; + + case BUFFERSTORAGE: + glGetError(); // errors before this allocation should be ignored + fences = new GLsync[SYNC_POINTS]; + for (u32 i = 0; i m_bugs; diff --git a/Source/Core/VideoCommon/Src/DriverDetails.h b/Source/Core/VideoCommon/Src/DriverDetails.h index 1b024cefca..dd8eeade69 100644 --- a/Source/Core/VideoCommon/Src/DriverDetails.h +++ b/Source/Core/VideoCommon/Src/DriverDetails.h @@ -141,6 +141,14 @@ namespace DriverDetails // Ended Version: -1 // If a shader includes a textureSize function call then the shader compiler will call abort() BUG_BROKENTEXTURESIZE, + // Bug: ARB_buffer_storage doesn't work with ARRAY_BUFFER type streams + // Affected devices: Geforce 4xx+ + // Started Version: -1 + // Ended Version: -1 + // The buffer_storage streaming method is required for greater speed gains in our buffer streaming + // It reduces what is needed for streaming to basically a memcpy call + // It seems to work for all buffer types except GL_ARRAY_BUFFER + BUG_BROKENBUFFERSTORAGE, }; // Initializes our internal vendor, device family, and driver version From d8ceb97a609389df7a2449b21626469b19a73e1a Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Fri, 27 Dec 2013 15:37:31 -0600 Subject: [PATCH 2/5] [buffer_storage] Require GLEW 1.10 which has the new OpenGL 4.4 methods. Fixes linux build. --- CMakeLists.txt | 2 +- CMakeTests/FindGLEW.cmake | 7 ++++--- Source/Core/VideoBackends/OGL/Src/Render.cpp | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a767693bd9..583ea9e711 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -661,7 +661,7 @@ else() if(NOT ${CMAKE_SYSTEM_NAME} MATCHES "Darwin") include(FindGLEW) endif() - if(NOT GLEW_FOUND OR NOT GLEW_HAS_1_9_METHODS) + if(NOT GLEW_FOUND OR NOT GLEW_HAS_1_10_METHODS) message("Using static GLEW from Externals") add_subdirectory(Externals/GLew) include_directories(Externals/GLew/include) diff --git a/CMakeTests/FindGLEW.cmake b/CMakeTests/FindGLEW.cmake index 4861292558..c91048e362 100644 --- a/CMakeTests/FindGLEW.cmake +++ b/CMakeTests/FindGLEW.cmake @@ -10,13 +10,14 @@ macro(test_glew) #include int main() { - #ifdef GLEW_ARB_shader_image_load_store + #ifdef GLEW_ARB_buffer_storage return 0; #else return 1; #endif }" - GLEW_HAS_1_9_METHODS) + GLEW_HAS_1_10_METHODS) + endmacro() if(PKG_CONFIG_FOUND AND NOT ${var}_FOUND) @@ -25,7 +26,7 @@ endif() if(GLEW_FOUND) test_glew() - if (GLEW_HAS_1_9_METHODS) + if (GLEW_HAS_1_10_METHODS) include_directories(${GLEW_INCLUDE_DIRS}) message("GLEW found") endif() diff --git a/Source/Core/VideoBackends/OGL/Src/Render.cpp b/Source/Core/VideoBackends/OGL/Src/Render.cpp index 694301b6f8..b2b5687047 100644 --- a/Source/Core/VideoBackends/OGL/Src/Render.cpp +++ b/Source/Core/VideoBackends/OGL/Src/Render.cpp @@ -547,7 +547,7 @@ Renderer::Renderer() g_ogl_config.gl_renderer, g_ogl_config.gl_version), 5000); - WARN_LOG(VIDEO,"Missing OGL Extensions: %s%s%s%s%s%s%s%s%s%s", + WARN_LOG(VIDEO,"Missing OGL Extensions: %s%s%s%s%s%s%s%s%s%s%s", g_ActiveConfig.backend_info.bSupportsDualSourceBlend ? "" : "DualSourceBlend ", g_ActiveConfig.backend_info.bSupportsGLSLUBO ? "" : "UniformBuffer ", g_ActiveConfig.backend_info.bSupportsPrimitiveRestart ? "" : "PrimitiveRestart ", From 77ba05136130cbb0c4bf9ea913779ada88426607 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Fri, 27 Dec 2013 22:58:36 -0600 Subject: [PATCH 3/5] [buffer_storage] Add the CLIENT_STORAGE_BIT since we access the buffer more frequently on the client side than the server side. That is exactly what the hint is for. --- Source/Core/VideoBackends/OGL/Src/StreamBuffer.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Source/Core/VideoBackends/OGL/Src/StreamBuffer.cpp b/Source/Core/VideoBackends/OGL/Src/StreamBuffer.cpp index 5e3d4dfd38..5aae51c686 100644 --- a/Source/Core/VideoBackends/OGL/Src/StreamBuffer.cpp +++ b/Source/Core/VideoBackends/OGL/Src/StreamBuffer.cpp @@ -220,8 +220,9 @@ void StreamBuffer::Init() // PERSISTANT_BIT to make sure that the buffer can be used while mapped // COHERENT_BIT is set so we don't have to use a MemoryBarrier on write + // CLIENT_STORAGE_BIT is set since we access the buffer more frequently on the client side then server side glBufferStorage(m_buffertype, m_size, NULL, - GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT | GL_MAP_COHERENT_BIT); + GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT | GL_MAP_COHERENT_BIT | GL_CLIENT_STORAGE_BIT); pointer = (u8*)glMapBufferRange(m_buffertype, 0, m_size, GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT | GL_MAP_COHERENT_BIT); if(!pointer) From 8fdcba0c06adc987bedc670ecbe91a34fee2562c Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Sat, 28 Dec 2013 05:15:43 -0600 Subject: [PATCH 4/5] [buffer_storage] Code formatting changes suggested by Bh44L --- Source/Core/VideoBackends/OGL/Src/StreamBuffer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Core/VideoBackends/OGL/Src/StreamBuffer.cpp b/Source/Core/VideoBackends/OGL/Src/StreamBuffer.cpp index 5aae51c686..95e8a8e72b 100644 --- a/Source/Core/VideoBackends/OGL/Src/StreamBuffer.cpp +++ b/Source/Core/VideoBackends/OGL/Src/StreamBuffer.cpp @@ -85,7 +85,7 @@ void StreamBuffer::Alloc ( size_t size, u32 stride ) case PINNED_MEMORY: case BUFFERSTORAGE: // insert waiting slots for used memory - for (size_t i = SLOT(m_used_iterator); i Date: Sat, 28 Dec 2013 05:16:39 -0600 Subject: [PATCH 5/5] [buffer_storage] Temporary fix for Android/GLES3 just like how everything else is. Cleanup for this will be in a new branch. --- Source/Core/VideoBackends/OGL/Src/GLUtil.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Source/Core/VideoBackends/OGL/Src/GLUtil.h b/Source/Core/VideoBackends/OGL/Src/GLUtil.h index 48466fe580..6ba2187ad0 100644 --- a/Source/Core/VideoBackends/OGL/Src/GLUtil.h +++ b/Source/Core/VideoBackends/OGL/Src/GLUtil.h @@ -30,10 +30,13 @@ #define GL_READ_WRITE 0x88BA #define GL_SRC1_ALPHA 0 #define GL_BGRA GL_RGBA +#define GL_MAP_COHERENT_BIT 0 +#define GL_MAP_PERSISTENT_BIT 0 #define glDrawElementsBaseVertex(...) #define glDrawRangeElementsBaseVertex(...) #define glRenderbufferStorageMultisampleCoverageNV(...) #define glViewportIndexedf(...) +#define glBufferStorage(...) #endif #else #define TEX2D GL_TEXTURE_RECTANGLE_ARB