Canvases painted into CG display list image buffers don't ever update

hortont424 · hortont424 · commit 7d40f6d46408 · 2023-01-06T17:24:58.000-08:00
https://bugs.webkit.org/show_bug.cgi?id=250196 rdar://98798050 Reviewed by Simon Fraser and Dean Jackson. WebKit has long accidentally depended on the combination of two somewhat unusual behavioral quirks in CGIOSurfaceContext: 1) (Source) If you make a CGImageRef from one CGIOSurfaceContext via CGIOSurfaceContextCreateImage, and mutate the original IOSurface under the hood (or in a different process) in a way that CGIOSurfaceContext does not know, CGIOSurfaceContextCreateImage will return the same CGImageRef when called later. 2) (Destination) If you make a CGImageRef from one CGIOSurfaceContext via CGIOSurfaceContextCreateImage, paint it into a different CGIOSurfaceContext, then mutate the original IOSurface, and paint the same CGImageRef again, the updated IOSurface contents will be used the second time. The second quirk has never worked with unaccelerated CoreGraphics bitmap context destinations. Instead, in the unaccelerated case, the CGImageRef acts as a snapshot of the surface at the time it was created. We've long had code to handle this, forcing CGIOSurfaceContextCreateImage to re-create the CGImageRef each time we paint it (by drawing an empty rect into the CGIOSurfaceContext), working around quirk #1 and thus bypassing quirk #2, if we're painting into an unaccelerated backing store. It turns out our CG display list backing store implementation behaves like a CG bitmap context (without quirk #2), and so currently any IOSurfaces painted into CG display list backing store from a CGImageRef created by CGIOSurfaceContextCreateImage (but not -CreateImageReference) become stale if painted multiple times. To avoid this, extend the workaround to apply to any destination context that claims that it needs the workaround, and use it whenever painting an IOSurface into anything other than a CGIOSurfaceContext. * Source/WebCore/platform/graphics/BifurcatedGraphicsContext.cpp: (WebCore::BifurcatedGraphicsContext::needsCachedNativeImageInvalidationWorkaround): * Source/WebCore/platform/graphics/BifurcatedGraphicsContext.h: Make BifurcatedGraphicsContext assume the more conservative mode of its two children. * Source/WebCore/platform/graphics/GraphicsContext.h: (WebCore::GraphicsContext::needsCachedNativeImageInvalidationWorkaround): Assume that by default, GraphicsContexts need the workaround. * Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp: (WebCore::GraphicsContextCG::needsCachedNativeImageInvalidationWorkaround): * Source/WebCore/platform/graphics/cg/GraphicsContextCG.h: GraphicsContextCG needs the workaround, except in the IOSurface->IOSurface case. * Source/WebCore/platform/graphics/cg/ImageBufferIOSurfaceBackend.cpp: (WebCore::ImageBufferIOSurfaceBackend::finalizeDrawIntoContext): Confer with the GraphicsContext about its need for the workaround instead of hardcoding the behavior here. * Source/WebKit/Shared/RemoteLayerTree/CGDisplayListImageBufferBackend.mm: CG display list graphics contexts need the workaround. Canonical link: https://commits.webkit.org/258586@main
diff --git a/Source/WebCore/platform/graphics/BifurcatedGraphicsContext.cpp b/Source/WebCore/platform/graphics/BifurcatedGraphicsContext.cpp
@@ -340,6 +340,11 @@ void BifurcatedGraphicsContext::drawNativeImage(NativeImage& nativeImage, const
     VERIFY_STATE_SYNCHRONIZATION();
 }
 
+bool BifurcatedGraphicsContext::needsCachedNativeImageInvalidationWorkaround(RenderingMode renderingMode)
+{
+    return m_primaryContext.needsCachedNativeImageInvalidationWorkaround(renderingMode) || m_secondaryContext.needsCachedNativeImageInvalidationWorkaround(renderingMode);
+}
+
 void BifurcatedGraphicsContext::drawSystemImage(SystemImage& systemImage, const FloatRect& destinationRect)
 {
     m_primaryContext.drawSystemImage(systemImage, destinationRect);
diff --git a/Source/WebCore/platform/graphics/BifurcatedGraphicsContext.h b/Source/WebCore/platform/graphics/BifurcatedGraphicsContext.h
@@ -98,6 +98,7 @@ class WEBCORE_EXPORT BifurcatedGraphicsContext : public GraphicsContext {
     void setMiterLimit(float) final;
 
     void drawNativeImage(NativeImage&, const FloatSize& selfSize, const FloatRect& destRect, const FloatRect& srcRect, const ImagePaintingOptions& = { }) final;
+    bool needsCachedNativeImageInvalidationWorkaround(RenderingMode) final;
     void drawSystemImage(SystemImage&, const FloatRect&) final;
     void drawPattern(NativeImage&, const FloatRect& destRect, const FloatRect& tileRect, const AffineTransform& patternTransform, const FloatPoint& phase, const FloatSize& spacing, const ImagePaintingOptions& = { }) final;
     ImageDrawResult drawImage(Image&, const FloatRect& destination, const FloatRect& source, const ImagePaintingOptions& = { ImageOrientation::FromImage }) final;
diff --git a/Source/WebCore/platform/graphics/GraphicsContext.h b/Source/WebCore/platform/graphics/GraphicsContext.h
@@ -248,6 +248,8 @@ class GraphicsContext {
 
     virtual void drawNativeImage(NativeImage&, const FloatSize& selfSize, const FloatRect& destRect, const FloatRect& srcRect, const ImagePaintingOptions& = { }) = 0;
 
+    virtual bool needsCachedNativeImageInvalidationWorkaround(RenderingMode) { return true; }
+
     WEBCORE_EXPORT virtual void drawSystemImage(SystemImage&, const FloatRect&);
 
     WEBCORE_EXPORT ImageDrawResult drawImage(Image&, const FloatPoint& destination, const ImagePaintingOptions& = { ImageOrientation::FromImage });
diff --git a/Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp b/Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp
@@ -374,6 +374,18 @@ void GraphicsContextCG::drawNativeImage(NativeImage& nativeImage, const FloatSiz
     LOG_WITH_STREAM(Images, stream << "GraphicsContextCG::drawNativeImage " << image.get() << " size " << imageSize << " into " << destRect << " took " << (MonotonicTime::now() - startTime).milliseconds() << "ms");
 }
 
+bool GraphicsContextCG::needsCachedNativeImageInvalidationWorkaround(RenderingMode imageRenderingMode)
+{
+    // Only accelerated images cache CGImageRefs underneath us (when returned by
+    // IOSurface::createImage), and thus need the workaround.
+    if (imageRenderingMode == RenderingMode::Unaccelerated)
+        return false;
+
+    // Accelerated destinations have "live" CGImageRefs, so we only need
+    // the workaround when painting into an unaccelerated context.
+    return renderingMode() == RenderingMode::Unaccelerated;
+}
+
 static void drawPatternCallback(void* info, CGContextRef context)
 {
     CGImageRef image = (CGImageRef)info;
diff --git a/Source/WebCore/platform/graphics/cg/GraphicsContextCG.h b/Source/WebCore/platform/graphics/cg/GraphicsContextCG.h
@@ -96,6 +96,7 @@ class WEBCORE_EXPORT GraphicsContextCG : public GraphicsContext {
 
     void drawNativeImage(NativeImage&, const FloatSize& selfSize, const FloatRect& destRect, const FloatRect& srcRect, const ImagePaintingOptions& = { }) final;
     void drawPattern(NativeImage&, const FloatRect& destRect, const FloatRect& tileRect, const AffineTransform& patternTransform, const FloatPoint& phase, const FloatSize& spacing, const ImagePaintingOptions& = { }) final;
+    bool needsCachedNativeImageInvalidationWorkaround(RenderingMode) override;
 
     using GraphicsContext::scale;
     void scale(const FloatSize&) final;
diff --git a/Source/WebCore/platform/graphics/cg/ImageBufferIOSurfaceBackend.cpp b/Source/WebCore/platform/graphics/cg/ImageBufferIOSurfaceBackend.cpp
@@ -195,9 +195,7 @@ RefPtr<NativeImage> ImageBufferIOSurfaceBackend::sinkIntoNativeImage()
 
 void ImageBufferIOSurfaceBackend::finalizeDrawIntoContext(GraphicsContext& destinationContext)
 {
-    // Accelerated to unaccelerated image buffers need complex caching. We trust that
-    // this is a one-off draw, and as such we clear the caches of the source image after each draw.
-    if (destinationContext.renderingMode() == RenderingMode::Unaccelerated)
+    if (destinationContext.needsCachedNativeImageInvalidationWorkaround(ImageBufferIOSurfaceBackend::renderingMode))
         invalidateCachedNativeImage();
 }
 
diff --git a/Source/WebKit/Shared/RemoteLayerTree/CGDisplayListImageBufferBackend.mm b/Source/WebKit/Shared/RemoteLayerTree/CGDisplayListImageBufferBackend.mm
@@ -84,6 +84,8 @@ void setCTM(const WebCore::AffineTransform& transform) final
 
     bool canUseShadowBlur() const final { return false; }
 
+    bool needsCachedNativeImageInvalidationWorkaround(WebCore::RenderingMode) override { return true; }
+
 protected:
     void setCGShadow(WebCore::RenderingMode renderingMode, const WebCore::FloatSize& offset, float blur, const WebCore::Color& color, bool shadowsIgnoreTransforms) override
     {

Original file line number	Diff line number	Diff line change
`@@ -340,6 +340,11 @@ void BifurcatedGraphicsContext::drawNativeImage(NativeImage& nativeImage, const`
`340`	`340`	`VERIFY_STATE_SYNCHRONIZATION();`
`341`	`341`	`}`
`342`	`342`
	`343`	`+bool BifurcatedGraphicsContext::needsCachedNativeImageInvalidationWorkaround(RenderingMode renderingMode)`
	`344`	`+{`
	`345`	`+ return m_primaryContext.needsCachedNativeImageInvalidationWorkaround(renderingMode) \|\| m_secondaryContext.needsCachedNativeImageInvalidationWorkaround(renderingMode);`
	`346`	`+}`
	`347`	`+`
`343`	`348`	`void BifurcatedGraphicsContext::drawSystemImage(SystemImage& systemImage, const FloatRect& destinationRect)`
`344`	`349`	`{`
`345`	`350`	`m_primaryContext.drawSystemImage(systemImage, destinationRect);`
Original file line number	Diff line number	Diff line change
`@@ -195,9 +195,7 @@ RefPtr<NativeImage> ImageBufferIOSurfaceBackend::sinkIntoNativeImage()`
`195`	`195`
`196`	`196`	`void ImageBufferIOSurfaceBackend::finalizeDrawIntoContext(GraphicsContext& destinationContext)`
`197`	`197`	`{`
`198`		`- // Accelerated to unaccelerated image buffers need complex caching. We trust that`
`199`		`- // this is a one-off draw, and as such we clear the caches of the source image after each draw.`
`200`		`- if (destinationContext.renderingMode() == RenderingMode::Unaccelerated)`
	`198`	`+ if (destinationContext.needsCachedNativeImageInvalidationWorkaround(ImageBufferIOSurfaceBackend::renderingMode))`
`201`	`199`	`invalidateCachedNativeImage();`
`202`	`200`	`}`
`203`	`201`
Original file line number	Diff line number	Diff line change
`@@ -84,6 +84,8 @@ void setCTM(const WebCore::AffineTransform& transform) final`
`84`	`84`
`85`	`85`	`bool canUseShadowBlur() const final { return false; }`
`86`	`86`
	`87`	`+ bool needsCachedNativeImageInvalidationWorkaround(WebCore::RenderingMode) override { return true; }`
	`88`	`+`
`87`	`89`	`protected:`
`88`	`90`	`void setCGShadow(WebCore::RenderingMode renderingMode, const WebCore::FloatSize& offset, float blur, const WebCore::Color& color, bool shadowsIgnoreTransforms) override`
`89`	`91`	`{`