renderer: enhanced shared surface handling with mutex implementation

Introduced a dedicated mutex for each surface instance
to ensure safe sharing between the loader, renderer, and engine.

This enhancement allows for secure modification and access to bitmap data,
addressing potential concurrency issues.

Multiple Picture instances can now safely share a single loader instance,
optimizing performance.

This change builds upon the previous Loader Cache improvements:
ff6ea4b6c4

src/loaders/external_jpg/tvgJpgLoader.cpp

@@ -53,7 +53,7 @@ JpgLoader::~JpgLoader()
     tjDestroy(jpegDecompressor);
 
     //This image is shared with raster engine.
-    tjFree(image);
+    tjFree(surface.buf8);
 }
 
 
@@ -84,7 +84,6 @@ bool JpgLoader::open(const string& path)
 
     w = static_cast<float>(width);
     h = static_cast<float>(height);
-    cs = ColorSpace::ARGB8888;
     ret = true;
 
     goto finalize;
@@ -115,7 +114,6 @@ bool JpgLoader::open(const char* data, uint32_t size, bool copy)
 
     w = static_cast<float>(width);
     h = static_cast<float>(height);
-    cs = ColorSpace::ARGB8888;
     this->size = size;
 
     return true;
@@ -130,8 +128,7 @@ bool JpgLoader::read()
 
     /* OPTIMIZE: We assume the desired colorspace is ColorSpace::ARGB
        How could we notice the renderer colorspace at this time? */
-    if (image) tjFree(image);
+    auto image = (unsigned char *)tjAlloc(static_cast<int>(w) * static_cast<int>(h) * tjPixelSize[TJPF_BGRX]);
-    image = (unsigned char *)tjAlloc(static_cast<int>(w) * static_cast<int>(h) * tjPixelSize[TJPF_BGRX]);
     if (!image) return false;
 
     //decompress jpg image
@@ -142,25 +139,15 @@ bool JpgLoader::read()
         return false;
     }
 
+    //setup the surface
+    surface.buf8 = image;
+    surface.stride = w;
+    surface.w = w;
+    surface.h = h;
+    surface.channelSize = sizeof(uint32_t);
+    surface.cs = ColorSpace::ARGB8888;
+    surface.premultiplied = true;
+
     clear();
     return true;
 }
-
-
-Surface* JpgLoader::bitmap()
-{
-    if (!image) return nullptr;
-
-    //TODO: It's better to keep this surface instance in the loader side
-    auto surface = new Surface;
-    surface->buf8 = image;
-    surface->stride = w;
-    surface->w = w;
-    surface->h = h;
-    surface->cs = cs;
-    surface->channelSize = sizeof(uint32_t);
-    surface->premultiplied = true;
-    surface->owner = true;
-
-    return surface;
-}

src/loaders/external_jpg/tvgJpgLoader.h

@@ -38,14 +38,11 @@ public:
     bool open(const char* data, uint32_t size, bool copy) override;
     bool read() override;
 
-    Surface* bitmap() override;
-
 private:
     void clear();
 
     tjhandle jpegDecompressor;
     unsigned char* data = nullptr;
-    unsigned char *image = nullptr;
     unsigned long size = 0;
     bool freeData = false;
 };

src/loaders/external_png/tvgPngLoader.cpp

@@ -48,7 +48,7 @@ PngLoader::PngLoader() : ImageLoader(FileType::Png)
 PngLoader::~PngLoader()
 {
     clear();
-    free((void*)content);
+    free((void*)surface.buf32);
 }
 
 
@@ -60,7 +60,6 @@ bool PngLoader::open(const string& path)
 
     w = (float)image->width;
     h = (float)image->height;
-    cs = ColorSpace::ARGB8888;
 
     return true;
 }
@@ -74,7 +73,6 @@ bool PngLoader::open(const char* data, uint32_t size, bool copy)
 
     w = (float)image->width;
     h = (float)image->height;
-    cs = ColorSpace::ARGB8888;
 
     return true;
 }
@@ -98,28 +96,17 @@ bool PngLoader::read()
         free(buffer);
         return false;
     }
-    content = reinterpret_cast<uint32_t*>(buffer);
+
+    //setup the surface
+    surface.buf32 = reinterpret_cast<uint32_t*>(buffer);
+    surface.stride = (uint32_t)w;
+    surface.w = (uint32_t)w;
+    surface.h = (uint32_t)h;
+    surface.channelSize = sizeof(uint32_t);
+    surface.cs = ColorSpace::ARGB8888;
+    surface.premultiplied = false;
 
     clear();
 
     return true;
 }
-
-
-Surface* PngLoader::bitmap()
-{
-    if (!content) return nullptr;
-
-    //TODO: It's better to keep this surface instance in the loader side
-    auto surface = new Surface;
-    surface->buf32 = content;
-    surface->stride = (uint32_t)w;
-    surface->w = (uint32_t)w;
-    surface->h = (uint32_t)h;
-    surface->cs = cs;
-    surface->channelSize = sizeof(uint32_t);
-    surface->owner = true;
-    surface->premultiplied = false;
-
-    return surface;
-}

src/loaders/external_png/tvgPngLoader.h

@@ -36,13 +36,10 @@ public:
     bool open(const char* data, uint32_t size, bool copy) override;
     bool read() override;
 
-    Surface* bitmap() override;
-
 private:
     void clear();
 
     png_imagep image = nullptr;
-    uint32_t* content = nullptr;
 };
 
 #endif //_TVG_PNG_LOADER_H_

src/loaders/external_webp/tvgWebpLoader.cpp

@@ -33,7 +33,16 @@
 
 void WebpLoader::run(unsigned tid)
 {
-    image = WebPDecodeBGRA(data, size, nullptr, nullptr);
+    surface.buf8 = WebPDecodeBGRA(data, size, nullptr, nullptr);
+
+    //setup the surface
+    surface.stride = (uint32_t)w;
+    surface.w = (uint32_t)w;
+    surface.h = (uint32_t)h;
+    surface.channelSize = sizeof(uint32_t);
+    surface.cs = ColorSpace::ARGB8888;
+    surface.premultiplied = false;
+
 }
 
 
@@ -54,7 +63,7 @@ WebpLoader::~WebpLoader()
     data = nullptr;
     size = 0;
     freeData = false;
-    WebPFree(image);
+    WebPFree(surface.buf8);
 }
 
 
@@ -82,7 +91,6 @@ bool WebpLoader::open(const string& path)
 
     w = static_cast<float>(width);
     h = static_cast<float>(height);
-    cs = ColorSpace::ARGB8888;
 
     ret = true;
 
@@ -109,7 +117,7 @@ bool WebpLoader::open(const char* data, uint32_t size, bool copy)
 
     w = static_cast<float>(width);
     h = static_cast<float>(height);
-    cs = ColorSpace::ARGB8888;
+    surface.cs = ColorSpace::ARGB8888;
     this->size = size;
     return true;
 }
@@ -122,6 +130,7 @@ bool WebpLoader::read()
     if (!data || w == 0 || h == 0) return false;
 
     TaskScheduler::request(this);
+
     return true;
 }
 
@@ -130,17 +139,5 @@ Surface* WebpLoader::bitmap()
 {
     this->done();
 
-    if (!image) return nullptr;
+    return ImageLoader::bitmap();
-
-    //TODO: It's better to keep this surface instance in the loader side
-    auto surface = new Surface;
-    surface->buf8 = image;
-    surface->stride = static_cast<uint32_t>(w);
-    surface->w = static_cast<uint32_t>(w);
-    surface->h = static_cast<uint32_t>(h);
-    surface->cs = cs;
-    surface->channelSize = sizeof(uint32_t);
-    surface->premultiplied = false;
-    surface->owner = true;
-    return surface;
 }

src/loaders/external_webp/tvgWebpLoader.h

@@ -42,7 +42,6 @@ private:
     void run(unsigned tid) override;
 
     unsigned char* data = nullptr;
-    unsigned char *image = nullptr;
     unsigned long size = 0;
     bool freeData = false;
 };

src/loaders/jpg/tvgJpgLoader.cpp

@@ -39,11 +39,13 @@ void JpgLoader::clear()
 
 void JpgLoader::run(unsigned tid)
 {
-    if (image) {
+    surface.buf8 = jpgdDecompress(decoder);
-        free(image);
+    surface.stride = static_cast<uint32_t>(w);
-        image = nullptr;
+    surface.w = static_cast<uint32_t>(w);
-    }
+    surface.h = static_cast<uint32_t>(h);
-    image = jpgdDecompress(decoder);
+    surface.cs = ColorSpace::ARGB8888;
+    surface.channelSize = sizeof(uint32_t);
+    surface.premultiplied = true;
 
     clear();
 }
@@ -62,7 +64,7 @@ JpgLoader::JpgLoader() : ImageLoader(FileType::Jpg)
 JpgLoader::~JpgLoader()
 {
     clear();
-    free(image);
+    free(surface.buf8);
 }
 
 
@@ -74,7 +76,6 @@ bool JpgLoader::open(const string& path)
 
     w = static_cast<float>(width);
     h = static_cast<float>(height);
-    cs = ColorSpace::ARGB8888;
 
     return true;
 }
@@ -98,7 +99,6 @@ bool JpgLoader::open(const char* data, uint32_t size, bool copy)
 
     w = static_cast<float>(width);
     h = static_cast<float>(height);
-    cs = ColorSpace::ARGB8888;
 
     return true;
 }
@@ -128,19 +128,5 @@ bool JpgLoader::close()
 Surface* JpgLoader::bitmap()
 {
     this->done();
-
+    return ImageLoader::bitmap();
-    if (!image) return nullptr;
-
-    //TODO: It's better to keep this surface instance in the loader side
-    auto surface = new Surface;
-    surface->buf8 = image;
-    surface->stride = static_cast<uint32_t>(w);
-    surface->w = static_cast<uint32_t>(w);
-    surface->h = static_cast<uint32_t>(h);
-    surface->cs = cs;
-    surface->channelSize = sizeof(uint32_t);
-    surface->premultiplied = true;
-    surface->owner = true;
-
-    return surface;
 }

src/loaders/jpg/tvgJpgLoader.h

@@ -32,7 +32,6 @@ class JpgLoader : public ImageLoader, public Task
 private:
     jpeg_decoder* decoder = nullptr;
     char* data = nullptr;
-    unsigned char *image = nullptr;
     bool freeData = false;
 
     void clear();

src/loaders/png/tvgPngLoader.cpp

@@ -32,16 +32,19 @@
 
 void PngLoader::run(unsigned tid)
 {
-    if (image) {
-        free(image);
-        image = nullptr;
-    }
     auto width = static_cast<unsigned>(w);
     auto height = static_cast<unsigned>(h);
 
-    if (lodepng_decode(&image, &width, &height, &state, data, size)) {
+    if (lodepng_decode(&surface.buf8, &width, &height, &state, data, size)) {
         TVGERR("PNG", "Failed to decode image");
     }
+
+    //setup the surface
+    surface.stride = width;
+    surface.w = width;
+    surface.h = height;
+    surface.channelSize = sizeof(uint32_t);
+    surface.premultiplied = false;
 }
 
 
@@ -58,7 +61,7 @@ PngLoader::PngLoader() : ImageLoader(FileType::Png)
 PngLoader::~PngLoader()
 {
     if (freeData) free(data);
-    free(image);
+    free(surface.buf8);
     lodepng_state_cleanup(&state);
 }
 
@@ -90,8 +93,8 @@ bool PngLoader::open(const string& path)
     w = static_cast<float>(width);
     h = static_cast<float>(height);
 
-    if (state.info_png.color.colortype == LCT_RGBA) cs = ColorSpace::ABGR8888;
+    if (state.info_png.color.colortype == LCT_RGBA) surface.cs = ColorSpace::ABGR8888;
-    else cs = ColorSpace::ARGB8888;
+    else surface.cs = ColorSpace::ARGB8888;
 
     ret = true;
 
@@ -122,7 +125,7 @@ bool PngLoader::open(const char* data, uint32_t size, bool copy)
     h = static_cast<float>(height);
     this->size = size;
 
-    cs = ColorSpace::ABGR8888;
+    surface.cs = ColorSpace::ABGR8888;
 
     return true;
 }
@@ -143,19 +146,5 @@ bool PngLoader::read()
 Surface* PngLoader::bitmap()
 {
     this->done();
-
+    return ImageLoader::bitmap();
-    if (!image) return nullptr;
-
-    //TODO: It's better to keep this surface instance in the loader side
-    auto surface = new Surface;
-    surface->buf8 = image;
-    surface->stride = static_cast<uint32_t>(w);
-    surface->w = static_cast<uint32_t>(w);
-    surface->h = static_cast<uint32_t>(h);
-    surface->cs = cs;
-    surface->channelSize = sizeof(uint32_t);
-    surface->premultiplied = false;
-    surface->owner = true;
-
-    return surface;
 }

src/loaders/png/tvgPngLoader.h

@@ -32,7 +32,6 @@ class PngLoader : public ImageLoader, public Task
 private:
     LodePNGState state;
     unsigned char* data = nullptr;
-    unsigned char *image = nullptr;
     unsigned long size = 0;
     bool freeData = false;
 

src/loaders/raw/tvgRawLoader.cpp

@@ -41,10 +41,7 @@ RawLoader::RawLoader() : ImageLoader(FileType::Raw)
 
 RawLoader::~RawLoader()
 {
-    if (copy && content) {
+    if (copy) free(surface.buf32);
-        free((void*)content);
-        content = nullptr;
-    }
 }
 
 
@@ -59,13 +56,19 @@ bool RawLoader::open(const uint32_t* data, uint32_t w, uint32_t h, bool copy)
     this->copy = copy;
 
     if (copy) {
-        content = (uint32_t*)malloc(sizeof(uint32_t) * w * h);
+        surface.buf32 = (uint32_t*)malloc(sizeof(uint32_t) * w * h);
-        if (!content) return false;
+        if (!surface.buf32) return false;
-        memcpy((void*)content, data, sizeof(uint32_t) * w * h);
+        memcpy((void*)surface.buf32, data, sizeof(uint32_t) * w * h);
     }
-    else content = const_cast<uint32_t*>(data);
+    else surface.buf32 = const_cast<uint32_t*>(data);
 
-    cs = ColorSpace::ARGB8888;
+    //setup the surface
+    surface.stride = w;
+    surface.w = w;
+    surface.h = h;
+    surface.cs = ColorSpace::ARGB8888;
+    surface.channelSize = sizeof(uint32_t);
+    surface.premultiplied = true;
 
     return true;
 }
@@ -74,24 +77,6 @@ bool RawLoader::open(const uint32_t* data, uint32_t w, uint32_t h, bool copy)
 bool RawLoader::read()
 {
     LoadModule::read();
+
     return true;
 }
-
-
-Surface* RawLoader::bitmap()
-{
-    if (!content) return nullptr;
-
-    //TODO: It's better to keep this surface instance in the loader side
-    auto surface = new Surface;
-    surface->buf32 = content;
-    surface->stride = static_cast<uint32_t>(w);
-    surface->w = static_cast<uint32_t>(w);
-    surface->h = static_cast<uint32_t>(h);
-    surface->cs = cs;
-    surface->channelSize = sizeof(uint32_t);
-    surface->premultiplied = true;
-    surface->owner = true;
-
-    return surface;
-}

src/loaders/raw/tvgRawLoader.h

@@ -26,7 +26,6 @@
 class RawLoader : public ImageLoader
 {
 public:
-    uint32_t* content = nullptr;
     bool copy = false;
 
     RawLoader();
@@ -35,8 +34,6 @@ public:
     using LoadModule::open;
     bool open(const uint32_t* data, uint32_t w, uint32_t h, bool copy);
     bool read() override;
-
-    Surface* bitmap() override;
 };
 
 

src/renderer/gl_engine/tvgGlRenderer.h

@@ -50,7 +50,7 @@ public:
         RT_None,
     };
 
-    Surface surface = {nullptr, 0, 0, 0, ColorSpace::Unsupported, true};
+    Surface surface;
 
     RenderData prepare(const RenderShape& rshape, RenderData data, const RenderTransform* transform, Array<RenderData>& clips, uint8_t opacity, RenderUpdateFlag flags, bool clipper) override;
     RenderData prepare(const Array<RenderData>& scene, RenderData data, const RenderTransform* transform, Array<RenderData>& clips, uint8_t opacity, RenderUpdateFlag flags) override;

src/renderer/sw_engine/tvgSwCommon.h

@@ -261,13 +261,26 @@ struct SwSurface : Surface
     SwAlpha alphas[4];                    //Alpha:2, InvAlpha:3, Luma:4, InvLuma:5
     SwBlender blender = nullptr;          //blender (optional)
     SwCompositor* compositor = nullptr;   //compositor (optional)
-    BlendMethod          blendMethod;     //blending method (uint8_t)
+    BlendMethod blendMethod;              //blending method (uint8_t)
 
     SwAlpha alpha(CompositeMethod method)
     {
         auto idx = (int)(method) - 2;       //0: None, 1: ClipPath
         return alphas[idx > 3 ? 0 : idx];   //CompositeMethod has only four Matting methods.
     }
+
+    SwSurface()
+    {
+    }
+
+    SwSurface(const SwSurface* rhs) : Surface(rhs)
+    {
+        join = rhs->join;
+        memcpy(alphas, rhs->alphas, sizeof(alphas));
+        blender = rhs->blender;
+        compositor = rhs->compositor;
+        blendMethod = rhs->blendMethod;
+     }
 };
 
 struct SwCompositor : Compositor

src/renderer/sw_engine/tvgSwRaster.cpp

@@ -1855,7 +1855,9 @@ void rasterUnpremultiply(Surface* surface)
 
 void rasterPremultiply(Surface* surface)
 {
-    if (surface->channelSize != sizeof(uint32_t)) return;
+    unique_lock<mutex> lock{surface->mtx};
+    if (surface->premultiplied || (surface->channelSize != sizeof(uint32_t))) return;
+    surface->premultiplied = true;
 
     TVGLOG("SW_ENGINE", "Premultiply [Size: %d x %d]", surface->w, surface->h);
 
@@ -1869,7 +1871,6 @@ void rasterPremultiply(Surface* surface)
             *dst = (c & 0xff000000) + ((((c >> 8) & 0xff) * a) & 0xff00) + ((((c & 0x00ff00ff) * a) >> 8) & 0x00ff00ff);
         }
     }
-    surface->premultiplied = true;
 }
 
 
@@ -1935,6 +1936,9 @@ bool rasterImage(SwSurface* surface, SwImage* image, const RenderMesh* mesh, con
 
 bool rasterConvertCS(Surface* surface, ColorSpace to)
 {
+    unique_lock<mutex> lock{surface->mtx};
+    if (surface->cs == to) return true;
+
     //TOOD: Support SIMD accelerations
     auto from = surface->cs;
 
@@ -1946,6 +1950,5 @@ bool rasterConvertCS(Surface* surface, ColorSpace to)
         surface->cs = to;
         return cRasterARGBtoABGR(surface);
     }
-
     return false;
 }

src/renderer/sw_engine/tvgSwRenderer.cpp

@@ -281,10 +281,8 @@ struct SwImageTask : SwTask
         auto clipRegion = bbox;
 
         //Convert colorspace if it's not aligned.
-        if (source->owner) {
+        rasterConvertCS(source, surface->cs);
-            if (source->cs != surface->cs) rasterConvertCS(source, surface->cs);
+        rasterPremultiply(source);
-            if (!source->premultiplied) rasterPremultiply(source);
-        }
 
         image.data = source->data;
         image.w = source->w;
@@ -436,7 +434,6 @@ bool SwRenderer::target(pixel_t* data, uint32_t stride, uint32_t w, uint32_t h,
     surface->cs = cs;
     surface->channelSize = CHANNEL_SIZE(cs);
     surface->premultiplied = true;
-    surface->owner = true;
 
     vport.x = vport.y = 0;
     vport.w = surface->w;
@@ -638,11 +635,8 @@ Compositor* SwRenderer::target(const RenderRegion& region, ColorSpace cs)
 
     //New Composition
     if (!cmp) {
-        cmp = new SwSurface;
-
         //Inherits attributes from main surface
-        *cmp = *surface;
+        cmp = new SwSurface(surface);
-
         cmp->compositor = new SwCompositor;
 
         //TODO: We can optimize compositor surface size from (surface->stride x surface->h) to Parameter(w x h)

src/renderer/tvgLoadModule.h

@@ -69,13 +69,18 @@ struct LoadModule
 struct ImageLoader : LoadModule
 {
     float w = 0, h = 0;                             //default image size
-    ColorSpace cs = ColorSpace::Unsupported;        //must be clarified at open()
+    Surface surface;
 
     ImageLoader(FileType type) : LoadModule(type) {}
 
     virtual bool animatable() { return false; }  //true if this loader supports animation.
-    virtual Surface* bitmap() { return nullptr; }
     virtual Paint* paint() { return nullptr; }
+
+    virtual Surface* bitmap()
+    {
+        if (surface.data) return &surface;
+        return nullptr;
+    }
 };
 
 

src/renderer/tvgPicture.h

@@ -83,7 +83,6 @@ struct Picture::Impl
     {
         LoaderMgr::retrieve(loader);
         delete(paint);
-        delete(surface);
     }
 
     bool dispose(RenderMethod& renderer)
@@ -206,11 +205,7 @@ struct Picture::Impl
             ++dup->loader->sharing;
         }
 
-        if (surface) {
+        dup->surface = surface;
-            dup->surface = new Surface;
-            *dup->surface = *surface;
-            dup->surface->owner = false;
-        }
         dup->w = w;
         dup->h = h;
         dup->resizing = resizing;

src/renderer/tvgRender.h

@@ -23,6 +23,7 @@
 #ifndef _TVG_RENDER_H_
 #define _TVG_RENDER_H_
 
+#include <mutex>
 #include "tvgCommon.h"
 #include "tvgArray.h"
 
@@ -49,17 +50,33 @@ enum ColorSpace
 struct Surface
 {
     union {
-        pixel_t* data;       //system based data pointer
+        pixel_t* data = nullptr;    //system based data pointer
-        uint32_t* buf32;     //for explicit 32bits channels
+        uint32_t* buf32;            //for explicit 32bits channels
-        uint8_t*  buf8;      //for explicit 8bits grayscale
+        uint8_t*  buf8;             //for explicit 8bits grayscale
     };
-    uint32_t stride;
+    mutex mtx;                      //used for thread safety
-    uint32_t w, h;
+    uint32_t stride = 0;
-    ColorSpace  cs;
+    uint32_t w = 0, h = 0;
-    uint8_t channelSize;
+    ColorSpace cs = ColorSpace::Unsupported;
+    uint8_t channelSize = 0;
+    bool premultiplied = 0;         //Alpha-premultiplied
+
+    Surface()
+    {
+    }
+
+    Surface(const Surface* rhs)
+    {
+        data = rhs->data;
+        stride = rhs->stride;
+        w = rhs->w;
+        h = rhs->h;
+        cs = rhs->cs;
+        channelSize = rhs->channelSize;
+        premultiplied = rhs->premultiplied;
+    }
+
 
-    bool premultiplied;      //Alpha-premultiplied
-    bool owner;              //Only owner could modify the buffer
 };
 
 struct Compositor

src/renderer/wg_engine/tvgWgRenderer.h

@@ -67,7 +67,7 @@ private:
     WgRenderTarget mRenderTarget;
     WGPUSurface mSurface{};
     WGPUSwapChain mSwapChain{};
-    Surface mTargetSurface = { nullptr, 0, 0, 0, ColorSpace::Unsupported, true };
+    Surface mTargetSurface;
 };
 
 #endif /* _TVG_WG_RENDERER_H_ */