Bug 188072 – Match GraphicsContext3D with correct virtual screen using registryID

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Justin Fan]

Match GraphicsContext3D with correct virtual screen using registryID

[Radar WebKit Bug Importer]

<rdar://problem/42634940>

[Justin Fan]

Created attachment 345861 [details]
Patch

[Justin Fan]

Going to add in Per Arne's original displayMask-matching path for versions of macOS older than 10.13 that cannot use kCGLRegistryID.

[Justin Fan]

Created attachment 345874 [details]
Patch

[Justin Fan]

Created attachment 345879 [details]
Patch

[EWS Watchlist]

Comment on attachment 345879 [details]
Patch

Attachment 345879 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8667685

New failing tests:
http/tests/security/canvas-remote-read-remote-video-blocked-no-crossorigin.html

[EWS Watchlist]

Created attachment 345892 [details]
Archive of layout-test-results from ews205 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews205  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Simon Fraser (smfr)]

Comment on attachment 345879 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=345879&action=review

> Source/WebCore/platform/PlatformScreen.h:107
> +int64_t primaryRegistryID();
> +int64_t registryIDForDisplay(PlatformDisplayID);
> +int64_t registryIDForDisplayMask(uint32_t);

Can we use a typedef for the registryID? We have way too many bare int64_t things.

[Justin Fan]

Created attachment 345933 [details]
Patch

[Dean Jackson]

Comment on attachment 345933 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=345933&action=review

> Source/WebCore/platform/ScreenProperties.h:49
> +    int64_t registryID { 0 };

Shouldn't this use the PlatformRegistryID typedef?

> Source/WebCore/platform/graphics/cocoa/GraphicsContext3DCocoa.mm:216
> +    // When the WebProcess does not have access to the WindowServer, there is no way for OpenGL to tell which GPU is connected to a display.
> +    // See code example at https://developer.apple.com/library/content/technotes/tn2229/_index.html#//apple_ref/doc/uid/DTS40008924-CH1-SUBSECTION7

Do we have access to the window server on 10.12?

[Justin Fan]

(In reply to Dean Jackson from comment #10)
> Comment on attachment 345933 [details]
> Patch
> 
> Shouldn't this use the PlatformRegistryID typedef?

Updated!

> Do we have access to the window server on 10.12?

Looked into it and we have access to window server before 10.14. I've added a comment for now and will ask Per Arne if he would do anything differently in that case.

[Justin Fan]

Created attachment 345939 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 345933 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=345933&action=review

> Source/WebCore/platform/PlatformScreen.h:61
> +using PlatformRegistryID = int64_t;

What is a platform registry? A registry of platforms? I think this needs a comment saying what it represents.

[Justin Fan]

Created attachment 345958 [details]
Patch

[Justin Fan]

Looks like I had a git mix-up. That test wasn't supposed to be in this patch :X

[Justin Fan]

Created attachment 345963 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 345963 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=345963&action=review

> Source/WebCore/platform/PlatformScreen.h:61
> +using GPURegistryID = int64_t; // A global identifier for a GPU used by CGL and Metal.

Can we call this a GPUIdentifier? We don't have to follow the bad naming choices for other frameworks.

[Justin Fan]

Created attachment 345982 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 345982 [details]
Patch

Clearing flags on attachment: 345982

Committed r234377: <https://trac.webkit.org/changeset/234377>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.