Changeset 251651 in webkit

Timestamp:

Oct 28, 2019 3:12:43 AM (5 years ago)

Author:

cturner@igalia.com

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):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/ImageBackingStore.h (modified) (6 diffs)
• platform/image-decoders/cairo/ImageBackingStoreCairo.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-10-28  Charlie Turner  <cturner@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  <cgarcia@igalia.com>
 

trunk/Source/WebCore/platform/graphics/ImageBackingStore.h

@@ -60 +60 @@
             return false;
 
-        Vector<char> buffer;
-        size_t bufferSize = size.area().unsafeGet() * sizeof(RGBA32);
-
-        if (!buffer.tryReserveCapacity(bufferSize))
+        m_pixels = RGBAPixelBufferThreadSafeRefCounted::create(size);
+
+        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);
@@ -87 +83 @@
     void clear()
     {
-        memset(m_pixelsPtr, 0, (m_size.area() * sizeof(RGBA32)).unsafeGet());
+        m_pixels->zeroPixelData();
     }
 
@@ -134 +130 @@
     {
         ASSERT(inBounds(IntPoint(x, y)));
-        return m_pixelsPtr + y * m_size.width() + x;
+        return m_pixels->pixelAt(x, y);
     }
 
@@ -190 +186 @@
 
 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)
@@ -202 +219 @@
     {
         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()));
     }
 
@@ -227 +242 @@
     }
 
-    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

trunk/Source/WebCore/platform/image-decoders/cairo/ImageBackingStoreCairo.cpp

@@ -35 +35 @@
     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;