Fix #4029: Expire vector display instances after SVG destruction

Client/mods/deathmatch/logic/CClientDisplay.cpp

@@ -10,25 +10,16 @@
 
 #include <StdInc.h>
 
-CClientDisplay::CClientDisplay(CClientDisplayManager* pDisplayManager, unsigned long ulID)
+CClientDisplay::CClientDisplay(unsigned long ulID)
 {
-    m_pDisplayManager = pDisplayManager;
     m_ulID = ulID;
 
     m_ulExpirationTime = 0;
     m_bVisible = true;
     m_Color = SColorRGBA(255, 255, 255, 255);
-
-    m_pDisplayManager->AddToList(this);
-}
-
-CClientDisplay::~CClientDisplay()
-{
-    // Remove us from the manager
-    m_pDisplayManager->RemoveFromList(this);
 }
 
 void CClientDisplay::SetColorAlpha(unsigned char ucAlpha)
 {
     m_Color.A = ucAlpha;
-}
+}

Client/mods/deathmatch/logic/CClientDisplay.h

@@ -22,18 +22,16 @@ enum eDisplayType
 
 class CClientDisplay
 {
-    friend class CClientDisplayManager;
-
 public:
-    CClientDisplay(class CClientDisplayManager* pDisplayManager, unsigned long ulID);
-    virtual ~CClientDisplay();
+    CClientDisplay(unsigned long ulID);
+    virtual ~CClientDisplay() = default;
 
-    unsigned long        GetID() { return m_ulID; }
+    unsigned long        GetID() const { return m_ulID; }
     virtual eDisplayType GetType() = 0;
 
-    unsigned long GetExpirationTime() { return m_ulExpirationTime; };
+    unsigned long GetExpirationTime() const { return m_ulExpirationTime; };
     void          SetExpirationTime(unsigned long ulTime) { m_ulExpirationTime = ulTime; };
-    unsigned long GetTimeTillExpiration() { return m_ulExpirationTime - CClientTime::GetTime(); };
+    unsigned long GetTimeTillExpiration() const { return m_ulExpirationTime - CClientTime::GetTime(); };
     void          SetTimeTillExpiration(unsigned long ulMs) { m_ulExpirationTime = CClientTime::GetTime() + ulMs; };
 
     virtual const CVector& GetPosition() { return m_vecPosition; };
@@ -49,9 +47,7 @@ class CClientDisplay
     virtual void Render() = 0;
 
 protected:
-    bool IsExpired() { return (m_ulExpirationTime != 0 && (CClientTime::GetTime() > m_ulExpirationTime)); };
-
-    CClientDisplayManager* m_pDisplayManager;
+    bool IsExpired() const { return (m_ulExpirationTime != 0 && (CClientTime::GetTime() > m_ulExpirationTime)); };
 
     unsigned long m_ulID;
     unsigned long m_ulExpirationTime;

Client/mods/deathmatch/logic/CClientDisplayManager.cpp

@@ -12,27 +12,21 @@
 
 using std::list;
 
-CClientDisplayManager::CClientDisplayManager()
-{
-    // Init
-    m_bCanRemoveFromList = true;
-}
-
-CClientDisplayManager::~CClientDisplayManager()
-{
-    RemoveAll();
-}
-
-CClientDisplay* CClientDisplayManager::Get(unsigned long ulID)
+std::shared_ptr<CClientDisplay> CClientDisplayManager::Get(unsigned long ulID)
 {
     // Find the display with the given id
-    list<CClientDisplay*>::const_iterator iter = m_List.begin();
-    for (; iter != m_List.end(); iter++)
+    auto iter = m_List.begin();
+
+    for (; iter != m_List.end(); iter++) // Iterate weak_ptr list
     {
-        if ((*iter)->GetID() == ulID)
+        if (const auto& display = (*iter).lock())  // Make sure the shared_ptr still exists
         {
-            return *iter;
+            if (display->GetID() == ulID)
+            {
+                return display;
+            }
         }
+
     }
 
     return NULL;
@@ -49,57 +43,38 @@ void CClientDisplayManager::DrawText2D(const char* szCaption, const CVector& vec
                                        static_cast<int>(fResHeight), rgbaColor, szCaption, fScale, fScale, 0);
 }
 
-void CClientDisplayManager::AddToList(CClientDisplay* pDisplay)
+void CClientDisplayManager::AddToList(const std::shared_ptr<CClientDisplay>& display)
 {
-    m_List.push_back(pDisplay);
+    m_List.push_back(display);
 }
 
-void CClientDisplayManager::RemoveAll()
+void CClientDisplayManager::DoPulse()
 {
-    // Delete all the items in the list
-    m_bCanRemoveFromList = false;
-    list<CClientDisplay*>::iterator iter = m_List.begin();
-    for (; iter != m_List.end(); iter++)
-    {
-        delete *iter;
-    }
+    // Render all our displays
+    auto iter = m_List.begin();
 
-    // Clear the list
-    m_List.clear();
-    m_bCanRemoveFromList = true;
-}
+    // Clean up expired weak_ptr
+    m_List.remove_if([](const std::weak_ptr<CClientDisplay>& wp) { return wp.expired(); });
 
-void CClientDisplayManager::RemoveFromList(CClientDisplay* pDisplay)
-{
-    if (m_bCanRemoveFromList)
+    for (; iter != m_List.end(); iter++) // Iterate weak_ptr list
     {
-        if (!m_List.empty())
+        if (const auto& display = (*iter).lock())            // Make sure the shared_ptr still exists
         {
-            m_List.remove(pDisplay);
+            display->Render();
         }
     }
 }
 
-void CClientDisplayManager::DoPulse()
+std::shared_ptr<CClientVectorGraphicDisplay> CClientDisplayManager::CreateVectorGraphicDisplay(CClientVectorGraphic* svg)
 {
-    // Render all our displays
-    m_bCanRemoveFromList = false;
-    list<CClientDisplay*>::iterator iter = m_List.begin();
-    while (iter != m_List.end())
-    {
-        CClientDisplay* pObject = *iter;
-        if (pObject->IsExpired())
-        {
-            // Delete it and remove it from the list
-            delete pObject;
-            iter = m_List.erase(iter);
-        }
-        else
-        {
-            ++iter;
-            pObject->Render();
-        }
-    }
+    auto display = std::make_shared<CClientVectorGraphicDisplay>(svg);
+    AddToList(display);
+    return display;
+}
 
-    m_bCanRemoveFromList = true;
+std::shared_ptr<CClientTextDisplay> CClientDisplayManager::CreateTextDisplay(int ID)
+{
+    auto display = std::make_shared<CClientTextDisplay>(ID);
+    AddToList(display);
+    return display;
 }

Client/mods/deathmatch/logic/CClientDisplayManager.h

@@ -13,31 +13,34 @@ class CClientDisplayManager;
 #pragma once
 
 #include "CClientManager.h"
-#include <list>
 
-class CClientDisplay;
+#include "CClientDisplay.h"
+#include "CClientVectorGraphicDisplay.h"
+#include "CClientTextDisplay.h"
+
+#include <list>
 
 class CClientDisplayManager
 {
     friend class CClientManager;
     friend class CClientDisplay;
 
 public:
-    CClientDisplayManager();
-    ~CClientDisplayManager();
+    CClientDisplayManager() = default;
+    ~CClientDisplayManager() = default;
 
     void DoPulse();
 
     unsigned int    Count() { return static_cast<unsigned int>(m_List.size()); };
-    CClientDisplay* Get(unsigned long ulID);
+    std::shared_ptr<CClientDisplay> Get(unsigned long ulID);
 
     void DrawText2D(const char* szCaption, const CVector& vecPosition, float fScale = 1.0f, RGBA rgbaColor = 0xFFFFFFFF);
 
-    void RemoveAll();
+    void AddToList(const std::shared_ptr<CClientDisplay>& display);
 
-    void AddToList(CClientDisplay* pDisplay);
-    void RemoveFromList(CClientDisplay* pDisplay);
+    std::shared_ptr<CClientVectorGraphicDisplay> CreateVectorGraphicDisplay(CClientVectorGraphic* svg);
+    std::shared_ptr<CClientTextDisplay>          CreateTextDisplay(int ID = 0xFFFFFFFF);
 
-    std::list<CClientDisplay*> m_List;
-    bool                       m_bCanRemoveFromList;
+    std::list<std::weak_ptr<CClientDisplay>> m_List;
 };
+

Client/mods/deathmatch/logic/CClientTextDisplay.cpp

@@ -14,18 +14,14 @@ using std::list;
 
 float CClientTextDisplay::m_fGlobalScale = 1.0f;
 
-CClientTextDisplay::CClientTextDisplay(CClientDisplayManager* pDisplayManager, int ID) : CClientDisplay(pDisplayManager, ID)
+CClientTextDisplay::CClientTextDisplay(int ID) : CClientDisplay(ID)
 {
     // Init
     m_fScale = 1;
     m_ulFormat = 0;
     m_bVisible = true;
 }
 
-CClientTextDisplay::~CClientTextDisplay()
-{
-}
-
 void CClientTextDisplay::SetCaption(const char* szCaption)
 {
     if (szCaption)

Client/mods/deathmatch/logic/CClientTextDisplay.h

@@ -16,13 +16,11 @@ class CClientTextDisplay;
 #include "CClientDisplayManager.h"
 #include <gui/CGUI.h>
 
-class CClientTextDisplay : public CClientDisplay
+class CClientTextDisplay final : public CClientDisplay
 {
-    friend class CClientDisplayManager;
-
 public:
-    CClientTextDisplay(CClientDisplayManager* pDisplayManager, int ID = 0xFFFFFFFF);
-    ~CClientTextDisplay();
+    CClientTextDisplay(int ID = 0xFFFFFFFF);
+    ~CClientTextDisplay() = default;
 
     eDisplayType GetType() { return DISPLAY_TEXT; }
 
@@ -34,24 +32,24 @@ class CClientTextDisplay : public CClientDisplay
     void SetColorAlpha(unsigned char ucAlpha);
     void SetShadowAlpha(unsigned char ucShadowAlpha);
 
-    float GetScale() { return m_fScale; };
+    float GetScale() const { return m_fScale; };
     void  SetScale(float fScale);
 
-    unsigned long GetFormat() { return m_ulFormat; };
+    unsigned long GetFormat() const { return m_ulFormat; };
     void          SetFormat(unsigned long ulFormat);
 
     void SetVisible(bool bVisible);
 
     void Render();
 
-    static void SetGlobalScale(float fScale) { m_fGlobalScale = fScale; }
+    static void SetGlobalScale(float fScale) { m_fGlobalScale = fScale; };
 
 private:
     SString m_strCaption;
     float   m_fScale;
 
     unsigned long m_ulFormat;
-    unsigned char m_ucShadowAlpha;
+    unsigned char m_ucShadowAlpha{};
 
     static float m_fGlobalScale;
 };

Client/mods/deathmatch/logic/CClientVectorGraphic.cpp

@@ -17,8 +17,7 @@ CClientVectorGraphic::CClientVectorGraphic(CClientManager* pManager, ElementID I
     SetTypeName("svg");
 
     m_pManager = pManager;
-
-    m_pVectorGraphicDisplay = std::make_unique<CClientVectorGraphicDisplay>(m_pManager->GetDisplayManager(), this);
+    m_pVectorGraphicDisplay = m_pManager->GetDisplayManager()->CreateVectorGraphicDisplay(this);
 
     // Generate the default XML document
     SString defaultXmlString = SString("<svg viewBox='0 0 %u %u'></svg>", pVectorGraphicItem->m_uiSizeX, pVectorGraphicItem->m_uiSizeY);
@@ -46,7 +45,7 @@ bool CClientVectorGraphic::LoadFromString(std::string strData)
 
 bool CClientVectorGraphic::SetDocument(CXMLNode* node)
 {
-    if (!node || !node->IsValid())
+    if (!m_pVectorGraphicDisplay || !node || !node->IsValid())
         return false;
 
     if (m_pXMLString && m_pXMLString->node != node)
@@ -80,11 +79,14 @@ bool CClientVectorGraphic::RemoveUpdateCallback()
 
 void CClientVectorGraphic::OnUpdate()
 {
+    if (!m_pVectorGraphicDisplay)
+        return;
+
     m_pVectorGraphicDisplay->UpdateTexture();
 
     if (std::holds_alternative<CLuaFunctionRef>(m_updateCallbackRef))
     {
-        auto func = std::get<CLuaFunctionRef>(m_updateCallbackRef);
+        auto& func = std::get<CLuaFunctionRef>(m_updateCallbackRef);
         auto state = func.GetLuaVM();
 
         if (VERIFY_FUNCTION(func) && state != NULL)

Client/mods/deathmatch/logic/CClientVectorGraphic.h

@@ -57,7 +57,7 @@ class CClientVectorGraphic final : public CClientTexture
     std::unique_ptr<SXMLString>        m_pXMLString = nullptr;
     CXMLNode*                          m_pXMLDocument = nullptr;
 
-    std::unique_ptr<CClientVectorGraphicDisplay> m_pVectorGraphicDisplay;
+    std::shared_ptr<CClientVectorGraphicDisplay> m_pVectorGraphicDisplay;
 
     std::variant<CLuaFunctionRef, bool> m_updateCallbackRef = false;
 

Client/mods/deathmatch/logic/CClientVectorGraphicDisplay.cpp

@@ -13,8 +13,8 @@
 
 using namespace lunasvg;
 
-CClientVectorGraphicDisplay::CClientVectorGraphicDisplay(CClientDisplayManager* pDisplayManager, CClientVectorGraphic* pVectorGraphic, int ID)
-    : CClientDisplay(pDisplayManager, ID)
+CClientVectorGraphicDisplay::CClientVectorGraphicDisplay(CClientVectorGraphic* pVectorGraphic, int ID)
+    : CClientDisplay(ID)
 {
     m_pVectorGraphic = pVectorGraphic;
     m_bVisible = true;
@@ -25,8 +25,11 @@ CClientVectorGraphicDisplay::CClientVectorGraphicDisplay(CClientDisplayManager*
 
 void CClientVectorGraphicDisplay::Render()
 {
+    // When the underlying vector graphic is deleted, this display will be destroyed automatically by the manager.
+    // CClientVectorGraphicDisplay::Render should be called as long as the display manager is still alive.
+    // see CClientDisplayManager::DoPulse
     if (!m_pVectorGraphic || m_pVectorGraphic->IsDestroyed())
-        return;
+        return SetTimeTillExpiration(1);
 
     if (!m_bVisible)
     {

Client/mods/deathmatch/logic/CClientVectorGraphicDisplay.h

@@ -20,7 +20,7 @@ class CClientVectorGraphicDisplay final : public CClientDisplay
     friend class CClientDisplayManager;
 
 public:
-    CClientVectorGraphicDisplay(CClientDisplayManager* pDisplayManager, CClientVectorGraphic* pVectorGraphic, int ID = DISPLAY_VECTORGRAPHIC);
+    CClientVectorGraphicDisplay(CClientVectorGraphic* pVectorGraphic, int ID = DISPLAY_VECTORGRAPHIC);
     ~CClientVectorGraphicDisplay() = default;
 
     eDisplayType GetType() { return DISPLAY_VECTORGRAPHIC; }
@@ -38,7 +38,6 @@ class CClientVectorGraphicDisplay final : public CClientDisplay
 private:
     void UnpremultiplyBitmap(lunasvg::Bitmap& bitmap);
 
-private:
     CClientVectorGraphic* m_pVectorGraphic;
 
     bool m_bIsCleared;