Bug 250899

Summary: Cocoa readPixels is slower in GPUP WebGL than in-process WebGL
Product: WebKit Reporter: Kimmo Kinnunen <kkinnunen>
Component: WebGLAssignee: Kimmo Kinnunen <kkinnunen>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, kbr, kkinnunen, mcatanzaro, webkit-bug-importer, zdobersek
Priority: P2 Keywords: InRadar
Version: Safari 15   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=252876
Bug Depends on: 251431    
Bug Blocks:    

Description Kimmo Kinnunen 2023-01-20 07:26:53 PST
GPUP WebGL readPixels is slower than in process
Should and could be improved
Comment 1 Radar WebKit Bug Importer 2023-01-20 07:27:02 PST
<rdar://problem/104475196>
Comment 2 Kimmo Kinnunen 2023-01-24 02:28:40 PST
Pull request: https://github.com/WebKit/WebKit/pull/9029
Comment 3 EWS 2023-01-30 03:27:20 PST
Committed 259550@main (4fbc3e1ca2be): <https://commits.webkit.org/259550@main>

Reviewed commits have been landed. Closing PR #9029 and removing active labels.
Comment 4 Michael Catanzaro 2023-01-30 07:38:28 PST
Hi, this won't work because the object is noncopyable on Unix:

/home/mcatanzaro/Projects/WebKit/Source/WebKit/Platform/SharedMemory.h:70:9: error: explicitly defaulted copy constructor is implicitly deleted [-Werror,-Wdefaulted-function-deleted]
        Handle(const Handle&) = default;
        ^
/home/mcatanzaro/Projects/WebKit/Source/WebKit/Platform/SharedMemory.h:98:36: note: copy constructor of 'Handle' is implicitly deleted because field 'm_handle' has a deleted copy constructor
        mutable UnixFileDescriptor m_handle;
                                   ^
/home/mcatanzaro/Projects/WebKit/WebKitBuild/gtk4/WTF/Headers/wtf/unix/UnixFileDescriptor.h:37:26: note: 'UnixFileDescriptor' has been explicitly marked deleted here
    WTF_MAKE_NONCOPYABLE(UnixFileDescriptor);
Comment 5 Michael Catanzaro 2023-01-30 07:39:39 PST
I'll try this:

diff --git a/Source/WebKit/Platform/SharedMemory.h b/Source/WebKit/Platform/SharedMemory.h
index 45a30661faa3..ff78942ba3d8 100644
--- a/Source/WebKit/Platform/SharedMemory.h
+++ b/Source/WebKit/Platform/SharedMemory.h
@@ -67,11 +67,14 @@ public:
     class Handle {
     public:
         Handle() = default;
-        Handle(const Handle&) = default;
         Handle(Handle&&) = default;
-        Handle& operator=(const Handle&) = default;
         Handle& operator=(Handle&&) = default;
 
+#if PLATFORM(COCOA)
+        Handle(const Handle&) = default;
+        Handle& operator=(const Handle&) = default;
+#endif
+
         bool isNull() const;
 
         size_t size() const { return m_size; }

and see if that works.
Comment 6 Michael Catanzaro 2023-01-30 07:44:09 PST
So that does seem to work. It doesn't feel great, though.
Comment 7 Michael Catanzaro 2023-01-30 07:57:44 PST
Zan has a better solution in https://github.com/WebKit/WebKit/pull/9310
Comment 8 Michael Catanzaro 2023-01-30 08:09:44 PST
(In reply to Michael Catanzaro from comment #7)
> Zan has a better solution in https://github.com/WebKit/WebKit/pull/9310

Well, I wound up leaving some negative feedback there.

Kimmo, any opinion on what to do here?