Bug 250899 - Cocoa readPixels is slower in GPUP WebGL than in-process WebGL
Summary: Cocoa readPixels is slower in GPUP WebGL than in-process WebGL
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: Safari 15
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kimmo Kinnunen
URL:
Keywords: InRadar
Depends on: 251431
Blocks:
  Show dependency treegraph
 
Reported: 2023-01-20 07:26 PST by Kimmo Kinnunen
Modified: 2023-05-22 05:22 PDT (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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?