From fe561c20622bab889d2d0c017210c1e20744ca7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Lemaire?= Date: Mon, 23 Jul 2012 16:56:24 +0000 Subject: [PATCH] paysages : Fixed previews possible segfaults by not changing data while chunks are processing them. git-svn-id: https://subversion.assembla.com/svn/thunderk/paysages@397 b1fd45b6-86a6-48da-8261-f70d1f35bdcc --- TODO | 3 -- gui_qt/basepreview.cpp | 74 ++++++++++++++++++++++++++++++++---- gui_qt/basepreview.h | 8 +++- lib_paysages/rayleigh.c | 2 + lib_paysages/system.c | 2 + lib_paysages/terrain.c | 6 +-- lib_paysages/terraincanvas.c | 3 +- 7 files changed, 80 insertions(+), 18 deletions(-) diff --git a/TODO b/TODO index 7bed803..731b504 100644 --- a/TODO +++ b/TODO @@ -22,9 +22,6 @@ Technology Preview 2 : => Add curve modes => Add logarithmic mode => Add zoom and scrolling -- Fix "RGB parameters out of range" (and segfault) on some previews, caused by data changing during a chunk computation. - => May need to change the updateData system. - => Previews need to be paused while updating data. - Lock some previews together (eg: terrain height and colored preview). - Find a new licence. diff --git a/gui_qt/basepreview.cpp b/gui_qt/basepreview.cpp index 68231d3..d38e083 100644 --- a/gui_qt/basepreview.cpp +++ b/gui_qt/basepreview.cpp @@ -93,6 +93,10 @@ public: { _preview->commitChunkTransaction(&pixbuf, _xstart, _ystart, _xsize, _ysize, revision); } + else + { + _preview->rollbackChunkTransaction(); + } } return changed; @@ -194,6 +198,23 @@ void PreviewDrawingManager::removeChunks(BasePreview* preview) logDebug(QString("[Previews] %1 chunks removed, %2 remaining").arg(removed).arg(_chunks.size())); } +void PreviewDrawingManager::suspendChunks(BasePreview* preview) +{ + _lock.lock(); + for (int i = 0; i < _updateQueue.size(); i++) + { + PreviewChunk* chunk; + chunk = _updateQueue.at(i); + if (chunk->isFrom(preview)) + { + _updateQueue.takeAt(i); + i--; + } + } + // TODO Interrupt currently processing chunks + _lock.unlock(); +} + void PreviewDrawingManager::updateChunks(BasePreview* preview) { for (int i = 0; i < _chunks.size(); i++) @@ -293,6 +314,8 @@ QWidget(parent) _width = width(); _height = height(); _revision = 0; + _transactions_count = 0; + _redraw_requested = false; _info = new QLabel(this); _info->setVisible(false); @@ -301,11 +324,12 @@ QWidget(parent) this->alive = true; QObject::connect(this, SIGNAL(contentChange()), this, SLOT(update())); - QObject::connect(this, SIGNAL(redrawRequested()), this, SLOT(handleRedraw())); this->setMinimumSize(256, 256); this->setMaximumSize(256, 256); this->resize(256, 256); + + startTimer(50); } BasePreview::~BasePreview() @@ -440,18 +464,31 @@ void BasePreview::toggleChangeEvent(QString, bool) void BasePreview::redraw() { - emit(redrawRequested()); + _drawing_manager->suspendChunks(this); + _redraw_requested = true; } QImage BasePreview::startChunkTransaction(int x, int y, int w, int h, int* revision) { + QImage result; + + _lock_drawing->lock(); + *revision = _revision; - return _pixbuf->copy(x, y, w, h); + result = _pixbuf->copy(x, y, w, h); + _transactions_count++; + + _lock_drawing->unlock(); + + return result; } -void BasePreview::commitChunkTransaction(QImage* chunk, int x, int y, int w, int h, int revision) +bool BasePreview::commitChunkTransaction(QImage* chunk, int x, int y, int w, int h, int revision) { + bool result; + _lock_drawing->lock(); + if (revision == _revision) { for (int ix = 0; ix < w; ix++) @@ -462,7 +499,26 @@ void BasePreview::commitChunkTransaction(QImage* chunk, int x, int y, int w, int } } emit contentChange(); + result = true; } + else + { + result = false; + } + + _transactions_count--; + + _lock_drawing->unlock(); + + return result; +} + +void BasePreview::rollbackChunkTransaction() +{ + _lock_drawing->lock(); + + _transactions_count--; + _lock_drawing->unlock(); } @@ -471,12 +527,16 @@ QColor BasePreview::getPixelColor(int x, int y) return getColor((double) (x - _width / 2) * scaling + xoffset, (double) (y - _height / 2) * scaling + yoffset); } -void BasePreview::handleRedraw() +void BasePreview::timerEvent(QTimerEvent*) { _lock_drawing->lock(); - updateData(); - invalidatePixbuf(128); + if (_redraw_requested && _transactions_count == 0) + { + updateData(); + invalidatePixbuf(128); + _redraw_requested = false; + } _lock_drawing->unlock(); } diff --git a/gui_qt/basepreview.h b/gui_qt/basepreview.h index f29b209..110a3a5 100644 --- a/gui_qt/basepreview.h +++ b/gui_qt/basepreview.h @@ -44,7 +44,8 @@ public: void redraw(); QImage startChunkTransaction(int x, int y, int w, int h, int* revision); - void commitChunkTransaction(QImage* chunk, int x, int y, int w, int h, int revision); + bool commitChunkTransaction(QImage* chunk, int x, int y, int w, int h, int revision); + void rollbackChunkTransaction(); QColor getPixelColor(int x, int y); @@ -72,6 +73,7 @@ private: void updateChunks(); void invalidatePixbuf(int value); + void timerEvent(QTimerEvent* event); void showEvent(QShowEvent* event); void resizeEvent(QResizeEvent* event); void paintEvent(QPaintEvent* event); @@ -94,6 +96,8 @@ private: int _height; int _revision; + int _transactions_count; + bool _redraw_requested; int mousex; int mousey; @@ -120,7 +124,6 @@ signals: void redrawRequested(); private slots: - void handleRedraw(); void choiceSelected(QAction* action); }; @@ -153,6 +156,7 @@ public: void addChunk(PreviewChunk* chunk); void removeChunks(BasePreview* preview); void updateChunks(BasePreview* preview); + void suspendChunks(BasePreview* preview); void updateAllChunks(); void performOneThreadJob(); int chunkCount(); diff --git a/lib_paysages/rayleigh.c b/lib_paysages/rayleigh.c index 5932510..c933bb3 100644 --- a/lib_paysages/rayleigh.c +++ b/lib_paysages/rayleigh.c @@ -2,6 +2,7 @@ #include +#if 0 static Vector3 _betaR = {5.5e-6, 13.0e-6, 22.4e-6}; /* Rayleigh scattering coefficients at sea level */ static Vector3 _betaM = {21e-6, 0.0, 0.0}; /* Mie scattering coefficients at sea level */ static double _Hr = 7994; /* Rayleigh scale height */ @@ -10,6 +11,7 @@ static double _radiusEarth = 6360e3; /* Earth radius */ static double _radiusAtmosphere = 6420e3; /* Atmosphere radius */ static double _sunIntensity = 20.0; /* Sun intensity */ static double _g = 0.76; /* Mean cosine */ +#endif /*typedef struct { diff --git a/lib_paysages/system.c b/lib_paysages/system.c index a0202d5..04f88af 100644 --- a/lib_paysages/system.c +++ b/lib_paysages/system.c @@ -1,5 +1,7 @@ #include "system.h" + #include +#include #include "IL/il.h" #include "IL/ilu.h" diff --git a/lib_paysages/terrain.c b/lib_paysages/terrain.c index 58a1ca9..592bb3f 100644 --- a/lib_paysages/terrain.c +++ b/lib_paysages/terrain.c @@ -51,16 +51,12 @@ TerrainDefinition terrainCreateDefinition() void terrainDeleteDefinition(TerrainDefinition* definition) { - int i; - noiseDeleteGenerator(definition->height_noise); layersDelete(definition->canvases); } void terrainCopyDefinition(TerrainDefinition* source, TerrainDefinition* destination) { - int i; - noiseCopy(source->height_noise, destination->height_noise); destination->height_factor = source->height_factor; destination->scaling = source->scaling; @@ -335,7 +331,7 @@ double terrainGetHeightNormalized(TerrainDefinition* definition, double x, doubl } else { - return _getHeight(definition, x, z) - definition->_min_height / (definition->_max_height - definition->_min_height); + return (_getHeight(definition, x, z) - definition->_min_height) / (definition->_max_height - definition->_min_height); } } diff --git a/lib_paysages/terraincanvas.c b/lib_paysages/terraincanvas.c index cab84db..8de4260 100644 --- a/lib_paysages/terraincanvas.c +++ b/lib_paysages/terraincanvas.c @@ -2,6 +2,7 @@ #include #include +#include TerrainCanvas* terrainCanvasCreate() { @@ -108,7 +109,7 @@ void terrainCanvasLoad(PackStream* stream, TerrainCanvas* canvas) double terrainCanvasGetLimits(TerrainCanvas* canvas, double* ymin, double* ymax) { - return heightmapGetLimits(canvas->height_map, ymin, ymax); + return heightmapGetLimits(&canvas->height_map, ymin, ymax); } void terrainCanvasRevertToTerrain(TerrainCanvas* canvas, TerrainDefinition* terrain, int only_masked)