Bug 226384 – Make ImageBuffer be able to put pixels directly into its backend from another ImageBuffer

• Component: Layout and Rendering
• Status: NEW

[Said Abou-Hallawa]

This can replace the call to ImageBuffer::draw() where alpha blending or scaling is required, for example the call in SourceGraphic::platformApplySoftware().

[Said Abou-Hallawa]

I meant: ...where alpha blending or scaling is NOT required.

[Said Abou-Hallawa]

Created attachment 430032 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 430032 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=430032&action=review

> Source/WebCore/platform/graphics/ImageBuffer.cpp:195
> +    if (auto pixelBuffer = sourceImage.getPixelBuffer(format, srcRect))

For buffers with GPU backing (IOSurfaces) does this trigger readback?

[Simon Fraser (smfr)]

Comment on attachment 430032 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=430032&action=review

> Source/WebCore/platform/graphics/ImageBuffer.cpp:187
> +    if (resolutionScale() != sourceImage.resolutionScale()) {

Doesn't this also need to check color profiles and maybe more?

[Said Abou-Hallawa]

Comment on attachment 430032 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=430032&action=review

>> Source/WebCore/platform/graphics/ImageBuffer.cpp:195
>> +    if (auto pixelBuffer = sourceImage.getPixelBuffer(format, srcRect))
> 
> For buffers with GPU backing (IOSurfaces) does this trigger readback?

Oh yes. I was thinking about filters in WebProcess. For GPUP case, I think we need to create a DL item which can only be used if both source and destination are in GPUP.

[Said Abou-Hallawa]

This code can be optimized more if we copy from the source backend to the destination backend directly without going through the PixelBuffer.

[Said Abou-Hallawa]

Comment on attachment 430032 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=430032&action=review

>> Source/WebCore/platform/graphics/ImageBuffer.cpp:187
>> +    if (resolutionScale() != sourceImage.resolutionScale()) {
> 
> Doesn't this also need to check color profiles and maybe more?

I think getPixelBuffer() will take care of these. The source PixelBufferFormat below is set to get the pixels in the destination colorSpace() and pixelFormat().

[Said Abou-Hallawa]

Created attachment 430046 [details]
Patch

[Said Abou-Hallawa]

Created attachment 430065 [details]
Patch

[Said Abou-Hallawa]

Created attachment 430066 [details]
Patch

[Sam Weinig]

Comment on attachment 430066 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=430066&action=review

> Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:2088
> +    PutImageBuffer(RenderingResourceIdentifier imageBufferIdentifier, const IntPoint& destPoint)

Does anything need to be done to ensure the sourceImage is not mutated before this can happen?

[Said Abou-Hallawa]

Created attachment 430110 [details]
Patch

[Said Abou-Hallawa]

Comment on attachment 430066 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=430066&action=review

>> Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:2088
>> +    PutImageBuffer(RenderingResourceIdentifier imageBufferIdentifier, const IntPoint& destPoint)
> 
> Does anything need to be done to ensure the sourceImage is not mutated before this can happen?

Yes you are right. PutImageBuffer should be a synchronous message after making sure all the pending items in both source and destination ImageBuffers are flushed to their backends.

[Said Abou-Hallawa]

Created attachment 430111 [details]
Patch

[Said Abou-Hallawa]

Created attachment 430196 [details]
Patch

[Said Abou-Hallawa]

Created attachment 430198 [details]
Patch

[Said Abou-Hallawa]

Created attachment 430199 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/78878491>