[webkit-changes] [295343] trunk/Source

Tue, 07 Jun 2022 06:44:42 -0700

Title: [295343] trunk/Source
Revision
295343
Author
commit-qu...@webkit.org
Date
2022-06-07 06:43:49 -0700 (Tue, 07 Jun 2022)

Log Message

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

Patch by Kimmo Kinnunen <kkinnu...@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

Modified Paths

Diff

Modified: trunk/Source/WebCore/platform/graphics/ConcreteImageBuffer.h (295342 => 295343)


--- trunk/Source/WebCore/platform/graphics/ConcreteImageBuffer.h	2022-06-07 13:08:34 UTC (rev 295342)
+++ trunk/Source/WebCore/platform/graphics/ConcreteImageBuffer.h	2022-06-07 13:43:49 UTC (rev 295343)
@@ -140,15 +140,6 @@
         return nullptr;
     }
 
-    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
     {
         auto* backend = ensureBackendCreated();
@@ -201,15 +192,6 @@
         return nullptr;
     }
 
-    RefPtr<Image> sinkIntoImage(PreserveResolution preserveResolution = PreserveResolution::No) override
-    {
-        if (auto* backend = ensureBackendCreated()) {
-            flushDrawingContext();
-            return backend->sinkIntoImage(preserveResolution);
-        }
-        return nullptr;
-    }
-
     void drawConsuming(GraphicsContext& destContext, const FloatRect& destRect, const FloatRect& srcRect, const ImagePaintingOptions& options) override
     {
         FloatRect adjustedSrcRect = srcRect;

Modified: trunk/Source/WebCore/platform/graphics/ImageBuffer.cpp (295342 => 295343)


--- trunk/Source/WebCore/platform/graphics/ImageBuffer.cpp	2022-06-07 13:08:34 UTC (rev 295342)
+++ trunk/Source/WebCore/platform/graphics/ImageBuffer.cpp	2022-06-07 13:43:49 UTC (rev 295343)
@@ -28,6 +28,7 @@
 #include "config.h"
 #include "ImageBuffer.h"
 
+#include "BitmapImage.h"
 #include "GraphicsContext.h"
 #include "HostWindow.h"
 #include "PlatformImageBuffer.h"
@@ -121,16 +122,50 @@
     return FloatRect(rect.location(), clampedSize(rect.size()));
 }
 
-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());
+}
+
 void ImageBuffer::drawConsuming(RefPtr<ImageBuffer> imageBuffer, GraphicsContext& context, const FloatRect& destRect, const FloatRect& srcRect, const ImagePaintingOptions& options)
 {
     imageBuffer->drawConsuming(context, destRect, srcRect, options);

Modified: trunk/Source/WebCore/platform/graphics/ImageBuffer.h (295342 => 295343)


--- trunk/Source/WebCore/platform/graphics/ImageBuffer.h	2022-06-07 13:08:34 UTC (rev 295342)
+++ trunk/Source/WebCore/platform/graphics/ImageBuffer.h	2022-06-07 13:43:49 UTC (rev 295343)
@@ -127,7 +127,7 @@
     virtual std::unique_ptr<ThreadSafeImageBufferFlusher> createFlusher() = 0;
 
     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;
 
     virtual void draw(GraphicsContext&, const FloatRect& destRect, const FloatRect& srcRect = FloatRect(0, 0, -1, -1), const ImagePaintingOptions& = { }) = 0;
@@ -162,7 +162,6 @@
     ImageBuffer() = default;
 
     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;
 };

Modified: trunk/Source/WebCore/platform/graphics/ImageBufferBackend.cpp (295342 => 295343)


--- trunk/Source/WebCore/platform/graphics/ImageBufferBackend.cpp	2022-06-07 13:08:34 UTC (rev 295342)
+++ trunk/Source/WebCore/platform/graphics/ImageBufferBackend.cpp	2022-06-07 13:43:49 UTC (rev 295343)
@@ -60,11 +60,6 @@
     return copyNativeImage(DontCopyBackingStore);
 }
 
-RefPtr<Image> ImageBufferBackend::sinkIntoImage(PreserveResolution preserveResolution)
-{
-    return copyImage(DontCopyBackingStore, preserveResolution);
-}
-
 void ImageBufferBackend::convertToLuminanceMask()
 {
     PixelBufferFormat format { AlphaPremultiplication::Unpremultiplied, PixelFormat::RGBA8, colorSpace() };

Modified: trunk/Source/WebCore/platform/graphics/ImageBufferBackend.h (295342 => 295343)


--- trunk/Source/WebCore/platform/graphics/ImageBufferBackend.h	2022-06-07 13:08:34 UTC (rev 295342)
+++ trunk/Source/WebCore/platform/graphics/ImageBufferBackend.h	2022-06-07 13:43:49 UTC (rev 295343)
@@ -111,10 +111,8 @@
 
     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&) { }

Modified: trunk/Source/WebCore/platform/graphics/cairo/ImageBufferCairoBackend.cpp (295342 => 295343)


--- trunk/Source/WebCore/platform/graphics/cairo/ImageBufferCairoBackend.cpp	2022-06-07 13:08:34 UTC (rev 295342)
+++ trunk/Source/WebCore/platform/graphics/cairo/ImageBufferCairoBackend.cpp	2022-06-07 13:43:49 UTC (rev 295343)
@@ -45,12 +45,6 @@
 
 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)
 {
     if (auto image = copyNativeImage(DontCopyBackingStore))

Modified: trunk/Source/WebCore/platform/graphics/cairo/ImageBufferCairoBackend.h (295342 => 295343)


--- trunk/Source/WebCore/platform/graphics/cairo/ImageBufferCairoBackend.h	2022-06-07 13:08:34 UTC (rev 295342)
+++ trunk/Source/WebCore/platform/graphics/cairo/ImageBufferCairoBackend.h	2022-06-07 13:43:49 UTC (rev 295343)
@@ -37,8 +37,6 @@
 
 class ImageBufferCairoBackend : public ImageBufferBackend {
 public:
-    RefPtr<Image> copyImage(BackingStoreCopy = CopyBackingStore, PreserveResolution = PreserveResolution::No) const override;
-
     void clipToMask(GraphicsContext&, const FloatRect& destRect) override;
 
     void transformToColorSpace(const DestinationColorSpace&) override;

Modified: trunk/Source/WebCore/platform/graphics/cg/ImageBufferCGBackend.cpp (295342 => 295343)


--- trunk/Source/WebCore/platform/graphics/cg/ImageBufferCGBackend.cpp	2022-06-07 13:08:34 UTC (rev 295342)
+++ trunk/Source/WebCore/platform/graphics/cg/ImageBufferCGBackend.cpp	2022-06-07 13:43:49 UTC (rev 295343)
@@ -92,45 +92,6 @@
     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);
-}
-
 void ImageBufferCGBackend::clipToMask(GraphicsContext& destContext, const FloatRect& destRect)
 {
     auto nativeImage = copyNativeImage(DontCopyBackingStore);

Modified: trunk/Source/WebCore/platform/graphics/cg/ImageBufferCGBackend.h (295342 => 295343)


--- trunk/Source/WebCore/platform/graphics/cg/ImageBufferCGBackend.h	2022-06-07 13:08:34 UTC (rev 295342)
+++ trunk/Source/WebCore/platform/graphics/cg/ImageBufferCGBackend.h	2022-06-07 13:43:49 UTC (rev 295343)
@@ -43,9 +43,6 @@
 protected:
     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;
 
     String toDataURL(const String& mimeType, std::optional<double> quality, PreserveResolution) const override;

Modified: trunk/Source/WebKit/WebProcess/GPU/graphics/ImageBufferShareableBitmapBackend.cpp (295342 => 295343)


--- trunk/Source/WebKit/WebProcess/GPU/graphics/ImageBufferShareableBitmapBackend.cpp	2022-06-07 13:08:34 UTC (rev 295342)
+++ trunk/Source/WebKit/WebProcess/GPU/graphics/ImageBufferShareableBitmapBackend.cpp	2022-06-07 13:43:49 UTC (rev 295343)
@@ -153,11 +153,6 @@
     return NativeImage::create(m_bitmap->createPlatformImage(copyBehavior));
 }
 
-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
 {
     return ImageBufferBackend::getPixelBuffer(outputFormat, srcRect, m_bitmap->data(), allocator);

Modified: trunk/Source/WebKit/WebProcess/GPU/graphics/ImageBufferShareableBitmapBackend.h (295342 => 295343)


--- trunk/Source/WebKit/WebProcess/GPU/graphics/ImageBufferShareableBitmapBackend.h	2022-06-07 13:08:34 UTC (rev 295342)
+++ trunk/Source/WebKit/WebProcess/GPU/graphics/ImageBufferShareableBitmapBackend.h	2022-06-07 13:43:49 UTC (rev 295343)
@@ -63,7 +63,6 @@
 #endif
 
     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;
     void putPixelBuffer(const WebCore::PixelBuffer&, const WebCore::IntRect& srcRect, const WebCore::IntPoint& destPoint, WebCore::AlphaPremultiplication destFormat) final;

Modified: trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h (295342 => 295343)


--- trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h	2022-06-07 13:08:34 UTC (rev 295342)
+++ trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h	2022-06-07 13:43:49 UTC (rev 295343)
@@ -195,21 +195,6 @@
         return WebCore::NativeImage::create(bitmap->createPlatformImage(WebCore::DontCopyBackingStore));
     }
 
-    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
     {
         ASSERT(&destContext != &context());
@@ -221,11 +206,6 @@
         return copyNativeImage();
     }
 
-    RefPtr<WebCore::Image> sinkIntoImage(WebCore::PreserveResolution preserveResolution = WebCore::PreserveResolution::No) final
-    {
-        return copyImage(WebCore::BackingStoreCopy::CopyBackingStore, preserveResolution);
-    }
-
     RefPtr<WebCore::Image> filteredImage(WebCore::Filter& filter) final
     {
         if (UNLIKELY(!m_remoteRenderingBackendProxy))

Modified: trunk/Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferRemoteIOSurfaceBackend.cpp (295342 => 295343)


--- trunk/Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferRemoteIOSurfaceBackend.cpp	2022-06-07 13:08:34 UTC (rev 295342)
+++ trunk/Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferRemoteIOSurfaceBackend.cpp	2022-06-07 13:43:49 UTC (rev 295343)
@@ -112,12 +112,6 @@
     return { };
 }
 
-RefPtr<Image> ImageBufferRemoteIOSurfaceBackend::copyImage(BackingStoreCopy, PreserveResolution) const
-{
-    RELEASE_ASSERT_NOT_REACHED();
-    return { };
-}
-
 String ImageBufferRemoteIOSurfaceBackend::toDataURL(const String&, std::optional<double>, PreserveResolution) const
 {
     RELEASE_ASSERT_NOT_REACHED();

Modified: trunk/Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferRemoteIOSurfaceBackend.h (295342 => 295343)


--- trunk/Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferRemoteIOSurfaceBackend.h	2022-06-07 13:08:34 UTC (rev 295342)
+++ trunk/Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferRemoteIOSurfaceBackend.h	2022-06-07 13:43:49 UTC (rev 295343)
@@ -60,7 +60,6 @@
 private:
     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;
     RefPtr<WebCore::PixelBuffer> getPixelBuffer(const WebCore::PixelBufferFormat& outputFormat, const WebCore::IntRect&, const WebCore::ImageBufferAllocator&) const final;

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to