common: improved instance memory handling

- prevent dangling instances in failure scenarios by utilizing a reference counter.
- update the test suite to treat null pointer arguments as invalid argument failures.

inc/thorvg.h

@@ -2116,7 +2116,7 @@ public:
      *
      * @note Experimental API
      */
-    Result set(const Picture* picture, std::function<bool(const Paint* paint, void* data)> func, void* data) noexcept;
+    Result set(Picture* picture, std::function<bool(const Paint* paint, void* data)> func, void* data) noexcept;
 
     /**
      * @brief Generate a unique ID (hash key) from a given name.

src/renderer/tvgAccessor.cpp

@@ -50,20 +50,28 @@ static bool accessChildren(Iterator* it, function<bool(const Paint* paint, void*
 /* External Class Implementation                                        */
 /************************************************************************/
 
-Result Accessor::set(const Picture* picture, function<bool(const Paint* paint, void* data)> func, void* data) noexcept
+Result Accessor::set(Picture* picture, function<bool(const Paint* paint, void* data)> func, void* data) noexcept
 {
     if (!picture || !func) return Result::InvalidArguments;
 
     //Use the Preorder Tree-Search
 
+    picture->ref();
+
     //Root
-    if (!func(picture, data)) return Result::Success;
+    if (!func(picture, data)) {
+        picture->unref();
+        return Result::Success;
+    }
 
     //Children
     if (auto it = IteratorAccessor::iterator(picture)) {
         accessChildren(it, func, data);
         delete(it);
     }
+
+    picture->unref(false);
+
     return Result::Success;
 }
 

src/renderer/tvgCanvas.cpp

@@ -68,6 +68,8 @@ Result Canvas::draw() noexcept
 Result Canvas::update(Paint* paint) noexcept
 {
     TVGLOG("RENDERER", "Update S. ------------------------------ Canvas(%p)", this);
+
+    if (pImpl->paints.empty() || pImpl->status == Status::Drawing) return Result::InsufficientCondition;
     auto ret = pImpl->update(paint, false);
     TVGLOG("RENDERER", "Update E. ------------------------------ Canvas(%p)", this);
 

src/renderer/tvgCanvas.h

@@ -60,10 +60,14 @@ struct Canvas::Impl
 
     Result push(Paint* paint)
     {
-        //You cannot push paints during rendering.
-        if (status == Status::Drawing) return Result::InsufficientCondition;
+        if (!paint) return Result::InvalidArguments;
+
+        //You cannot push paints during rendering.
+        if (status == Status::Drawing) {
+            TVG_DELETE(paint);
+            return Result::InsufficientCondition;
+        }
 
-        if (!paint) return Result::MemoryCorruption;
         paint->ref();
         paints.push_back(paint);
 
@@ -88,8 +92,6 @@ struct Canvas::Impl
 
     Result update(Paint* paint, bool force)
     {
-        if (paints.empty() || status == Status::Drawing) return Result::InsufficientCondition;
-
         Array<RenderData> clips;
         auto flag = RenderUpdateFlag::None;
         if (status == Status::Damaged || force) flag = RenderUpdateFlag::All;
@@ -109,8 +111,11 @@ struct Canvas::Impl
 
     Result draw()
     {
+        if (status == Status::Drawing || paints.empty()) return Result::InsufficientCondition;
+
         if (status == Status::Damaged) update(nullptr, false);
-        if (status == Status::Drawing || paints.empty() || !renderer->preRender()) return Result::InsufficientCondition;
+
+        if (!renderer->preRender()) return Result::InsufficientCondition;
 
         bool rendered = false;
         for (auto paint : paints) {

src/renderer/tvgCommon.h

@@ -80,6 +80,9 @@ uint16_t THORVG_VERSION_NUMBER();
 #define PP(A) (((Paint*)(A))->pImpl)   //Access to pimpl.
 
 
+#define TVG_DELETE(PAINT) \
+  if (PAINT->refCnt() == 0) delete(PAINT)
+
 //for debugging
 #if 0
 #include <sys/time.h>

src/renderer/tvgPaint.cpp

@@ -163,8 +163,8 @@ Paint* Paint::Impl::duplicate(Paint* ret)
 
     ret->pImpl->opacity = opacity;
 
-    if (maskData) ret->pImpl->mask(ret, maskData->target->duplicate(), maskData->method);
-    if (clipper) ret->pImpl->clip(clipper->duplicate());
+    if (maskData) ret->mask(maskData->target->duplicate(), maskData->method);
+    if (clipper) ret->clip(clipper->duplicate());
 
     return ret;
 }
@@ -357,7 +357,7 @@ bool Paint::Impl::bounds(float* x, float* y, float* w, float* h, bool transforme
 void Paint::Impl::reset()
 {
     if (clipper) {
-        delete(clipper);
+        clipper->unref();
         clipper = nullptr;
     }
 
@@ -446,6 +446,7 @@ Result Paint::clip(Paint* clipper) noexcept
 {
     if (clipper && clipper->type() != Type::Shape) {
         TVGERR("RENDERER", "Clipping only supports the Shape!");
+        TVG_DELETE(clipper);
         return Result::NonSupport;
     }
     pImpl->clip(clipper);
@@ -455,9 +456,8 @@ Result Paint::clip(Paint* clipper) noexcept
 
 Result Paint::mask(Paint* target, MaskMethod method) noexcept
 {
-    if (pImpl->mask(this, target, method)) return Result::Success;
-
-    delete(target);
+    if (pImpl->mask(target, method)) return Result::Success;
+    if (target) TVG_DELETE(target);
     return Result::InvalidArguments;
 }
 

src/renderer/tvgPaint.h

@@ -119,7 +119,7 @@ namespace tvg
             clipper->ref();
         }
 
-        bool mask(Paint* source, Paint* target, MaskMethod method)
+        bool mask(Paint* target, MaskMethod method)
         {
             //Invalid case
             if ((!target && method != MaskMethod::None) || (target && method == MaskMethod::None)) return false;
@@ -138,7 +138,7 @@ namespace tvg
             }
             target->ref();
             maskData->target = target;
-            maskData->source = source;
+            maskData->source = paint;
             maskData->method = method;
             return true;
         }

src/renderer/tvgSaver.cpp

@@ -40,7 +40,7 @@ struct Saver::Impl
     ~Impl()
     {
         delete(saveModule);
-        delete(bg);
+        if (bg) bg->unref();
     }
 };
 
@@ -102,11 +102,11 @@ Saver::~Saver()
 
 Result Saver::save(Paint* paint, const char* filename, uint32_t quality) noexcept
 {
-    if (!paint) return Result::MemoryCorruption;
+    if (!paint) return Result::InvalidArguments;
 
     //Already on saving another resource.
     if (pImpl->saveModule) {
-        if (paint->refCnt() == 0) delete(paint);
+        TVG_DELETE(paint);
         return Result::InsufficientCondition;
     }
 
@@ -115,19 +115,22 @@ Result Saver::save(Paint* paint, const char* filename, uint32_t quality) noexcep
             pImpl->saveModule = saveModule;
             return Result::Success;
         } else {
-            if (paint->refCnt() == 0) delete(paint);
+            TVG_DELETE(paint);
             delete(saveModule);
             return Result::Unknown;
         }
     }
-    if (paint->refCnt() == 0) delete(paint);
+    TVG_DELETE(paint);
     return Result::NonSupport;
 }
 
 
 Result Saver::background(Paint* paint) noexcept
 {
-    delete(pImpl->bg);
+    if (!paint) return Result::InvalidArguments;
+
+    if (pImpl->bg) TVG_DELETE(pImpl->bg);
+    paint->ref();
     pImpl->bg = paint;
 
     return Result::Success;
@@ -136,7 +139,7 @@ Result Saver::background(Paint* paint) noexcept
 
 Result Saver::save(Animation* animation, const char* filename, uint32_t quality, uint32_t fps) noexcept
 {
-    if (!animation) return Result::MemoryCorruption;
+    if (!animation) return Result::InvalidArguments;
 
     //animation holds the picture, it must be 1 at the bottom.
     auto remove = animation->picture()->refCnt() <= 1 ? true : false;

src/renderer/tvgScene.cpp

@@ -69,7 +69,7 @@ Type Scene::type() const noexcept
 
 Result Scene::push(Paint* paint) noexcept
 {
-    if (!paint) return Result::MemoryCorruption;
+    if (!paint) return Result::InvalidArguments;
     paint->ref();
     pImpl->paints.push_back(paint);
 

src/renderer/tvgShape.cpp

@@ -212,7 +212,7 @@ Result Shape::fill(uint8_t r, uint8_t g, uint8_t b, uint8_t a) noexcept
 
 Result Shape::fill(Fill* f) noexcept
 {
-    if (!f) return Result::MemoryCorruption;
+    if (!f) return Result::InvalidArguments;
 
     if (pImpl->rs.fill && pImpl->rs.fill != f) delete(pImpl->rs.fill);
     pImpl->rs.fill = f;

src/renderer/tvgShape.h

@@ -280,7 +280,7 @@ struct Shape::Impl
 
     Result strokeFill(Fill* f)
     {
-        if (!f) return Result::MemoryCorruption;
+        if (!f) return Result::InvalidArguments;
 
         if (!rs.stroke) rs.stroke = new RenderStroke();
         if (rs.stroke->fill && rs.stroke->fill != f) delete(rs.stroke->fill);

src/savers/gif/tvgGifSaver.cpp

@@ -96,7 +96,7 @@ bool GifSaver::close()
 {
     this->done();
 
-    delete(bg);
+    if (bg) bg->unref();
     bg = nullptr;
 
     //animation holds the picture, it must be 1 at the bottom.
@@ -143,7 +143,10 @@ bool GifSaver::save(Animation* animation, Paint* bg, const char* filename, TVG_U
 
     this->animation = animation;
 
-    if (bg) this->bg = bg->duplicate();
+    if (bg) {
+        bg->ref();
+        this->bg = bg;
+    }
     this->fps = static_cast<float>(fps);
 
     TaskScheduler::request(this);

test/capi/capiLinearGradient.cpp

@@ -71,7 +71,7 @@ TEST_CASE("Linear Gradient in shape", "[capiLinearGradient]")
     REQUIRE(tvg_shape_get_gradient(shape, &gradient_ret) == TVG_RESULT_SUCCESS);
     REQUIRE(gradient_ret);
 
-    REQUIRE(tvg_shape_set_gradient(shape, NULL) == TVG_RESULT_MEMORY_CORRUPTION);
+    REQUIRE(tvg_shape_set_gradient(shape, NULL) == TVG_RESULT_INVALID_ARGUMENT);
     REQUIRE(tvg_paint_del(shape) == TVG_RESULT_SUCCESS);
 }
 

test/capi/capiRadialGradient.cpp

@@ -72,7 +72,7 @@ TEST_CASE("Set gradient in shape", "[capiRadialGradient]")
     REQUIRE(tvg_shape_get_gradient(shape, &gradient_ret) == TVG_RESULT_SUCCESS);
     REQUIRE(gradient_ret);
 
-    REQUIRE(tvg_shape_set_gradient(shape, NULL) == TVG_RESULT_MEMORY_CORRUPTION);
+    REQUIRE(tvg_shape_set_gradient(shape, NULL) == TVG_RESULT_INVALID_ARGUMENT);
     REQUIRE(tvg_paint_del(shape) == TVG_RESULT_SUCCESS);
 }
 

test/capi/capiSwCanvas.cpp

@@ -85,6 +85,7 @@ TEST_CASE("Canvas draw", "[capiSwCanvas]")
     REQUIRE(tvg_swcanvas_set_target(canvas, buffer, 200, 200, 200, TVG_COLORSPACE_ARGB8888) == TVG_RESULT_SUCCESS);
 
     REQUIRE(tvg_canvas_draw(canvas) == TVG_RESULT_INSUFFICIENT_CONDITION);
+
     REQUIRE(tvg_canvas_sync(canvas) == TVG_RESULT_INSUFFICIENT_CONDITION);
 
     Tvg_Paint* paint = tvg_shape_new();

test/capi/capiText.cpp

@@ -153,11 +153,11 @@ TEST_CASE("Set gradient text fill", "[capiText]")
 
     REQUIRE(tvg_font_load(TEST_DIR"/Arial.ttf") == TVG_RESULT_SUCCESS);
 
-    REQUIRE(tvg_text_set_gradient(text, NULL) == TVG_RESULT_MEMORY_CORRUPTION);
+    REQUIRE(tvg_text_set_gradient(text, NULL) == TVG_RESULT_INVALID_ARGUMENT);
 
     REQUIRE(tvg_text_set_font(text, "Arial", 10.0f, "") == TVG_RESULT_SUCCESS);
 
-    REQUIRE(tvg_text_set_gradient(text, NULL) == TVG_RESULT_MEMORY_CORRUPTION);
+    REQUIRE(tvg_text_set_gradient(text, NULL) == TVG_RESULT_INVALID_ARGUMENT);
     REQUIRE(tvg_text_set_gradient(NULL, NULL) == TVG_RESULT_INVALID_ARGUMENT);
     REQUIRE(tvg_text_set_gradient(text, gradientLin) == TVG_RESULT_SUCCESS);
     REQUIRE(tvg_text_set_gradient(text, gradientRad) == TVG_RESULT_SUCCESS);

test/testScene.cpp

@@ -53,10 +53,10 @@ TEST_CASE("Pushing Paints Into Scene", "[tvgScene]")
     REQUIRE(scene->push(paints[2]) == Result::Success);
 
     //Pushing Null Pointer
-    REQUIRE(scene->push(nullptr) == Result::MemoryCorruption);
+    REQUIRE(scene->push(nullptr) == Result::InvalidArguments);
 
     //Pushing Invalid Paint
-    REQUIRE(scene->push(nullptr) == Result::MemoryCorruption);
+    REQUIRE(scene->push(nullptr) == Result::InvalidArguments);
 
     //Check list of paints
     auto list = scene->paints();