From 73b4470a0fa7a513e161f98dde95bbb35997e763 Mon Sep 17 00:00:00 2001
From: Chris Tallon <chris@vomp.tv>
Date: Fri, 15 May 2020 23:33:24 +0100
Subject: [PATCH] Bug fix: Don't inc/dec ref counts for invalid VectorHandles

---
 osdopenvg.cc     |  4 ++++
 osdvector.cc     | 40 ++++++++++++++++++++++++++++++++++++++--
 osdvector.h      |  4 ++++
 surfacevector.cc | 15 +++++++++++++++
 4 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/osdopenvg.cc b/osdopenvg.cc
index 757704b..4fdb27f 100644
--- a/osdopenvg.cc
+++ b/osdopenvg.cc
@@ -651,6 +651,10 @@ void OsdOpenVG::threadMethod()
 
       if ((time1 - lastrendertime) > 200)  //5 fps for OSD updates are enough, avoids tearing
       {
+        #if DEV
+        dumpStyles();
+        #endif
+
         InternalRendering();
         lastrendertime = getTimeMS();
 
diff --git a/osdvector.cc b/osdvector.cc
index 237f056..9657d8a 100644
--- a/osdvector.cc
+++ b/osdvector.cc
@@ -277,6 +277,11 @@ void OsdVector::updateOrAddSurface(const SurfaceVector* surf, float x, float y,
   std::lock_guard<std::mutex> lg(surfaces_mutex);
   SurfacesIterator si;
 
+  #if DEV
+  Log::getInstance()->log("OsdVector", Log::CRAZY, "updateOrAddSurface, surfaces.length %i", surfaces.size());
+  dumpStyles();
+  #endif
+
   // First determine it is already in our system
   for(si = surfaces.begin(); si != surfaces.end(); si++)
   {
@@ -323,6 +328,11 @@ void OsdVector::updateOrAddSurface(const SurfaceVector* surf, float x, float y,
   incrementAllRefCounts(si->commands);
 
   cleanupOrphanedRefs();
+
+  #if DEV
+  Log::getInstance()->log("OsdVector", Log::CRAZY, "After UOAS:");
+  dumpStyles();
+  #endif
 }
 
 void OsdVector::removeSurface(const SurfaceVector* surf)
@@ -344,7 +354,9 @@ void OsdVector::decrementAllRefCounts(std::vector<SVGCommand>& commands)
 {
   for (SVGCommand& command : commands)
   {
-    decrementDrawStyleHandleRefCount(command.getHandle()); // FIXME BUG BUG BUG
+    VectorHandle handle = command.getHandle();
+    if (handle != 0) // command might not have a handle
+      decrementDrawStyleHandleRefCount(handle);
 
     ImageIndex ii = command.getImageIndex();
     if (ii) removeImageRef(ii);
@@ -358,7 +370,9 @@ void OsdVector::incrementAllRefCounts(std::vector<SVGCommand>& commands)
 {
   for (SVGCommand& command : commands)
   {
-    incrementDrawStyleHandleRefCount(command.getHandle()); // FIXME BUG BUG BUG
+    VectorHandle handle = command.getHandle();
+    if (handle != 0) // command might not have a handle
+      incrementDrawStyleHandleRefCount(handle);
 
     ImageIndex ii = command.getImageIndex();
     if (ii) incImageRef(ii);
@@ -608,6 +622,10 @@ VectorHandle OsdVector::getDrawStyleHandle(const DrawStyle& c)
   surfaces_mutex.lock();
   VectorHandle style_handle = 0;
 
+  #if DEV
+  dumpStyles();
+  #endif
+
   if (!drawstyleHandles_lastit_valid || (drawstyleHandles_lastit->first != c))
   {
     drawstyleHandles_lastit_valid = false;
@@ -649,6 +667,24 @@ VectorHandle OsdVector::getDrawStyleHandle(const DrawStyle& c)
   return style_handle;
 }
 
+#if DEV
+void OsdVector::dumpStyles()
+{
+  std::map<DrawStyle, VectorHandle>::iterator i;
+  for(i = drawstyleHandles.begin(); i != drawstyleHandles.end(); i++)
+  {
+    const DrawStyle* test = &(i->first);
+    Log::getInstance()->log("OsdVector", Log::DEBUG, "DumpStyles: %p %i", test , i->second);
+  }
+
+  std::map<VectorHandle, int>::iterator i2;
+  for (i2 = drawstyleHandlesRefCounts.begin(); i2 != drawstyleHandlesRefCounts.end(); i2++)
+  {
+    Log::getInstance()->log("OsdVector", Log::DEBUG, "DumpStylesRef: %i %i", i2->first, i2->second);
+  }
+}
+#endif
+
 LoadIndex OsdVector::getTVMediaRef(TVMediaInfo& tvmedia, ImageIndex& image)
 {
   ImageIndex image_handle = 0;
diff --git a/osdvector.h b/osdvector.h
index 1797c25..713879e 100644
--- a/osdvector.h
+++ b/osdvector.h
@@ -385,6 +385,10 @@ class OsdVector : public Osd
     virtual void executeDrawCommand(SVGCommand& command) = 0;
 
     void drawSurfaces();
+
+    #if DEV
+    void dumpStyles();
+    #endif
 };
 
 #endif
diff --git a/surfacevector.cc b/surfacevector.cc
index 1dc153e..cf2f9b0 100644
--- a/surfacevector.cc
+++ b/surfacevector.cc
@@ -56,6 +56,21 @@ float SurfaceVector::getCharWidth(wchar_t c)
   return osd->getCharWidth(c);
 }
 
+/*
+ * By current design it's not permitted to have a surface without the first drawing being to
+ * fill with a background colour. The fillblt calls removeCommands which deletes all the old
+ * commands before the background colour becomes the new first command, then all the rest are
+ * redrawn on top. If you want to change this in future then do this as the first thing in
+ * Boxx::draw():
+ *   surface->clear();
+ * and have this function in here:
+ *   void SurfaceVector::clear()
+ *   {
+ *     removeCommands(0, 0, swidth, sheight);
+ *   }
+ * Or preferably a simpler one that doesn't do all the comparison work.
+*/
+
 int SurfaceVector::drawText(const char* text, int x, int y, const DrawStyle& c)
 {
   return drawText(text, x, y, 0, c);
-- 
2.39.2