Changeset 295343 in webkit

Timestamp:

Jun 7, 2022, 6:43:49 AM (4 years ago)

Author:

commit-queue@webkit.org

Message:

ImageBuffer::copyImage consumes GPUP memory and is redundant
​https://bugs.webkit.org/show_bug.cgi?id=241127

Patch by Kimmo Kinnunen <​kkinnunen@apple.com> on 2022-06-07
ImageBuffer::copyImage overrides:

• Always ends up returning BitmapImage
• Always ends up constructing BitmapImage from copyNativeImage
• Is implemented erroneously in many backends, ignoring the preserve resolution scale flag
• Is implemented correctly in ImageBufferCGBackend, but will allocate the destination buffer when trying to honor the PreserveResolution::No. This will cause a large unattributed allocation in GPUP.

Instead, implement ImageBuffer::copyImage for all the backends and overrides
with ImageBuffer::copyNativeImage().

Implement the scaling conversion, PreserveResolution::No, as a normal WebCore GraphicsContext
operation.

Reviewed by Said Abou-Hallawa.

• Source/WebCore/platform/graphics/ConcreteImageBuffer.h:
• Source/WebCore/platform/graphics/ImageBuffer.cpp:

(WebCore::ImageBuffer::copyImage const):
(WebCore::ImageBuffer::sinkIntoImage):

• Source/WebCore/platform/graphics/ImageBuffer.h:
• Source/WebCore/platform/graphics/ImageBufferBackend.cpp:

(WebCore::ImageBufferBackend::sinkIntoImage): Deleted.

• Source/WebCore/platform/graphics/ImageBufferBackend.h:
• Source/WebCore/platform/graphics/cairo/ImageBufferCairoBackend.cpp:

(WebCore::ImageBufferCairoBackend::copyImage const): Deleted.

• Source/WebCore/platform/graphics/cairo/ImageBufferCairoBackend.h:
• Source/WebCore/platform/graphics/cg/ImageBufferCGBackend.cpp:

(WebCore::createBitmapImageAfterScalingIfNeeded): Deleted.
(WebCore::ImageBufferCGBackend::copyImage const): Deleted.
(WebCore::ImageBufferCGBackend::sinkIntoImage): Deleted.

• Source/WebCore/platform/graphics/cg/ImageBufferCGBackend.h:
• Source/WebKit/WebProcess/GPU/graphics/ImageBufferShareableBitmapBackend.cpp:

(WebKit::ImageBufferShareableBitmapBackend::copyImage const): Deleted.

• Source/WebKit/WebProcess/GPU/graphics/ImageBufferShareableBitmapBackend.h:
• Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h:
• Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferRemoteIOSurfaceBackend.cpp:

(WebKit::ImageBufferRemoteIOSurfaceBackend::copyImage const): Deleted.

• Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferRemoteIOSurfaceBackend.h:

Canonical link: ​https://commits.webkit.org/251359@main

Location:

trunk/Source

Files:

• WebCore/platform/graphics/ConcreteImageBuffer.h (modified) (2 diffs)
• WebCore/platform/graphics/ImageBuffer.cpp (modified) (2 diffs)
• WebCore/platform/graphics/ImageBuffer.h (modified) (2 diffs)
• WebCore/platform/graphics/ImageBufferBackend.cpp (modified) (1 diff)
• WebCore/platform/graphics/ImageBufferBackend.h (modified) (1 diff)
• WebCore/platform/graphics/cairo/ImageBufferCairoBackend.cpp (modified) (1 diff)
• WebCore/platform/graphics/cairo/ImageBufferCairoBackend.h (modified) (1 diff)
• WebCore/platform/graphics/cg/ImageBufferCGBackend.cpp (modified) (1 diff)
• WebCore/platform/graphics/cg/ImageBufferCGBackend.h (modified) (1 diff)
• WebKit/WebProcess/GPU/graphics/ImageBufferShareableBitmapBackend.cpp (modified) (1 diff)
• WebKit/WebProcess/GPU/graphics/ImageBufferShareableBitmapBackend.h (modified) (1 diff)
• WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h (modified) (2 diffs)
• WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferRemoteIOSurfaceBackend.cpp (modified) (1 diff)
• WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferRemoteIOSurfaceBackend.h (modified) (1 diff)

trunk/Source/WebCore/platform/graphics/ConcreteImageBuffer.h

@@ -141 +141 @@
     }
 
-    RefPtr<Image> copyImage(BackingStoreCopy copyBehavior = CopyBackingStore, PreserveResolution preserveResolution = PreserveResolution::No) const override
-    {
-        if (auto* backend = ensureBackendCreated()) {
-            const_cast<ConcreteImageBuffer&>(*this).flushDrawingContext();
-            return backend->copyImage(copyBehavior, preserveResolution);
-        }
-        return nullptr;
-    }
-
     RefPtr<Image> filteredImage(Filter& filter) override
     {
@@ -198 +189 @@
             flushDrawingContext();
             return backend->sinkIntoNativeImage();
-        }
-        return nullptr;
-    }
-
-    RefPtr<Image> sinkIntoImage(PreserveResolution preserveResolution = PreserveResolution::No) override
-    {
-        if (auto* backend = ensureBackendCreated()) {
-            flushDrawingContext();
-            return backend->sinkIntoImage(preserveResolution);
         }
         return nullptr;

trunk/Source/WebCore/platform/graphics/ImageBuffer.cpp

@@ -29 +29 @@
 #include "ImageBuffer.h"
 
+#include "BitmapImage.h"
 #include "GraphicsContext.h"
 #include "HostWindow.h"
@@ -122 +123 @@
 }
 
-RefPtr<NativeImage> ImageBuffer::sinkIntoNativeImage(RefPtr<ImageBuffer> imageBuffer)
+RefPtr<NativeImage> ImageBuffer::sinkIntoNativeImage(RefPtr<ImageBuffer> source)
 {
-    return imageBuffer->sinkIntoNativeImage();
+    if (!source)
+        return nullptr;
+    return source->sinkIntoNativeImage();
 }
 
-RefPtr<Image> ImageBuffer::sinkIntoImage(RefPtr<ImageBuffer> imageBuffer, PreserveResolution preserveResolution)
+RefPtr<Image> ImageBuffer::copyImage(BackingStoreCopy copyBehavior, PreserveResolution preserveResolution) const
 {
-    return imageBuffer->sinkIntoImage(preserveResolution);
+    RefPtr<NativeImage> image;
+    if (resolutionScale() == 1 || preserveResolution == PreserveResolution::Yes)
+        image = copyNativeImage(copyBehavior);
+    else {
+        auto copyBuffer = context().createImageBuffer(logicalSize(), 1.f, colorSpace());
+        if (!copyBuffer)
+            return nullptr;
+        copyBuffer->context().drawImageBuffer(const_cast<ImageBuffer&>(*this), FloatPoint { }, CompositeOperator::Copy);
+        image = ImageBuffer::sinkIntoNativeImage(WTFMove(copyBuffer));
+    }
+    if (!image)
+        return nullptr;
+    return BitmapImage::create(image.releaseNonNull());
+}
+
+RefPtr<Image> ImageBuffer::sinkIntoImage(RefPtr<ImageBuffer> source, PreserveResolution preserveResolution)
+{
+    if (!source)
+        return nullptr;
+    RefPtr<NativeImage> image;
+    if (source->resolutionScale() == 1 || preserveResolution == PreserveResolution::Yes)
+        image = sinkIntoNativeImage(WTFMove(source));
+    else {
+        auto copySize = source->logicalSize();
+        auto copyBuffer = source->context().createImageBuffer(copySize, 1.f, source->colorSpace());
+        if (!copyBuffer)
+            return nullptr;
+        drawConsuming(WTFMove(source), copyBuffer->context(), FloatRect { { }, copySize }, FloatRect { 0, 0, -1, -1 }, CompositeOperator::Copy);
+        image = ImageBuffer::sinkIntoNativeImage(WTFMove(copyBuffer));
+    }
+    if (!image)
+        return nullptr;
+    return BitmapImage::create(image.releaseNonNull());
 }
 

trunk/Source/WebCore/platform/graphics/ImageBuffer.h

@@ -128 +128 @@
 
     virtual RefPtr<NativeImage> copyNativeImage(BackingStoreCopy = CopyBackingStore) const = 0;
-    virtual RefPtr<Image> copyImage(BackingStoreCopy = CopyBackingStore, PreserveResolution = PreserveResolution::No) const = 0;
+    WEBCORE_EXPORT RefPtr<Image> copyImage(BackingStoreCopy = CopyBackingStore, PreserveResolution = PreserveResolution::No) const;
     virtual RefPtr<Image> filteredImage(Filter&) = 0;
 
@@ -163 +163 @@
 
     virtual RefPtr<NativeImage> sinkIntoNativeImage() = 0;
-    virtual RefPtr<Image> sinkIntoImage(PreserveResolution = PreserveResolution::No) = 0;
     virtual void drawConsuming(GraphicsContext&, const FloatRect& destRect, const FloatRect& srcRect, const ImagePaintingOptions&) = 0;
 };

trunk/Source/WebCore/platform/graphics/ImageBufferBackend.cpp

@@ -59 +59 @@
 {
     return copyNativeImage(DontCopyBackingStore);
-}
-
-RefPtr<Image> ImageBufferBackend::sinkIntoImage(PreserveResolution preserveResolution)
-{
-    return copyImage(DontCopyBackingStore, preserveResolution);
 }
 

trunk/Source/WebCore/platform/graphics/ImageBufferBackend.h

@@ -112 +112 @@
     virtual void finalizeDrawIntoContext(GraphicsContext&) { }
     virtual RefPtr<NativeImage> copyNativeImage(BackingStoreCopy) const = 0;
-    virtual RefPtr<Image> copyImage(BackingStoreCopy, PreserveResolution) const = 0;
 
     WEBCORE_EXPORT virtual RefPtr<NativeImage> sinkIntoNativeImage();
-    WEBCORE_EXPORT virtual RefPtr<Image> sinkIntoImage(PreserveResolution);
 
     virtual void clipToMask(GraphicsContext&, const FloatRect&) { }

trunk/Source/WebCore/platform/graphics/cairo/ImageBufferCairoBackend.cpp

@@ -45 +45 @@
 
 namespace WebCore {
-
-RefPtr<Image> ImageBufferCairoBackend::copyImage(BackingStoreCopy copyBehavior, PreserveResolution) const
-{
-    // BitmapImage will release the passed in surface on destruction
-    return BitmapImage::create(copyNativeImage(copyBehavior));
-}
 
 void ImageBufferCairoBackend::clipToMask(GraphicsContext& destContext, const FloatRect& destRect)

trunk/Source/WebCore/platform/graphics/cairo/ImageBufferCairoBackend.h

@@ -38 +38 @@
 class ImageBufferCairoBackend : public ImageBufferBackend {
 public:
-    RefPtr<Image> copyImage(BackingStoreCopy = CopyBackingStore, PreserveResolution = PreserveResolution::No) const override;
-
     void clipToMask(GraphicsContext&, const FloatRect& destRect) override;
 

trunk/Source/WebCore/platform/graphics/cg/ImageBufferCGBackend.cpp

@@ -91 +91 @@
         return sRGBColorSpaceRef();
     return imageBufferColorSpace.platformColorSpace();
-}
-
-static RefPtr<Image> createBitmapImageAfterScalingIfNeeded(RefPtr<NativeImage>&& image, const IntSize& logicalSize, const IntSize& backendSize, float resolutionScale, PreserveResolution preserveResolution)
-{
-    if (!image)
-        return nullptr;
-
-    if (resolutionScale == 1 || preserveResolution == PreserveResolution::Yes)
-        image = NativeImage::create(createCroppedImageIfNecessary(image->platformImage().get(), backendSize));
-    else {
-        auto context = adoptCF(CGBitmapContextCreate(0, logicalSize.width(), logicalSize.height(), 8, 4 * logicalSize.width(), colorSpaceForBitmap(image->colorSpace()), static_cast<uint32_t>(kCGImageAlphaPremultipliedFirst) | static_cast<uint32_t>(kCGBitmapByteOrder32Host)));
-        CGContextSetBlendMode(context.get(), kCGBlendModeCopy);
-        CGContextClipToRect(context.get(), FloatRect(FloatPoint::zero(), logicalSize));
-        FloatSize imageSizeInUserSpace = logicalSize;
-        CGContextDrawImage(context.get(), FloatRect(FloatPoint::zero(), imageSizeInUserSpace), image->platformImage().get());
-        image = NativeImage::create(adoptCF(CGBitmapContextCreateImage(context.get())));
-    }
-
-    if (!image)
-        return nullptr;
-
-    return BitmapImage::create(WTFMove(image));
-}
-
-RefPtr<Image> ImageBufferCGBackend::copyImage(BackingStoreCopy copyBehavior, PreserveResolution preserveResolution) const
-{
-    RefPtr<NativeImage> image;
-    if (resolutionScale() == 1 || preserveResolution == PreserveResolution::Yes)
-        image = copyNativeImage(copyBehavior);
-    else
-        image = copyNativeImage(DontCopyBackingStore);
-    return createBitmapImageAfterScalingIfNeeded(WTFMove(image), logicalSize(), backendSize(), resolutionScale(), preserveResolution);
-}
-
-RefPtr<Image> ImageBufferCGBackend::sinkIntoImage(PreserveResolution preserveResolution)
-{
-    // Get the backend size before sinking the it into a NativeImage. sinkIntoNativeImage() sets the IOSurface to null if it's an accelerated backend.
-    auto backendSize = this->backendSize();
-    return createBitmapImageAfterScalingIfNeeded(sinkIntoNativeImage(), logicalSize(), backendSize, resolutionScale(), preserveResolution);
 }
 

trunk/Source/WebCore/platform/graphics/cg/ImageBufferCGBackend.h

@@ -44 +44 @@
     using ImageBufferBackend::ImageBufferBackend;
 
-    RefPtr<Image> copyImage(BackingStoreCopy = CopyBackingStore, PreserveResolution = PreserveResolution::No) const override;
-    RefPtr<Image> sinkIntoImage(PreserveResolution) override;
-
     void clipToMask(GraphicsContext&, const FloatRect& destRect) override;
 

trunk/Source/WebKit/WebProcess/GPU/graphics/ImageBufferShareableBitmapBackend.cpp

@@ -154 +154 @@
 }
 
-RefPtr<Image> ImageBufferShareableBitmapBackend::copyImage(BackingStoreCopy, PreserveResolution) const
-{
-    return m_bitmap->createImage();
-}
-
 RefPtr<PixelBuffer> ImageBufferShareableBitmapBackend::getPixelBuffer(const PixelBufferFormat& outputFormat, const IntRect& srcRect, const ImageBufferAllocator& allocator) const
 {

trunk/Source/WebKit/WebProcess/GPU/graphics/ImageBufferShareableBitmapBackend.h

@@ -64 +64 @@
 
     RefPtr<WebCore::NativeImage> copyNativeImage(WebCore::BackingStoreCopy = WebCore::CopyBackingStore) const final;
-    RefPtr<WebCore::Image> copyImage(WebCore::BackingStoreCopy = WebCore::CopyBackingStore, WebCore::PreserveResolution = WebCore::PreserveResolution::No) const final;
 
     RefPtr<WebCore::PixelBuffer> getPixelBuffer(const WebCore::PixelBufferFormat& outputFormat, const WebCore::IntRect&, const WebCore::ImageBufferAllocator&) const final;

trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h

@@ -196 +196 @@
     }
 
-    RefPtr<WebCore::Image> copyImage(WebCore::BackingStoreCopy copyBehavior = WebCore::CopyBackingStore, WebCore::PreserveResolution preserveResolution = WebCore::PreserveResolution::No) const final
-    {
-        if (UNLIKELY(!m_remoteRenderingBackendProxy))
-            return { };
-
-        if (canMapBackingStore())
-            return BaseConcreteImageBuffer::copyImage(copyBehavior, preserveResolution);
-
-        const_cast<RemoteImageBufferProxy*>(this)->flushDrawingContext();
-        auto bitmap = m_remoteRenderingBackendProxy->getShareableBitmap(m_renderingResourceIdentifier, preserveResolution);
-        if (!bitmap)
-            return { };
-        return bitmap->createImage();
-    }
-
     void drawConsuming(WebCore::GraphicsContext& destContext, const WebCore::FloatRect& destRect, const WebCore::FloatRect& srcRect, const WebCore::ImagePaintingOptions& options) final
     {
@@ -220 +205 @@
     {
         return copyNativeImage();
-    }
-
-    RefPtr<WebCore::Image> sinkIntoImage(WebCore::PreserveResolution preserveResolution = WebCore::PreserveResolution::No) final
-    {
-        return copyImage(WebCore::BackingStoreCopy::CopyBackingStore, preserveResolution);
     }
 

trunk/Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferRemoteIOSurfaceBackend.cpp

@@ -113 +113 @@
 }
 
-RefPtr<Image> ImageBufferRemoteIOSurfaceBackend::copyImage(BackingStoreCopy, PreserveResolution) const
-{
-    RELEASE_ASSERT_NOT_REACHED();
-    return { };
-}
-
 String ImageBufferRemoteIOSurfaceBackend::toDataURL(const String&, std::optional<double>, PreserveResolution) const
 {

trunk/Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferRemoteIOSurfaceBackend.h

@@ -61 +61 @@
     WebCore::IntSize backendSize() const final;
     RefPtr<WebCore::NativeImage> copyNativeImage(WebCore::BackingStoreCopy) const final;
-    RefPtr<WebCore::Image> copyImage(WebCore::BackingStoreCopy, WebCore::PreserveResolution) const final;
     String toDataURL(const String& mimeType, std::optional<double> quality, WebCore::PreserveResolution) const final;
     Vector<uint8_t> toData(const String& mimeType, std::optional<double> quality) const final;