Title: [269526] trunk/Source/WebKit
Revision
269526
Author
wenson_hs...@apple.com
Date
2020-11-06 11:29:32 -0800 (Fri, 06 Nov 2020)

Log Message

Encoding PutImageData should not serialize separate IPC attachments for the image data
https://bugs.webkit.org/show_bug.cgi?id=218649

Reviewed by Simon Fraser.

The argument coder for `WebCore::ImageData` currently sends its image data by allocating a separate shared
memory buffer and sending it over to the GPU process via a separate `IPC::Attachment`. This isn't compatible
with the new "concurrent display list" model for processing display list items, wherein all out-of-line items
are encoded as raw bytes directly into reusable shared memory buffers and decoded from the corresponding buffer
in the GPU process.

Instead, encode and decode image data directly into and out of the main IPC data buffer. This also addresses a
couple of existing FIXMEs in the ImageData coder methods by avoiding the need for redundant copies to and from
temporary `WebCore::SharedBuffer`s.

* Shared/WebCoreArgumentCoders.cpp:
(IPC::ArgumentCoder<Ref<WebCore::ImageData>>::encode):
(IPC::ArgumentCoder<Ref<WebCore::ImageData>>::decode):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (269525 => 269526)


--- trunk/Source/WebKit/ChangeLog	2020-11-06 18:54:52 UTC (rev 269525)
+++ trunk/Source/WebKit/ChangeLog	2020-11-06 19:29:32 UTC (rev 269526)
@@ -1,5 +1,26 @@
 2020-11-06  Wenson Hsieh  <wenson_hs...@apple.com>
 
+        Encoding PutImageData should not serialize separate IPC attachments for the image data
+        https://bugs.webkit.org/show_bug.cgi?id=218649
+
+        Reviewed by Simon Fraser.
+
+        The argument coder for `WebCore::ImageData` currently sends its image data by allocating a separate shared
+        memory buffer and sending it over to the GPU process via a separate `IPC::Attachment`. This isn't compatible
+        with the new "concurrent display list" model for processing display list items, wherein all out-of-line items
+        are encoded as raw bytes directly into reusable shared memory buffers and decoded from the corresponding buffer
+        in the GPU process.
+
+        Instead, encode and decode image data directly into and out of the main IPC data buffer. This also addresses a
+        couple of existing FIXMEs in the ImageData coder methods by avoiding the need for redundant copies to and from
+        temporary `WebCore::SharedBuffer`s.
+
+        * Shared/WebCoreArgumentCoders.cpp:
+        (IPC::ArgumentCoder<Ref<WebCore::ImageData>>::encode):
+        (IPC::ArgumentCoder<Ref<WebCore::ImageData>>::decode):
+
+2020-11-06  Wenson Hsieh  <wenson_hs...@apple.com>
+
         [Concurrent display lists] Encode display list items directly into shared memory
         https://bugs.webkit.org/show_bug.cgi?id=218406
 

Modified: trunk/Source/WebKit/Shared/WebCoreArgumentCoders.cpp (269525 => 269526)


--- trunk/Source/WebKit/Shared/WebCoreArgumentCoders.cpp	2020-11-06 18:54:52 UTC (rev 269525)
+++ trunk/Source/WebKit/Shared/WebCoreArgumentCoders.cpp	2020-11-06 19:29:32 UTC (rev 269526)
@@ -3188,28 +3188,30 @@
 
 void ArgumentCoder<Ref<WebCore::ImageData>>::encode(Encoder& encoder, const Ref<WebCore::ImageData>& imageData)
 {
-    // FIXME: Copying from the ImageData to the SharedBuffer is slow. Invent some way for the SharedBuffer to be populated directly.
-    auto sharedBuffer = WebCore::SharedBuffer::create(imageData->data()->data(), imageData->data()->byteLength());
     encoder << imageData->size();
-    encoder << sharedBuffer;
+
+    auto rawData = imageData->data();
+    encoder << static_cast<uint64_t>(rawData->byteLength());
+    encoder.encodeFixedLengthData(rawData->data(), rawData->byteLength(), 1);
 }
 
 Optional<Ref<WebCore::ImageData>> ArgumentCoder<Ref<WebCore::ImageData>>::decode(Decoder& decoder)
 {
     Optional<IntSize> imageDataSize;
-    Optional<Ref<SharedBuffer>> data;
-
     decoder >> imageDataSize;
     if (!imageDataSize)
         return WTF::nullopt;
 
-    decoder >> data;
-    if (!data)
+    Optional<uint64_t> dataLength;
+    decoder >> dataLength;
+    if (!dataLength)
         return WTF::nullopt;
 
-    // FIXME: Copying from the SharedBuffer into the ImageData is slow. Invent some way for the ImageData to simply just retain the SharedBuffer, and use it internally.
-    // Alternatively, we could create an overload for putImageData() which operates on the SharedBuffer directly.
-    auto imageData = ImageData::create(*imageDataSize, Uint8ClampedArray::create(reinterpret_cast<const uint8_t*>((*data)->data()), (*data)->size()));
+    auto rawData = Uint8ClampedArray::createUninitialized(*dataLength);
+    if (!decoder.decodeFixedLengthData(rawData->data(), rawData->length(), 1))
+        return WTF::nullopt;
+
+    auto imageData = ImageData::create(*imageDataSize, WTFMove(rawData));
     if (!imageData)
         return WTF::nullopt;
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to