Title: [251651] trunk/Source/WebCore
Revision
251651
Author
ctur...@igalia.com
Date
2019-10-28 03:12:43 -0700 (Mon, 28 Oct 2019)

Log Message

ImageDecoders: use a thread safe data buffer for Cairo backing store
https://bugs.webkit.org/show_bug.cgi?id=201727

Reviewed by Carlos Garcia Campos.

When an image resource gets cached and replaces an existing image,
CachedImage::didReplaceSharedBufferContents is called, which
destroys the decoder in the BitmapImage class. This decoder can be
initialized from any thread via
ImageSource::ensureDecoderAvailable. On GTK/WPE, this dispatches
to a ScalableImageDecoder, which contains a vector of
ScalableImageDecoderFrame's, which contain ImageBackingStore's,
which for reasons related to Cairo, contain a RefCounted
SharedBuffer of pixel data.

The problem is that the CachedImage's decoders can be
created/destroyed on different threads, so a thread-safe buffer
class is required to hold these data, and pass them safely into
cairo_image_surface_create_for_data rather than a SharedBuffer
which must be created/destroyed on the main-thread.

Covered by existing tests.

* platform/graphics/ImageBackingStore.h: Create a small
thread-safe utility class to hold the RGBA pixel data.
(WebCore::ImageBackingStore::setSize):
(WebCore::ImageBackingStore::clear):
(WebCore::ImageBackingStore::pixelAt const):
(WebCore::ImageBackingStore::ThreadSafeRGBAPixelBuffer::create):
(WebCore::ImageBackingStore::ThreadSafeRGBAPixelBuffer::zeroPixelData):
(WebCore::ImageBackingStore::ThreadSafeRGBAPixelBuffer::pixelAt const):
(WebCore::ImageBackingStore::ThreadSafeRGBAPixelBuffer::data const):
(WebCore::ImageBackingStore::ThreadSafeRGBAPixelBuffer::isValid const):
(WebCore::ImageBackingStore::ThreadSafeRGBAPixelBuffer::RGBAPixelBufferThreadSafeRefCounted):
(WebCore::ImageBackingStore::ImageBackingStore):
* platform/image-decoders/cairo/ImageBackingStoreCairo.cpp:
(WebCore::ImageBackingStore::image const):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (251650 => 251651)


--- trunk/Source/WebCore/ChangeLog	2019-10-28 09:24:57 UTC (rev 251650)
+++ trunk/Source/WebCore/ChangeLog	2019-10-28 10:12:43 UTC (rev 251651)
@@ -1,3 +1,43 @@
+2019-10-28  Charlie Turner  <ctur...@igalia.com>
+
+        ImageDecoders: use a thread safe data buffer for Cairo backing store
+        https://bugs.webkit.org/show_bug.cgi?id=201727
+
+        Reviewed by Carlos Garcia Campos.
+
+        When an image resource gets cached and replaces an existing image,
+        CachedImage::didReplaceSharedBufferContents is called, which
+        destroys the decoder in the BitmapImage class. This decoder can be
+        initialized from any thread via
+        ImageSource::ensureDecoderAvailable. On GTK/WPE, this dispatches
+        to a ScalableImageDecoder, which contains a vector of
+        ScalableImageDecoderFrame's, which contain ImageBackingStore's,
+        which for reasons related to Cairo, contain a RefCounted
+        SharedBuffer of pixel data.
+
+        The problem is that the CachedImage's decoders can be
+        created/destroyed on different threads, so a thread-safe buffer
+        class is required to hold these data, and pass them safely into
+        cairo_image_surface_create_for_data rather than a SharedBuffer
+        which must be created/destroyed on the main-thread.
+
+        Covered by existing tests.
+
+        * platform/graphics/ImageBackingStore.h: Create a small
+        thread-safe utility class to hold the RGBA pixel data.
+        (WebCore::ImageBackingStore::setSize):
+        (WebCore::ImageBackingStore::clear):
+        (WebCore::ImageBackingStore::pixelAt const):
+        (WebCore::ImageBackingStore::ThreadSafeRGBAPixelBuffer::create):
+        (WebCore::ImageBackingStore::ThreadSafeRGBAPixelBuffer::zeroPixelData):
+        (WebCore::ImageBackingStore::ThreadSafeRGBAPixelBuffer::pixelAt const):
+        (WebCore::ImageBackingStore::ThreadSafeRGBAPixelBuffer::data const):
+        (WebCore::ImageBackingStore::ThreadSafeRGBAPixelBuffer::isValid const):
+        (WebCore::ImageBackingStore::ThreadSafeRGBAPixelBuffer::RGBAPixelBufferThreadSafeRefCounted):
+        (WebCore::ImageBackingStore::ImageBackingStore):
+        * platform/image-decoders/cairo/ImageBackingStoreCairo.cpp:
+        (WebCore::ImageBackingStore::image const):
+
 2019-10-28  Carlos Garcia Campos  <cgar...@igalia.com>
 
         [GTK] Simplify the Input Method implementation

Modified: trunk/Source/WebCore/platform/graphics/ImageBackingStore.h (251650 => 251651)


--- trunk/Source/WebCore/platform/graphics/ImageBackingStore.h	2019-10-28 09:24:57 UTC (rev 251650)
+++ trunk/Source/WebCore/platform/graphics/ImageBackingStore.h	2019-10-28 10:12:43 UTC (rev 251651)
@@ -59,15 +59,11 @@
         if (size.isEmpty())
             return false;
 
-        Vector<char> buffer;
-        size_t bufferSize = size.area().unsafeGet() * sizeof(RGBA32);
+        m_pixels = RGBAPixelBufferThreadSafeRefCounted::create(size);
 
-        if (!buffer.tryReserveCapacity(bufferSize))
+        if (!m_pixels->isValid())
             return false;
 
-        buffer.grow(bufferSize);
-        m_pixels = SharedBuffer::create(WTFMove(buffer));
-        m_pixelsPtr = reinterpret_cast<RGBA32*>(const_cast<char*>(m_pixels->data()));
         m_size = size;
         m_frameRect = IntRect(IntPoint(), m_size);
         clear();
@@ -86,7 +82,7 @@
 
     void clear()
     {
-        memset(m_pixelsPtr, 0, (m_size.area() * sizeof(RGBA32)).unsafeGet());
+        m_pixels->zeroPixelData();
     }
 
     void clearRect(const IntRect& rect)
@@ -133,7 +129,7 @@
     RGBA32* pixelAt(int x, int y) const
     {
         ASSERT(inBounds(IntPoint(x, y)));
-        return m_pixelsPtr + y * m_size.width() + x;
+        return m_pixels->pixelAt(x, y);
     }
 
     void setPixel(RGBA32* dest, unsigned r, unsigned g, unsigned b, unsigned a)
@@ -189,6 +185,27 @@
     }
 
 private:
+    class RGBAPixelBufferThreadSafeRefCounted : public ThreadSafeRefCounted<RGBAPixelBufferThreadSafeRefCounted> {
+    public:
+        static Ref<RGBAPixelBufferThreadSafeRefCounted> create(const IntSize& initialSize) { return adoptRef(*new RGBAPixelBufferThreadSafeRefCounted(initialSize)); }
+        void zeroPixelData() { m_pixels.fill(0); }
+        RGBA32* pixelAt(int x, int y) const { return const_cast<unsigned*>(&m_pixels.data()[y * m_size.width() + x]); }
+        const RGBA32* data() const { return m_pixels.data(); }
+        bool isValid() const { return m_isValid; }
+    private:
+        RGBAPixelBufferThreadSafeRefCounted(const IntSize& initialSize)
+            : m_size(initialSize)
+        {
+            unsigned bufferSize = initialSize.area().unsafeGet();
+            m_isValid = m_pixels.tryReserveCapacity(bufferSize);
+            if (m_isValid)
+                m_pixels.resize(bufferSize);
+        }
+        IntSize m_size;
+        Vector<RGBA32> m_pixels;
+        bool m_isValid { false };
+    };
+
     ImageBackingStore(const IntSize& size, bool premultiplyAlpha = true)
         : m_premultiplyAlpha(premultiplyAlpha)
     {
@@ -201,8 +218,6 @@
         , m_premultiplyAlpha(other.m_premultiplyAlpha)
     {
         ASSERT(!m_size.isEmpty() && !isOverSize(m_size));
-        m_pixels = SharedBuffer::create(other.m_pixels->data(), other.m_pixels->size());
-        m_pixelsPtr = reinterpret_cast<RGBA32*>(const_cast<char*>(m_pixels->data()));
     }
 
     bool inBounds(const IntPoint& point) const
@@ -226,8 +241,8 @@
         return makeRGBA(r, g, b, a);
     }
 
-    RefPtr<SharedBuffer> m_pixels;
-    RGBA32* m_pixelsPtr { nullptr };
+    RefPtr<RGBAPixelBufferThreadSafeRefCounted> m_pixels;
+
     IntSize m_size;
     IntRect m_frameRect; // This will always just be the entire buffer except for GIF and PNG frames
     bool m_premultiplyAlpha { true };

Modified: trunk/Source/WebCore/platform/image-decoders/cairo/ImageBackingStoreCairo.cpp (251650 => 251651)


--- trunk/Source/WebCore/platform/image-decoders/cairo/ImageBackingStoreCairo.cpp	2019-10-28 09:24:57 UTC (rev 251650)
+++ trunk/Source/WebCore/platform/image-decoders/cairo/ImageBackingStoreCairo.cpp	2019-10-28 10:12:43 UTC (rev 251651)
@@ -34,10 +34,10 @@
 {
     m_pixels->ref();
     RefPtr<cairo_surface_t> surface = adoptRef(cairo_image_surface_create_for_data(
-        reinterpret_cast<unsigned char*>(const_cast<uint32_t*>(m_pixelsPtr)),
+        reinterpret_cast<unsigned char*>(const_cast<uint32_t*>(m_pixels->data())),
         CAIRO_FORMAT_ARGB32, size().width(), size().height(), size().width() * sizeof(uint32_t)));
     static cairo_user_data_key_t s_surfaceDataKey;
-    cairo_surface_set_user_data(surface.get(), &s_surfaceDataKey, m_pixels.get(), [](void* data) { static_cast<SharedBuffer*>(data)->deref(); });
+    cairo_surface_set_user_data(surface.get(), &s_surfaceDataKey, m_pixels.get(), [](void* data) { static_cast<RGBAPixelBufferThreadSafeRefCounted*>(data)->deref(); });
 
     return surface;
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to