Bug 242879 - REGRESSION(r294536): Drawing the ImageBuffer should not invalidate its cached copied images
Summary: REGRESSION(r294536): Drawing the ImageBuffer should not invalidate its cached...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-07-18 18:13 PDT by Said Abou-Hallawa
Modified: 2022-07-25 23:11 PDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Said Abou-Hallawa 2022-07-18 18:13:33 PDT
When copying the IOSurface to a CGImage, the memory of this CGImage is allocated as copy-on-write. Before any draws committed to the IOSurface, the all copied CGImages are invalidated such that they have their memory copied so they are not affected by the new changes. Because changing the pixels of the IOSurface directly through putPixelBuffer() does not invalidate the IOSurface image cache, ImageBufferIOSurfaceBackend::invalidateCachedNativeImage() handles this case by a simple hack: draw an empty rect to the IOSurface context. invalidateCachedNativeImage() relies on the flag m_mayHaveOutstandingBackingStoreReferences which is set to true when a CGImage is copied from the backend.

When drawing an accelerated ImageBuffer, the backend IOSurface is copied to a CGImage then this it is drawn to the context and finally it is destroyed. But creating it causes the flag m_mayHaveOutstandingBackingStoreReferences to be set to true. Any subsequent calls to putPixelBuffer() will be preceded by invalidating the cache of the copied images. This invalidation is not needed because the copied image is volatile. It will be destroyed before any subsequent draws or putPixelBuffer() calls.

Consider this sequence:

ImageBuffer::getPixelBuffer()
ImageBuffer::putPixelBuffer()
ImageBuffer::draw(); // Calls copyNativeImage(), sets m_mayHaveOutstandingBackingStoreReferences = true
ImageBuffer::getPixelBuffer()
ImageBuffer::putPixelBuffer(); // Because m_mayHaveOutstandingBackingStoreReferences, IOSurface image cache is invalidated.
ImageBuffer::draw(); // Calls copyNativeImage(), sets m_mayHaveOutstandingBackingStoreReferences = true
Comment 1 Said Abou-Hallawa 2022-07-18 18:13:53 PDT
rdar://93775177
Comment 2 Said Abou-Hallawa 2022-07-18 18:27:22 PDT
Pull request: https://github.com/WebKit/WebKit/pull/2533
Comment 3 EWS 2022-07-25 23:11:15 PDT
Committed 252813@main (c7880bea0dca): <https://commits.webkit.org/252813@main>

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