From b318ff2a39fec32c0d7c76ec6d2c69c1a12d61c9 Mon Sep 17 00:00:00 2001 From: kerwizzy Date: Wed, 5 Jun 2019 19:00:16 -0400 Subject: [PATCH] Mandelbrot: Fix race conditions and missing initializations. Change-Id: I64e7bb68631320a7f8d80be25b4011a0332e0348 Reviewed-on: https://review.haiku-os.org/c/1480 Reviewed-by: waddlesplash --- src/apps/mandelbrot/FractalEngine.cpp | 79 +++++++++++++++++++-------- src/apps/mandelbrot/FractalEngine.h | 10 +++- src/apps/mandelbrot/Mandelbrot.cpp | 2 +- 3 files changed, 65 insertions(+), 26 deletions(-) diff --git a/src/apps/mandelbrot/FractalEngine.cpp b/src/apps/mandelbrot/FractalEngine.cpp index bdae067502..846b93e5d5 100644 --- a/src/apps/mandelbrot/FractalEngine.cpp +++ b/src/apps/mandelbrot/FractalEngine.cpp @@ -17,7 +17,7 @@ #include "Colorsets.h" -// #define TRACE_MANDELBROT_ENGINE +//#define TRACE_MANDELBROT_ENGINE #ifdef TRACE_MANDELBROT_ENGINE # include # define TRACE(x...) printf(x) @@ -34,6 +34,9 @@ FractalEngine::FractalEngine(BHandler* parent, BLooper* looper) fRenderBufferLen(0), fSubsampling(2), fMessenger(parent, looper), + fRenderStopping(false), + fRenderStopped(true), + fResizing(false), fIterations(1024), fColorset(Colorset_Royal) { @@ -99,12 +102,13 @@ void FractalEngine::MessageReceived(BMessage* msg) break; case MSG_RESIZE: { - TRACE("Got MSG_RESIZE threads rendering\n"); - if (fStopRender) { + TRACE("Got MSG_RESIZE\n"); + if (fResizing) { // Will be true throughout this whole handler. Set false at the end TRACE("Breaking out of MSG_RESIZE handler\n"); break; } + fResizing = true; StopRender(); delete fRenderBuffer; @@ -113,21 +117,26 @@ void FractalEngine::MessageReceived(BMessage* msg) fWidth = msg->GetUInt16("width", 320); fHeight = msg->GetUInt16("height", 240); - TRACE("Creating new buffer. width %u height %u\n", fWidth, fHeight); fRenderBufferLen = fWidth * fHeight * 3; fRenderBuffer = new uint8[fRenderBufferLen]; + TRACE("New buffer width %u height %u ptr = %p\n", + fWidth, fHeight, fRenderBuffer); + memset(fRenderBuffer, 0, fRenderBufferLen); BMessage message(MSG_BUFFER_CREATED); fMessenger.SendMessage(&message); - fStopRender = false; + fResizing = false; break; } case MSG_RENDER: { TRACE("Got MSG_RENDER.\n"); + if (fResizing) + break; + + // Stop the render if one is already running StopRender(); - fStopRender = false; Render(msg->GetDouble("locationX", 0), msg->GetDouble("locationY", 0), msg->GetDouble("size", 0.005)); break; @@ -138,6 +147,7 @@ void FractalEngine::MessageReceived(BMessage* msg) int32 threadsStopped; get_sem_count(fRenderStoppedSem, &threadsStopped); + TRACE("threadsStopped = %d\n",threadsStopped); if (threadsStopped == fThreadCount) { TRACE("Done rendering!\n"); BMessage message(MSG_RENDER_COMPLETE); @@ -155,37 +165,57 @@ void FractalEngine::MessageReceived(BMessage* msg) void FractalEngine::WriteToBitmap(BBitmap* bitmap) { + Lock(); + BSize size = bitmap->Bounds().Size(); + if (size.IntegerWidth() != fWidth || size.IntegerHeight() != fHeight) { + // some resize happened and now this won't work. + Unlock(); + return; + } + TRACE("Drawing from = %p\n",fRenderBuffer); bitmap->ImportBits(fRenderBuffer, fRenderBufferLen, fWidth * 3, 0, B_RGB24); + Unlock(); } void FractalEngine::StopRender() { - if (fRenderStopped) + if (fRenderStopped || fRenderStopping) { + // if fRenderStopped is true, then render is already stopped, + // so we can't stop it again! + // if fStopRender is true, then the fRenderStoppedSem are already + // trying to be acquired, so stuff would break if we tried to acquire + // them again. return; - fRenderStopped = true; - // true if another call to StopRender() won't work properly because - // the fRenderStoppedSem semaphores have already been acquired. + } + fRenderStopping = true; TRACE("Stopping render...\n"); - fStopRender = true; - for (uint i = 0; i < fThreadCount; i++) + for (uint i = 0; i < fThreadCount; i++) { + TRACE("Stopping thread %d\n",i); acquire_sem(fRenderStoppedSem); + // wait till all the threads are stopped... + } + int32 threadsStopped; + get_sem_count(fRenderStoppedSem, &threadsStopped); + TRACE("stopped sem count after stop = %d\n",threadsStopped); + fRenderStopping = false; + fRenderStopped = true; TRACE("Render stopped.\n"); - // note that fStopRender is NOT set to false at the end here. - // This is to allow the message handlers to use this variable to - // block duplication of stuff. } void FractalEngine::Render(double locationX, double locationY, double size) { + if (fRenderStopping) + debugger("Error: Render shouldn't be called while fRenderStopping = true\n"); + if (!fRenderStopped) + debugger("Error: Render already running\n"); + fRenderStopped = false; // This means that future Render calls will need to call stop render - if (fStopRender) - debugger("Error: Render shouldn't be called while fStopRender = true\n"); fLocationX = locationX; fLocationY = locationY; @@ -211,8 +241,6 @@ status_t FractalEngine::RenderThread(void* data) } while (true) { - release_sem(engine->fRenderStoppedSem); - TRACE("Thread %d awaiting semaphore...\n", threadNum); acquire_sem(engine->fRenderSem); TRACE("Thread %d got semaphore!\n", threadNum); @@ -237,17 +265,24 @@ status_t FractalEngine::RenderThread(void* data) // halfHeight-(halfHeight-1)-1 = 0 } - if (engine->fStopRender) { + if (engine->fRenderStopping) { TRACE("Thread %d stopping\n", threadNum); - break; // Restart the loop to update width, height, etc. + + // Restart the loop to release fRenderStoppedSem and tell + // the main thread that this thread is stopped, as well as + // to update width, height, etc. + break; } } - if (!engine->fStopRender) { + if (!engine->fRenderStopping) { + // if we got here, then this thread has finished rendering. BMessage message(FractalEngine::MSG_THREAD_RENDER_COMPLETE); message.AddUInt8("thread", threadNum); engine->PostMessage(&message); } + + release_sem(engine->fRenderStoppedSem); } return B_OK; } diff --git a/src/apps/mandelbrot/FractalEngine.h b/src/apps/mandelbrot/FractalEngine.h index ba21a33a92..ea9a059864 100644 --- a/src/apps/mandelbrot/FractalEngine.h +++ b/src/apps/mandelbrot/FractalEngine.h @@ -54,14 +54,18 @@ private: uint8 fThreadCount; thread_id fRenderThreads[MAX_RENDER_THREADS]; sem_id fRenderSem; + // released to tell threads to start running sem_id fRenderStoppedSem; + // released by threads when done rendering or otherwise stopped - bool fStopRender; + bool fRenderStopping; + // true when the render is trying to be stopped bool fRenderStopped; + bool fResizing; - double fLocationX; - double fLocationY; + double fLocationX, fLocationY; double fSize; + // the width on the complex plane of a single pixel uint16 fIterations; diff --git a/src/apps/mandelbrot/Mandelbrot.cpp b/src/apps/mandelbrot/Mandelbrot.cpp index fe8a30cfc1..9fac754046 100644 --- a/src/apps/mandelbrot/Mandelbrot.cpp +++ b/src/apps/mandelbrot/Mandelbrot.cpp @@ -28,7 +28,7 @@ // #pragma mark - FractalView -// #define TRACE_MANDELBROT_VIEW +//#define TRACE_MANDELBROT_VIEW #ifdef TRACE_MANDELBROT_VIEW # include # define TRACE(x...) printf(x)