Changes after colleagues comments.

This commit is contained in:
Vladimir Byko-Ianko 2015-04-01 08:50:05 +03:00 committed by Alex Zolotarev
parent cbbbeea23b
commit 98b7c1ed8c
8 changed files with 86 additions and 60 deletions

View file

@ -59,14 +59,11 @@ namespace graphics
return;
}
/// When the binders leave the scope the buffer objects will be unbound.
shared_ptr<gl::BufferObject::Binder> 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();

View file

@ -54,13 +54,10 @@ namespace graphics
if (m_storage.isValid())
{
/// When the binders leave the scope the buffer object will be unbound.
shared_ptr<gl::BufferObject::Binder> 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());

View file

@ -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<Binder> 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<Binder> & 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<Binder> & 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<Binder> & 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
}
}
}

View file

@ -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<vector<unsigned char> > m_sharedBuffer;
bool m_bound;
public:
class Binder;
void makeCurrent(shared_ptr<Binder> & 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<Binder> & 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<Binder> & bindBuf);
void unlock(shared_ptr<Binder> & 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();

View file

@ -186,8 +186,8 @@ namespace graphics
prg->setVertexDecl(Vertex::getVertexDecl());
/// When the binders leave the scope the buffer object will be unbound.
shared_ptr<gl::BufferObject::Binder> 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<gl::BufferObject::Binder> 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

View file

@ -403,7 +403,7 @@ namespace graphics
}
}
void Program::makeCurrent(shared_ptr<gl::BufferObject::Binder> & bindVerticesBuf, shared_ptr<gl::BufferObject::Binder> & 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();
}

View file

@ -66,6 +66,7 @@ namespace graphics
math::Matrix<float, N, N> 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<gl::BufferObject::Binder> & bindVerticesBuf, shared_ptr<gl::BufferObject::Binder> & bindIndicesBuf);
void makeCurrent(gl::BufferObject::Binder & verticesBufBinder, gl::BufferObject::Binder & indicesBufBinder);
void riseChangedFlag();
};
}

View file

@ -70,10 +70,8 @@ namespace graphics
if (m_useSingleThreadedOGL)
{
/// When the binders leave the scope the buffer object will be unbound.
shared_ptr<gl::BufferObject::Binder> 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<gl::BufferObject::Binder> 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();
}
}