Bug 192471 – [CG] Don't copy the ImageBuffer pixels data when drawing to an accelerated context

• Component: Images
• Status: RESOLVED WONTFIX

[Said Abou-Hallawa]

This basically will roll out the change <http://trac.webkit.org/changeset/106095>.

ImageBuffer::copyNativeImage() has two copy behaviors:

1. DontCopyBackingStore: this passes a CGDataProvider to CGImageCreate() which is named m_data.dataProvider. The m_data.dataProvider holds the image pixels data which is named ‘m_data.data’. The same m_data.dataProvider is also backs the ImageBuffer context which is named ‘m_data.context’. Remember m_data.dataProvider is ref-counted pointer of type RetainPtr<CGDataProviderRef>.

2. CopyBackingStore: this passes the ‘m_data.context’ to CGBitmapContextCreateImage(). But this is actually does not copy the pixel data from the context. According to the documentation the returned CGImageRef follows copy-on-write semantics. That means it will copy the data only when the context changes. This does not include freeing the pixels data if the context was created by CGBitmapContextCreate().

The description of bug in https://bugs.webkit.org/show_bug.cgi?id=77185 says that “Thus, especially in the case of accelerated canvas there can be image tearing so we need to always do a deep copy if the destination context is accelerated.” So the worry was freeing the Image buffer not changing its contents. But this is confusing because DontCopyBackingStore is safer than CopyBackingStore in this regard. The image buffer may be deleted but its m_data.data will stay alive if there is a CGDataProvider referencing it.

[Said Abou-Hallawa]

<rdar://problem/38045885>

[Said Abou-Hallawa]

Created attachment 356750 [details]
Patch

[Said Abou-Hallawa]

*** Bug 183271 has been marked as a duplicate of this bug. ***

[Simon Fraser (smfr)]

Comment on attachment 356750 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        [CG] Don't copy the ImageBuffer pixels data when drawing to an accelerated context

This doesn't describe the symptoms. Bug 183271 has a much better title.

> Source/WebCore/ChangeLog:8
> +        This rolls out r106095.

How does this not revert the synchronization bug that r106095 was fixing?

> Source/WebCore/ChangeLog:11
> +        underlaying pixels data will be held in memory till the image is drawn

underlying

> Source/WebCore/ChangeLog:14
> +        The underlaying pixels data will be freed only when the CGDataProvider

underlying pixel data

[Simon Fraser (smfr)]

I don't think we need this any more.