forked from 0ad/0ad
MOVABLE idiom, const CGUI struct maps, in place move construction instead of copying temporaries during CGUI XML loading and GenerateText.
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:
parent
f804226b89
commit
3028551b91
@ -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)
|
||||
|
@ -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
|
||||
|
@ -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)
|
||||
{
|
||||
|
@ -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);
|
||||
|
@ -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())
|
||||
{
|
||||
/*
|
||||
|
@ -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);
|
||||
}
|
||||
|
@ -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
|
||||
|
@ -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;
|
||||
|
@ -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 */
|
||||
//--------------------------------------------------------
|
||||
|
@ -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
|
||||
|
Loading…
Reference in New Issue
Block a user