Use NONCOPYABLE for most GUI classes and structs to have the compiler indicate unintended copies, refs 3028551b91 / D2163.

That is CChartData, CGUIList, CGUISeries, COListColumn, GUITooltip,
SGUIMessage, SSpriteCall, STextCall, SFeedback, IGUISetting,
CGUISetting, GUI, IGUIObject, IGUIScrollBar.
Drop copying GetSetting and SetSetting template functions for CGUIList,
CGUISeries, CClientArea, CGUIString.
Stop copying COListColumn.
Drop copying GUI<CClientArea>::GetSetting call in
IGUIObject::UpdateCachedSize() and four copying
GUI<CGUIString>::GetSetting calls in SetupText() functions.
Delete unused GUIRenderer IGLState class from 849f50a500 obsolete since
1f5b8f1c9a.

Differential Revision: https://code.wildfiregames.com/D2164
This was SVN commit r22638.

source/gui/CButton.cpp

@@ -68,12 +68,12 @@ void CButton::SetupText()
 		// TODO Gee: (2004-08-14) Default should not be hard-coded, but be in styles!
 		font = L"default";
 
-	CGUIString caption;
+	CGUIString* caption = nullptr;
-	GUI<CGUIString>::GetSetting(this, "caption", caption);
+	GUI<CGUIString>::GetSettingPointer(this, "caption", caption);
 
 	float buffer_zone = 0.f;
 	GUI<float>::GetSetting(this, "buffer_zone", buffer_zone);
-	*m_GeneratedTexts[0] = GetGUI()->GenerateText(caption, font, m_CachedActualSize.GetWidth(), buffer_zone, this);
+	*m_GeneratedTexts[0] = GetGUI()->GenerateText(*caption, font, m_CachedActualSize.GetWidth(), buffer_zone, this);
 
 	CalculateTextPosition(m_CachedActualSize, m_TextPos, *m_GeneratedTexts[0]);
 }

source/gui/CChart.h

@@ -27,6 +27,11 @@
 
 struct CChartData
 {
+	// Avoid copying the container.
+	NONCOPYABLE(CChartData);
+	MOVABLE(CChartData);
+	CChartData() = default;
+
 	CGUIColor m_Color;
 	std::vector<CVector2D> m_Points;
 };

source/gui/CCheckBox.cpp

@@ -80,12 +80,12 @@ void CCheckBox::SetupText()
 	float square_side;
 	GUI<float>::GetSetting(this, "square_side", square_side);
 
-	CGUIString caption;
+	CGUIString* caption = nullptr;
-	GUI<CGUIString>::GetSetting(this, "caption", caption);
+	GUI<CGUIString>::GetSettingPointer(this, "caption", caption);
 
 	float buffer_zone = 0.f;
 	GUI<float>::GetSetting(this, "buffer_zone", buffer_zone);
-	*m_GeneratedTexts[0] = GetGUI()->GenerateText(caption, font, m_CachedActualSize.GetWidth()-square_side, 0.f, this);
+	*m_GeneratedTexts[0] = GetGUI()->GenerateText(*caption, font, m_CachedActualSize.GetWidth()-square_side, 0.f, this);
 }
 
 void CCheckBox::HandleMessage(SGUIMessage& Message)

source/gui/CGUI.cpp

@@ -488,6 +488,8 @@ const SGUIScrollBarStyle* CGUI::GetScrollBarStyle(const CStr& style) const
 // private struct used only in GenerateText(...)
 struct SGenerateTextImage
 {
+	// Only primitve members, so move assignments are the same as copy assignments.
+
 	float m_YFrom,			// The image's starting location in Y
 		  m_YTo,			// The image's end location in Y
 		  m_Indentation;	// The image width in other words
@@ -602,8 +604,8 @@ SGUIText CGUI::GenerateText(const CGUIString& string, const CStrW& FontW, const
 					// Check if image is the lowest thing.
 					Text.m_Size.cy = std::max(Text.m_Size.cy, Image.m_YTo);
 
-					Images[j].push_back(Image);
+					Images[j].emplace_back(Image);
-					Text.m_SpriteCalls.push_back(std::move(SpriteCall));
+					Text.m_SpriteCalls.emplace_back(std::move(SpriteCall));
 				}
 			}
 		}

source/gui/CGUIList.h

@@ -1,4 +1,4 @@
-/* Copyright (C) 2009 Wildfire Games.
+/* Copyright (C) 2019 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -26,6 +26,12 @@ class CGUIList
 public: // struct:ish (but for consistency I call it _C_GUIList, and
 		//  for the same reason it is a class, so that confusion doesn't
 		//  appear when forward declaring.
+
+	// Avoid copying the vector.
+	NONCOPYABLE(CGUIList);
+	MOVABLE(CGUIList);
+	CGUIList() = default;
+
 	/**
 	 * List of items (as text), the post-processed result is stored in
 	 *  the IGUITextOwner structure of this class.

source/gui/CGUISeries.h

@@ -1,4 +1,4 @@
-/* Copyright (C) 2017 Wildfire Games.
+/* Copyright (C) 2019 Wildfire Games.
 * This file is part of 0 A.D.
 *
 * 0 A.D. is free software: you can redistribute it and/or modify
@@ -19,13 +19,15 @@
 #ifndef INCLUDED_CGUISERIES
 #define INCLUDED_CGUISERIES
 
-#include "GUItext.h"
 #include "maths/Vector2D.h"
 
-
 class CGUISeries
 {
 public:
+	NONCOPYABLE(CGUISeries);
+	MOVABLE(CGUISeries);
+	CGUISeries() = default;
+
 	std::vector<std::vector<CVector2D>> m_Series;
 };
 

source/gui/COList.cpp

@@ -297,20 +297,18 @@ bool COList::HandleAdditionalChildren(const XMBElement& child, CXeromyces* pFile
 			}
 		}
 
-		m_Columns.push_back(column);
-
 		AddSetting<CGUIList>("list_" + column.m_Id);
 		AddSetting<bool>("hidden_" + column.m_Id);
 		GUI<bool>::SetSetting(this, "hidden_" + column.m_Id, hidden);
 
+		m_Columns.emplace_back(std::move(column));
+
 		SetupText();
 
 		return true;
 	}
-	else
+
-	{
+	return false;
-		return false;
-	}
 }
 
 void COList::DrawList(const int& selected, const CStr& _sprite, const CStr& _sprite_selected, const CStr& _textcolor)

source/gui/COList.h

@@ -25,11 +25,15 @@
  */
 struct COListColumn
 {
-  CGUIColor m_TextColor;
+	// Avoid copying the strings.
-  CStr m_Id;
+	NONCOPYABLE(COListColumn);
-  float m_Width;
+	MOVABLE(COListColumn);
-  CStrW m_Heading;
+	COListColumn() = default;
 
+	CGUIColor m_TextColor;
+	CStr m_Id;
+	float m_Width;
+	CStrW m_Heading;
 };
 
 /**

source/gui/CText.cpp

@@ -75,9 +75,10 @@ void CText::SetupText()
 		// TODO Gee: (2004-08-14) Don't define standard like this. Do it with the default style.
 		font = L"default";
 
-	CGUIString caption;
+	CGUIString* caption = nullptr;
+	GUI<CGUIString>::GetSettingPointer(this, "caption", caption);
+
 	bool scrollbar;
-	GUI<CGUIString>::GetSetting(this, "caption", caption);
 	GUI<bool>::GetSetting(this, "scrollbar", scrollbar);
 
 	float width = m_CachedActualSize.GetWidth();
@@ -88,7 +89,7 @@ void CText::SetupText()
 
     float buffer_zone = 0.f;
 	GUI<float>::GetSetting(this, "buffer_zone", buffer_zone);
-	*m_GeneratedTexts[0] = GetGUI()->GenerateText(caption, font, width, buffer_zone, this);
+	*m_GeneratedTexts[0] = GetGUI()->GenerateText(*caption, font, width, buffer_zone, this);
 
 	if (!scrollbar)
 		CalculateTextPosition(m_CachedActualSize, m_TextPos, *m_GeneratedTexts[0]);

source/gui/CTooltip.cpp

@@ -75,13 +75,13 @@ void CTooltip::SetupText()
 	float buffer_zone = 0.f;
 	GUI<float>::GetSetting(this, "buffer_zone", buffer_zone);
 
-	CGUIString caption;
+	CGUIString* caption = nullptr;
-	GUI<CGUIString>::GetSetting(this, "caption", caption);
+	GUI<CGUIString>::GetSettingPointer(this, "caption", caption);
 
 	float max_width = 0.f;
 	GUI<float>::GetSetting(this, "maxwidth", max_width);
 
-	*m_GeneratedTexts[0] = GetGUI()->GenerateText(caption, font, max_width, buffer_zone, this);
+	*m_GeneratedTexts[0] = GetGUI()->GenerateText(*caption, font, max_width, buffer_zone, this);
 
 
 	// Position the tooltip relative to the mouse:

source/gui/GUIManager.cpp

@@ -96,7 +96,7 @@ void CGUIManager::PushPage(const CStrW& pageName, shared_ptr<ScriptInterface::St
 {
 	// Push the page prior to loading its contents, because that may push
 	// another GUI page on init which should be pushed on top of this new page.
-	m_PageStack.emplace_back(SGUIPage(pageName, initData));
+	m_PageStack.emplace_back(pageName, initData);
 	m_PageStack.back().LoadPage(m_ScriptRuntime);
 	ResetCursor();
 }

source/gui/GUIManager.h

@@ -148,6 +148,8 @@ public:
 private:
 	struct SGUIPage
 	{
+		// COPYABLE, because event handlers may invalidate page stack iterators by open or close pages,
+		// and event handlers need to be called for the entire stack.
 		SGUIPage(const CStrW& pageName, const shared_ptr<ScriptInterface::StructuredClone> initData);
 		void LoadPage(shared_ptr<ScriptRuntime> scriptRuntime);
 

source/gui/GUIRenderer.h

@@ -32,14 +32,6 @@ struct SGUIImage;
 
 namespace GUIRenderer
 {
-	class IGLState
-	{
-	public:
-		virtual ~IGLState() {};
-		virtual void Set(const CTexturePtr& tex) = 0;
-		virtual void Unset() = 0;
-	};
-
 	struct SDrawCall
 	{
 		SDrawCall(const SGUIImage* image) : m_Image(image) {}

source/gui/GUITooltip.h

@@ -1,4 +1,4 @@
-/* Copyright (C) 2015 Wildfire Games.
+/* Copyright (C) 2019 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -27,6 +27,8 @@ class CGUI;
 class GUITooltip
 {
 public:
+	NONCOPYABLE(GUITooltip);
+
 	GUITooltip();
 	void Update(IGUIObject* Nearest, const CPos& MousePos, CGUI* GUI);
 

source/gui/GUIbase.h

@@ -95,6 +95,9 @@ enum EGUIMessageType
  */
 struct SGUIMessage
 {
+	// This should be passed as a const reference or pointer.
+	NONCOPYABLE(SGUIMessage);
+
 	SGUIMessage(EGUIMessageType _type) : type(_type), skipped(false) {}
 	SGUIMessage(EGUIMessageType _type, const CStr& _value) : type(_type), value(_value), skipped(false) {}
 
@@ -173,6 +176,8 @@ struct SGUIIcon
 class CClientArea
 {
 public:
+	// COPYABLE, since there are only primitives involved, making move and copy identical,
+	// and since some temporaries cannot be avoided.
 	CClientArea();
 	CClientArea(const CStr& Value);
 	CClientArea(const CRect& pixel, const CRect& percent);

source/gui/GUItext.cpp

@@ -165,7 +165,7 @@ void CGUIString::GenerateTextCall(const CGUI* pGUI, SFeedback& Feedback, CStrInt
 				TextCall.m_pSpriteCall = &Feedback.m_SpriteCalls.back();
 
 				// Add text call
-				Feedback.m_TextCalls.push_back(TextCall);
+				Feedback.m_TextCalls.emplace_back(std::move(TextCall));
 
 				break;
 			}
@@ -227,7 +227,7 @@ void CGUIString::GenerateTextCall(const CGUI* pGUI, SFeedback& Feedback, CStrInt
 				Feedback.m_NewLine = true;
 
 			// Add text-chunk
-			Feedback.m_TextCalls.push_back(TextCall);
+			Feedback.m_TextCalls.emplace_back(std::move(TextCall));
 		}
 	}
 }

source/gui/GUItext.h

@@ -60,6 +60,10 @@ struct SGUIText
 	 */
 	struct SSpriteCall
 	{
+		// The CGUISpriteInstance makes this uncopyable to avoid invalidating its draw cache
+		NONCOPYABLE(SSpriteCall);
+		MOVABLE(SSpriteCall);
+
 		SSpriteCall() : m_CellID(0) {}
 
 		/**
@@ -90,6 +94,9 @@ struct SGUIText
 	 */
 	struct STextCall
 	{
+		NONCOPYABLE(STextCall);
+		MOVABLE(STextCall);
+
 		STextCall() :
 			m_UseCustomColor(false),
 			m_Bold(false), m_Italic(false), m_Underlined(false),
@@ -248,6 +255,11 @@ public:
 	 */
 	struct SFeedback
 	{
+		// Avoid copying the vector and list containers.
+		NONCOPYABLE(SFeedback);
+		MOVABLE(SFeedback);
+		SFeedback() = default;
+
 		// Constants
 		static const int Left = 0;
 		static const int Right = 1;

source/gui/GUItypes.h

@@ -25,20 +25,23 @@ places, and to make it much easier to add a new type). Just do
 to handle every possible type.
 */
 
+#ifndef GUITYPE_IGNORE_COPYABLE
 TYPE(bool)
 TYPE(i32)
 TYPE(u32)
 TYPE(float)
 TYPE(CGUIColor)
-TYPE(CClientArea)
-TYPE(CGUIString)
-#ifndef GUITYPE_IGNORE_CGUISpriteInstance
-TYPE(CGUISpriteInstance)
-#endif
 TYPE(CStr)
 TYPE(CStrW)
 TYPE(EAlign)
 TYPE(EVAlign)
 TYPE(CPos)
+#endif
+
+#ifndef GUITYPE_IGNORE_NONCOPYABLE
+TYPE(CClientArea)
 TYPE(CGUIList)
 TYPE(CGUISeries)
+TYPE(CGUISpriteInstance)
+TYPE(CGUIString)
+#endif

source/gui/GUIutil.cpp

@@ -388,22 +388,21 @@ PSRETURN GUI<T>::SetSettingWrap(IGUIObject* pObject, const CStr& Setting, const
 }
 
 // Instantiate templated functions:
+// These functions avoid copies by working with a pointer and move semantics.
 #define TYPE(T) \
 	template PSRETURN GUI<T>::GetSettingPointer(const IGUIObject* pObject, const CStr& Setting, T*& Value); \
-	template PSRETURN GUI<T>::GetSetting(const IGUIObject* pObject, const CStr& Setting, T& Value); \
 	template PSRETURN GUI<T>::SetSetting(IGUIObject* pObject, const CStr& Setting, T& Value, const bool& SkipMessage); \
-	template PSRETURN GUI<T>::SetSetting(IGUIObject* pObject, const CStr& Setting, const T& Value, const bool& SkipMessage); \
 	template class CGUISetting<T>; \
 
-#define GUITYPE_IGNORE_CGUISpriteInstance
 #include "GUItypes.h"
-#undef GUITYPE_IGNORE_CGUISpriteInstance
 #undef TYPE
 
-// Don't instantiate GetSetting<CGUISpriteInstance> - this will cause linker errors if
+// Copying functions - discouraged except for primitives.
-// you attempt to retrieve a sprite using GetSetting, since that copies the sprite
+#define TYPE(T) \
-// and will mess up the caching performed by DrawSprite. You have to use GetSettingPointer
+	template PSRETURN GUI<T>::GetSetting(const IGUIObject* pObject, const CStr& Setting, T& Value); \
-// instead. (This is mainly useful to stop me accidentally using the wrong function.)
+	template PSRETURN GUI<T>::SetSetting(IGUIObject* pObject, const CStr& Setting, const T& Value, const bool& SkipMessage); \
-template PSRETURN GUI<CGUISpriteInstance>::GetSettingPointer(const IGUIObject* pObject, const CStr& Setting, CGUISpriteInstance*& Value);
+
-template PSRETURN GUI<CGUISpriteInstance>::SetSetting(IGUIObject* pObject, const CStr& Setting, CGUISpriteInstance& Value, const bool& SkipMessage);
+#define GUITYPE_IGNORE_NONCOPYABLE
-template class CGUISetting<CGUISpriteInstance>;
+#include "GUItypes.h"
+#undef GUITYPE_IGNORE_NONCOPYABLE
+#undef TYPE

source/gui/GUIutil.h

@@ -46,7 +46,10 @@ template<typename T> class GUI;
 class IGUISetting
 {
 public:
-	virtual ~IGUISetting() {};
+	NONCOPYABLE(IGUISetting);
+
+	IGUISetting() = default;
+	virtual ~IGUISetting() = default;
 
 	/**
 	 * Parses the given string and assigns to the setting value. Used for parsing XML attributes.
@@ -70,6 +73,8 @@ class CGUISetting : public IGUISetting
 	friend class GUI<T>;
 
 public:
+	NONCOPYABLE(CGUISetting);
+
 	CGUISetting(IGUIObject& pObject, const CStr& Name);
 
 	/**
@@ -125,6 +130,7 @@ class GUI
 	friend class IGUIObject;
 
 public:
+	NONCOPYABLE(GUI);
 
 	// Like GetSetting (below), but doesn't make a copy of the value
 	// (so it can be modified later)

source/gui/IGUIObject.cpp

@@ -238,16 +238,16 @@ void IGUIObject::UpdateCachedSize()
 	float aspectratio = 0.f;
 	GUI<float>::GetSetting(this, "aspectratio", aspectratio);
 
-	CClientArea ca;
+	CClientArea* ca;
-	GUI<CClientArea>::GetSetting(this, "size", ca);
+	GUI<CClientArea>::GetSettingPointer(this, "size", ca);
 
 	// If absolute="false" and the object has got a parent,
 	//  use its cached size instead of the screen. Notice
 	//  it must have just been cached for it to work.
 	if (absolute == false && m_pParent && !IsRootObject())
-		m_CachedActualSize = ca.GetClientArea(m_pParent->m_CachedActualSize);
+		m_CachedActualSize = ca->GetClientArea(m_pParent->m_CachedActualSize);
 	else
-		m_CachedActualSize = ca.GetClientArea(CRect(0.f, 0.f, g_xres / g_GuiScale, g_yres / g_GuiScale));
+		m_CachedActualSize = ca->GetClientArea(CRect(0.f, 0.f, g_xres / g_GuiScale, g_yres / g_GuiScale));
 
 	// In a few cases, GUI objects have to resize to fill the screen
 	// but maintain a constant aspect ratio.

source/gui/IGUIObject.h

@@ -60,6 +60,8 @@ class IGUIObject
 	friend bool JSI_IGUIObject::getComputedSize(JSContext* cx, uint argc, JS::Value* vp);
 
 public:
+	NONCOPYABLE(IGUIObject);
+
 	IGUIObject(CGUI* pGUI);
 	virtual ~IGUIObject();
 

source/gui/IGUIScrollBar.h

@@ -160,6 +160,8 @@ struct SGUIScrollBarStyle
 class IGUIScrollBar
 {
 public:
+	NONCOPYABLE(IGUIScrollBar);
+
 	IGUIScrollBar(CGUI* pGUI);
 	virtual ~IGUIScrollBar();
 

source/gui/MiniMap.cpp

@@ -316,6 +316,7 @@ void CMiniMap::DrawViewRect(CMatrix3D transform)
 
 struct MinimapUnitVertex
 {
+	// This struct is copyable for convenience and because to move is to copy for primitives.
 	u8 r, g, b, a;
 	float x, y;
 };