Mandelbrot: Fix race conditions and missing initializations.

Change-Id: I64e7bb68631320a7f8d80be25b4011a0332e0348
Reviewed-on: https://review.haiku-os.org/c/1480
Reviewed-by: waddlesplash <[email protected]>

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 <stdio.h>
 #	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;
 }

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;
 

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 <stdio.h>
 #	define TRACE(x...) printf(x)