WebCore::IOSurface has WebCore::GraphicsContext as its property, it s…

…hould be the other way around

https://bugs.webkit.org/show_bug.cgi?id=240417

Patch by Kimmo Kinnunen <kkinnunen@apple.com> on 2022-05-15
Reviewed by Simon Fraser.

Do not store WebCore::GraphicsContext in WebCore::IOSurface.
E.g. graphics context is not the property of the bitmap that
is being drawn to. The backing bitmap is a property of the context.

Instead, store the GraphicsContextCG at the place where the user is,
e.g. in ImageBufferIOSurfaceBackend. This allows the context client
to create the context with the options the client needs.

* platform/graphics/cg/GraphicsContextCG.cpp:
(WebCore::GraphicsContextCG::GraphicsContextCG):
(WebCore::GraphicsContextCG::drawNativeImage):
* platform/graphics/cg/GraphicsContextCG.h:
* platform/graphics/cg/IOSurfacePool.cpp:
(WebCore::IOSurfacePool::willAddSurface):
* platform/graphics/cg/ImageBufferIOSurfaceBackend.cpp:
(WebCore::ImageBufferIOSurfaceBackend::~ImageBufferIOSurfaceBackend):
(WebCore::ImageBufferIOSurfaceBackend::context const):
(WebCore::ImageBufferIOSurfaceBackend::releaseGraphicsContext):
* platform/graphics/cg/ImageBufferIOSurfaceBackend.h:
* platform/graphics/cocoa/IOSurface.h:
* platform/graphics/cocoa/IOSurface.mm:
(WebCore::IOSurface::create):
(WebCore::IOSurface::releasePlatformGraphicsContext):
(WebCore::IOSurface::ensureGraphicsContext): Deleted.
(WebCore::IOSurface::releaseGraphicsContext): Deleted.

Canonical link: https://commits.webkit.org/250573@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294215 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog

@@ -1,3 +1,36 @@
+2022-05-15  Kimmo Kinnunen  <kkinnunen@apple.com>
+
+        WebCore::IOSurface has WebCore::GraphicsContext as its property, it should be the other way around
+        https://bugs.webkit.org/show_bug.cgi?id=240417
+
+        Reviewed by Simon Fraser.
+
+        Do not store WebCore::GraphicsContext in WebCore::IOSurface.
+        E.g. graphics context is not the property of the bitmap that
+        is being drawn to. The backing bitmap is a property of the context.
+
+        Instead, store the GraphicsContextCG at the place where the user is,
+        e.g. in ImageBufferIOSurfaceBackend. This allows the context client
+        to create the context with the options the client needs.
+
+        * platform/graphics/cg/GraphicsContextCG.cpp:
+        (WebCore::GraphicsContextCG::GraphicsContextCG):
+        (WebCore::GraphicsContextCG::drawNativeImage):
+        * platform/graphics/cg/GraphicsContextCG.h:
+        * platform/graphics/cg/IOSurfacePool.cpp:
+        (WebCore::IOSurfacePool::willAddSurface):
+        * platform/graphics/cg/ImageBufferIOSurfaceBackend.cpp:
+        (WebCore::ImageBufferIOSurfaceBackend::~ImageBufferIOSurfaceBackend):
+        (WebCore::ImageBufferIOSurfaceBackend::context const):
+        (WebCore::ImageBufferIOSurfaceBackend::releaseGraphicsContext):
+        * platform/graphics/cg/ImageBufferIOSurfaceBackend.h:
+        * platform/graphics/cocoa/IOSurface.h:
+        * platform/graphics/cocoa/IOSurface.mm:
+        (WebCore::IOSurface::create):
+        (WebCore::IOSurface::releasePlatformGraphicsContext):
+        (WebCore::IOSurface::ensureGraphicsContext): Deleted.
+        (WebCore::IOSurface::releaseGraphicsContext): Deleted.
+
 2022-05-08  Yusuke Suzuki  <ysuzuki@apple.com>
 
         Put ThreadGlobalData in Thread

Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp

@@ -267,8 +267,8 @@ void GraphicsContextCG::drawNativeImage(NativeImage& nativeImage, const FloatSiz
             // image. We need to convert subimageRect to physical image coordinates.
             if (auto transform = options.orientation().transformFromDefault(imageSize).inverse())
                 physicalSubimageRect = transform.value().mapRect(physicalSubimageRect);
-        }
 
+        }
 #if CACHE_SUBIMAGES
         return SubimageCacheWithTimer::getSubimage(image, physicalSubimageRect);
 #else

Source/WebCore/platform/graphics/cg/GraphicsContextCG.h

@@ -32,10 +32,16 @@
 
 namespace WebCore {
 
+#if HAVE(IOSURFACE)
+class IOSurface;
+#endif
+
 class WEBCORE_EXPORT GraphicsContextCG : public GraphicsContext {
 public:
     GraphicsContextCG(CGContextRef);
-
+#if HAVE(IOSURFACE)
+    explicit GraphicsContextCG(IOSurface&);
+#endif
 #if PLATFORM(WIN)
     GraphicsContextCG(HDC, bool hasAlpha = false); // FIXME: To be removed.
 #endif

Source/WebCore/platform/graphics/cg/IOSurfacePool.cpp

@@ -82,7 +82,7 @@ void IOSurfacePool::willAddSurface(IOSurface& surface, bool inUse)
     CachedSurfaceDetails& details = m_surfaceDetails.add(&surface, CachedSurfaceDetails()).iterator->value;
     details.resetLastUseTime();
 
-    surface.releaseGraphicsContext();
+    surface.releasePlatformGraphicsContext();
 
     size_t surfaceBytes = surface.totalBytes();
 

Source/WebCore/platform/graphics/cg/ImageBufferIOSurfaceBackend.cpp

@@ -125,17 +125,17 @@ ImageBufferIOSurfaceBackend::ImageBufferIOSurfaceBackend(const Parameters& param
 
 ImageBufferIOSurfaceBackend::~ImageBufferIOSurfaceBackend()
 {
+    m_context = std::nullopt; // Make sure the context does not interfere with moving to pool.
     IOSurface::moveToPool(WTFMove(m_surface), m_ioSurfacePool.get());
 }
 
 GraphicsContext& ImageBufferIOSurfaceBackend::context() const
 {
-    GraphicsContext& context = m_surface->ensureGraphicsContext();
-    if (m_needsSetupContext) {
-        m_needsSetupContext = false;
+    if (!m_context) {
+        m_context.emplace(*m_surface);
         applyBaseTransformToContext();
     }
-    return context;
+    return *m_context;
 }
 
 void ImageBufferIOSurfaceBackend::flushContext()
@@ -238,8 +238,8 @@ bool ImageBufferIOSurfaceBackend::isInUse() const
 
 void ImageBufferIOSurfaceBackend::releaseGraphicsContext()
 {
-    m_needsSetupContext = true;
-    return m_surface->releaseGraphicsContext();
+    m_context = std::nullopt;
+    m_surface->releasePlatformGraphicsContext();
 }
 
 bool ImageBufferIOSurfaceBackend::setVolatile()

Source/WebCore/platform/graphics/cg/ImageBufferIOSurfaceBackend.h

@@ -27,13 +27,17 @@
 
 #if HAVE(IOSURFACE)
 
+#include "GraphicsContextCG.h"
 #include "ImageBufferCGBackend.h"
 #include "IOSurface.h"
 #include "IOSurfacePool.h"
+#include <optional>
 #include <wtf/IsoMalloc.h>
 
 namespace WebCore {
 
+class GraphicsContextCG;
+
 class WEBCORE_EXPORT ImageBufferIOSurfaceBackend : public ImageBufferCGBackend {
     WTF_MAKE_ISO_ALLOCATED(ImageBufferIOSurfaceBackend);
     WTF_MAKE_NONCOPYABLE(ImageBufferIOSurfaceBackend);
@@ -91,7 +95,7 @@ class WEBCORE_EXPORT ImageBufferIOSurfaceBackend : public ImageBufferCGBackend {
     IOSurfaceSeed m_lastSeedWhenDrawingImage { 0 };
     mutable bool m_requiresDrawAfterPutPixelBuffer { false };
 
-    mutable bool m_needsSetupContext { false };
+    mutable std::optional<GraphicsContextCG> m_context;
     VolatilityState m_volatilityState { VolatilityState::NonVolatile };
 
     RefPtr<IOSurfacePool> m_ioSurfacePool;

Source/WebCore/platform/graphics/cocoa/GraphicsContextCocoa.mm

@@ -29,6 +29,7 @@
 #import "DisplayListRecorder.h"
 #import "GraphicsContextCG.h"
 #import "GraphicsContextPlatformPrivateCG.h"
+#import "IOSurface.h"
 #import "IntRect.h"
 #import <pal/spi/cg/CoreGraphicsSPI.h>
 #import <pal/spi/mac/NSGraphicsSPI.h>
@@ -115,6 +116,12 @@ static void drawFocusRingToContext(CGContextRef context, CGPathRef focusRingPath
     drawFocusRing(context, color);
 }
 
+GraphicsContextCG::GraphicsContextCG(IOSurface& iosurface)
+    : GraphicsContextCG(iosurface.ensurePlatformContext())
+{
+    setIsAcceleratedContext(true);
+}
+
 void GraphicsContextCG::drawFocusRing(const Path& path, float, float, const Color& color)
 {
     if (path.isNull())

Source/WebCore/platform/graphics/cocoa/IOSurface.h

@@ -43,7 +43,6 @@ class TextStream;
 
 namespace WebCore {
 
-class GraphicsContext;
 class HostWindow;
 class IOSurfacePool;
 class ProcessIdentity;
@@ -127,8 +126,10 @@ class IOSurface final {
     id asLayerContents() const { return (__bridge id)m_surface.get(); }
 #endif
     IOSurfaceRef surface() const { return m_surface.get(); }
-    WEBCORE_EXPORT GraphicsContext& ensureGraphicsContext();
     WEBCORE_EXPORT CGContextRef ensurePlatformContext(const HostWindow* = nullptr);
+    // The graphics context cached on the surface counts as a "user", so to get
+    // an accurate result from isInUse(), it needs to be released.
+    WEBCORE_EXPORT void releasePlatformGraphicsContext();
 
     // Querying volatility can be expensive, so in cases where the surface is
     // going to be used immediately, use the return value of setVolatile to
@@ -150,10 +151,6 @@ class IOSurface final {
 
     WEBCORE_EXPORT bool isInUse() const;
 
-    // The graphics context cached on the surface counts as a "user", so to get
-    // an accurate result from isInUse(), it needs to be released.
-    WEBCORE_EXPORT void releaseGraphicsContext();
-
 #if HAVE(IOSURFACE_ACCELERATOR)
     WEBCORE_EXPORT static bool allowConversionFromFormatToFormat(Format, Format);
     WEBCORE_EXPORT static void convertToFormat(IOSurfacePool*, std::unique_ptr<WebCore::IOSurface>&& inSurface, Format, Function<void(std::unique_ptr<WebCore::IOSurface>)>&&);
@@ -172,7 +169,6 @@ class IOSurface final {
     IntSize m_size;
     size_t m_totalBytes;
 
-    std::unique_ptr<GraphicsContext> m_graphicsContext;
     RetainPtr<CGContextRef> m_cgContext;
 
     RetainPtr<IOSurfaceRef> m_surface;

Source/WebCore/platform/graphics/cocoa/IOSurface.mm

@@ -27,7 +27,6 @@
 #import "IOSurface.h"
 
 #import "DestinationColorSpace.h"
-#import "GraphicsContextCG.h"
 #import "HostWindow.h"
 #import "IOSurfacePool.h"
 #import "Logging.h"
@@ -47,7 +46,7 @@
 {
     if (pool) {
         if (auto cachedSurface = pool->takeSurface(size, colorSpace, pixelFormat)) {
-            cachedSurface->releaseGraphicsContext();
+            cachedSurface->releasePlatformGraphicsContext();
             LOG_WITH_STREAM(IOSurface, stream << "IOSurface::create took from pool: " << *cachedSurface);
             return cachedSurface;
         }
@@ -352,17 +351,6 @@ static IntSize computeMaximumSurfaceSize()
     return m_cgContext.get();
 }
 
-GraphicsContext& IOSurface::ensureGraphicsContext()
-{
-    if (m_graphicsContext)
-        return *m_graphicsContext;
-
-    m_graphicsContext = makeUnique<GraphicsContextCG>(ensurePlatformContext());
-    m_graphicsContext->setIsAcceleratedContext(true);
-
-    return *m_graphicsContext;
-}
-
 SetNonVolatileResult IOSurface::state() const
 {
     uint32_t previousState = 0;
@@ -438,9 +426,8 @@ static IntSize computeMaximumSurfaceSize()
     return IOSurfaceIsInUse(m_surface.get());
 }
 
-void IOSurface::releaseGraphicsContext()
+void IOSurface::releasePlatformGraphicsContext()
 {
-    m_graphicsContext = nullptr;
     m_cgContext = nullptr;
 }