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

*IMPORTANT FIX* Fixes possible crashes/segfaults when converting OpenGL image...

*IMPORTANT FIX* Fixes possible crashes/segfaults when converting OpenGL image representations to local image representations:
 * ImageRepresentationLocal::tryConvert() does not deadlock anymore when called from OpenGL context and converting from GL representation
 * Fixed possible double deletion after converting from OpenGL to local representation due to ambiguous pointer ownership
 * fixed possible out of bounds array write in tgt::Texture::downloadTextureToBuffer()
parent d3b27cb2
......@@ -301,7 +301,7 @@ namespace campvis {
template<typename BASETYPE, size_t NUMCHANNELS>
campvis::GenericImageRepresentationLocal<BASETYPE, NUMCHANNELS>::~GenericImageRepresentationLocal() {
delete _data;
delete [] _data;
}
template<typename BASETYPE, size_t NUMCHANNELS>
......
......@@ -294,5 +294,10 @@ namespace campvis {
_texture->unbind();
}
const WeaklyTypedPointer ImageRepresentationGL::getWeaklyTypedPointerCopy() const {
void* ptr = _texture->downloadTextureToBuffer(_texture->getFormat(), _texture->getDataType());
return WeaklyTypedPointer(WeaklyTypedPointer::baseType(_texture->getDataType()), _texture->getNumChannels(), ptr);
}
}
\ No newline at end of file
......@@ -135,11 +135,21 @@ namespace campvis {
/**
* Returns a WeaklyTypedPointer to the data of this representation.
* You do \b not own the pointer - do \b not modify its content!
* \see ImageRepresentationGL::getWeaklyTypedPointerCopy()
* \note Make sure to call this method from a valid OpenGL context.
* \return A WeaklyTypedPointer to the data of this representation. Neither you own nor you may modify its data!
*/
const WeaklyTypedPointer getWeaklyTypedPointer() const;
/**
* Returns a WeaklyTypedPointer to the data of this representation.
* Caller will own the pointer - take care to eventually delete it!
* \see ImageRepresentationGL::getWeaklyTypedPointer()
* \note Make sure to call this method from a valid OpenGL context.
* \return A WeaklyTypedPointer to the data of this representation. Caller takes ownership!
*/
const WeaklyTypedPointer getWeaklyTypedPointerCopy() const;
protected:
/**
......
......@@ -64,19 +64,31 @@ namespace campvis {
return create(tester->getParent(), tester->getImageData());
}
else if (const ImageRepresentationGL* tester = dynamic_cast<const ImageRepresentationGL*>(source)) {
// FIXME: this here deadlocks, if called from OpenGL context (GLJobProc)!!!
tgt::GLCanvas* context = GLJobProc.iKnowWhatImDoingGetArbitraryContext();
ImageRepresentationLocal* toReturn = 0;
GLJobProc.pause();
try {
tgt::GLContextScopedLock lock(context);
WeaklyTypedPointer wtp = tester->getWeaklyTypedPointer();
toReturn = create(source->getParent(), wtp);
if (GLJobProc.isCurrentThreadOpenGlThread()) {
try {
WeaklyTypedPointer wtp = tester->getWeaklyTypedPointerCopy();
toReturn = create(source->getParent(), wtp);
}
catch (...) {
LERROR("An unknown error occured during conversion...");
}
}
catch (...) {
LERROR("An unknown error occured during conversion...");
else {
tgt::GLCanvas* context = GLJobProc.iKnowWhatImDoingGetArbitraryContext();
GLJobProc.pause();
try {
tgt::GLContextScopedLock lock(context);
WeaklyTypedPointer wtp = tester->getWeaklyTypedPointerCopy();
toReturn = create(source->getParent(), wtp);
}
catch (...) {
LERROR("An unknown error occured during conversion...");
}
GLJobProc.resume();
}
GLJobProc.resume();
return toReturn;
}
......
......@@ -32,6 +32,8 @@
namespace campvis {
std::thread::id OpenGLJobProcessor::_this_thread_id;
OpenGLJobProcessor::OpenGLJobProcessor()
{
_pause = 0;
......@@ -57,6 +59,8 @@ namespace campvis {
}
void OpenGLJobProcessor::run() {
_this_thread_id = std::this_thread::get_id();
std::unique_lock<tbb::mutex> lock(tgt::GlContextManager::getRef().getGlMutex());
clock_t lastCleanupTime = clock() * 1000 / CLOCKS_PER_SEC;
......@@ -230,5 +234,9 @@ namespace campvis {
}
}
bool OpenGLJobProcessor::isCurrentThreadOpenGlThread() const {
return std::this_thread::get_id() == _this_thread_id;
}
}
......@@ -130,6 +130,11 @@ namespace campvis {
*/
tgt::GLCanvas* iKnowWhatImDoingGetArbitraryContext();
/**
* Checks whether calling thread is OpenGL thread.
* \return std::this_thread::get_id() == _this_thread_id
*/
bool isCurrentThreadOpenGlThread() const;
protected:
/**
......@@ -179,6 +184,9 @@ namespace campvis {
std::condition_variable _evaluationCondition; ///< conditional wait to be used when there are currently no jobs to process
tbb::atomic<tgt::GLCanvas*> _currentContext; ///< current active OpenGL context
private:
static std::thread::id _this_thread_id;
};
}
......
......@@ -33,6 +33,10 @@ namespace tgt {
std::cerr << "glewInit failed, error: " << glewGetErrorString(err) << std::endl;
exit(EXIT_FAILURE);
}
glPixelStorei(GL_UNPACK_ALIGNMENT, 1);
glPixelStorei(GL_PACK_ALIGNMENT, 1);
releaseCurrentContext();
return toReturn;
......
......@@ -611,6 +611,7 @@ void Texture::downloadTextureToBuffer(GLubyte* pixels, size_t numBytesAllocated)
LWARNINGC("tgt.texture", "downloadTextureToBuffer: allocated buffer is too small");
}
else {
glPixelStorei(GL_PACK_ALIGNMENT, 1);
glGetTexImage(type_, 0, format_, dataType_, pixels);
}
}
......@@ -621,6 +622,7 @@ GLubyte* Texture::downloadTextureToBuffer(GLint format, GLenum dataType) const {
int arraySize = hmul(dimensions_) * calcBpp(format, dataType);
GLubyte* pixels = new GLubyte[arraySize];
glPixelStorei(GL_PACK_ALIGNMENT, 1);
glGetTexImage(type_, 0, format, dataType, pixels);
return pixels;
}
......
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