Codechange: use _cur_palette the same in all the drivers

It was a bit of a mixed bag. With this change, gfx.cpp is in
control who accesses _cur_palette from the video-drivers.

src/gfx.cpp

@@ -1201,6 +1201,30 @@ void GfxInitPalettes()
 	DoPaletteAnimations();
 }
 
+/**
+ * Copy the current palette if the palette was updated.
+ * Used by video-driver to get a current up-to-date version of the palette,
+ * to avoid two threads accessing the same piece of memory (with a good chance
+ * one is already updating the palette while the other is drawing based on it).
+ * @param local_palette The location to copy the palette to.
+ * @param force_copy Whether to ignore if there is an update for the palette.
+ * @return True iff a copy was done.
+ */
+bool CopyPalette(Palette &local_palette, bool force_copy)
+{
+	if (!force_copy && _cur_palette.count_dirty == 0) return false;
+
+	local_palette = _cur_palette;
+	_cur_palette.count_dirty = 0;
+
+	if (force_copy) {
+		local_palette.first_dirty = 0;
+		local_palette.count_dirty = 256;
+	}
+
+	return true;
+}
+
 #define EXTR(p, q) (((uint16)(palette_animation_counter * (p)) * (q)) >> 16)
 #define EXTR2(p, q) (((uint16)(~palette_animation_counter * (p)) * (q)) >> 16)
 

src/gfx_func.h

@@ -121,6 +121,7 @@ void DrawDirtyBlocks();
 void AddDirtyBlock(int left, int top, int right, int bottom);
 void MarkWholeScreenDirty();
 
+bool CopyPalette(Palette &local_palette, bool force_copy = false);
 void GfxInitPalettes();
 void CheckBlitter();
 

src/video/allegro_v.cpp

@@ -43,6 +43,7 @@ static BITMAP *_allegro_screen;
 #define MAX_DIRTY_RECTS 100
 static PointDimension _dirty_rects[MAX_DIRTY_RECTS];
 static int _num_dirty_rects;
+static Palette _local_palette; ///< Current palette to use for drawing.
 
 void VideoDriver_Allegro::MakeDirty(int left, int top, int width, int height)
 {
@@ -80,9 +81,9 @@ static void UpdatePalette(uint start, uint count)
 
 	uint end = start + count;
 	for (uint i = start; i != end; i++) {
-		pal[i].r = _cur_palette.palette[i].r / 4;
-		pal[i].g = _cur_palette.palette[i].g / 4;
-		pal[i].b = _cur_palette.palette[i].b / 4;
+		pal[i].r = _local_palette.palette[i].r / 4;
+		pal[i].g = _local_palette.palette[i].g / 4;
+		pal[i].b = _local_palette.palette[i].b / 4;
 		pal[i].filler = 0;
 	}
 
@@ -96,25 +97,24 @@ static void InitPalette()
 
 void VideoDriver_Allegro::CheckPaletteAnim()
 {
-	if (_cur_palette.count_dirty != 0) {
-		Blitter *blitter = BlitterFactory::GetCurrentBlitter();
+	if (!CopyPalette(_local_palette)) return;
 
-		switch (blitter->UsePaletteAnimation()) {
-			case Blitter::PALETTE_ANIMATION_VIDEO_BACKEND:
-				UpdatePalette(_cur_palette.first_dirty, _cur_palette.count_dirty);
-				break;
+	Blitter *blitter = BlitterFactory::GetCurrentBlitter();
 
-			case Blitter::PALETTE_ANIMATION_BLITTER:
-				blitter->PaletteAnimate(_cur_palette);
-				break;
+	switch (blitter->UsePaletteAnimation()) {
+		case Blitter::PALETTE_ANIMATION_VIDEO_BACKEND:
+			UpdatePalette(_local_palette.first_dirty, _local_palette.count_dirty);
+			break;
 
-			case Blitter::PALETTE_ANIMATION_NONE:
-				break;
+		case Blitter::PALETTE_ANIMATION_BLITTER:
+			blitter->PaletteAnimate(_local_palette);
+			break;
 
-			default:
-				NOT_REACHED();
-		}
-		_cur_palette.count_dirty = 0;
+		case Blitter::PALETTE_ANIMATION_NONE:
+			break;
+
+		default:
+			NOT_REACHED();
 	}
 }
 

src/video/cocoa/cocoa_ogl.mm

@@ -37,6 +37,8 @@
 #import <OpenGL/OpenGL.h>
 #import <OpenGL/gl3.h>
 
+static Palette _local_palette; ///< Current palette to use for drawing.
+
 
 /**
  * Important notice regarding all modifications!!!!!!!
@@ -304,17 +306,15 @@ void VideoDriver_CocoaOpenGL::Paint()
 {
 	PerformanceMeasurer framerate(PFE_VIDEO);
 
-	if (_cur_palette.count_dirty != 0) {
+	if (CopyPalette(_local_palette)) {
 		Blitter *blitter = BlitterFactory::GetCurrentBlitter();
 
 		/* Always push a changed palette to OpenGL. */
 		CGLSetCurrentContext(this->gl_context);
-		OpenGLBackend::Get()->UpdatePalette(_cur_palette.palette, _cur_palette.first_dirty, _cur_palette.count_dirty);
+		OpenGLBackend::Get()->UpdatePalette(_local_palette.palette, _local_palette.first_dirty, _local_palette.count_dirty);
 		if (blitter->UsePaletteAnimation() == Blitter::PALETTE_ANIMATION_BLITTER) {
-			blitter->PaletteAnimate(_cur_palette);
+			blitter->PaletteAnimate(_local_palette);
 		}
-
-		_cur_palette.count_dirty = 0;
 	}
 
 	[ CATransaction begin ];

src/video/cocoa/cocoa_v.mm

@@ -72,6 +72,7 @@
 #endif
 
 bool _cocoa_video_started = false;
+static Palette _local_palette; ///< Current palette to use for drawing.
 
 extern bool _tab_is_down;
 
@@ -714,9 +715,9 @@ void VideoDriver_CocoaQuartz::UpdatePalette(uint first_color, uint num_colors)
 
 	for (uint i = first_color; i < first_color + num_colors; i++) {
 		uint32 clr = 0xff000000;
-		clr |= (uint32)_cur_palette.palette[i].r << 16;
-		clr |= (uint32)_cur_palette.palette[i].g << 8;
-		clr |= (uint32)_cur_palette.palette[i].b;
+		clr |= (uint32)_local_palette.palette[i].r << 16;
+		clr |= (uint32)_local_palette.palette[i].g << 8;
+		clr |= (uint32)_local_palette.palette[i].b;
 		this->palette[i] = clr;
 	}
 
@@ -725,25 +726,24 @@ void VideoDriver_CocoaQuartz::UpdatePalette(uint first_color, uint num_colors)
 
 void VideoDriver_CocoaQuartz::CheckPaletteAnim()
 {
-	if (_cur_palette.count_dirty != 0) {
-		Blitter *blitter = BlitterFactory::GetCurrentBlitter();
+	if (!CopyPalette(_local_palette)) return;
 
-		switch (blitter->UsePaletteAnimation()) {
-			case Blitter::PALETTE_ANIMATION_VIDEO_BACKEND:
-				this->UpdatePalette(_cur_palette.first_dirty, _cur_palette.count_dirty);
-				break;
+	Blitter *blitter = BlitterFactory::GetCurrentBlitter();
 
-			case Blitter::PALETTE_ANIMATION_BLITTER:
-				blitter->PaletteAnimate(_cur_palette);
-				break;
+	switch (blitter->UsePaletteAnimation()) {
+		case Blitter::PALETTE_ANIMATION_VIDEO_BACKEND:
+			this->UpdatePalette(_local_palette.first_dirty, _local_palette.count_dirty);
+			break;
 
-			case Blitter::PALETTE_ANIMATION_NONE:
-				break;
+		case Blitter::PALETTE_ANIMATION_BLITTER:
+			blitter->PaletteAnimate(_local_palette);
+			break;
 
-			default:
-				NOT_REACHED();
-		}
-		_cur_palette.count_dirty = 0;
+		case Blitter::PALETTE_ANIMATION_NONE:
+			break;
+
+		default:
+			NOT_REACHED();
 	}
 }
 

src/video/sdl2_default_v.cpp

@@ -62,9 +62,7 @@ void VideoDriver_SDL_Default::MakePalette()
 		if (_sdl_palette == nullptr) usererror("SDL2: Couldn't allocate palette: %s", SDL_GetError());
 	}
 
-	_cur_palette.first_dirty = 0;
-	_cur_palette.count_dirty = 256;
-	this->local_palette = _cur_palette;
+	CopyPalette(this->local_palette, true);
 	this->UpdatePalette();
 
 	if (_sdl_surface != _sdl_real_surface) {
@@ -96,9 +94,9 @@ void VideoDriver_SDL_Default::Paint()
 {
 	PerformanceMeasurer framerate(PFE_VIDEO);
 
-	if (IsEmptyRect(this->dirty_rect) && _cur_palette.count_dirty == 0) return;
+	if (IsEmptyRect(this->dirty_rect) && this->local_palette.count_dirty == 0) return;
 
-	if (_cur_palette.count_dirty != 0) {
+	if (this->local_palette.count_dirty != 0) {
 		Blitter *blitter = BlitterFactory::GetCurrentBlitter();
 
 		switch (blitter->UsePaletteAnimation()) {
@@ -117,7 +115,7 @@ void VideoDriver_SDL_Default::Paint()
 			default:
 				NOT_REACHED();
 		}
-		_cur_palette.count_dirty = 0;
+		this->local_palette.count_dirty = 0;
 	}
 
 	SDL_Rect r = { this->dirty_rect.left, this->dirty_rect.top, this->dirty_rect.right - this->dirty_rect.left, this->dirty_rect.bottom - this->dirty_rect.top };

src/video/sdl2_opengl_v.cpp

@@ -146,9 +146,7 @@ bool VideoDriver_SDL_OpenGL::AllocateBackingStore(int w, int h, bool force)
 	SDL_GL_SwapWindow(this->sdl_window);
 	_screen.dst_ptr = this->GetVideoPointer();
 
-	_cur_palette.first_dirty = 0;
-	_cur_palette.count_dirty = 256;
-	this->local_palette = _cur_palette;
+	CopyPalette(this->local_palette, true);
 
 	return res;
 }
@@ -173,7 +171,7 @@ void VideoDriver_SDL_OpenGL::Paint()
 {
 	PerformanceMeasurer framerate(PFE_VIDEO);
 
-	if (_cur_palette.count_dirty != 0) {
+	if (this->local_palette.count_dirty != 0) {
 		Blitter *blitter = BlitterFactory::GetCurrentBlitter();
 
 		/* Always push a changed palette to OpenGL. */
@@ -182,7 +180,7 @@ void VideoDriver_SDL_OpenGL::Paint()
 			blitter->PaletteAnimate(this->local_palette);
 		}
 
-		_cur_palette.count_dirty = 0;
+		this->local_palette.count_dirty = 0;
 	}
 
 	OpenGLBackend::Get()->Paint();

src/video/sdl2_v.cpp

@@ -43,9 +43,7 @@ void VideoDriver_SDL_Base::MakeDirty(int left, int top, int width, int height)
 
 void VideoDriver_SDL_Base::CheckPaletteAnim()
 {
-	if (_cur_palette.count_dirty == 0) return;
-
-	this->local_palette = _cur_palette;
+	if (!CopyPalette(this->local_palette)) return;
 	this->MakeDirty(0, 0, _screen.width, _screen.height);
 }
 
@@ -131,10 +129,7 @@ void VideoDriver_SDL_Base::ClientSizeChanged(int w, int h, bool force)
 {
 	/* Allocate backing store of the new size. */
 	if (this->AllocateBackingStore(w, h, force)) {
-		/* Mark all palette colours dirty. */
-		_cur_palette.first_dirty = 0;
-		_cur_palette.count_dirty = 256;
-		this->local_palette = _cur_palette;
+		CopyPalette(this->local_palette, true);
 
 		BlitterFactory::GetCurrentBlitter()->PostResize();
 

src/video/sdl2_v.h

@@ -45,7 +45,7 @@ public:
 
 protected:
 	struct SDL_Window *sdl_window; ///< Main SDL window.
-	Palette local_palette; ///< Copy of _cur_palette.
+	Palette local_palette; ///< Current palette to use for drawing.
 	bool buffer_locked; ///< Video buffer was locked by the main thread.
 	Rect dirty_rect; ///< Rectangle encompassing the dirty area of the video buffer.
 

src/video/sdl_v.cpp

@@ -106,35 +106,30 @@ static void UpdatePalette(bool init = false)
 
 static void InitPalette()
 {
-	_local_palette = _cur_palette;
-	_local_palette.first_dirty = 0;
-	_local_palette.count_dirty = 256;
+	CopyPalette(_local_palette, true);
 	UpdatePalette(true);
 }
 
 void VideoDriver_SDL::CheckPaletteAnim()
 {
-	_local_palette = _cur_palette;
+	if (!CopyPalette(_local_palette)) return;
 
-	if (_cur_palette.count_dirty != 0) {
-		Blitter *blitter = BlitterFactory::GetCurrentBlitter();
+	Blitter *blitter = BlitterFactory::GetCurrentBlitter();
 
-		switch (blitter->UsePaletteAnimation()) {
-			case Blitter::PALETTE_ANIMATION_VIDEO_BACKEND:
-				UpdatePalette();
-				break;
+	switch (blitter->UsePaletteAnimation()) {
+		case Blitter::PALETTE_ANIMATION_VIDEO_BACKEND:
+			UpdatePalette();
+			break;
 
-			case Blitter::PALETTE_ANIMATION_BLITTER:
-				blitter->PaletteAnimate(_local_palette);
-				break;
+		case Blitter::PALETTE_ANIMATION_BLITTER:
+			blitter->PaletteAnimate(_local_palette);
+			break;
 
-			case Blitter::PALETTE_ANIMATION_NONE:
-				break;
+		case Blitter::PALETTE_ANIMATION_NONE:
+			break;
 
-			default:
-				NOT_REACHED();
-		}
-		_cur_palette.count_dirty = 0;
+		default:
+			NOT_REACHED();
 	}
 }
 

src/video/win32_v.cpp

@@ -42,8 +42,7 @@ bool _window_maximize;
 static Dimension _bck_resolution;
 DWORD _imm_props;
 
-/** Local copy of the palette for use in the drawing thread. */
-static Palette _local_palette;
+static Palette _local_palette; ///< Current palette to use for drawing.
 
 bool VideoDriver_Win32Base::ClaimMousePointer()
 {
@@ -812,9 +811,7 @@ void VideoDriver_Win32Base::MakeDirty(int left, int top, int width, int height)
 
 void VideoDriver_Win32Base::CheckPaletteAnim()
 {
-	if (_cur_palette.count_dirty == 0) return;
-
-	_local_palette = _cur_palette;
+	if (!CopyPalette(_local_palette)) return;
 	this->MakeDirty(0, 0, _screen.width, _screen.height);
 }
 
@@ -878,10 +875,7 @@ void VideoDriver_Win32Base::ClientSizeChanged(int w, int h, bool force)
 {
 	/* Allocate backing store of the new size. */
 	if (this->AllocateBackingStore(w, h, force)) {
-		/* Mark all palette colours dirty. */
-		_cur_palette.first_dirty = 0;
-		_cur_palette.count_dirty = 256;
-		_local_palette = _cur_palette;
+		CopyPalette(_local_palette, true);
 
 		BlitterFactory::GetCurrentBlitter()->PostResize();
 
@@ -1078,9 +1072,7 @@ bool VideoDriver_Win32GDI::AfterBlitterChange()
 
 void VideoDriver_Win32GDI::MakePalette()
 {
-	_cur_palette.first_dirty = 0;
-	_cur_palette.count_dirty = 256;
-	_local_palette = _cur_palette;
+	CopyPalette(_local_palette, true);
 
 	LOGPALETTE *pal = (LOGPALETTE*)alloca(sizeof(LOGPALETTE) + (256 - 1) * sizeof(PALETTEENTRY));
 
@@ -1135,7 +1127,7 @@ void VideoDriver_Win32GDI::Paint()
 	HBITMAP old_bmp = (HBITMAP)SelectObject(dc2, this->dib_sect);
 	HPALETTE old_palette = SelectPalette(dc, this->gdi_palette, FALSE);
 
-	if (_cur_palette.count_dirty != 0) {
+	if (_local_palette.count_dirty != 0) {
 		Blitter *blitter = BlitterFactory::GetCurrentBlitter();
 
 		switch (blitter->UsePaletteAnimation()) {
@@ -1154,7 +1146,7 @@ void VideoDriver_Win32GDI::Paint()
 			default:
 				NOT_REACHED();
 		}
-		_cur_palette.count_dirty = 0;
+		_local_palette.count_dirty = 0;
 	}
 
 	BitBlt(dc, 0, 0, this->width, this->height, dc2, 0, 0, SRCCOPY);
@@ -1474,7 +1466,7 @@ void VideoDriver_Win32OpenGL::Paint()
 {
 	PerformanceMeasurer framerate(PFE_VIDEO);
 
-	if (_cur_palette.count_dirty != 0) {
+	if (_local_palette.count_dirty != 0) {
 		Blitter *blitter = BlitterFactory::GetCurrentBlitter();
 
 		/* Always push a changed palette to OpenGL. */
@@ -1483,7 +1475,7 @@ void VideoDriver_Win32OpenGL::Paint()
 			blitter->PaletteAnimate(_local_palette);
 		}
 
-		_cur_palette.count_dirty = 0;
+		_local_palette.count_dirty = 0;
 	}
 
 	OpenGLBackend::Get()->Paint();