1
0
forked from 0ad/0ad

CGUISpriteInstance non-copyable and FromJSVal / ToJSVal.

Make CGUISpriteInstance non-copyable to further harden Philips
protection from 8f4f8e240f against unintentional copies of its DrawCall
cache such as in c19f3608a5.
Remove its copy constructor from 849f50a500, make it movable, and until
it becomes otherwise necessary, force move assignment when sprites are
assigned.
Improves the fixes of the compiler warnings about deprecated implicit
copy constructors in 8a32b0b3d4 by avoiding the copies instead of
copying explicitly.
Add ToJSVal, FromJSVal for CGUISprinteInstance to make
JSI_IGUIObject::getProperty and setProperty more consistent.
Rename Sprite operator= to SetName to reduce ambiguity.
Pass CRect by reference in CGUISpriteInstance::Draw.

Differential Revision: https://code.wildfiregames.com/D2133
Comments By: wraitii
This was SVN commit r22570.
This commit is contained in:
elexis 2019-07-28 22:40:58 +00:00
parent 9fcfdb0324
commit 0a7d0ecdde
8 changed files with 84 additions and 43 deletions

View File

@ -608,7 +608,7 @@ SGUIText CGUI::GenerateText(const CGUIString& string, const CStrW& FontW, const
Text.m_Size.cy = std::max(Text.m_Size.cy, Image.m_YTo);
Images[j].push_back(Image);
Text.m_SpriteCalls.push_back(SpriteCall);
Text.m_SpriteCalls.push_back(std::move(SpriteCall));
}
}
}
@ -784,7 +784,10 @@ SGUIText CGUI::GenerateText(const CGUIString& string, const CStrW& FontW, const
// Sprite call can exist within only a newline segment,
// therefore we need this.
Text.m_SpriteCalls.insert(Text.m_SpriteCalls.end(), Feedback2.m_SpriteCalls.begin(), Feedback2.m_SpriteCalls.end());
Text.m_SpriteCalls.insert(
Text.m_SpriteCalls.end(),
std::make_move_iterator(Feedback2.m_SpriteCalls.begin()),
std::make_move_iterator(Feedback2.m_SpriteCalls.end()));
break;
}
else if (x > width_range[To] && j == temp_from)
@ -800,8 +803,15 @@ SGUIText CGUI::GenerateText(const CGUIString& string, const CStrW& FontW, const
}
// Add the whole Feedback2.m_TextCalls to our m_TextCalls.
Text.m_TextCalls.insert(Text.m_TextCalls.end(), Feedback2.m_TextCalls.begin(), Feedback2.m_TextCalls.end());
Text.m_SpriteCalls.insert(Text.m_SpriteCalls.end(), Feedback2.m_SpriteCalls.begin(), Feedback2.m_SpriteCalls.end());
Text.m_TextCalls.insert(
Text.m_TextCalls.end(),
std::make_move_iterator(Feedback2.m_TextCalls.begin()),
std::make_move_iterator(Feedback2.m_TextCalls.end()));
Text.m_SpriteCalls.insert(
Text.m_SpriteCalls.end(),
std::make_move_iterator(Feedback2.m_SpriteCalls.begin()),
std::make_move_iterator(Feedback2.m_SpriteCalls.end()));
if (j == (int)string.m_Words.size()-2)
done = true;
@ -1677,7 +1687,7 @@ void CGUI::Xeromyces_ReadScrollBarStyle(XMBElement Element, CXeromyces* pFile)
scrollbar.m_SpriteBarVerticalPressed = attr_value;
}
m_ScrollBarStyles[name] = scrollbar;
m_ScrollBarStyles[name] = std::move(scrollbar);
}
void CGUI::Xeromyces_ReadIcon(XMBElement Element, CXeromyces* pFile)

View File

@ -30,7 +30,7 @@ void CGUISprite::AddImage(SGUIImage* image)
m_Images.push_back(image);
}
void CGUISpriteInstance::Draw(CRect Size, int CellID, std::map<CStr, CGUISprite*>& Sprites, float Z) const
void CGUISpriteInstance::Draw(const CRect& Size, int CellID, std::map<CStr, CGUISprite*>& Sprites, float Z) const
{
if (m_CachedSize != Size || m_CachedCellID != CellID)
{
@ -41,12 +41,6 @@ void CGUISpriteInstance::Draw(CRect Size, int CellID, std::map<CStr, CGUISprite*
GUIRenderer::Draw(m_DrawCallCache, Z);
}
void CGUISpriteInstance::Invalidate()
{
m_CachedSize = CRect();
m_CachedCellID = -1;
}
bool CGUISpriteInstance::IsEmpty() const
{
return m_SpriteName.empty();
@ -66,21 +60,10 @@ CGUISpriteInstance::CGUISpriteInstance(const CStr& SpriteName)
{
}
CGUISpriteInstance::CGUISpriteInstance(const CGUISpriteInstance& Sprite)
: m_SpriteName(Sprite.m_SpriteName), m_CachedCellID(-1)
{
}
CGUISpriteInstance& CGUISpriteInstance::operator=(const CGUISpriteInstance& Sprite)
{
return this->operator=(Sprite.m_SpriteName);
}
CGUISpriteInstance& CGUISpriteInstance::operator=(const CStr& SpriteName)
void CGUISpriteInstance::SetName(const CStr& SpriteName)
{
m_SpriteName = SpriteName;
m_CachedSize = CRect();
m_DrawCallCache.clear();
Invalidate();
return *this;
m_CachedCellID = -1;
}

View File

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

View File

@ -159,7 +159,7 @@ void CGUIString::GenerateTextCall(const CGUI* pGUI, SFeedback& Feedback, CStrInt
SpriteCall.m_CellID = icon.m_CellID;
// Add sprite call
Feedback.m_SpriteCalls.push_back(SpriteCall);
Feedback.m_SpriteCalls.push_back(std::move(SpriteCall));
// Finalize text call
TextCall.m_pSpriteCall = &Feedback.m_SpriteCalls.back();

View File

@ -325,6 +325,24 @@ PSRETURN GUI<T>::GetSetting(const IGUIObject* pObject, const CStr& Setting, T& V
return ret;
}
template <typename T>
PSRETURN GUI<T>::SetSetting(IGUIObject* pObject, const CStr& Setting, T& Value, const bool& SkipMessage)
{
return SetSettingWrap(pObject, Setting, Value, SkipMessage,
[&pObject, &Setting, &Value]() {
*static_cast<T*>(pObject->m_Settings[Setting].m_pSetting) = std::move(Value);
});
}
template <typename T>
PSRETURN GUI<T>::SetSetting(IGUIObject* pObject, const CStr& Setting, const T& Value, const bool& SkipMessage)
{
return SetSettingWrap(pObject, Setting, Value, SkipMessage,
[&pObject, &Setting, &Value]() {
*static_cast<T*>(pObject->m_Settings[Setting].m_pSetting) = Value;
});
}
// Helper function for SetSetting
template <typename T>
bool IsBoolTrue(const T&)
@ -338,7 +356,7 @@ bool IsBoolTrue<bool>(const bool& v)
}
template <typename T>
PSRETURN GUI<T>::SetSetting(IGUIObject* pObject, const CStr& Setting, const T& Value, const bool& SkipMessage)
PSRETURN GUI<T>::SetSettingWrap(IGUIObject* pObject, const CStr& Setting, const T& Value, const bool& SkipMessage, const std::function<void()>& valueSet)
{
ENSURE(pObject != NULL);
@ -354,12 +372,9 @@ PSRETURN GUI<T>::SetSetting(IGUIObject* pObject, const CStr& Setting, const T& V
CheckType<T>(pObject, Setting);
#endif
// Set value
*(T*)pObject->m_Settings[Setting].m_pSetting = Value;
valueSet();
//
// Some settings needs special attention at change
//
// If setting was "size", we need to re-cache itself and all children
if (Setting == "size")
@ -386,6 +401,7 @@ PSRETURN GUI<T>::SetSetting(IGUIObject* pObject, const CStr& Setting, const T& V
#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);
#define GUITYPE_IGNORE_CGUISpriteInstance
#include "GUItypes.h"
@ -397,4 +413,4 @@ PSRETURN GUI<T>::SetSetting(IGUIObject* pObject, const CStr& Setting, const T& V
// 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<CGUISpriteInstance>::GetSettingPointer(const IGUIObject* pObject, const CStr& Setting, CGUISpriteInstance*& Value);
template PSRETURN GUI<CGUISpriteInstance>::SetSetting(IGUIObject* pObject, const CStr& Setting, const CGUISpriteInstance& Value, const bool& SkipMessage);
template PSRETURN GUI<CGUISpriteInstance>::SetSetting(IGUIObject* pObject, const CStr& Setting, CGUISpriteInstance& Value, const bool& SkipMessage);

View File

@ -37,6 +37,8 @@ GUI util
#include "gui/GUIbase.h"
#include "gui/IGUIObject.h"
#include <functional>
class CClientArea;
class CGUIString;
class CMatrix3D;
@ -90,11 +92,18 @@ public:
* This is the official way of setting a setting, no other
* way should only cautiously be used!
*
* This variant will use the move-assignment.
*
* @param pObject Object pointer
* @param Setting Setting by name
* @param Value Sets value to this, note type T!
* @param SkipMessage Does not send a GUIM_SETTINGS_UPDATED if true
*/
static PSRETURN SetSetting(IGUIObject* pObject, const CStr& Setting, T& Value, const bool& SkipMessage = false);
/**
* This variant will copy the value.
*/
static PSRETURN SetSetting(IGUIObject* pObject, const CStr& Setting, const T& Value, const bool& SkipMessage = false);
/**
@ -147,6 +156,12 @@ public:
private:
/**
* Changes the value of the setting by calling the valueSet functon that performs either a copy or move assignment.
* Updates some internal data depending on the setting changed.
*/
static PSRETURN SetSettingWrap(IGUIObject* pObject, const CStr& Setting, const T& Value, const bool& SkipMessage, const std::function<void()>& valueSet);
// templated typedef of function pointer
typedef void (IGUIObject::*void_Object_pFunction_argT)(const T& arg);
typedef void (IGUIObject::*void_Object_pFunction_argRefT)(T& arg);
@ -238,7 +253,6 @@ private:
RecurseObject(RR, obj, pFunc);
}
private:
/**
* Checks restrictions for the iteration, for instance if
* you tell the recursor to avoid all hidden objects, it

View File

@ -286,3 +286,18 @@ template<> bool ScriptInterface::FromJSVal<EAlign>(JSContext* cx, JS::HandleValu
}
return true;
}
template<> void ScriptInterface::ToJSVal<CGUISpriteInstance>(JSContext* cx, JS::MutableHandleValue ret, const CGUISpriteInstance& val)
{
ToJSVal(cx, ret, val.GetName());
}
template<> bool ScriptInterface::FromJSVal<CGUISpriteInstance>(JSContext* cx, JS::HandleValue v, CGUISpriteInstance& out)
{
std::string name;
if (!FromJSVal(cx, v, name))
return false;
out.SetName(name);
return true;
}

View File

@ -203,7 +203,7 @@ bool JSI_IGUIObject::getProperty(JSContext* cx, JS::HandleObject obj, JS::Handle
{
CGUISpriteInstance* value;
GUI<CGUISpriteInstance>::GetSettingPointer(e, propName, value);
ScriptInterface::ToJSVal(cx, vp, value->GetName());
ScriptInterface::ToJSVal(cx, vp, *value);
break;
}
@ -324,11 +324,11 @@ bool JSI_IGUIObject::setProperty(JSContext* cx, JS::HandleObject obj, JS::Handle
case GUIST_CGUISpriteInstance:
{
std::string value;
CGUISpriteInstance value;
if (!ScriptInterface::FromJSVal(cx, vp, value))
return false;
GUI<CGUISpriteInstance>::SetSetting(e, propName, CGUISpriteInstance(value));
GUI<CGUISpriteInstance>::SetSetting(e, propName, value);
break;
}