diff --git a/graphics/blitter.cpp b/graphics/blitter.cpp index b898da31d4..3d33be28ff 100644 --- a/graphics/blitter.cpp +++ b/graphics/blitter.cpp @@ -59,14 +59,11 @@ namespace graphics return; } - /// When the binders leave the scope the buffer objects will be unbound. - shared_ptr bindIndicesBuf, bindVericesBuf; - /// TODO : Bad lock/unlock checking pattern. Should refactor if (!storage.m_vertices->isLocked()) - storage.m_vertices->lock(bindVericesBuf); + storage.m_vertices->lock(); if (!storage.m_indices->isLocked()) - storage.m_indices->lock(bindIndicesBuf); + storage.m_indices->lock(); gl::Vertex * pointsData = (gl::Vertex*)storage.m_vertices->data(); diff --git a/graphics/geometry_pipeline.cpp b/graphics/geometry_pipeline.cpp index c38b2847f4..1dacdb93ca 100644 --- a/graphics/geometry_pipeline.cpp +++ b/graphics/geometry_pipeline.cpp @@ -54,13 +54,10 @@ namespace graphics if (m_storage.isValid()) { - /// When the binders leave the scope the buffer object will be unbound. - shared_ptr bindVerticesBuf, bindIndicesBuf; - if (!m_storage.m_vertices->isLocked()) - m_storage.m_vertices->lock(bindVerticesBuf); + m_storage.m_vertices->lock(); if (!m_storage.m_indices->isLocked()) - m_storage.m_indices->lock(bindIndicesBuf); + m_storage.m_indices->lock(); m_decl->initStream(&m_vertexStream, (unsigned char*)m_storage.m_vertices->data()); diff --git a/graphics/opengl/buffer_object.cpp b/graphics/opengl/buffer_object.cpp index f4e914daab..237c855573 100644 --- a/graphics/opengl/buffer_object.cpp +++ b/graphics/opengl/buffer_object.cpp @@ -13,19 +13,17 @@ namespace graphics { namespace gl { - BufferObject::Binder::Binder(unsigned int target, unsigned int id) - : m_target(target) + void BufferObject::Binder::Reset(BufferObject * bufferObj) { - OGLCHECK(glBindBufferFn(target, id)); + ASSERT(bufferObj, ()); + m_bufferObj = bufferObj; + bufferObj->Bind(); } BufferObject::Binder::~Binder() { -#ifdef OMIM_OS_IPHONE - OGLCHECK(glBindBufferFn(m_target, 0)); -#else - UNUSED_VALUE(m_target); -#endif + if (m_bufferObj != nullptr) + m_bufferObj->Unbind(); } BufferObject::BufferObject(unsigned target) @@ -44,7 +42,8 @@ namespace graphics m_size(0), m_gpuData(0), m_isLocked(false), - m_isUsingMapBuffer(false) + m_isUsingMapBuffer(false), + m_bound(false) { if (g_isBufferObjectsSupported) OGLCHECK(glGenBuffersFn(1, &m_id)); @@ -61,9 +60,9 @@ namespace graphics m_size = size; - /// When bindBuf leaves the scope the buffer object will be unbound. - shared_ptr bindBuf; - makeCurrent(bindBuf); + /// When binder leaves the scope the buffer object will be unbound. + Binder binder; + makeCurrent(binder); if (g_isBufferObjectsSupported) { OGLCHECK(glBufferDataFn(m_target, m_size, 0, GL_DYNAMIC_DRAW)); @@ -98,7 +97,7 @@ namespace graphics return m_gpuData; } - void * BufferObject::lock(shared_ptr & bindBuf) + void * BufferObject::lock() { ASSERT(!m_isLocked, ()); m_isLocked = true; @@ -106,7 +105,7 @@ namespace graphics if (g_isMapBufferSupported) { m_isUsingMapBuffer = true; - makeCurrent(bindBuf); + Bind(); OGLCHECK(glBufferDataFn(m_target, m_size, 0, GL_DYNAMIC_DRAW)); if (graphics::gl::g_hasContext) m_gpuData = glMapBufferFn(m_target, GL_WRITE_ONLY_MWM); @@ -128,7 +127,7 @@ namespace graphics return m_gpuData; } - void BufferObject::unlock(shared_ptr & bindBuf) + void BufferObject::unlock() { ASSERT(m_isLocked, ()); m_isLocked = false; @@ -137,7 +136,7 @@ namespace graphics { ASSERT(m_gpuData != 0, ("BufferObject is not locked")); - makeCurrent(bindBuf); + Bind(); if (g_isMapBufferSupported && m_isUsingMapBuffer) { @@ -160,6 +159,7 @@ namespace graphics m_gpuData = 0; } + Unbind(); } void BufferObject::discard() @@ -183,15 +183,35 @@ namespace graphics return 0; } - void BufferObject::makeCurrent(shared_ptr & bindBuf) + void BufferObject::makeCurrent(Binder & binder) { - if (!g_isBufferObjectsSupported) - return; - /*#ifndef OMIM_OS_ANDROID if (m_id != current()) #endif*/ - bindBuf.reset(new BufferObject::Binder(m_target, m_id)); + binder.Reset(this); + } + + void BufferObject::Bind() + { + if (!g_isBufferObjectsSupported) + return; + if (m_bound) + return; + OGLCHECK(glBindBufferFn(m_target, m_id)); + m_bound = true; + } + + void BufferObject::Unbind() + { + if (!m_bound) + return; + + m_bound = false; +#ifdef OMIM_OS_IPHONE + OGLCHECK(glBindBufferFn(m_target, 0)); +#else + UNUSED_VALUE(m_target); +#endif } } } diff --git a/graphics/opengl/buffer_object.hpp b/graphics/opengl/buffer_object.hpp index cabf301218..574db63a81 100644 --- a/graphics/opengl/buffer_object.hpp +++ b/graphics/opengl/buffer_object.hpp @@ -1,5 +1,7 @@ #pragma once +#include "../../base/macros.hpp" + #include "../../std/vector.hpp" #include "../../std/shared_ptr.hpp" @@ -18,21 +20,26 @@ namespace graphics bool m_isLocked; bool m_isUsingMapBuffer; shared_ptr > m_sharedBuffer; + bool m_bound; public: class Binder; - void makeCurrent(shared_ptr & bindBuf); + /// Bind the buffer object to the current thread. + /// The buffer object will be unbound from the current thread on calling the destructor of Binder in case of iOS. + void makeCurrent(Binder & binder); + /// This class is wrapper for binding and unbinding gl buffers. class Binder { - friend void BufferObject::makeCurrent(shared_ptr & bindBuf); - unsigned int const m_target; + friend void BufferObject::makeCurrent(Binder & binder); + BufferObject * m_bufferObj; + + void Reset(BufferObject * bufferObj); - Binder(unsigned int target, unsigned int id); - Binder(const Binder &) = delete; - Binder const & operator=(const Binder &) = delete; public: + Binder() : m_bufferObj(nullptr) {} ~Binder(); + DISALLOW_COPY_AND_MOVE(Binder); }; BufferObject(unsigned target); @@ -42,10 +49,20 @@ namespace graphics void resize(size_t size); size_t size() const; + /// Multithreading notes for Bind and Unbind methods: + /// 1. If method Bind calls from a thread, method Unbind shell be called from the same thread. + /// 2. After the buffer object is unbound (Unbind calls) the instance of it can be reused from another thread. + /// Bind the buffer object to the current thread. + void Bind(); + /// Unbind the buffer object from the current thread only for iOS. + void Unbind(); - - void * lock(shared_ptr & bindBuf); - void unlock(shared_ptr & bindBuf); + /// This method locks the instance of a buffer object and return a pointer to a GPU buffer. + /// Notes. The buffer object is being bound to the caller thread in this method. + /// It shell not be unbound untill unlock is called. + void * lock(); + /// Inform gl that the instance of a buffer object is released and unbind it. + void unlock(); void discard(); void * glPtr(); diff --git a/graphics/opengl/geometry_renderer.cpp b/graphics/opengl/geometry_renderer.cpp index 8df5253f5a..ae7bf60176 100644 --- a/graphics/opengl/geometry_renderer.cpp +++ b/graphics/opengl/geometry_renderer.cpp @@ -186,8 +186,8 @@ namespace graphics prg->setVertexDecl(Vertex::getVertexDecl()); /// When the binders leave the scope the buffer object will be unbound. - shared_ptr bindVerticesBuf, bindIndicesBuf; - prg->makeCurrent(bindVerticesBuf, bindIndicesBuf); + gl::BufferObject::Binder verticesBufBinder, indicesBufBinder; + prg->makeCurrent(verticesBufBinder, indicesBufBinder); if (m_texture) m_texture->makeCurrent(); @@ -281,10 +281,8 @@ namespace graphics { if (m_storage.m_vertices && m_storage.m_indices) { - /// When the binders leave the scope the buffer object will be unbound. - shared_ptr bindVerticesBuf, bindIndicesBuf; - m_storage.m_vertices->unlock(bindVerticesBuf); - m_storage.m_indices->unlock(bindIndicesBuf); + m_storage.m_vertices->unlock(); + m_storage.m_indices->unlock(); /// In multithreaded resource usage scenarios the suggested way to see /// resource update made in one thread to the another thread is diff --git a/graphics/opengl/program.cpp b/graphics/opengl/program.cpp index 063ec5db8a..89c87a300a 100644 --- a/graphics/opengl/program.cpp +++ b/graphics/opengl/program.cpp @@ -403,7 +403,7 @@ namespace graphics } } - void Program::makeCurrent(shared_ptr & bindVerticesBuf, shared_ptr & bindIndicesBuf) + void Program::makeCurrent(gl::BufferObject::Binder & verticesBufBinder, gl::BufferObject::Binder & indicesBufBinder) { if (m_changed) { @@ -412,11 +412,11 @@ namespace graphics m_changed = false; } - m_storage.m_vertices->makeCurrent(bindVerticesBuf); + m_storage.m_vertices->makeCurrent(verticesBufBinder); applyAttributes(); - m_storage.m_indices->makeCurrent(bindIndicesBuf); + m_storage.m_indices->makeCurrent(indicesBufBinder); applyUniforms(); } diff --git a/graphics/opengl/program.hpp b/graphics/opengl/program.hpp index db04d7172e..ff1dbd9ae4 100644 --- a/graphics/opengl/program.hpp +++ b/graphics/opengl/program.hpp @@ -66,6 +66,7 @@ namespace graphics math::Matrix const & m); public: + DECLARE_EXCEPTION(Exception, RootException); DECLARE_EXCEPTION(LinkException, Exception); @@ -94,7 +95,7 @@ namespace graphics void setStorage(Storage const & storage); - void makeCurrent(shared_ptr & bindVerticesBuf, shared_ptr & bindIndicesBuf); + void makeCurrent(gl::BufferObject::Binder & verticesBufBinder, gl::BufferObject::Binder & indicesBufBinder); void riseChangedFlag(); }; } diff --git a/graphics/resource_manager.cpp b/graphics/resource_manager.cpp index aed6bafbd3..43a1ee6ce5 100644 --- a/graphics/resource_manager.cpp +++ b/graphics/resource_manager.cpp @@ -70,10 +70,8 @@ namespace graphics if (m_useSingleThreadedOGL) { - /// When the binders leave the scope the buffer object will be unbound. - shared_ptr bindVerticesBuf, bindIndicesBuf; - res.m_indices->lock(bindIndicesBuf); - res.m_vertices->lock(bindVerticesBuf); + res.m_indices->lock(); + res.m_vertices->lock(); } return res; } @@ -82,17 +80,15 @@ namespace graphics { if (m_useSingleThreadedOGL) { - /// When the binders leave the scope the buffer object will be unbound. - shared_ptr bindVerticesBuf, bindIndicesBuf; if (e.m_indices->isLocked()) - e.m_indices->unlock(bindIndicesBuf); + e.m_indices->unlock(); - e.m_indices->lock(bindIndicesBuf); + e.m_indices->lock(); if (e.m_vertices->isLocked()) - e.m_vertices->unlock(bindVerticesBuf); + e.m_vertices->unlock(); - e.m_vertices->lock(bindVerticesBuf); + e.m_vertices->lock(); } }