Skip to content

GitLab

Commit c56f094b authored by Jakob Weiss's avatar Jakob Weiss
Browse files

minor fixes and clarifications regarding ScopedTypedData

parent ce377bcc
Hide whitespace changes
Inline Side-by-side
Showing with 93 additions and 62 deletions
core/datastructures/datacontainer.cpp
View file @ c56f094b
......@@ -44,10 +44,10 @@ namespace campvis {
DataHandle DataContainer::addData(const std::string& name, AbstractData* data) {
if (name.empty()) {
LERROR("Tried to add data with empty name to DataContainer.");
return DataHandle(0);
return DataHandle(nullptr);
}
cgtAssert(data != 0, "The Data must not be 0.");
cgtAssert(data != nullptr, "The Data must not be 0.");
cgtAssert(!name.empty(), "The data's name must not be empty.");
DataHandle dh(data);
......@@ -61,7 +61,7 @@ namespace campvis {
return;
}
cgtAssert(dh.getData() != 0, "The data in the DataHandle must not be 0!");
cgtAssert(dh, "The data in the DataHandle must not be 0!");
cgtAssert(!name.empty(), "The data's name must not be empty.");
_handles.erase(name);
_handles.insert(std::make_pair(name, dh));
......@@ -81,7 +81,7 @@ namespace campvis {
return a->second;
}
else {
return DataHandle(0);
return DataHandle(nullptr);
}
}
......
core/datastructures/datahandle.cpp
View file @ c56f094b
......@@ -28,7 +28,7 @@
#include "core/datastructures/abstractdata.h"
namespace campvis {
DataHandle::DataHandle(AbstractData* data)
DataHandle::DataHandle(AbstractData* data)
: _timestamp(clock())
{
if (data) {
......@@ -42,7 +42,7 @@ namespace campvis {
}
}
DataHandle::DataHandle(const DataHandle& rhs)
DataHandle::DataHandle(const DataHandle& rhs)
: _ptr(rhs._ptr)
, _timestamp(rhs._timestamp)
{
......
core/datastructures/datahandle.h
View file @ c56f094b
......@@ -36,18 +36,17 @@ namespace campvis {
/**
* A DataHandle is responsible to manage the lifetime of an AbstractData instance.
* Therefore, it implements a reference counting technique in cooperation with AbstractData.
*
* This class can be considered as thread safe under the following conditions:
* * A single DataHandle instance must not be accessed from different threads.
* * Concurrent access to the same AbstractData instance via different DataHandles is safe.
*
* \note For clarity: An AbstractData instance can be referenced by multiple DataHandles. As soon
* Therefore, it implements a reference counting technique and automatically deletes instances when
* no other references exist anymore.
*
* This class can be considered as thread safe, since through the getData() and getWritableData() thread-safe
* access is ensured.
*
* \note For clarity: An AbstractData instance can be referenced by multiple DataHandles. As soon
* as it is afterwards reference by 0 DataHandles, the AbstractData instance will be destroyed.
* Also remember that a DataHandle takes ownership of the given AbstractData instance. So do
* not delete it once it has been assigned to a DataHandle (respectively DataContainer) or mess
* with its reference counting!
* \note Reference counting implementation inspired from Scott Meyers: More Effective C++, Item 29
*/
class CAMPVIS_CORE_API DataHandle {
template<class T, bool isWritable> friend class ScopedTypedData;
......@@ -83,7 +82,7 @@ namespace campvis {
/**
* Grants const access to the managed AbstractData instance.
* \return _data;
* \return const pointer proxy class managing access
*/
template <class T = AbstractData>
ScopedTypedData<T, false> getData() const {
......@@ -91,8 +90,8 @@ namespace campvis {
}
/**
* Grants const access to the managed AbstractData instance.
* \return _data;
* Grants writable access to the managed AbstractData instance.
* \return non-const pointer proxy class managing the access
*/
template <class T = AbstractData>
ScopedTypedData<T, true> getWritableData() {
......@@ -113,11 +112,11 @@ namespace campvis {
}
/**
* Explicit cast to bool, returns true if pointer is valid. Allows to check DataHandles
* Explicit cast to bool, returns true if pointer is valid. Allows to check DataHandles
* conveniently using an if(dh) { [DataHandle is valid] } statement.
* \return !empty()
*/
operator bool() const {
explicit operator bool() const {
return !empty();
}
......
core/datastructures/scopedtypeddata.h
View file @ c56f094b
......@@ -31,19 +31,20 @@
#include "core/datastructures/datacontainer.h"
#include <typeinfo>
#include <type_traits>
namespace campvis {
/**
* Proxy class for scoped strongly-typed access to the data of a DataContainer.
* From the outside ScopedTypedData<T> behaves exactly like a const T*, but internally it preserves the
* reference counting of a DataHandle. Use this class when you want temporary access to a strongly-typed
* data item in a DataContainer but don't want to to the dynamic_cast yourself.
* reference counting of a DataHandle. This also manages the read/write access for every data handle by acquiring
* the lock type specified in the template parameter (default is read-only).
*
* \tparam T Base class of the DataHandle data to test for
*/
template<typename T, bool isWritable = false>
class ScopedTypedData : public AbstractData::LockType {
class ScopedTypedData : private AbstractData::LockType {
public:
/**
* Creates a new DataHandle to the data item with the key \a name in \a dc, that behaves like a T*.
......@@ -57,10 +58,10 @@ namespace campvis {
, data(nullptr)
{
if (dh) {
data = dynamic_cast<const T*>(dh._ptr.get());
data = dynamic_cast<T*>(dh._ptr.get());
if (data == nullptr) {
if (!silent)
LDEBUGC("CAMPVis.core.ScopedTypedData (Readonly)", "Found DataHandle with id '" << name << "', but it is of wrong type (" << typeid(dh._ptr.get()).name() << " instead of " << typeid(T).name() << ").");
LDEBUGC("CAMPVis.core.ScopedTypedData", "[" << (isWritable ? "Writable" : "Readonly") << "] Found DataHandle with id '" << name << "', but it is of wrong type(" << typeid(dh._ptr.get()).name() << " instead of " << typeid(T).name() << ").");
dh = DataHandle(0);
}
......@@ -71,7 +72,7 @@ namespace campvis {
}
else {
if (!silent)
LDEBUGC("CAMPVis.core.ScopedTypedData (Readonly)", "Could not find a DataHandle with id '" << name << "' in DataContainer '" << dc.getName() << "'.");
LDEBUGC("CAMPVis.core.ScopedTypedData", "[" << (isWritable ? "Writable" : "Readonly") << "] Could not find a DataHandle with id '" << name << "' in DataContainer '" << dc.getName() << "'.");
}
};
......@@ -80,10 +81,10 @@ namespace campvis {
, dh(handle)
, data(nullptr)
{
data = dynamic_cast<const T*>(dh._ptr.get());
data = dynamic_cast<T*>(dh._ptr.get());
if (data == nullptr) {
if (!silent)
LDEBUGC("CAMPVis.core.ScopedTypedData (Readonly)", "DataHandle is of wrong type (" << typeid(dh._ptr.get()).name() << " instead of " << typeid(T).name() << ").");
LDEBUGC("CAMPVis.core.ScopedTypedData", "[" << (isWritable ? "Writable" : "Readonly") << "] DataHandle is of wrong type (" << typeid(dh._ptr.get()).name() << " instead of " << typeid(T).name() << ").");
dh = DataHandle(nullptr);
}
......@@ -94,25 +95,26 @@ namespace campvis {
/**
* Move-Constructor that moves the lock over
* \note: Since tbb locks do not support move-construction, this implementation is not very efficient
* \note: Since tbb locks do not support move-construction, this implementation *will* release the lock during the handoff
*/
ScopedTypedData(ScopedTypedData && other)
: AbstractData::LockType()
, dh(other.dh)
, data(other.data)
{
other.release(); // force a release - otherwise we might deadlock
if (data != nullptr) { // we have a valid handle, so we need to lock it
acquire(dh._ptr->_mutex, isWritable);
}
other.dh = DataHandle(nullptr);
other.data = nullptr;
other.release();
}
~ScopedTypedData() {
if(dh)
release(); // we release manually to make sure to release before the datahandle is destroyed - otherwise the mutex is destroyed before it is released
if (dh)
release(); // we release manually to make sure to release before the datahandle is destroyed - otherwise the mutex might be destroyed before it is released
}
/**
......@@ -131,7 +133,27 @@ namespace campvis {
return data;
}
/**
// Note: The following enable_if construct yields an internal compiler error for VC 2013, which is why we explicitly specialize
// the whole class template below
///**
//* Implicit conversion operator to T*. Only applies for writable ScopedTypedData instances.
//* \return The data in the DataHandle, may be 0 when no DataHandle was found, or the data is of the wrong type.
//*/
//template< typename U = T, typename = typename std::enable_if< isWritable >::type >
//operator T*() {
// return data;
//}
///**
//* Implicit arrow operator to T*. Only applies for writable ScopedTypedData instances.
//* \return The data in the DataHandle, may be 0 when no DataHandle was found, or the data is of the wrong type.
//*/
//template< typename U = T, typename = typename std::enable_if< isWritable >::type >
//T* operator->() {
// return data;
//}
/**
* Gets the DataHandle.
* \return dh
*/
......@@ -139,19 +161,23 @@ namespace campvis {
return dh;
}
private:
protected:
/// Not copy-constructable
ScopedTypedData(const ScopedTypedData& rhs);
/// Not assignable
ScopedTypedData& operator=(const ScopedTypedData& rhs);
DataHandle dh; ///< DataHandle
const T* data; ///< strongly-typed pointer to data, may be 0
DataHandle dh; ///< a DataHandle to ensure data's lifetime until this accessor is destroyed
T* data; ///< strongly-typed pointer to data, may be 0
};
/**
* Full template specialization for isWritable = true. Could be avoided with the std::enable_if SFINAE magic but
* unfortunately this causes compiler crashes for VC2013
*/
template<typename T>
class ScopedTypedData<T, true> : public AbstractData::LockType {
class ScopedTypedData<T, true> : private AbstractData::LockType {
const bool isWritable = true;
public:
/**
* Creates a new DataHandle to the data item with the key \a name in \a dc, that behaves like a T*.
......@@ -168,18 +194,18 @@ namespace campvis {
data = dynamic_cast<T*>(dh._ptr.get());
if (data == nullptr) {
if (!silent)
LDEBUGC("CAMPVis.core.ScopedTypedData (Writable)", "Found DataHandle with id '" << name << "', but it is of wrong type (" << typeid(dh._ptr.get()).name() << " instead of " << typeid(T).name() << ").");
LDEBUGC("CAMPVis.core.ScopedTypedData", "[" << (isWritable ? "Writable" : "Readonly") << "] Found DataHandle with id '" << name << "', but it is of wrong type(" << typeid(dh._ptr.get()).name() << " instead of " << typeid(T).name() << ").");
dh = DataHandle(nullptr);
dh = DataHandle(0);
}
else { // we have a valid handle, so we need to lock it
// lock(data->_mutex);
acquire(dh._ptr->_mutex, true);
// lock_shared(dh->_mutex);
acquire(dh._ptr->_mutex, isWritable);
}
}
else {
if (!silent)
LDEBUGC("CAMPVis.core.ScopedTypedData (Writable)", "Could not find a DataHandle with id '" << name << "' in DataContainer '" << dc.getName() << "'.");
LDEBUGC("CAMPVis.core.ScopedTypedData", "[" << (isWritable ? "Writable" : "Readonly") << "] Could not find a DataHandle with id '" << name << "' in DataContainer '" << dc.getName() << "'.");
}
};
......@@ -191,40 +217,41 @@ namespace campvis {
data = dynamic_cast<T*>(dh._ptr.get());
if (data == nullptr) {
if (!silent)
LDEBUGC("CAMPVis.core.ScopedTypedData (Writable)", "DataHandle is of wrong type (" << typeid(dh._ptr.get()).name() << " instead of " << typeid(T).name() << ").");
LDEBUGC("CAMPVis.core.ScopedTypedData", "[" << (isWritable ? "Writable" : "Readonly") << "] DataHandle is of wrong type (" << typeid(dh._ptr.get()).name() << " instead of " << typeid(T).name() << ").");
dh = DataHandle(nullptr);
}
else { // we have a valid handle, so we need to lock it
// lock(data->_mutex);
acquire(dh._ptr->_mutex, true);
acquire(dh._ptr->_mutex, isWritable);
}
};
/**
*/
* Move-Constructor that moves the lock over
* \note: Since tbb locks do not support move-construction, this implementation is not very efficient
*/
ScopedTypedData(ScopedTypedData && other)
: AbstractData::LockType()
, dh(other.dh)
, data(other.data)
{
other.release(); // force a release - otherwise we might deadlock
if (data != nullptr) { // we have a valid handle, so we need to lock it
other.release(); // we need to release the other lock first because we cannot move it directly
acquire(dh._ptr->_mutex, true);
acquire(dh._ptr->_mutex, isWritable);
}
other.dh = DataHandle(nullptr);
other.data = nullptr;
}
~ScopedTypedData() {
if(dh)
release(); // we release manually to make sure to release before the datahandle is destroyed - otherwise the mutex is destroyed before it is released
if (dh)
release(); // we release manually to make sure to release before the datahandle is destroyed - otherwise the mutex might be destroyed before it is released
}
/**
* Implicit conversion operator to const T*.
* Implicit conversion operator to T*. Only applies for writable ScopedTypedData instances.
* \return The data in the DataHandle, may be 0 when no DataHandle was found, or the data is of the wrong type.
*/
operator T*() {
......@@ -232,31 +259,31 @@ namespace campvis {
}
/**
* Implicit arrow operator to T*.
* Implicit arrow operator to T*. Only applies for writable ScopedTypedData instances.
* \return The data in the DataHandle, may be 0 when no DataHandle was found, or the data is of the wrong type.
*/
T* operator->() {
return data;
}
/**
* Gets the DataHandle.
* \return dh
*/
DataHandle getDataHandle() {
DataHandle getDataHandle() const {
return dh;
}
private:
protected:
/// Not copy-constructable
ScopedTypedData(const ScopedTypedData& rhs);
/// Not assignable
ScopedTypedData& operator=(const ScopedTypedData& rhs);
DataHandle dh; ///< DataHandle
T* data; ///< strongly-typed pointer to data, may be 0
DataHandle dh; ///< a DataHandle to ensure data's lifetime until this accessor is destroyed
T* data; ///< strongly-typed pointer to data, may be 0
};
}
#endif // SCOPEDTYPEDDATA_H__
\ No newline at end of file
core/pipeline/visualizationprocessor.cpp
View file @ c56f094b
......@@ -72,23 +72,26 @@ namespace campvis {
void VisualizationProcessor::createAndAttachTexture(GLint internalFormat, GLenum attachment, ScopedTypedData<RenderData, true>* renderData /*= nullptr*/) {
cgtAssert(_fbo->isActive(), "Trying to attach a texture while FBO is not bound!");
// acqiure a new TextureUnit, so that we don't mess with other currently bound textures during texture upload...
// acquire a new TextureUnit, so that we don't mess with other currently bound textures during texture upload...
cgt::TextureUnit rtUnit;
rtUnit.activate();
// Set OpenGL pixel alignment to 1 to avoid problems with NPOT textures
glPixelStorei(GL_UNPACK_ALIGNMENT, 1);
// Check the render data if we can reuse a texture
cgt::Texture* tex = nullptr;
if (renderData) {
if (renderData) {`
// check if we can get a GL representation of the desired texture
ImageRepresentationGL* glRep;
if (attachment == GL_DEPTH_ATTACHMENT)
if(auto id = (*renderData)->getDepthTexture())
glRep = const_cast<ImageRepresentationGL*>(id->getRepresentation<ImageRepresentationGL>(false));
else
if(auto id =(*renderData)->getColorTexture(_fbo->getNumColorAttachments()))
if(auto id =(*renderData)->getColorTexture(_fbo->getNumColorAttachments())) //_fbo->getNumColorAttachments() is the index of the texture that is to be created
glRep = const_cast<ImageRepresentationGL*>(id->getRepresentation<ImageRepresentationGL>(false));
// check if the texture is compatible with what we want, and in that case detach from the old RenderTargetData
if (glRep) {
tex = const_cast<cgt::Texture*>(glRep->getTexture());
if (tex->getType() != GL_TEXTURE_2D || tex->getDimensions() != getRenderTargetSize()
......@@ -96,13 +99,15 @@ namespace campvis {
tex = nullptr;
else {
// somehow detach the cgt::Texture from the ImageRepresentationGL
// yes this is horrible code
// yes this is horrible - however, we know that renderData has write access, ergo
// we can safely do this const_cast
const_cast<ImageData*>(glRep->getParent())->removeRepresentationUnsafe<ImageRepresentationGL>();
}
}
}
// if we could not get the texture from a previous render target, create a new one
if (!tex) {
// create texture
tex = new cgt::Texture(GL_TEXTURE_2D, getRenderTargetSize(), internalFormat, cgt::Texture::LINEAR);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment