Commit de26ca34 authored by Christian Schulte zu Berge's avatar Christian Schulte zu Berge
Browse files

Merge branch 'refactor-various' into 'development'

Refactor various
parents 35f624c6 c3b1de22
......@@ -33,7 +33,6 @@
#include "tgt/texturereadertga.h"
#include "tgt/qt/qtapplication.h"
#include "tgt/qt/qtthreadedcanvas.h"
#include "tbb/compat/thread"
#include "application/campvispainter.h"
#include "application/gui/mainwindow.h"
......@@ -44,6 +43,7 @@
#include "core/tools/quadrenderer.h"
#include "core/pipeline/abstractpipeline.h"
#include "core/pipeline/visualizationprocessor.h"
#include "modules/pipelinefactory.h"
#ifdef CAMPVIS_HAS_SCRIPTING
......
......@@ -125,7 +125,6 @@ namespace campvis {
void CampVisPainter::init() {
try {
// TODO: Remove hardcoded paths, and use ShdrMgr.addPath() at some central location
_copyShader = ShdrMgr.load("core/glsl/passthrough.vert", "core/glsl/copyimage.frag", "");
_copyShader->setAttributeLocation(0, "in_Position");
_copyShader->setAttributeLocation(1, "in_TexCoords");
......
......@@ -26,8 +26,9 @@
#define CAMPVISPAINTER_H__
#include "sigslot/sigslot.h"
#include "tbb/atomic.h"
#include "tbb/compat/condition_variable"
#include <ext/threading.h>
#include <tbb/atomic.h>
#include "tgt/logmanager.h"
#include "tgt/painter.h"
......
......@@ -32,9 +32,9 @@ namespace campvis {
{
setSliderProperties(_spinBox->singleStep(), _spinBox->minimum(), _spinBox->maximum());
connect(this, SIGNAL(s_minChanged(double)), this, SLOT(onMinChanged(double)), Qt::QueuedConnection);
connect(this, SIGNAL(s_maxChanged(double)), this, SLOT(onMaxChanged(double)), Qt::QueuedConnection);
connect(this, SIGNAL(s_singleStepChanged(double)), this, SLOT(onSingleStepChanged(double)), Qt::QueuedConnection);
connect(this, SIGNAL(s_minChanged(double)), this, SLOT(onMinChanged(double)));
connect(this, SIGNAL(s_maxChanged(double)), this, SLOT(onMaxChanged(double)));
connect(this, SIGNAL(s_singleStepChanged(double)), this, SLOT(onSingleStepChanged(double)));
connect(_spinBox, SIGNAL(valueChanged(double)), this, SLOT(onSpinBoxValueChanged(double)));
connect(_slider, SIGNAL(valueChanged(int)), this, SLOT(onSliderValueChanged(int)));
......
......@@ -32,9 +32,9 @@ namespace campvis {
{
setSliderProperties(_spinBox->singleStep(), _spinBox->minimum(), _spinBox->maximum());
connect(this, SIGNAL(s_minChanged(int)), this, SLOT(onMinChanged(int)), Qt::QueuedConnection);
connect(this, SIGNAL(s_maxChanged(int)), this, SLOT(onMaxChanged(int)), Qt::QueuedConnection);
connect(this, SIGNAL(s_singleStepChanged(int)), this, SLOT(onSingleStepChanged(int)), Qt::QueuedConnection);
connect(this, SIGNAL(s_minChanged(int)), this, SLOT(onMinChanged(int)));
connect(this, SIGNAL(s_maxChanged(int)), this, SLOT(onMaxChanged(int)));
connect(this, SIGNAL(s_singleStepChanged(int)), this, SLOT(onSingleStepChanged(int)));
connect(_spinBox, SIGNAL(valueChanged(int)), this, SLOT(onSpinBoxValueChanged(int)));
connect(_slider, SIGNAL(valueChanged(int)), this, SLOT(onSliderValueChanged(int)));
......
......@@ -135,6 +135,8 @@ namespace campvis {
_scriptingConsoleWidget = new ScriptingWidget(this);
ui.scriptingConsoleDock->setWidget(_scriptingConsoleWidget);
connect(_scriptingConsoleWidget, SIGNAL(s_commandExecuted(const QString&)), this, SLOT(onLuaCommandExecuted(const QString&)));
#else
ui.scriptingConsoleDock->setVisible(false);
#endif
_dcInspectorWidget = new DataContainerInspectorWidget();
......
......@@ -240,7 +240,7 @@ namespace campvis {
_lblIntensityRight = new QLabel(QString::number(gtf->getIntensityDomain().y), this);
_layout->addWidget(_lblIntensityRight, 4, 3, 1, 1, Qt::AlignRight);
QVBoxLayout* buttonLayout = new QVBoxLayout(); // TODO: check whether buttonLayout will be deleted by Qt's GC!
QVBoxLayout* buttonLayout = new QVBoxLayout();
_layout->addLayout(buttonLayout, 1, 4, 1, 3, Qt::AlignTop);
_btnAddGeometry = new QPushButton(tr("Add Geometry"), this);
......
......@@ -149,15 +149,13 @@ IF(WIN32)
ELSEIF(UNIX)
LIST(APPEND CampvisGlobalDefinitions "-DUNIX")
LIST(APPEND CampvisGlobalDefinitions "-Wall -Wno-unused-local-typedefs -Wno-unused-variable")
LIST(APPEND CampvisGlobalDefinitions "-Wall -Wno-unused-local-typedefs -Wno-unused-variable -Wno-unknown-pragmas")
LIST(APPEND CampvisGlobalDefinitions "-D__STDC_CONSTANT_MACROS")
ENDIF(WIN32)
IF(CMAKE_COMPILER_IS_GNUCXX)
# enable C++11 support in GCC
LIST(APPEND CMAKE_CXX_FLAGS "-std=c++11")
# however: we want to use the TBB implementation of threads
LIST(APPEND CampvisGlobalDefinitions "-DTBB_IMPLEMENT_CPP0X")
ENDIF()
# tgt configuration
......
......@@ -57,8 +57,6 @@ namespace campvis {
/**
* Abstract base class for data handled by a DataHandle and stored in a DataContainer.
*
* \todo
*/
class CAMPVIS_CORE_API AbstractData {
friend class DataHandle;
......
......@@ -49,8 +49,6 @@ namespace campvis {
* also ensures (hopefully) that nobody can do messy things, such as changing the data while some other
* thread is reading it. Theoretically this should be possible, but a correct implementation would require
* some brain fuck.
*
* \todo Check thread-safety
*/
class CAMPVIS_CORE_API DataContainer {
public:
......
......@@ -28,11 +28,13 @@
#include "core/datastructures/abstractdata.h"
#include <string>
#include <memory>
namespace campvis {
/**
* Class that generically wraps around a pointer of the template type and takes ownership of it.
* Ownership is handled through shared pointers, as the clone() method only returns a shallow copy.
* \tparam T Type of the pointer this AbstractData wraps around.
*/
template<typename T>
......@@ -40,7 +42,7 @@ namespace campvis {
public:
/**
* Creates a new GenericPointerData and initializes its pointer with \a data.
* \param data The initial pointer for this data, may be 0, GenericPointerData takes ownerwhip.
* \param data The initial pointer for this data, may be 0, GenericPointerData takes ownership.
*/
explicit GenericPointerData(T* data)
: AbstractData()
......@@ -48,10 +50,19 @@ namespace campvis {
{};
/**
* Destructor, deletes the pointer.
* Creates a new GenericPointerData and initializes its pointer with \a data.
* \param data The initial pointer for this data, may be 0, GenericPointerData takes ownership.
*/
explicit GenericPointerData(std::shared_ptr<T> data)
: AbstractData()
, _data(data)
{};
/**
* Destructor
*/
virtual ~GenericPointerData() {
delete _data;
};
/**
......@@ -72,10 +83,10 @@ namespace campvis {
/**
* Sets the data to \a data.
* \param data The new pointer for this data, may be 0, GenericPointerData takes ownerwhip.
* \param data The new pointer for this data, may be 0, GenericPointerData takes ownership.
*/
void setData(T* data) {
_data = data;
_data = std::shared_ptr<T>(data);
};
......@@ -83,8 +94,7 @@ namespace campvis {
* Prototype - clone method, some people call this virtual constructor...
* \return A SHALLOW copy of this object.
*/
virtual AbstractData* clone() const {
// FIXME: This is only a shallow copy - not what you expect from clone!
virtual GenericPointerData<T>* clone() const {
return new GenericPointerData<T>(_data);
};
......@@ -105,7 +115,7 @@ namespace campvis {
};
protected:
T* _data; ///< Pointer to the data.
std::shared_ptr<T> _data; ///< Shared pointer to the data.
};
}
......
......@@ -33,8 +33,6 @@ namespace campvis {
/**
* Subclass of ImageData offering access to image data stored in binary form on the local harddisk.
*
* \todo Number of channels
*/
class CAMPVIS_CORE_API ImageRepresentationDisk : public GenericAbstractImageRepresentation<ImageRepresentationDisk> {
public:
......
......@@ -68,7 +68,6 @@ namespace campvis {
}
{
// TODO: there is probably a more elegant method...
tbb::spin_mutex::scoped_lock(mutex);
_normalizedIntensityRange.nibble(localMin);
_normalizedIntensityRange.nibble(localMax);
......
......@@ -37,8 +37,6 @@ namespace campvis {
/**
* Abstract base class for storing image data in the local memory.
*
* \todo implement padding, add some kind of cool iterators
*/
class CAMPVIS_CORE_API ImageRepresentationLocal : public GenericAbstractImageRepresentation<ImageRepresentationLocal> {
public:
......
......@@ -37,8 +37,6 @@ namespace campvis {
/**
* Abstract base class for data handled by a DataHandle and stored in a DataContainer.
*
* \todo
*/
class CAMPVIS_CORE_API LightSourceData : public AbstractData {
public:
......
......@@ -22,9 +22,10 @@
//
// ================================================================================================
#include <tbb/compat/thread>
#include "tgt/assert.h"
#include <ext/threading.h>
#include "abstractprocessor.h"
#include "core/properties/abstractproperty.h"
#include "core/tools/job.h"
......@@ -132,26 +133,31 @@ namespace campvis {
void AbstractProcessor::process(DataContainer& data, bool unlockInExtraThread) {
if (hasInvalidShader()) {
updateShader();
validate(INVALID_SHADER);
}
if (hasInvalidProperties()) {
updateProperties(data);
validate(INVALID_PROPERTIES);
}
// use a scoped lock for exception safety
AbstractProcessor::ScopedLock lock(this, unlockInExtraThread);
tgtAssert(_locked == true, "Processor not locked, this should not happen!");
if (hasInvalidShader())
updateShader();
if (hasInvalidProperties())
updateProperties(data);
if (hasInvalidResult())
if (hasInvalidResult()) {
updateResult(data);
validate(INVALID_RESULT);
}
}
void AbstractProcessor::updateShader() {
LDEBUG("Called non-overriden updateShader() in " << getName() << ". Did you forget to override your method?");
validate(INVALID_SHADER);
}
void AbstractProcessor::updateProperties(DataContainer& dc) {
LDEBUG("Called non-overriden updateProperties() in " << getName() << ". Did you forget to override your method?");
validate(INVALID_PROPERTIES);
}
void AbstractProcessor::addProperty(AbstractProperty& prop) {
......
......@@ -58,6 +58,25 @@ namespace campvis {
*/
class CAMPVIS_CORE_API AbstractProcessor : public HasPropertyCollection {
public:
/**
* Scoped lock of an AbstractProcessor that automatically unlocks the processor on destruction.
* Useful for exception safety.
*/
struct CAMPVIS_CORE_API ScopedLock {
/**
* Constructs a new Scoped lock, locking \a p and unlocking \a p on destruction.
* \param p Processor to lock
* \param unlockInExtraThread Unlock \a p in extra thread (since this might be an expensive operation)
*/
ScopedLock(AbstractProcessor* p, bool unlockInExtraThread);
/// Destructor, unlocks the processor
~ScopedLock();
AbstractProcessor* _p; ///< The processor to lock
bool _unlockInExtraThread; ///< Unlock _p in extra thread (since this might be an expensive operation)
};
/**
* Available invalidation levels
*/
......@@ -275,26 +294,6 @@ namespace campvis {
sigslot::signal1<AbstractProcessor*> s_validated;
protected:
/**
* Scoped lock of an AbstractProcessor that automatically unlocks the processor on destruction.
* Useful for exception safety.
*/
struct CAMPVIS_CORE_API ScopedLock {
/**
* Constructs a new Scoped lock, locking \a p and unlocking \a p on destruction.
* \param p Processor to lock
* \param unlockInExtraThread Unlock \a p in extra thread (since this might be an expensive operation)
*/
ScopedLock(AbstractProcessor* p, bool unlockInExtraThread);
/// Destructor, unlocks the processor
~ScopedLock();
AbstractProcessor* _p; ///< The processor to lock
bool _unlockInExtraThread; ///< Unlock _p in extra thread (since this might be an expensive operation)
};
/**
* Gets called from default process() method when having an invalidation level of INVALID_SHADER.
*
......
......@@ -171,8 +171,6 @@ namespace campvis {
else {
LERROR("No suitable input image found.");
}
validate(INVALID_RESULT);
}
std::string RaycastingProcessor::generateHeader() const {
......@@ -183,15 +181,11 @@ namespace campvis {
void RaycastingProcessor::updateProperties(DataContainer& dc) {
ScopedTypedData<ImageData> img(dc, p_sourceImageID.getValue());
p_transferFunction.setImageHandle(img.getDataHandle());
validate(AbstractProcessor::INVALID_PROPERTIES);
}
void RaycastingProcessor::updateShader() {
_shader->setHeaders(generateHeader());
_shader->rebuild();
validate(AbstractProcessor::INVALID_SHADER);
}
}
......@@ -39,9 +39,6 @@
namespace campvis {
/**
* Abstract base class for CAMPVis Property.
*
* \todo Add PropertyWidgets, add clone()?
* Think about a reasonable locking mechanism and implement that
*/
class CAMPVIS_CORE_API AbstractProperty {
public:
......
......@@ -35,7 +35,6 @@ namespace campvis {
* Generic class for value-based properties.
*
* \tparam T Base type of the property's value.
* \todo Add PropertyWidgets, review use of mutex
*/
template<typename T>
class GenericProperty : public AbstractProperty {
......@@ -179,11 +178,9 @@ namespace campvis {
template<typename T>
void campvis::GenericProperty<T>::setFrontValue(const T& value) {
_value = value;
// TODO: think about the correct/reasonable order of observer notification
// thread-safety might play a role thereby...
for (std::set<AbstractProperty*>::iterator it = _sharedProperties.begin(); it != _sharedProperties.end(); ++it) {
// We ensure all shared properties to be of type GenericProperty<T> in the addSharedProperty overload.
// Hence, static_cast ist safe.
// Hence, static_cast is safe.
GenericProperty<T>* child = static_cast< GenericProperty<T>* >(*it);
child->setValue(value);
}
......
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