ImageBuffer::copyImage is redundant, should use copyNativeImage instead

https://bugs.webkit.org/show_bug.cgi?id=261944
rdar://115891269

Reviewed by Matt Woodrow.

Remove ImageBuffer::copyImaage(). WebCore::Image is higher level
type. Callers should call sinkIntoNativeImage and then convert the
NativeImage into Image, if needed. Note: most callers should use
NativeImage, as that is the lowest level source image.

These are identical:
copyImage(CopyBackingStore, PreserveResolution::Yes) -> copyNativeImage()
copyImage(DontCopyBackingStore, PreserveResolution::No) + scale 1
 -> createNativeImageReference()

Other variants wouldn't have corresponding NativeImage call, but they
do not exist.

This is work towards simplifying ImageBuffer by making it operate
with NativeImage as its primary primitive image source.

* Source/WebCore/html/CustomPaintCanvas.cpp:
(WebCore::CustomPaintCanvas::copiedImage const):
Fix an issue where the code would nullptr deref if Context2D call
would try to reset the transform to default.
The idea of CanvasBase::baseTransform() is that it would return the
transform to be set, but this cannot happen because ImageBuffer has not
been created at the time of the call. The transform reset is recorded.

* Source/WebCore/html/HTMLCanvasElement.cpp:
(WebCore::HTMLCanvasElement::copiedImage const):
* Source/WebCore/html/ImageBitmap.cpp:
(WebCore::ImageBitmap::createCompletionHandler):
* Source/WebCore/html/OffscreenCanvas.cpp:
(WebCore::OffscreenCanvas::copiedImage const):
* Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:
(WebCore::WebGLRenderingContextBase::texImageSource):
(WebCore::WebGLRenderingContextBase::drawImageIntoBuffer):
(WebCore::WebGLRenderingContextBase::videoFrameToImage):
* Source/WebCore/html/canvas/WebGLRenderingContextBase.h:
* Source/WebCore/platform/graphics/GraphicsContext.cpp:
(WebCore::GraphicsContext::drawPattern):
* Source/WebCore/platform/graphics/GraphicsContextGL.cpp:
(WebCore::GraphicsContextGL::videoFrameToImage):
* Source/WebCore/platform/graphics/ImageBuffer.cpp:
(WebCore::ImageBuffer::copyImage const): Deleted.
* Source/WebCore/platform/graphics/ImageBuffer.h:
* Source/WebCore/platform/graphics/nicosia/cairo/NicosiaCairoOperationRecorder.cpp:
(Nicosia::CairoOperationRecorder::drawImageBuffer):
(Nicosia::CairoOperationRecorder::drawFilteredImageBuffer):
(Nicosia::CairoOperationRecorder::clipToImageBuffer):
* Source/WebCore/platform/graphics/texmap/BitmapTexture.cpp:
(WebCore::BitmapTexture::updateContents):
* Source/WebCore/platform/graphics/texmap/BitmapTexture.h:
* Source/WebCore/platform/graphics/texmap/BitmapTextureGL.cpp:
(WebCore::BitmapTextureGL::updateContents):
* Source/WebCore/platform/graphics/texmap/BitmapTextureGL.h:
* Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm:
(WebCore::MockRealtimeVideoSourceMac::updateSampleBuffer):

Canonical link: https://commits.webkit.org/268643@main

Source/WebCore/html/CustomPaintCanvas.cpp

@@ -28,6 +28,7 @@
 
 #if ENABLE(CSS_PAINTING_API)
 
+#include "BitmapImage.h"
 #include "CanvasRenderingContext.h"
 #include "DisplayListDrawingContext.h"
 #include "DisplayListRecorder.h"
@@ -82,15 +83,25 @@ void CustomPaintCanvas::replayDisplayList(GraphicsContext& target)
     target.drawImageBuffer(*image, clipBounds);
 }
 
+AffineTransform CustomPaintCanvas::baseTransform() const
+{
+    // The base transform of the display list.
+    // FIXME: this is actually correct, but the display list will not behave correctly with respect to
+    // playback. The GraphicsContext should be fixed to start at identity transform, and the
+    // device transform should be a separate concept that the display list or context2d cannot reset.
+    return { };
+}
+
 Image* CustomPaintCanvas::copiedImage() const
 {
     if (!width() || !height())
         return nullptr;
-    m_copiedBuffer = ImageBuffer::create(size(), RenderingPurpose::Unspecified, 1, DestinationColorSpace::SRGB(), PixelFormat::BGRA8);
-    if (!m_copiedBuffer)
-        return nullptr;
-    replayDisplayListImpl(m_copiedBuffer->context());
-    m_copiedImage = m_copiedBuffer->copyImage(DontCopyBackingStore, PreserveResolution::Yes);
+    m_copiedImage = nullptr;
+    auto buffer = ImageBuffer::create(size(), RenderingPurpose::Unspecified, 1, DestinationColorSpace::SRGB(), PixelFormat::BGRA8);
+    if (buffer) {
+        replayDisplayListImpl(buffer->context());
+        m_copiedImage = BitmapImage::create(ImageBuffer::sinkIntoNativeImage(buffer));
+    }
     return m_copiedImage.get();
 }
 

Source/WebCore/html/CustomPaintCanvas.h

@@ -65,7 +65,7 @@ class CustomPaintCanvas final : public RefCounted<CustomPaintCanvas>, public Can
 
     void didDraw(const std::optional<FloatRect>&, ShouldApplyPostProcessingToDirtyRect) final { }
 
-    AffineTransform baseTransform() const final { ASSERT(m_copiedBuffer); return m_copiedBuffer->baseTransform(); }
+    AffineTransform baseTransform() const final;
     Image* copiedImage() const final;
     void clearCopiedImage() const final;
 
@@ -87,7 +87,6 @@ class CustomPaintCanvas final : public RefCounted<CustomPaintCanvas>, public Can
 
     std::unique_ptr<CanvasRenderingContext> m_context;
     mutable std::unique_ptr<DisplayList::DrawingContext> m_recordingContext;
-    mutable RefPtr<ImageBuffer> m_copiedBuffer;
     mutable RefPtr<Image> m_copiedImage;
 };
 

Source/WebCore/html/HTMLCanvasElement.cpp

@@ -28,6 +28,7 @@
 #include "config.h"
 #include "HTMLCanvasElement.h"
 
+#include "BitmapImage.h"
 #include "Blob.h"
 #include "BlobCallback.h"
 #include "CanvasGradient.h"
@@ -923,7 +924,7 @@ Image* HTMLCanvasElement::copiedImage() const
     if (!m_copiedImage && buffer()) {
         if (m_context)
             m_context->paintRenderingResultsToCanvas();
-        m_copiedImage = buffer()->copyImage(CopyBackingStore, PreserveResolution::Yes);
+        m_copiedImage = BitmapImage::create(buffer()->copyNativeImage());
     }
     return m_copiedImage.get();
 }

Source/WebCore/html/ImageBitmap.cpp

@@ -656,7 +656,7 @@ void ImageBitmap::createCompletionHandler(ScriptExecutionContext& scriptExecutio
         return;
     }
 
-    auto imageForRender = existingImageBitmap->buffer()->copyImage();
+    auto imageForRender = BitmapImage::create(existingImageBitmap->buffer()->copyNativeImage());
 
     FloatRect destRect(FloatPoint(), outputSize);
     bitmapData->context().drawImage(*imageForRender, destRect, sourceRectangle.releaseReturnValue(), { interpolationQualityForResizeQuality(options.resizeQuality), options.resolvedImageOrientation(ImageOrientation::Orientation::None) });

Source/WebCore/html/OffscreenCanvas.cpp

@@ -28,6 +28,7 @@
 
 #if ENABLE(OFFSCREEN_CANVAS)
 
+#include "BitmapImage.h"
 #include "CSSValuePool.h"
 #include "CanvasRenderingContext.h"
 #include "Chrome.h"
@@ -430,7 +431,7 @@ Image* OffscreenCanvas::copiedImage() const
     if (!m_copiedImage && buffer()) {
         if (m_context)
             m_context->paintRenderingResultsToCanvas();
-        m_copiedImage = buffer()->copyImage(CopyBackingStore, PreserveResolution::Yes);
+        m_copiedImage = BitmapImage::create(buffer()->copyNativeImage());
     }
     return m_copiedImage.get();
 }

Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp

@@ -29,6 +29,7 @@
 #if ENABLE(WEBGL)
 
 #include "ANGLEInstancedArrays.h"
+#include "BitmapImage.h"
 #include "CachedImage.h"
 #include "Chrome.h"
 #include "DiagnosticLoggingClient.h"
@@ -3532,7 +3533,7 @@ ExceptionOr<void> WebGLRenderingContextBase::texImageSource(TexImageFunctionID f
         return { };
 
     // Fallback pure SW path.
-    RefPtr<Image> image = buffer->copyImage(DontCopyBackingStore);
+    RefPtr<Image> image = BitmapImage::create(buffer->createNativeImageReference());
     // The premultiplyAlpha and flipY pixel unpack parameters are ignored for ImageBitmaps.
     if (image)
         texImageImpl(functionID, target, level, internalformat, xoffset, yoffset, zoffset, format, type, image.get(), GraphicsContextGL::DOMSource::Image, false, source.premultiplyAlpha(), source.forciblyPremultiplyAlpha(), sourceImageRect, depth, unpackImageHeight);
@@ -3702,7 +3703,7 @@ ExceptionOr<void> WebGLRenderingContextBase::texImageSource(TexImageFunctionID f
     }
 
     // Fallback pure SW path.
-    RefPtr<Image> image = videoFrameToImage(source, DontCopyBackingStore, functionName);
+    RefPtr<Image> image = videoFrameToImage(source, functionName);
     if (!image)
         return { };
     texImageImpl(functionID, target, level, internalformat, xoffset, yoffset, zoffset, format, type, image.get(), GraphicsContextGL::DOMSource::Video, m_unpackFlipY, m_unpackPremultiplyAlpha, false, inputSourceImageRect, depth, unpackImageHeight);
@@ -4466,12 +4467,14 @@ RefPtr<Image> WebGLRenderingContextBase::drawImageIntoBuffer(Image& image, int w
     FloatRect srcRect(FloatPoint(), image.size());
     FloatRect destRect(FloatPoint(), size);
     buf->context().drawImage(image, destRect, srcRect);
-    return buf->copyImage(DontCopyBackingStore);
+    // FIXME: createNativeImageReference() does not make sense for GPUP.
+    // Instead, should fix by GPUP side upload.
+    return BitmapImage::create(buf->createNativeImageReference());
 }
 
 #if ENABLE(VIDEO)
 
-RefPtr<Image> WebGLRenderingContextBase::videoFrameToImage(HTMLVideoElement& video, BackingStoreCopy backingStoreCopy, const char* functionName)
+RefPtr<Image> WebGLRenderingContextBase::videoFrameToImage(HTMLVideoElement& video, const char* functionName)
 {
     RefPtr<ImageBuffer> imageBuffer;
     // FIXME: When texImage2D is passed an HTMLVideoElement, implementations
@@ -4518,7 +4521,7 @@ RefPtr<Image> WebGLRenderingContextBase::videoFrameToImage(HTMLVideoElement& vid
         }
         video.paintCurrentFrameInContext(imageBuffer->context(), { { }, videoSize });
     }
-    RefPtr<Image> image = imageBuffer->copyImage(backingStoreCopy);
+    RefPtr<Image> image = BitmapImage::create(imageBuffer->createNativeImageReference());
     if (!image) {
         synthesizeGLError(GraphicsContextGL::OUT_OF_MEMORY, functionName, "out of memory");
         return nullptr;

Source/WebCore/html/canvas/WebGLRenderingContextBase.h

@@ -534,7 +534,7 @@ class WebGLRenderingContextBase : public GraphicsContextGL::Client, public GPUBa
     RefPtr<Image> drawImageIntoBuffer(Image&, int width, int height, int deviceScaleFactor, const char* functionName);
 
 #if ENABLE(VIDEO)
-    RefPtr<Image> videoFrameToImage(HTMLVideoElement&, BackingStoreCopy, const char* functionName);
+    RefPtr<Image> videoFrameToImage(HTMLVideoElement&, const char* functionName);
 #endif
 
     WebGLTexture::TextureExtensionFlag textureExtensionFlags() const;

Source/WebCore/platform/graphics/GraphicsContext.cpp

@@ -403,8 +403,8 @@ void GraphicsContext::drawPattern(ImageBuffer& image, const FloatRect& destRect,
 {
     FloatRect scaledSource = source;
     scaledSource.scale(image.resolutionScale());
-    if (auto copiedImage = image.copyImage(this == &image.context() ? CopyBackingStore : DontCopyBackingStore))
-        copiedImage->drawPattern(*this, destRect, scaledSource, patternTransform, phase, spacing, options);
+    if (auto nativeImage = nativeImageForDrawing(image))
+        drawPattern(*nativeImage, destRect, source, patternTransform, phase, spacing, options);
 }
 
 void GraphicsContext::drawControlPart(ControlPart& part, const FloatRoundedRect& borderRect, float deviceScaleFactor, const ControlStyle& style)

Source/WebCore/platform/graphics/GraphicsContextGL.cpp

@@ -30,6 +30,7 @@
 
 #if ENABLE(WEBGL)
 
+#include "BitmapImage.h"
 #include "FormatConverter.h"
 #include "GCGLSpan.h"
 #include "GraphicsContext.h"
@@ -652,9 +653,8 @@ RefPtr<Image> GraphicsContextGL::videoFrameToImage(VideoFrame& frame)
     auto imageBuffer = ImageBuffer::create(size, RenderingPurpose::Unspecified, 1, DestinationColorSpace::SRGB(), PixelFormat::BGRA8);
     if (!imageBuffer)
         return { };
-
     imageBuffer->context().paintVideoFrame(frame, { { }, size }, true);
-    return imageBuffer->copyImage(DontCopyBackingStore);
+    return BitmapImage::create(ImageBuffer::sinkIntoNativeImage(WTFMove(imageBuffer)));
 }
 #endif
 

Source/WebCore/platform/graphics/ImageBuffer.cpp

@@ -385,14 +385,6 @@ RefPtr<NativeImage> ImageBuffer::sinkIntoNativeImage(RefPtr<ImageBuffer> source)
     return source->sinkIntoNativeImage();
 }
 
-RefPtr<Image> ImageBuffer::copyImage(BackingStoreCopy copyBehavior, PreserveResolution preserveResolution) const
-{
-    auto image = copyImageBufferToNativeImage(const_cast<ImageBuffer&>(*this), copyBehavior, preserveResolution);
-    if (!image)
-        return nullptr;
-    return BitmapImage::create(image.releaseNonNull());
-}
-
 void ImageBuffer::convertToLuminanceMask()
 {
     if (auto* backend = ensureBackendCreated())

Source/WebCore/platform/graphics/ImageBuffer.h

@@ -162,7 +162,6 @@ class ImageBuffer : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<Image
     // Useful when caller can guarantee the use of the NativeImage ends "immediately", before the next draw to this ImageBuffer.
     WEBCORE_EXPORT virtual RefPtr<NativeImage> createNativeImageReference() const;
 
-    WEBCORE_EXPORT RefPtr<Image> copyImage(BackingStoreCopy = CopyBackingStore, PreserveResolution = PreserveResolution::No) const;
     WEBCORE_EXPORT virtual RefPtr<NativeImage> filteredNativeImage(Filter&);
     RefPtr<NativeImage> filteredNativeImage(Filter&, Function<void(GraphicsContext&)> drawCallback);
 

Source/WebCore/platform/graphics/nicosia/cairo/NicosiaCairoOperationRecorder.cpp

@@ -564,11 +564,7 @@ void CairoOperationRecorder::drawImageBuffer(ImageBuffer& buffer, const FloatRec
         }
     };
 
-    RefPtr<Image> image = buffer.copyImage(DontCopyBackingStore);
-    if (!image)
-        return;
-
-    auto nativeImage = image->nativeImageForCurrentFrame();
+    auto nativeImage = buffer.createNativeImageReference();
     if (!nativeImage)
         return;
 
@@ -602,11 +598,7 @@ void CairoOperationRecorder::drawFilteredImageBuffer(ImageBuffer* srcImage, cons
     if (!imageBuffer)
         return;
 
-    RefPtr<Image> image = imageBuffer->copyImage(DontCopyBackingStore);
-    if (!image)
-        return;
-
-    auto nativeImage = image->nativeImageForCurrentFrame();
+    auto nativeImage = imageBuffer->createNativeImageReference();
     if (!nativeImage)
         return;
 
@@ -1161,11 +1153,7 @@ void CairoOperationRecorder::clipToImageBuffer(ImageBuffer& buffer, const FloatR
         }
     };
 
-    RefPtr<Image> image = buffer.copyImage(DontCopyBackingStore);
-    if (!image)
-        return;
-
-    if (auto nativeImage = image->nativeImageForCurrentFrame())
+    if (auto nativeImage = buffer.createNativeImageReference())
         append(createCommand<ClipToImageBuffer>(nativeImage->platformImage(), destRect));
 }
 

Source/WebCore/platform/graphics/texmap/BitmapTexture.cpp

@@ -54,7 +54,7 @@ void BitmapTexture::updateContents(GraphicsLayer* sourceLayer, const IntRect& ta
 
     sourceLayer->paintGraphicsLayerContents(context, sourceRect);
 
-    RefPtr<Image> image = imageBuffer->copyImage(DontCopyBackingStore);
+    auto image = ImageBuffer::sinkIntoNativeImage(WTFMove(imageBuffer));
     if (!image)
         return;
 

Source/WebCore/platform/graphics/texmap/BitmapTexture.h

@@ -37,7 +37,7 @@ namespace WebCore {
 
 class FilterOperations;
 class GraphicsLayer;
-class Image;
+class NativeImage;
 class TextureMapper;
 
 // A 2D texture that can be the target of software or GL rendering.
@@ -60,7 +60,7 @@ class BitmapTexture : public RefCounted<BitmapTexture> {
     virtual bool isBackedByOpenGL() const { return false; }
 
     virtual IntSize size() const = 0;
-    virtual void updateContents(Image*, const IntRect&, const IntPoint& offset) = 0;
+    virtual void updateContents(NativeImage*, const IntRect&, const IntPoint& offset) = 0;
     void updateContents(GraphicsLayer*, const IntRect& target, const IntPoint& offset, float scale = 1);
     virtual void updateContents(const void*, const IntRect& target, const IntPoint& offset, int bytesPerLine) = 0;
     virtual bool isValid() const = 0;

Source/WebCore/platform/graphics/texmap/BitmapTextureGL.cpp

@@ -142,11 +142,8 @@ void BitmapTextureGL::updateContents(const void* srcData, const IntRect& targetR
     }
 }
 
-void BitmapTextureGL::updateContents(Image* image, const IntRect& targetRect, const IntPoint& offset)
+void BitmapTextureGL::updateContents(NativeImage* frameImage, const IntRect& targetRect, const IntPoint& offset)
 {
-    if (!image)
-        return;
-    auto frameImage = image->nativeImageForCurrentFrame();
     if (!frameImage)
         return;
 

Source/WebCore/platform/graphics/texmap/BitmapTextureGL.h

@@ -26,7 +26,6 @@
 #include "BitmapTexture.h"
 #include "ClipStack.h"
 #include "FilterOperation.h"
-#include "Image.h"
 #include "IntSize.h"
 #include "TextureMapperContextAttributes.h"
 #include "TextureMapperGL.h"
@@ -37,6 +36,7 @@ namespace WebCore {
 class TextureMapper;
 class TextureMapperGL;
 class FilterOperation;
+class NativeImage;
 
 #if OS(WINDOWS)
 #define USE_TEXMAP_DEPTH_STENCIL_BUFFER 1
@@ -62,7 +62,7 @@ class BitmapTextureGL : public BitmapTexture {
     virtual uint32_t id() const { return m_id; }
     uint32_t textureTarget() const { return GL_TEXTURE_2D; }
     IntSize textureSize() const { return m_textureSize; }
-    void updateContents(Image*, const IntRect&, const IntPoint&) override;
+    void updateContents(NativeImage*, const IntRect&, const IntPoint&) override;
     void updateContents(const void*, const IntRect& target, const IntPoint& sourceOffset, int bytesPerLine) override;
     bool isBackedByOpenGL() const override { return true; }
 
@@ -101,10 +101,6 @@ class BitmapTextureGL : public BitmapTexture {
     TextureMapperContextAttributes m_contextAttributes;
     TextureMapperGL::Flags m_colorConvertFlags { TextureMapperGL::NoFlag };
 
-#if ENABLE(WEBGL)
-    RefPtr<Image> m_pendingContents { nullptr };
-#endif
-
     void clearIfNeeded();
     void createFboIfNeeded();
 

Source/WebCore/platform/graphics/texmap/TextureMapperTile.cpp

@@ -45,7 +45,8 @@ void TextureMapperTile::updateContents(TextureMapper& textureMapper, Image* imag
         m_texture->reset(targetRect.size(), image->currentFrameKnownToBeOpaque() ? 0 : BitmapTexture::SupportsAlpha);
     }
 
-    m_texture->updateContents(image, targetRect, sourceOffset);
+    auto nativeImage = image->nativeImageForCurrentFrame();
+    m_texture->updateContents(nativeImage.get(), targetRect, sourceOffset);
 }
 
 void TextureMapperTile::updateContents(TextureMapper& textureMapper, GraphicsLayer* sourceLayer, const IntRect& dirtyRect, float scale)

Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm

@@ -92,7 +92,7 @@
     }
 
     PlatformImagePtr platformImage;
-    if (auto nativeImage = imageBuffer->copyImage()->nativeImage())
+    if (auto nativeImage = imageBuffer->copyNativeImage())
         platformImage = nativeImage->platformImage();
 
     auto presentationTime = MediaTime::createWithDouble((elapsedTime() + 100_ms).seconds());