Index: ps/trunk/source/gui/CButton.cpp =================================================================== --- ps/trunk/source/gui/CButton.cpp +++ ps/trunk/source/gui/CButton.cpp @@ -68,12 +68,12 @@ // TODO Gee: (2004-08-14) Default should not be hard-coded, but be in styles! font = L"default"; - CGUIString caption; - GUI::GetSetting(this, "caption", caption); + CGUIString* caption = nullptr; + GUI::GetSettingPointer(this, "caption", caption); float buffer_zone = 0.f; GUI::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]); } Index: ps/trunk/source/gui/CChart.h =================================================================== --- ps/trunk/source/gui/CChart.h +++ ps/trunk/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 m_Points; }; Index: ps/trunk/source/gui/CCheckBox.cpp =================================================================== --- ps/trunk/source/gui/CCheckBox.cpp +++ ps/trunk/source/gui/CCheckBox.cpp @@ -80,12 +80,12 @@ float square_side; GUI::GetSetting(this, "square_side", square_side); - CGUIString caption; - GUI::GetSetting(this, "caption", caption); + CGUIString* caption = nullptr; + GUI::GetSettingPointer(this, "caption", caption); float buffer_zone = 0.f; GUI::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) Index: ps/trunk/source/gui/CGUI.cpp =================================================================== --- ps/trunk/source/gui/CGUI.cpp +++ ps/trunk/source/gui/CGUI.cpp @@ -488,6 +488,8 @@ // 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 @@ // 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); - Text.m_SpriteCalls.push_back(std::move(SpriteCall)); + Images[j].emplace_back(Image); + Text.m_SpriteCalls.emplace_back(std::move(SpriteCall)); } } } Index: ps/trunk/source/gui/CGUIList.h =================================================================== --- ps/trunk/source/gui/CGUIList.h +++ ps/trunk/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 @@ 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. Index: ps/trunk/source/gui/CGUISeries.h =================================================================== --- ps/trunk/source/gui/CGUISeries.h +++ ps/trunk/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> m_Series; }; Index: ps/trunk/source/gui/COList.h =================================================================== --- ps/trunk/source/gui/COList.h +++ ps/trunk/source/gui/COList.h @@ -25,11 +25,15 @@ */ struct COListColumn { - CGUIColor m_TextColor; - CStr m_Id; - float m_Width; - CStrW m_Heading; - + // Avoid copying the strings. + NONCOPYABLE(COListColumn); + MOVABLE(COListColumn); + COListColumn() = default; + + CGUIColor m_TextColor; + CStr m_Id; + float m_Width; + CStrW m_Heading; }; /** Index: ps/trunk/source/gui/COList.cpp =================================================================== --- ps/trunk/source/gui/COList.cpp +++ ps/trunk/source/gui/COList.cpp @@ -297,20 +297,18 @@ } } - m_Columns.push_back(column); - AddSetting("list_" + column.m_Id); AddSetting("hidden_" + column.m_Id); GUI::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) Index: ps/trunk/source/gui/CText.cpp =================================================================== --- ps/trunk/source/gui/CText.cpp +++ ps/trunk/source/gui/CText.cpp @@ -75,9 +75,10 @@ // 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::GetSettingPointer(this, "caption", caption); + bool scrollbar; - GUI::GetSetting(this, "caption", caption); GUI::GetSetting(this, "scrollbar", scrollbar); float width = m_CachedActualSize.GetWidth(); @@ -88,7 +89,7 @@ float buffer_zone = 0.f; GUI::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]); Index: ps/trunk/source/gui/CTooltip.cpp =================================================================== --- ps/trunk/source/gui/CTooltip.cpp +++ ps/trunk/source/gui/CTooltip.cpp @@ -75,13 +75,13 @@ float buffer_zone = 0.f; GUI::GetSetting(this, "buffer_zone", buffer_zone); - CGUIString caption; - GUI::GetSetting(this, "caption", caption); + CGUIString* caption = nullptr; + GUI::GetSettingPointer(this, "caption", caption); float max_width = 0.f; GUI::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: Index: ps/trunk/source/gui/GUIManager.h =================================================================== --- ps/trunk/source/gui/GUIManager.h +++ ps/trunk/source/gui/GUIManager.h @@ -148,6 +148,8 @@ 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 initData); void LoadPage(shared_ptr scriptRuntime); Index: ps/trunk/source/gui/GUIManager.cpp =================================================================== --- ps/trunk/source/gui/GUIManager.cpp +++ ps/trunk/source/gui/GUIManager.cpp @@ -96,7 +96,7 @@ { // 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(); } Index: ps/trunk/source/gui/GUIRenderer.h =================================================================== --- ps/trunk/source/gui/GUIRenderer.h +++ ps/trunk/source/gui/GUIRenderer.h @@ -32,14 +32,6 @@ 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) {} Index: ps/trunk/source/gui/GUITooltip.h =================================================================== --- ps/trunk/source/gui/GUITooltip.h +++ ps/trunk/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 GUITooltip { public: + NONCOPYABLE(GUITooltip); + GUITooltip(); void Update(IGUIObject* Nearest, const CPos& MousePos, CGUI* GUI); Index: ps/trunk/source/gui/GUIbase.h =================================================================== --- ps/trunk/source/gui/GUIbase.h +++ ps/trunk/source/gui/GUIbase.h @@ -95,6 +95,9 @@ */ 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 @@ 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); Index: ps/trunk/source/gui/GUItext.h =================================================================== --- ps/trunk/source/gui/GUItext.h +++ ps/trunk/source/gui/GUItext.h @@ -60,6 +60,10 @@ */ 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 STextCall { + NONCOPYABLE(STextCall); + MOVABLE(STextCall); + STextCall() : m_UseCustomColor(false), m_Bold(false), m_Italic(false), m_Underlined(false), @@ -248,6 +255,11 @@ */ 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; Index: ps/trunk/source/gui/GUItext.cpp =================================================================== --- ps/trunk/source/gui/GUItext.cpp +++ ps/trunk/source/gui/GUItext.cpp @@ -165,7 +165,7 @@ 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 @@ Feedback.m_NewLine = true; // Add text-chunk - Feedback.m_TextCalls.push_back(TextCall); + Feedback.m_TextCalls.emplace_back(std::move(TextCall)); } } } Index: ps/trunk/source/gui/GUItypes.h =================================================================== --- ps/trunk/source/gui/GUItypes.h +++ ps/trunk/source/gui/GUItypes.h @@ -25,20 +25,23 @@ 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 Index: ps/trunk/source/gui/GUIutil.h =================================================================== --- ps/trunk/source/gui/GUIutil.h +++ ps/trunk/source/gui/GUIutil.h @@ -46,7 +46,10 @@ 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 @@ friend class GUI; public: + NONCOPYABLE(CGUISetting); + CGUISetting(IGUIObject& pObject, const CStr& Name); /** @@ -125,6 +130,7 @@ friend class IGUIObject; public: + NONCOPYABLE(GUI); // Like GetSetting (below), but doesn't make a copy of the value // (so it can be modified later) Index: ps/trunk/source/gui/GUIutil.cpp =================================================================== --- ps/trunk/source/gui/GUIutil.cpp +++ ps/trunk/source/gui/GUIutil.cpp @@ -388,22 +388,21 @@ } // Instantiate templated functions: +// These functions avoid copies by working with a pointer and move semantics. #define TYPE(T) \ template PSRETURN GUI::GetSettingPointer(const IGUIObject* pObject, const CStr& Setting, T*& Value); \ - template PSRETURN GUI::GetSetting(const IGUIObject* pObject, const CStr& Setting, T& Value); \ template PSRETURN GUI::SetSetting(IGUIObject* pObject, const CStr& Setting, T& Value, const bool& SkipMessage); \ - template PSRETURN GUI::SetSetting(IGUIObject* pObject, const CStr& Setting, const T& Value, const bool& SkipMessage); \ template class CGUISetting; \ -#define GUITYPE_IGNORE_CGUISpriteInstance #include "GUItypes.h" -#undef GUITYPE_IGNORE_CGUISpriteInstance #undef TYPE -// Don't instantiate GetSetting - this will cause linker errors if -// you attempt to retrieve a sprite using GetSetting, since that copies the sprite -// and will mess up the caching performed by DrawSprite. You have to use GetSettingPointer -// instead. (This is mainly useful to stop me accidentally using the wrong function.) -template PSRETURN GUI::GetSettingPointer(const IGUIObject* pObject, const CStr& Setting, CGUISpriteInstance*& Value); -template PSRETURN GUI::SetSetting(IGUIObject* pObject, const CStr& Setting, CGUISpriteInstance& Value, const bool& SkipMessage); -template class CGUISetting; +// Copying functions - discouraged except for primitives. +#define TYPE(T) \ + template PSRETURN GUI::GetSetting(const IGUIObject* pObject, const CStr& Setting, T& Value); \ + template PSRETURN GUI::SetSetting(IGUIObject* pObject, const CStr& Setting, const T& Value, const bool& SkipMessage); \ + +#define GUITYPE_IGNORE_NONCOPYABLE +#include "GUItypes.h" +#undef GUITYPE_IGNORE_NONCOPYABLE +#undef TYPE Index: ps/trunk/source/gui/IGUIObject.h =================================================================== --- ps/trunk/source/gui/IGUIObject.h +++ ps/trunk/source/gui/IGUIObject.h @@ -60,6 +60,8 @@ friend bool JSI_IGUIObject::getComputedSize(JSContext* cx, uint argc, JS::Value* vp); public: + NONCOPYABLE(IGUIObject); + IGUIObject(CGUI* pGUI); virtual ~IGUIObject(); Index: ps/trunk/source/gui/IGUIObject.cpp =================================================================== --- ps/trunk/source/gui/IGUIObject.cpp +++ ps/trunk/source/gui/IGUIObject.cpp @@ -238,16 +238,16 @@ float aspectratio = 0.f; GUI::GetSetting(this, "aspectratio", aspectratio); - CClientArea ca; - GUI::GetSetting(this, "size", ca); + CClientArea* ca; + GUI::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. Index: ps/trunk/source/gui/IGUIScrollBar.h =================================================================== --- ps/trunk/source/gui/IGUIScrollBar.h +++ ps/trunk/source/gui/IGUIScrollBar.h @@ -160,6 +160,8 @@ class IGUIScrollBar { public: + NONCOPYABLE(IGUIScrollBar); + IGUIScrollBar(CGUI* pGUI); virtual ~IGUIScrollBar(); Index: ps/trunk/source/gui/MiniMap.cpp =================================================================== --- ps/trunk/source/gui/MiniMap.cpp +++ ps/trunk/source/gui/MiniMap.cpp @@ -316,6 +316,7 @@ 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; };