- Changed the sentinels in the thinker lists into a proper thinker. The old

way wasn't playing well with the write barriers.
- Fixed: DFrameBuffer::WriteSavePic needs to fix the canvas in place while
  using it.
- Fixed: The sound system was updated every frame, which is a complete waste
  of time. Doing it only once each tic is quite sufficient, since nothing
  really moves between tics, even if the display makes it look otherwise.


SVN r799 (trunk)

docs/rh-log.txt

@@ -1,3 +1,12 @@
+March 12, 2008
+- Changed the sentinels in the thinker lists into a proper thinker. The old
+  way wasn't playing well with the write barriers.
+- Fixed: DFrameBuffer::WriteSavePic needs to fix the canvas in place while
+  using it.
+- Fixed: The sound system was updated every frame, which is a complete waste
+  of time. Doing it only once each tic is quite sufficient, since nothing
+  really moves between tics, even if the display makes it look otherwise.
+
 March 12, 2008 (Changes by Graf Zahl)
 - Fixed: The SimpleCanvas in FCanvasTexture must be declared as a soft
   root to the garbage collector.

src/d_main.cpp

@@ -771,6 +771,8 @@ void D_DoomLoop ()
 				C_Ticker ();
 				M_Ticker ();
 				G_Ticker ();
+				// [RH] Use the consoleplayer's camera to update sounds
+				S_UpdateSounds (players[consoleplayer].camera);	// move positional sounds
 				gametic++;
 				maketic++;
 				GC::CheckGC ();
@@ -780,8 +782,6 @@ void D_DoomLoop ()
 			{
 				TryRunTics (); // will run at least one tic
 			}
-			// [RH] Use the consoleplayer's camera to update sounds
-			S_UpdateSounds (players[consoleplayer].camera);	// move positional sounds
 			// Update display, next frame, with current state.
 			I_StartTic ();
 			D_Display ();

src/d_net.cpp

@@ -1825,6 +1825,8 @@ void TryRunTics (void)
 	}
 
 	// run the count tics
+	if (counts > 0)
+	{
 		while (counts--)
 		{
 			if (gametic > lowtic)
@@ -1845,6 +1847,8 @@ void TryRunTics (void)
 
 			NetUpdate ();	// check for new console commands
 		}
+		S_UpdateSounds (players[consoleplayer].camera);	// move positional sounds
+	}
 }
 
 void Net_NewMakeTic (void)

src/dobject.cpp

@@ -412,7 +412,7 @@ DObject::~DObject ()
 
 void DObject::Destroy ()
 {
-	ObjectFlags |= OF_EuthanizeMe;
+	ObjectFlags = (ObjectFlags & ~OF_Fixed) | OF_EuthanizeMe;
 }
 
 size_t DObject::PropagateMark()

src/dobject.h

@@ -226,6 +226,7 @@ enum EObjectFlags
 	// Other flags
 	OF_JustSpawned		= 1 << 8,		// Thinker was spawned this tic
 	OF_SerialSuccess	= 1 << 9,		// For debugging Serialize() calls
+	OF_Sentinel			= 1 << 10,		// Object is serving as the sentinel in a ring list
 };
 
 template<class T> class TObjPtr;

src/dthinker.cpp

@@ -44,24 +44,71 @@ static cycle_t ThinkCycles;
 extern cycle_t BotSupportCycles;
 extern cycle_t BotWTG;
 
-IMPLEMENT_CLASS (DThinker)
+IMPLEMENT_POINTY_CLASS (DThinker)
+ DECLARE_POINTER(NextThinker)
+ DECLARE_POINTER(PrevThinker)
+END_POINTERS
 
-static Node *NextToThink;
+static DThinker *NextToThink;
 
-List DThinker::Thinkers[MAX_STATNUM+1];
+FThinkerList DThinker::Thinkers[MAX_STATNUM+1];
-List DThinker::FreshThinkers[MAX_STATNUM+1];
+FThinkerList DThinker::FreshThinkers[MAX_STATNUM+1];
 bool DThinker::bSerialOverride = false;
 
-void DThinker::SaveList(FArchive &arc, Node *node)
+void FThinkerList::AddTail(DThinker *thinker)
 {
-	if (node->Succ != NULL)
+	if (Sentinel == NULL)
 	{
-		do
+		Sentinel = new DThinker(DThinker::NO_LINK);
+		Sentinel->ObjectFlags |= OF_Sentinel;
+		Sentinel->NextThinker = Sentinel;
+		Sentinel->PrevThinker = Sentinel;
+		GC::WriteBarrier(Sentinel);
+	}
+	DThinker *tail = Sentinel->PrevThinker;
+	assert(tail->NextThinker == Sentinel);
+	thinker->PrevThinker = tail;
+	thinker->NextThinker = Sentinel;
+	tail->NextThinker = thinker;
+	Sentinel->PrevThinker = thinker;
+	GC::WriteBarrier(thinker, tail);
+	GC::WriteBarrier(thinker, Sentinel);
+	GC::WriteBarrier(tail, thinker);
+	GC::WriteBarrier(Sentinel, thinker);
+}
+
+DThinker *FThinkerList::GetHead() const
 {
-			DThinker *thinker = static_cast<DThinker *>(node);
+	if (Sentinel == NULL || Sentinel->NextThinker == Sentinel)
-			arc << thinker;
+	{
-			node = node->Succ;
+		return NULL;
-		} while (node->Succ != NULL);
+	}
+	return Sentinel->NextThinker;
+}
+
+DThinker *FThinkerList::GetTail() const
+{
+	if (Sentinel == NULL || Sentinel->PrevThinker == Sentinel)
+	{
+		return NULL;
+	}
+	return Sentinel->PrevThinker;
+}
+
+bool FThinkerList::IsEmpty() const
+{
+	return Sentinel == NULL || Sentinel->NextThinker == NULL;
+}
+
+void DThinker::SaveList(FArchive &arc, DThinker *node)
+{
+	if (node != NULL)
+	{
+		while (!(node->ObjectFlags & OF_Sentinel))
+		{
+			arc << node;
+			node = node->NextThinker;
+		}
 	}
 }
 
@@ -92,8 +139,8 @@ void DThinker::SerializeAll(FArchive &arc, bool hubLoad)
 			{
 				stat = i;
 				arc << stat;
-				SaveList(arc, Thinkers[i].Head);
+				SaveList(arc, Thinkers[i].GetHead());
-				SaveList(arc, FreshThinkers[i].Head);
+				SaveList(arc, FreshThinkers[i].GetHead());
 				thinker = NULL;
 				arc << thinker;		// Save a final NULL for this list
 			}
@@ -146,7 +193,8 @@ DThinker::DThinker (int statnum) throw()
 {
 	if (bSerialOverride)
 	{ // The serializer will insert us into the right list
-		Succ = NULL;
+		NextThinker = NULL;
+		PrevThinker = NULL;
 		return;
 	}
 
@@ -155,46 +203,43 @@ DThinker::DThinker (int statnum) throw()
 	{
 		statnum = MAX_STATNUM;
 	}
-	if (FreshThinkers[statnum].TailPred->Pred != NULL)
-	{
-		GC::WriteBarrier(static_cast<DThinker*>(FreshThinkers[statnum].Tail, this));
-	}
-	else
-	{
-		GC::WriteBarrier(this);
-	}
 	FreshThinkers[statnum].AddTail (this);
 }
 
+DThinker::DThinker(no_link_type foo) throw()
+{
+	foo;	// Avoid unused argument warnings.
+}
+
 DThinker::~DThinker ()
 {
-	if (Succ != NULL)
+	assert(NextThinker == NULL && PrevThinker == NULL);
-	{
-		Remove ();
-	}
 }
 
 void DThinker::Destroy ()
 {
-	if (this == NextToThink)
+	if (NextThinker != NULL)
-		NextToThink = Succ;
-
-	if (Succ != NULL)
 	{
 		Remove();
-		Succ = NULL;
 	}
 	Super::Destroy();
 }
 
 void DThinker::Remove()
 {
-	if (Pred->Pred != NULL && Succ->Succ != NULL)
+	if (this == NextToThink)
 	{
-		GC::WriteBarrier(static_cast<DThinker *>(Pred), static_cast<DThinker *>(Succ));
+		NextToThink = NextThinker;
-		GC::WriteBarrier(static_cast<DThinker *>(Succ), static_cast<DThinker *>(Pred));
 	}
-	Node::Remove();
+	DThinker *prev = PrevThinker;
+	DThinker *next = NextThinker;
+	assert(prev != NULL && next != NULL);
+	prev->NextThinker = next;
+	next->PrevThinker = prev;
+	GC::WriteBarrier(prev, next);
+	GC::WriteBarrier(next, prev);
+	NextThinker = NULL;
+	PrevThinker = NULL;
 }
 
 void DThinker::PostBeginPlay ()
@@ -203,27 +248,27 @@ void DThinker::PostBeginPlay ()
 
 DThinker *DThinker::FirstThinker (int statnum)
 {
-	Node *node;
+	DThinker *node;
 
 	if ((unsigned)statnum > MAX_STATNUM)
 	{
 		statnum = MAX_STATNUM;
 	}
-	node = Thinkers[statnum].Head;
+	node = Thinkers[statnum].GetHead();
-	if (node->Succ == NULL)
+	if (node == NULL)
 	{
-		node = FreshThinkers[statnum].Head;
+		node = FreshThinkers[statnum].GetHead();
-		if (node->Succ == NULL)
+		if (node == NULL)
 		{
 			return NULL;
 		}
 	}
-	return static_cast<DThinker *>(node);
+	return node;
 }
 
 void DThinker::ChangeStatNum (int statnum)
 {
-	List *list;
+	FThinkerList *list;
 
 	if ((unsigned)statnum > MAX_STATNUM)
 	{
@@ -238,14 +283,6 @@ void DThinker::ChangeStatNum (int statnum)
 	{
 		list = &Thinkers[statnum];
 	}
-	if (list->TailPred->Pred != NULL)
-	{
-		GC::WriteBarrier(static_cast<DThinker*>(list->Tail, this));
-	}
-	else
-	{
-		GC::WriteBarrier(this);
-	}
 	list->AddTail(this);
 }
 
@@ -254,24 +291,11 @@ void DThinker::MarkRoots()
 {
 	for (int i = 0; i <= MAX_STATNUM; ++i)
 	{
-		DThinker *t = static_cast<DThinker *>(Thinkers[i].Head);
+		GC::Mark(Thinkers[i].Sentinel);
-		GC::Mark(t);
+		GC::Mark(FreshThinkers[i].Sentinel);
-		t = static_cast<DThinker *>(FreshThinkers[i].Head);
-		GC::Mark(t);
 	}
 }
 
-size_t DThinker::PropagateMark()
-{
-	// Mark the next thinker in my list
-	if (Succ != NULL && Succ->Succ != NULL)
-	{
-		DThinker *t = static_cast<DThinker *>(Succ);
-		GC::Mark(t);
-	}
-	return Super::PropagateMark();
-}
-
 // Destroy every thinker
 void DThinker::DestroyAllThinkers ()
 {
@@ -281,8 +305,8 @@ void DThinker::DestroyAllThinkers ()
 	{
 		if (i != STAT_TRAVELLING)
 		{
-			DestroyThinkersInList (Thinkers[i].Head);
+			DestroyThinkersInList (Thinkers[i]);
-			DestroyThinkersInList (FreshThinkers[i].Head);
+			DestroyThinkersInList (FreshThinkers[i]);
 		}
 	}
 	GC::FullGC();
@@ -306,31 +330,39 @@ void DThinker::DestroyMostThinkers ()
 	GC::FullGC();
 }
 
-void DThinker::DestroyThinkersInList (Node *node)
+void DThinker::DestroyThinkersInList (FThinkerList &list)
 {
-	while (node->Succ != NULL)
+	if (list.Sentinel != NULL)
 	{
-		Node *next = node->Succ;
+		DThinker *node = list.Sentinel->NextThinker;
-		static_cast<DThinker *> (node)->Destroy ();
+		while (node != list.Sentinel)
+		{
+			DThinker *next = node->NextThinker;
+			node->Destroy();
 			node = next;
 		}
+		list.Sentinel->Destroy();
+		list.Sentinel = NULL;
+	}
 }
 
-void DThinker::DestroyMostThinkersInList (List &list, int stat)
+void DThinker::DestroyMostThinkersInList (FThinkerList &list, int stat)
 {
 	if (stat != STAT_PLAYER)
 	{
-		DestroyThinkersInList (list.Head);
+		DestroyThinkersInList (list);
 	}
-	else
+	else if (list.Sentinel != NULL)
 	{
-		Node *node = list.Head;
+		DThinker *node = list.Sentinel->NextThinker;
-		while (node->Succ != NULL)
+		while (node != list.Sentinel)
 		{
-			Node *next = node->Succ;
+			DThinker *next = node->NextThinker;
 			node->Remove();
 			node = next;
 		}
+		list.Sentinel->Destroy();
+		list.Sentinel = NULL;
 	}
 }
 
@@ -361,42 +393,38 @@ void DThinker::RunThinkers ()
 	unclock (ThinkCycles);
 }
 
-int DThinker::TickThinkers (List *list, List *dest)
+int DThinker::TickThinkers (FThinkerList *list, FThinkerList *dest)
 {
 	int count = 0;
-	Node *node = list->Head;
+	DThinker *node = list->GetHead();
 
-	while (node->Succ != NULL)
+	if (node == NULL)
+	{
+		return 0;
+	}
+
+	while (node != list->Sentinel)
 	{
 		++count;
-		NextToThink = node->Succ;
+		NextToThink = node->NextThinker;
-		DThinker *thinker = static_cast<DThinker *> (node);
+		if (node->ObjectFlags & OF_JustSpawned)
-		if (thinker->ObjectFlags & OF_JustSpawned)
 		{
-			thinker->ObjectFlags &= ~OF_JustSpawned;
+			node->ObjectFlags &= ~OF_JustSpawned;
 			if (dest != NULL)
 			{ // Move thinker from this list to the destination list
 				node->Remove();
-				if (dest->TailPred->Pred != NULL)
-				{
-					GC::WriteBarrier(static_cast<DThinker*>(dest->Tail, thinker));
-				}
-				else
-				{
-					GC::WriteBarrier(thinker);
-				}
 				dest->AddTail(node);
 			}
-			thinker->PostBeginPlay ();
+			node->PostBeginPlay();
 		}
 		else if (dest != NULL)
 		{ // Move thinker from this list to the destination list
 			I_Error("There is a thinker in the fresh list that has already ticked.\n");
 		}
 
-		if (!(thinker->ObjectFlags & OF_EuthanizeMe))
+		if (!(node->ObjectFlags & OF_EuthanizeMe))
 		{ // Only tick thinkers not scheduled for destruction
-			thinker->Tick ();
+			node->Tick ();
 		}
 		node = NextToThink;
 	}
@@ -420,7 +448,7 @@ FThinkerIterator::FThinkerIterator (const PClass *type, int statnum)
 		m_SearchStats = false;
 	}
 	m_ParentType = type;
-	m_CurrThinker = DThinker::Thinkers[m_Stat].Head;
+	m_CurrThinker = DThinker::Thinkers[m_Stat].GetHead();
 	m_SearchingFresh = false;
 }
 
@@ -437,43 +465,48 @@ FThinkerIterator::FThinkerIterator (const PClass *type, int statnum, DThinker *p
 		m_SearchStats = false;
 	}
 	m_ParentType = type;
-	if (prev == NULL || prev->Succ == NULL)
+	if (prev == NULL || (prev->NextThinker->ObjectFlags & OF_Sentinel))
 	{
 		Reinit();
 	}
 	else
 	{
-		m_CurrThinker = prev->Succ;
+		m_CurrThinker = prev->NextThinker;
 		m_SearchingFresh = false;
 	}
 }
 
 void FThinkerIterator::Reinit ()
 {
-	m_CurrThinker = DThinker::Thinkers[m_Stat].Head;
+	m_CurrThinker = DThinker::Thinkers[m_Stat].GetHead();
 	m_SearchingFresh = false;
 }
 
 DThinker *FThinkerIterator::Next ()
 {
-	if (m_ParentType == NULL) return NULL;
+	if (m_ParentType == NULL)
+	{
+		return NULL;
+	}
 	do
 	{
 		do
 		{
-			while (m_CurrThinker->Succ)
+			if (m_CurrThinker != NULL)
 			{
-				DThinker *thinker = static_cast<DThinker *> (m_CurrThinker);
+				while (!(m_CurrThinker->ObjectFlags & OF_Sentinel))
+				{
+					DThinker *thinker = m_CurrThinker;
+					m_CurrThinker = thinker->NextThinker;
 					if (thinker->IsKindOf(m_ParentType))
 					{
-					m_CurrThinker = m_CurrThinker->Succ;
 						return thinker;
 					}
-				m_CurrThinker = m_CurrThinker->Succ;
+				}
 			}
 			if ((m_SearchingFresh = !m_SearchingFresh))
 			{
-				m_CurrThinker = DThinker::FreshThinkers[m_Stat].Head;
+				m_CurrThinker = DThinker::FreshThinkers[m_Stat].GetHead();
 			}
 		} while (m_SearchingFresh);
 		if (m_SearchStats)
@@ -484,7 +517,7 @@ DThinker *FThinkerIterator::Next ()
 				m_Stat = STAT_FIRST_THINKING;
 			}
 		}
-		m_CurrThinker = DThinker::Thinkers[m_Stat].Head;
+		m_CurrThinker = DThinker::Thinkers[m_Stat].GetHead();
 		m_SearchingFresh = false;
 	} while (m_SearchStats && m_Stat != STAT_FIRST_THINKING);
 	return NULL;

src/dthinker.h

@@ -36,7 +36,6 @@
 
 #include <stdlib.h>
 #include "dobject.h"
-#include "lists.h"
 
 class AActor;
 class player_s;
@@ -48,15 +47,25 @@ class FThinkerIterator;
 
 enum { MAX_STATNUM = 127 };
 
-// Doubly linked list of thinkers
+// Doubly linked ring list of thinkers
-class DThinker : public DObject, private Node
+struct FThinkerList
+{
+	FThinkerList() : Sentinel(0) {}
+	void AddTail(DThinker *thinker);
+	DThinker *GetHead() const;
+	DThinker *GetTail() const;
+	bool IsEmpty() const;
+
+	DThinker *Sentinel;
+};
+
+class DThinker : public DObject
 {
 	DECLARE_CLASS (DThinker, DObject)
-
+	HAS_OBJECT_POINTERS
 public:
 	DThinker (int statnum = MAX_STATNUM) throw();
 	void Destroy ();
-	size_t PropagateMark();
 	virtual ~DThinker ();
 	virtual void Tick ();
 	virtual void PostBeginPlay ();	// Called just before the first tick
@@ -73,18 +82,23 @@ public:
 	static DThinker *FirstThinker (int statnum);
 
 private:
-	static void DestroyThinkersInList (Node *first);
+	enum no_link_type { NO_LINK };
-	static void DestroyMostThinkersInList (List &list, int stat);
+	DThinker(no_link_type) throw();
-	static int TickThinkers (List *list, List *dest);	// Returns: # of thinkers ticked
+	static void DestroyThinkersInList (FThinkerList &list);
-	static void SaveList(FArchive &arc, Node *node);
+	static void DestroyMostThinkersInList (FThinkerList &list, int stat);
+	static int TickThinkers (FThinkerList *list, FThinkerList *dest);	// Returns: # of thinkers ticked
+	static void SaveList(FArchive &arc, DThinker *node);
 	void Remove();
 
-	static List Thinkers[MAX_STATNUM+1];		// Current thinkers
+	static FThinkerList Thinkers[MAX_STATNUM+1];		// Current thinkers
-	static List FreshThinkers[MAX_STATNUM+1];	// Newly created thinkers
+	static FThinkerList FreshThinkers[MAX_STATNUM+1];	// Newly created thinkers
 	static bool bSerialOverride;
 
+	friend struct FThinkerList;
 	friend class FThinkerIterator;
 	friend class DObject;
+
+	DThinker *NextThinker, *PrevThinker;
 };
 
 class FThinkerIterator
@@ -92,7 +106,7 @@ class FThinkerIterator
 protected:
 	const PClass *m_ParentType;
 private:
-	Node *m_CurrThinker;
+	DThinker *m_CurrThinker;
 	BYTE m_Stat;
 	bool m_SearchStats;
 	bool m_SearchingFresh;

src/v_video.cpp

@@ -1385,11 +1385,13 @@ void DFrameBuffer::WriteSavePic (player_t *player, FILE *file, int width, int he
 	PalEntry palette[256];
 
 	// Take a snapshot of the player's view
+	pic->ObjectFlags |= OF_Fixed;
 	pic->Lock ();
 	R_RenderViewToCanvas (player->mo, pic, 0, 0, width, height);
 	GetFlashedPalette (palette);
 	M_CreatePNG (file, pic->GetBuffer(), palette, SS_PAL, width, height, pic->GetPitch());
 	pic->Unlock ();
+	pic->Destroy();
 	pic->ObjectFlags |= OF_YesReallyDelete;
 	delete pic;
 }