Stan/0ad
1
0
Fork 0
forked from 0ad/0ad
Code Pull Requests Activity

MOVABLE idiom, const CGUI struct maps, in place move construction instead of copying temporaries during CGUI XML loading and GenerateText.

Browse Source 
Introduce MOVABLE idiom indicating that a class can use move semantics.
Make values of CGUI struct maps holding XML data const to ensure at the
root that the data is not modified.
Use NONCOPYABLE and MOVABLE for SGUIIcon and SGUIStyle to enforce the
non-copy policy on the compiler level (until someone changes the GUI
design to make modifications needed).

As indicated by that:
  Replace copy operations by in place move operations for these CGUI
struct maps in the CGUI Xeromyces XML loading functions.
  Replace copy operations by const references for CSize and SGUIIcon in
CGUIString::GenerateTextCall and CGUI::GenerateText.
  This avoids copying of non primitive members, such as the strings and
containers of strings.

Further related cleanup to be kept separated for auditability.

Differential Revision: https://code.wildfiregames.com/D2163
Few comments on IRC by: wraitii, Itms
Tested on: gcc 9, Jenkins, partially VS2015

Refs #1984,
  NONCOPYABLE CGUISpriteInstances: 0a7d0ecdde, 8f4f8e240f, c19f3608a5
  NONCOPYABLE Image, Sprite: fb8032043b
  NONCOPYABLE GUI page: 94c57085e9
  NONCOPYABLE GUIManager: 7c2e9027c2
  NONCOPYABLE macro: 16ccae10cd

This was SVN commit r22637.
This commit is contained in:
elexis 2019-08-09 17:25:55 +00:00
parent f804226b89
commit 3028551b91
10 changed files with 53 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
source/gui/CGUI.cpp
View File

@ -383,7 +383,7 @@ void CGUI::Destroy()
}
m_pAllObjects.clear();
for (const std::pair<CStr, CGUISprite*>& p : m_Sprites)
for (const std::pair<CStr, const CGUISprite*>& p : m_Sprites)
delete p.second;
m_Sprites.clear();
m_Icons.clear();
@ -478,7 +478,7 @@ void CGUI::SetFocusedObject(IGUIObject* pObject)
const SGUIScrollBarStyle* CGUI::GetScrollBarStyle(const CStr& style) const
{
std::map<CStr, SGUIScrollBarStyle>::const_iterator it = m_ScrollBarStyles.find(style);
std::map<CStr, const SGUIScrollBarStyle>::const_iterator it = m_ScrollBarStyles.find(style);
if (it == m_ScrollBarStyles.end())
return nullptr;
@ -594,9 +594,9 @@ SGUIText CGUI::GenerateText(const CGUIString& string, const CStrW& FontW, const
_y = y;
// Get Size from Icon database
SGUIIcon icon = GetIcon(imgname);
const SGUIIcon& icon = GetIcon(imgname);
CSize size = icon.m_Size;
const CSize& size = icon.m_Size;
Image.SetupSpriteCall((j == CGUIString::SFeedback::Left), SpriteCall, Width, _y, size, icon.m_SpriteName, BufferZone, icon.m_CellID);
// Check if image is the lowest thing.
@ -1423,7 +1423,8 @@ void CGUI::Xeromyces_ReadSprite(XMBElement Element, CXeromyces* pFile)
delete effects;
m_Sprites[name] = Sprite;
m_Sprites.erase(name);
m_Sprites.emplace(name, Sprite);
}
void CGUI::Xeromyces_ReadImage(XMBElement Element, CXeromyces* pFile, CGUISprite& parent)
@ -1595,10 +1596,11 @@ void CGUI::Xeromyces_ReadStyle(XMBElement Element, CXeromyces* pFile)
if (attr_name == "name")
name = attr.Value;
else
style.m_SettingsDefaults[attr_name] = attr.Value.FromUTF8();
style.m_SettingsDefaults.emplace(attr_name, attr.Value.FromUTF8());
}
m_Styles[name] = style;
m_Styles.erase(name);
m_Styles.emplace(name, std::move(style));
}
void CGUI::Xeromyces_ReadScrollBarStyle(XMBElement Element, CXeromyces* pFile)
@ -1680,7 +1682,8 @@ void CGUI::Xeromyces_ReadScrollBarStyle(XMBElement Element, CXeromyces* pFile)
scrollbar.m_SpriteBarVerticalPressed = attr_value;
}
m_ScrollBarStyles[name] = std::move(scrollbar);
m_ScrollBarStyles.erase(name);
m_ScrollBarStyles.emplace(name, std::move(scrollbar));
}
void CGUI::Xeromyces_ReadIcon(XMBElement Element, CXeromyces* pFile)
@ -1720,7 +1723,8 @@ void CGUI::Xeromyces_ReadIcon(XMBElement Element, CXeromyces* pFile)
debug_warn(L"Invalid data - DTD shouldn't allow this");
}
m_Icons[name] = icon;
m_Icons.erase(name);
m_Icons.emplace(name, std::move(icon));
}
void CGUI::Xeromyces_ReadTooltip(XMBElement Element, CXeromyces* pFile)

19
source/gui/CGUI.h
View File

@ -42,6 +42,11 @@ extern const double SELECT_DBLCLICK_RATE;
*/
struct SGUIStyle
{
// Take advantage of moving the entire map instead and avoiding unintended copies.
NONCOPYABLE(SGUIStyle);
MOVABLE(SGUIStyle);
SGUIStyle() = default;
std::map<CStr, CStrW> m_SettingsDefaults;
};
@ -235,9 +240,9 @@ public:
bool IconExists(const CStr& str) const { return (m_Icons.count(str) != 0); }
/**
* Get Icon (a copy, can never be changed)
* Get Icon (a const reference, can never be changed)
*/
SGUIIcon GetIcon(const CStr& str) const { return m_Icons.find(str)->second; }
const SGUIIcon& GetIcon(const CStr& str) const { return m_Icons.find(str)->second; }
/**
* Get pre-defined color (if it exists)
@ -628,19 +633,21 @@ private:
//--------------------------------------------------------
// Databases
// These are loaded from XML files and marked as noncopyable and const to
// rule out unintentional modification and copy, especially during Draw calls.
//--------------------------------------------------------
// Sprites
std::map<CStr, CGUISprite*> m_Sprites;
std::map<CStr, const CGUISprite*> m_Sprites;
// Styles
std::map<CStr, SGUIStyle> m_Styles;
std::map<CStr, const SGUIStyle> m_Styles;
// Scroll-bar styles
std::map<CStr, SGUIScrollBarStyle> m_ScrollBarStyles;
std::map<CStr, const SGUIScrollBarStyle> m_ScrollBarStyles;
// Icons
std::map<CStr, SGUIIcon> m_Icons;
std::map<CStr, const SGUIIcon> m_Icons;
};
#endif // INCLUDED_CGUI

2
source/gui/CGUISprite.cpp
View File

@ -30,7 +30,7 @@ void CGUISprite::AddImage(SGUIImage* image)
m_Images.push_back(image);
}
void CGUISpriteInstance::Draw(const CRect& Size, int CellID, std::map<CStr, CGUISprite*>& Sprites, float Z) const
void CGUISpriteInstance::Draw(const CRect& Size, int CellID, std::map<CStr, const CGUISprite*>& Sprites, float Z) const
{
if (m_CachedSize != Size || m_CachedCellID != CellID)
{

10
source/gui/CGUISprite.h
View File

@ -157,16 +157,14 @@ public:
// calculations between draw calls.
class CGUISpriteInstance
{
NONCOPYABLE(CGUISpriteInstance);
public:
NONCOPYABLE(CGUISpriteInstance);
MOVABLE(CGUISpriteInstance);
CGUISpriteInstance();
CGUISpriteInstance(const CStr& SpriteName);
CGUISpriteInstance(CGUISpriteInstance&&) = default;
CGUISpriteInstance& operator=(CGUISpriteInstance&&) = default;
void Draw(const CRect& Size, int CellID, std::map<CStr, CGUISprite*>& Sprites, float Z) const;
void Draw(const CRect& Size, int CellID, std::map<CStr, const CGUISprite*>& Sprites, float Z) const;
bool IsEmpty() const;
const CStr& GetName() const { return m_SpriteName; }
void SetName(const CStr& SpriteName);

4
source/gui/GUIRenderer.cpp
View File

@ -59,7 +59,7 @@ DrawCalls& DrawCalls::operator=(const DrawCalls&)
}
void GUIRenderer::UpdateDrawCallCache(DrawCalls& Calls, const CStr& SpriteName, const CRect& Size, int CellID, std::map<CStr, CGUISprite*>& Sprites)
void GUIRenderer::UpdateDrawCallCache(DrawCalls& Calls, const CStr& SpriteName, const CRect& Size, int CellID, std::map<CStr, const CGUISprite*>& Sprites)
{
// This is called only when something has changed (like the size of the
// sprite), so it doesn't need to be particularly efficient.
@ -74,7 +74,7 @@ void GUIRenderer::UpdateDrawCallCache(DrawCalls& Calls, const CStr& SpriteName,
return;
std::map<CStr, CGUISprite*>::iterator it(Sprites.find(SpriteName));
std::map<CStr, const CGUISprite*>::iterator it(Sprites.find(SpriteName));
if (it == Sprites.end())
{
/*

2
source/gui/GUIRenderer.h
View File

@ -79,7 +79,7 @@ namespace GUIRenderer
namespace GUIRenderer
{
void UpdateDrawCallCache(DrawCalls& Calls, const CStr& SpriteName, const CRect& Size, int CellID, std::map<CStr, CGUISprite*>& Sprites);
void UpdateDrawCallCache(DrawCalls& Calls, const CStr& SpriteName, const CRect& Size, int CellID, std::map<CStr, const CGUISprite*>& Sprites);
void Draw(DrawCalls& Calls, float Z);
}

4
source/gui/GUIbase.h
View File

@ -148,6 +148,10 @@ typedef std::vector<IGUIObject*> vector_pObjects;
// you use them in text owned by different objects... Such as CText.
struct SGUIIcon
{
// This struct represents an immutable type, so ensure to avoid copying the strings.
NONCOPYABLE(SGUIIcon);
MOVABLE(SGUIIcon);
SGUIIcon() : m_CellID(0) {}
// Sprite name of icon

4
source/gui/GUItext.cpp
View File

@ -124,9 +124,9 @@ void CGUIString::GenerateTextCall(const CGUI* pGUI, SFeedback& Feedback, CStrInt
SGUIText::SSpriteCall SpriteCall;
// Get Icon from icon database in pGUI
SGUIIcon icon = pGUI->GetIcon(path);
const SGUIIcon& icon = pGUI->GetIcon(path);
CSize size = icon.m_Size;
const CSize& size = icon.m_Size;
// append width, and make maximum height the height.
Feedback.m_Size.cx += size.cx;

5
source/gui/IGUIScrollBar.h
View File

@ -49,6 +49,11 @@ A GUI ScrollBar
*/
struct SGUIScrollBarStyle
{
// CGUISpriteInstance makes this NONCOPYABLE implicitly, make it explicit
NONCOPYABLE(SGUIScrollBarStyle);
MOVABLE(SGUIScrollBarStyle);
SGUIScrollBarStyle() = default;
//--------------------------------------------------------
/** @name General Settings */
//--------------------------------------------------------

8
source/lib/code_annotation.h
View File

@ -218,6 +218,14 @@ switch(x % 2)
className(const className&) = delete; \
className& operator=(const className&) = delete
/**
* Indicates that move semantics can be used, so that a NONCOPYABLE class can still be assigned by taking over the reference to the value.
* Make sure to use the macro with the necessary access modifier.
*/
#define MOVABLE(className) \
className(className&&) = default; \
className& operator=(className&&) = default
#if ICC_VERSION
# define ASSUME_ALIGNED(ptr, multiple) __assume_aligned(ptr, multiple)
#else