Bug 240100 – [GPU Process] [iOS] REGRESSION(r293570): Snapshot rendering is not scaled with the device scale factor

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Said Abou-Hallawa]

In r293570, the GPU Process was enabled for snapshot images. Before r293570, a local ImageBuffer was created in snapshotFrameRectWithClip(). Then in WebFrame::createSelectionSnapshot(), the pixels of the ImageBuffer were copied to a WebImage, which was backed by a ShareableBitmap.

r293570 made snapshotFrameRectWithClip() create a remote ImageBuffer which is moved to a WebImage in WebFrame::createSelectionSnapshot(). This remote ImageBuffer has to have ImageBufferShareableBitmapBackend so it can provide ShareableBitmap without allocating any extra memory.

The problem is in initializing the GraphicsContext of ImageBufferShareableBitmapBackend. The scaling factor is not set to the CTM of the ImageBufferShareableBitmapBackend context. See the comment in ImageBufferShareableBitmapBackend constructor and notice that this function does not call applyBaseTransformToContext().

A simple approach to fix this bug is to make snapshotFrameRectWithClip() handle the scaling outside the ImageBuffer creation. This is similar to what we do in GraphicsContext::createAlignedImageBuffer() where we scale the size and create the ImageBuffer with scaleFactor = 1.

[Said Abou-Hallawa]

rdar://92635752

[Said Abou-Hallawa]

Created attachment 458839 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 458839 [details]
Patch

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

> Source/WebCore/page/FrameSnapshotting.cpp:124
> -    auto buffer = ImageBuffer::create(imageRect.size(), purpose, scaleFactor, options.colorSpace, options.pixelFormat, { }, { hostWindow });
> +    auto buffer = ImageBuffer::create(scaledImageRect.size(), purpose, 1, options.colorSpace, options.pixelFormat, { }, { hostWindow });

It's odd that ImageBuffer::create() takes a scale factor argument, but apparently that doesn't behave the way we want?

[Said Abou-Hallawa]

Comment on attachment 458839 [details]
Patch

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

>> Source/WebCore/page/FrameSnapshotting.cpp:124
>> +    auto buffer = ImageBuffer::create(scaledImageRect.size(), purpose, 1, options.colorSpace, options.pixelFormat, { }, { hostWindow });
> 
> It's odd that ImageBuffer::create() takes a scale factor argument, but apparently that doesn't behave the way we want?

I think it is okay that we want the scaling of the ImageBuffer to happen at the creation time. And it is working for all the other backends. But there is a problem with ImageBufferShareableBitmapBackend which I can't figure it out so far. 

This patch is correct and it fixes the snapshot bug. But we do have a bug with creating scaled unaccelerated remote ImageBuffer which we never needed before. All the canvas ImageBuffers are not scaled and all the layer ImageBuffers are accelerated. So I will file another bug for this issue which can be fixed later.

[EWS]

Committed r293825 (250298@main): <https://commits.webkit.org/250298@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 458839 [details].