Skip to content

Commit

Permalink
REGRESSION(268243@main) ~9/21: MotionMark Images 15% on multiple models
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=262032
rdar://115907669

Reviewed by Matt Woodrow.

Before 268243@main, call ImageBuffer::copyImageForDrawing() would do the work:
  if (can use ref)
    <do the work of create native image reference>
  else
    <do the work of copy native image>

After 268243@main, the invocation was moved to GraphicsContext::nativeImageForDrawing():
  if (can use ref)
     imageBuffer->createNativeImageReference();
  else
     imageBuffer->copyNativeImage();

ImageBufferShareableMappedIOSurfaceBackend happened to have an
copyNativeImage() override that caused a difference in behavior.

Remove this code altogether, it was a leftover from eariler days
working around the issues of backbuffer reading from WP and updating
from GPUP. This code was not needed anymore. It is not correct, as
IOSurface seed cannot be used to determine read cache invalidation
accurately. The seed is updated during HW rasterization, which happens
at undefined time. The drawing commands might be on their way from WP
-> GPUP or from GPUP -> CG, and not yet issued at the HW rasterization
level.

The read cache is cleared when the drawing context, e.g.
RemoteDisplayListRecorderProxy detects the first modification after a
copyNativeImage() call.
RemoteImageBufferProxy::backingStoreWillChange() notification will
invalidate the read caches.

* Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferShareableMappedIOSurfaceBackend.h:
* Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferShareableMappedIOSurfaceBackend.cpp:

Canonical link: https://commits.webkit.org/268438@main
  • Loading branch information
kkinnunen-apple committed Sep 26, 2023
1 parent 03ad7af commit de7538e
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -85,18 +85,6 @@ std::optional<ImageBufferBackendHandle> ImageBufferShareableMappedIOSurfaceBacke
return ImageBufferBackendHandle(m_surface->createSendRight());
}

RefPtr<NativeImage> ImageBufferShareableMappedIOSurfaceBackend::copyNativeImage()
{
auto currentSeed = m_surface->seed();

// Invalidate the cached image before getting a new one because GPUProcess might
// have drawn something to the IOSurface since last time this function was called.
if (std::exchange(m_lastSeedWhenCopyingImage, currentSeed) != currentSeed)
invalidateCachedNativeImage();

return ImageBufferIOSurfaceBackend::copyNativeImage();
}

String ImageBufferShareableMappedIOSurfaceBackend::debugDescription() const
{
TextStream stream;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,7 @@ class ImageBufferShareableMappedIOSurfaceBackend final : public WebCore::ImageBu
// ImageBufferBackendSharing
ImageBufferBackendSharing* toBackendSharing() final { return this; }

RefPtr<WebCore::NativeImage> copyNativeImage() final;

String debugDescription() const final;

WebCore::IOSurfaceSeed m_lastSeedWhenCopyingImage { 0 };
};

} // namespace WebKit
Expand Down

0 comments on commit de7538e

Please sign in to comment.