common: fix a potential memory leak after clear()

Paints must clear canvas engine data if they were dismissed from the canvas,

1. Canvas::clear(free = false) must retain all the paints from the paints hierarchy
so that user keeps the all dangled paints lifecycle.
In this scenario, it could leak the engine data of paints, this patch fixes it.

2. Previously, t just keeps the immediate paints lives of canvas, but not them of children of scene nor picture.
This patch changes a policy which was not considered seriously,
Now it keeps the all paints lives through the tree-hieararchy.

3. Also changed the Scene::clear() behavior identical to Canvas::clear() for consistency.

@API Modification:

From: Result Scene::clear() noexcept;
To: Result Scene::clear(bool free = true) noexcept;

inc/thorvg.h

@@ -1095,12 +1095,18 @@ public:
 
     /**
      * @brief Sets the total number of the paints pushed into the scene to be zero.
+     * Depending on the value of the @p free argument, the paints are freed or not.
      *
+     * @param[in] free If @c true, the memory occupied by paints is deallocated, otherwise it is not.
+     *
+     * @return Result::Success when succeed
+     *
+     * @warning If you don't free the paints they become dangled. They are supposed to be reused, otherwise you are responsible for their lives. Thus please use the @p free argument only when you know how it works, otherwise it's not recommended.
      * @warning Please do not use it, this API is not official one. It could be modified in the next version.
      *
      * @BETA_API
      */
-    Result clear() noexcept;
+    Result clear(bool free = true) noexcept;
 
     /**
      * @brief Creates a new Scene object.

inc/thorvg_capi.h

@@ -1724,12 +1724,21 @@ TVG_EXPORT Tvg_Result tvg_scene_push(Tvg_Paint* scene, Tvg_Paint* paint);
 
 
 /*!
-* \brief Sets the total number of the paints pushed into the scene to be zero. (BETA version)
+* \brief Clears a Tvg_Scene objects from pushed paints.
 *
-* \warning Please do not use it, this API is not official one. It could be modified in the next version.
+* Tvg_Paint objects stored in the scene are released if @p free is set to @c true, otherwise the memory is not deallocated and
+* all paints should be released manually in order to avoid memory leaks.
+*
+* \param[in] scene The Tvg_Scene object to be cleared.
+* \param[in] free If @c true the memory occupied by paints is deallocated, otherwise it is not.
+*
+* \return Tvg_Result enumeration.
+* \retval TVG_RESULT_SUCCESS Succeed.
+* \retval TVG_RESULT_INVALID_ARGUMENT An invalid Tvg_Canvas pointer.
+*
+* \warning Please use the @p free argument only when you know how it works, otherwise it's not recommended.
 */
-TVG_EXPORT Tvg_Result tvg_scene_clear(Tvg_Paint* scene);
-
+TVG_EXPORT Tvg_Result tvg_scene_clear(Tvg_Paint* scene, bool free);
 
 /** \} */   // end defgroup ThorVGCapi_Scene
 

src/bindings/capi/tvgCapi.cpp

@@ -564,10 +564,10 @@ TVG_EXPORT Tvg_Result tvg_scene_push(Tvg_Paint* scene, Tvg_Paint* paint)
 }
 
 
-TVG_EXPORT Tvg_Result tvg_scene_clear(Tvg_Paint* scene)
+TVG_EXPORT Tvg_Result tvg_scene_clear(Tvg_Paint* scene, bool free)
 {
     if (!scene) return TVG_RESULT_INVALID_ARGUMENT;
-    return (Tvg_Result) reinterpret_cast<Scene*>(scene)->clear();
+    return (Tvg_Result) reinterpret_cast<Scene*>(scene)->clear(free);
 }
 
 

src/lib/tvgCanvasImpl.h

@@ -59,11 +59,9 @@ struct Canvas::Impl
         if (!renderer || !renderer->clear()) return Result::InsufficientCondition;
 
         //free paints
-        if (free) {
-            for (auto paint = paints.data; paint < (paints.data + paints.count); ++paint) {
-                (*paint)->pImpl->dispose(*renderer);
-                delete(*paint);
-            }
+        for (auto paint = paints.data; paint < (paints.data + paints.count); ++paint) {
+            (*paint)->pImpl->dispose(*renderer);
+            if (free) delete(*paint);
         }
 
         paints.clear();

src/lib/tvgPictureImpl.h

@@ -44,15 +44,17 @@ struct Picture::Impl
     {
     }
 
+    ~Impl()
+    {
+        if (paint) delete(paint);
+    }
+
     bool dispose(RenderMethod& renderer)
     {
         bool ret = true;
         if (paint) {
             ret = paint->pImpl->dispose(renderer);
-            delete(paint);
-            paint = nullptr;
-        }
-        else if (pixels) {
+        } else if (pixels) {
             ret =  renderer.dispose(rdata);
             rdata = nullptr;
         }

src/lib/tvgScene.cpp

@@ -62,9 +62,9 @@ Result Scene::reserve(uint32_t size) noexcept
 }
 
 
-Result Scene::clear() noexcept
+Result Scene::clear(bool free) noexcept
 {
-    pImpl->paints.clear();
+    pImpl->clear(free);
 
     return Result::Success;
 }

src/lib/tvgSceneImpl.h

@@ -33,14 +33,22 @@ struct Scene::Impl
 {
     Array<Paint*> paints;
     uint8_t opacity;            //for composition
+    RenderMethod* renderer = nullptr;    //keep it for explicit clear
+
+    ~Impl()
+    {
+        for (auto paint = paints.data; paint < (paints.data + paints.count); ++paint) {
+            delete(*paint);
+        }
+    }
 
     bool dispose(RenderMethod& renderer)
     {
         for (auto paint = paints.data; paint < (paints.data + paints.count); ++paint) {
             (*paint)->pImpl->dispose(renderer);
-            delete(*paint);
         }
-        paints.clear();
+
+        this->renderer = nullptr;
 
         return true;
     }
@@ -69,6 +77,9 @@ struct Scene::Impl
 
         /* FXIME: it requires to return list of children engine data
            This is necessary for scene composition */
+
+        this->renderer = &renderer;
+
         return nullptr;
     }
 
@@ -158,6 +169,22 @@ struct Scene::Impl
 
         return ret.release();
     }
+
+    void clear(bool free)
+    {
+        //Clear render target before drawing
+        if (!renderer || !renderer->clear()) {
+            paints.clear();
+            return;
+        }
+        //free paints
+        for (auto paint = paints.data; paint < (paints.data + paints.count); ++paint) {
+            (*paint)->pImpl->dispose(*renderer);
+            if (free) delete(*paint);
+        }
+        paints.clear();
+        renderer = nullptr;
+    }
 };
 
 #endif //_TVG_SCENE_IMPL_H_