[GPU Process] [Filters] Make PixelBuffer an abstract class #1261

shallawa · 2022-06-03T00:43:25Z

`76d7b2f`

[GPU Process] [Filters] Make PixelBuffer an abstract class
https://bugs.webkit.org/show_bug.cgi?id=240809
<rdar://problem/94117981 >

Reviewed by Simon Fraser.

Hide the current underlying memory of PixelBuffer which is 'Uint8ClampedArray'
and move it to a new subclass named ByteArrayPixelBuffer.

All clients should use the virtual methods 'bytes()' and 'sizeInBytes()' to have
access to the pixel bytes. All calls to 'PixelBuffer::tryCreate()' should be
replaced with 'ByteArrayPixelBuffer::tryCreate()'.

In future patches and when GPUProcess applies software filters, we are going to
create new subclass of PixelBuffer for the result FilterImages. This new sub-
class will use SharedMemory as its underlying pixel data and it will attribute
this SharedMemory to WebProcess.

* Source/WebCore/Headers.cmake:
* Source/WebCore/Sources.txt:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
* Source/WebCore/bindings/js/SerializedScriptValue.cpp:
(WebCore::CloneSerializer::dumpImageBitmap):
(WebCore::CloneDeserializer::readImageBitmap):
* Source/WebCore/html/HTMLCanvasElement.cpp:
(WebCore::HTMLCanvasElement::getImageData):
* Source/WebCore/html/ImageData.cpp:
(WebCore::ImageData::create):
(WebCore::ImageData::pixelBuffer const):
* Source/WebCore/html/ImageData.h:
* Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp:
(WebCore::CanvasRenderingContext2DBase::getImageData const):
* Source/WebCore/page/PageColorSampler.cpp:
(WebCore::sampleColor):
* Source/WebCore/platform/graphics/GraphicsContextGL.cpp:
(WebCore::GraphicsContextGL::extractPixelBuffer):
* Source/WebCore/platform/graphics/ImageBufferAllocator.cpp:
(WebCore::ImageBufferAllocator::createPixelBuffer const):
* Source/WebCore/platform/graphics/ImageBufferBackend.cpp:
(WebCore::ImageBufferBackend::convertToLuminanceMask):
(WebCore::ImageBufferBackend::getPixelBuffer const):
(WebCore::ImageBufferBackend::putPixelBuffer):
* Source/WebCore/platform/graphics/PixelBuffer.cpp:
(WebCore::PixelBuffer::supportedPixelFormat):
(WebCore::PixelBuffer::PixelBuffer):
(WebCore::PixelBuffer::tryCreateForDecoding): Deleted.
(WebCore::PixelBuffer::tryCreate): Deleted.
(WebCore::PixelBuffer::create): Deleted.
(WebCore::PixelBuffer::bytes const): Deleted.
(WebCore::PixelBuffer::sizeInBytes const): Deleted.
(WebCore::PixelBuffer::createScratchPixelBuffer const): Deleted.
(WebCore::operator<<): Deleted.
* Source/WebCore/platform/graphics/PixelBuffer.h:
(WebCore::PixelBuffer::size const):
(WebCore::PixelBuffer::isByteArrayPixelBuffer const):
(WebCore::PixelBuffer::data const): Deleted.
(WebCore::PixelBuffer::takeData): Deleted.
(WebCore::PixelBuffer::encode const): Deleted.
(WebCore::PixelBuffer::decode): Deleted.
* Source/WebCore/platform/graphics/ShadowBlur.cpp:
(WebCore::ShadowBlur::blurShadowBuffer):
* Source/WebCore/platform/graphics/ByteArrayPixelBuffer.cpp: Added.
(WebCore::ByteArrayPixelBuffer::create):
(WebCore::ByteArrayPixelBuffer::tryCreate):
(WebCore::ByteArrayPixelBuffer::tryCreateForDecoding):
(WebCore::ByteArrayPixelBuffer::ByteArrayPixelBuffer):
(WebCore::ByteArrayPixelBuffer::bytes const):
(WebCore::ByteArrayPixelBuffer::sizeInBytes const):
(WebCore::ByteArrayPixelBuffer::createScratchPixelBuffer const):
* Source/WebCore/platform/graphics/ByteArrayPixelBuffer.h: Added.
(WebCore::ByteArrayPixelBuffer::data const):
(WebCore::ByteArrayPixelBuffer::takeData):
(WebCore::ByteArrayPixelBuffer::encode const):
(WebCore::ByteArrayPixelBuffer::decode):
(isType):
* Source/WebCore/platform/graphics/angle/GraphicsContextGLANGLE.cpp:
(WebCore::GraphicsContextGLANGLE::readPixelsForPaintResults):
(WebCore::GraphicsContextGLANGLE::paintRenderingResultsToPixelBuffer):
* Source/WebCore/platform/graphics/cg/GraphicsContextGLCG.cpp:
(WebCore::GraphicsContextGL::paintToCanvas):
* Source/WebCore/platform/graphics/cg/ImageBufferCGBackend.cpp:
(WebCore::ImageBufferCGBackend::copyCGImageForEncoding const):
* Source/WebCore/platform/graphics/cg/ImageBufferUtilitiesCG.cpp:
(WebCore::encode):
* Source/WebCore/platform/graphics/cv/VideoFrameCV.mm:
(WebCore::VideoFrameCV::createFromPixelBuffer):
* Source/WebCore/platform/graphics/filters/FilterImage.cpp:
(WebCore::copyImageBytes):
* Source/WebCore/rendering/shapes/Shape.cpp:
(WebCore::Shape::createRasterShape):
* Source/WebKit/GPUProcess/graphics/RemoteGraphicsContextGL.cpp:
(WebKit::RemoteGraphicsContextGL::paintPixelBufferToImageBuffer):
* Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp:
(WebKit::RemoteRenderingBackend::getPixelBufferForImageBuffer):
* Source/WebKit/Platform/IPC/PixelBufferReference.h:
(IPC::PixelBufferReference::encode const):
(IPC::PixelBufferReference::decode):
* Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h:
* Tools/TestWebKitAPI/Tests/WebCore/ImageBufferTests.cpp:
(TestWebKitAPI::imageBufferPixelIs):

Canonical link: https://commits.webkit.org/251282@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295227 268f45cc-cd09-0410-ab3c-d52691b4dbfc

smfr · 2022-06-03T03:48:01Z

Source/WebCore/platform/graphics/Uint8ClampedPixelBuffer.h

+
+namespace WebCore {
+
+class Uint8ClampedPixelBuffer : public PixelBuffer {


Uint8ClampedPixelBuffer is a bit of a mouthful. How about ByteArrayPixelBuffer?

The name of the class was changed.

smfr · 2022-06-03T03:52:48Z

Source/WebCore/bindings/js/SerializedScriptValue.cpp

@@ -1116,7 +1117,7 @@ class CloneSerializer : CloneBase {
            return;
        }

-        auto arrayBuffer = pixelBuffer->data().possiblySharedBuffer();
+        auto arrayBuffer = downcast<Uint8ClampedPixelBuffer>(*pixelBuffer).data().possiblySharedBuffer();


Do we know it's a Uint8ClampedPixelBuffer here?

Yes it should be of type ByteArrayPixelBuffer since we are passing the default ImageBufferAllocator to getPixelBuffer(). But I changed the if-statement which comes before this code fromif (!pixelBuffer)to if (!is<ByteArrayPixelBuffer>(pixelBuffer)) to make it clear that this is what we expect.

smfr · 2022-06-03T04:01:00Z

Source/WebCore/html/HTMLCanvasElement.cpp

@@ -766,7 +766,8 @@ RefPtr<ImageData> HTMLCanvasElement::getImageData()
    if (is<WebGLRenderingContextBase>(m_context)) {
        if (RuntimeEnabledFeatures::sharedFeatures().webAPIStatisticsEnabled())
            ResourceLoadObserver::shared().logCanvasRead(document());
-        return ImageData::create(downcast<WebGLRenderingContextBase>(*m_context).paintRenderingResultsToPixelBuffer());
+        auto pixelBuffer = downcast<WebGLRenderingContextBase>(*m_context).paintRenderingResultsToPixelBuffer();
+        return ImageData::create(static_pointer_cast<Uint8ClampedPixelBuffer>(WTFMove(pixelBuffer)));


I guess we know that WebGLRenderingContextBase will return a Uint8ClampedPixelBuffer?

Yes it should be of type ByteArrayPixelBuffer because GraphicsContextGLANGLE::readPixelsForPaintResults() calls ByteArrayPixelBuffer::tryCreate() for the returned PixelBuffer. But I added the new if-statement before this casting

if (!is<ByteArrayPixelBuffer>(pixelBuffer)) return nullptr;

to make it clear that this is the expectation.

smfr · 2022-06-03T04:04:41Z

Source/WebCore/platform/graphics/PixelBuffer.h


-    Ref<JSC::Uint8ClampedArray>&& takeData() { return WTFMove(m_data); }
+    virtual uint8_t* bytes() const = 0;


Every call to item() now calls the bytes() virtual function. I think this would be a measurable performance hit for filter effect code that can do this inside tight loops.

Also since item() is exported, it won't be inlined.

Can we fetch the start pointer once via a virtual function at creation time, and after that just index off it?

You are right.

I changed ByteArrayPixelBuffer to pass the raw data pointer and the sizeInBytes to the PixelBuffer constructor and I made the methods bytes() and sizeInBytes() non virtual. I think I can't call virtual methods from the base class constructor.

smfr · 2022-06-03T04:11:08Z

Source/WebCore/platform/graphics/Uint8ClampedPixelBuffer.cpp

+    return create(format, size, data.releaseNonNull());
+}
+
+RefPtr<Uint8ClampedPixelBuffer> Uint8ClampedPixelBuffer::tryCreateForDecoding(const PixelBufferFormat& format, const IntSize& size, unsigned dataByteLength)


Not new, but it's not clear to me how a "for decoding" buffer is different.

tryCreateForDecoding() was renamed to tryCreate()

webkit-early-warning-system · 2022-06-03T20:53:41Z

Committed r295227 (251282@main): https://commits.webkit.org/251282@main

Reviewed commits have been landed. Closing PR #1261 and removing active labels.

shallawa self-assigned this Jun 3, 2022

shallawa added Layout and Rendering For bugs with layout and rendering of Web pages. WebKit Nightly Build labels Jun 3, 2022

shallawa requested review from weinig and smfr June 3, 2022 00:44

shallawa force-pushed the eng/GPU-Process-Filters-Make-PixelBuffer-an-abstract-class branch from a401f0e to e75e0ff Compare June 3, 2022 01:36

webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label Jun 3, 2022

smfr reviewed Jun 3, 2022

View reviewed changes

shallawa removed merging-blocked Applied to prevent a change from being merged Layout and Rendering For bugs with layout and rendering of Web pages. WebKit Nightly Build labels Jun 3, 2022

shallawa force-pushed the eng/GPU-Process-Filters-Make-PixelBuffer-an-abstract-class branch from e75e0ff to fb00f36 Compare June 3, 2022 06:00

shallawa added Layout and Rendering For bugs with layout and rendering of Web pages. WebKit Nightly Build labels Jun 3, 2022

shallawa force-pushed the eng/GPU-Process-Filters-Make-PixelBuffer-an-abstract-class branch from fb00f36 to bfadc14 Compare June 3, 2022 06:10

webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label Jun 3, 2022

shallawa removed merging-blocked Applied to prevent a change from being merged Layout and Rendering For bugs with layout and rendering of Web pages. WebKit Nightly Build labels Jun 3, 2022

shallawa force-pushed the eng/GPU-Process-Filters-Make-PixelBuffer-an-abstract-class branch from bfadc14 to 48e7647 Compare June 3, 2022 07:40

shallawa added Layout and Rendering For bugs with layout and rendering of Web pages. WebKit Nightly Build labels Jun 3, 2022

shallawa force-pushed the eng/GPU-Process-Filters-Make-PixelBuffer-an-abstract-class branch from 48e7647 to 8d8ee87 Compare June 3, 2022 17:07

smfr approved these changes Jun 3, 2022

View reviewed changes

shallawa added the merge-queue Applied to send a pull request to merge-queue label Jun 3, 2022

webkit-early-warning-system closed this Jun 3, 2022

webkit-early-warning-system force-pushed the eng/GPU-Process-Filters-Make-PixelBuffer-an-abstract-class branch from 8d8ee87 to 76d7b2f Compare June 3, 2022 20:53

webkit-early-warning-system merged commit 76d7b2f into WebKit:main Jun 3, 2022

webkit-early-warning-system removed the merge-queue Applied to send a pull request to merge-queue label Jun 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GPU Process] [Filters] Make PixelBuffer an abstract class #1261

[GPU Process] [Filters] Make PixelBuffer an abstract class #1261

shallawa commented Jun 3, 2022 •

edited by webkit-early-warning-system

smfr Jun 3, 2022

shallawa Jun 3, 2022

smfr Jun 3, 2022

shallawa Jun 3, 2022

smfr Jun 3, 2022

shallawa Jun 3, 2022

smfr Jun 3, 2022

shallawa Jun 3, 2022

smfr Jun 3, 2022

shallawa Jun 3, 2022

webkit-early-warning-system commented Jun 3, 2022


		namespace WebCore {

		class Uint8ClampedPixelBuffer : public PixelBuffer {


		Ref<JSC::Uint8ClampedArray>&& takeData() { return WTFMove(m_data); }
		virtual uint8_t* bytes() const = 0;

[GPU Process] [Filters] Make PixelBuffer an abstract class #1261

[GPU Process] [Filters] Make PixelBuffer an abstract class #1261

Conversation

shallawa commented Jun 3, 2022 • edited by webkit-early-warning-system

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webkit-early-warning-system commented Jun 3, 2022

shallawa commented Jun 3, 2022 •

edited by webkit-early-warning-system