Attachment 345939 Details for Bug 188072 – Patch

[patch] Patch

bug-188072-20180727124223.patch (text/plain), 16.02 KB, created by Justin Fan on 2018-07-27 12:42:23 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Justin Fan

Created: 2018-07-27 12:42:23 PDT

Size: 16.02 KB

patch

obsolete

>Subversion Revision: 234224
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 81c20c0363bd63df5bfddeb1ac7eadfabea8a2e0..4961918ca3011975b8d6150c4af9476f76214aac 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,37 @@
>+2018-07-26  Justin Fan  <justin_fan@apple.com>
>+
>+        Match GraphicsContext3D with correct virtual screen using registryID
>+        https://bugs.webkit.org/show_bug.cgi?id=188072
>+        <rdar://problem/42634940>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Follow up to https://bugs.webkit.org/show_bug.cgi?id=187750. Rather than matching rendererIDs, 
>+        which can vary between processes, match GPU to display using registryID that is unique to a GPU, 
>+        which was added to CGL in MacOS 10.13. 
>+
>+        No new tests. Manually tested on MacBookPro13,3 and iMacPro1,1 with Apple DisplayPort Cinema Display
>+        and RX 580 external GPU. 
>+
>+        * platform/PlatformScreen.h:
>+        * platform/ScreenProperties.h:
>+        (WebCore::ScreenData::encode const):
>+        (WebCore::ScreenData::decode):
>+        * platform/graphics/cocoa/GraphicsContext3DCocoa.mm:
>+        (WebCore::setGPUByRegistryID):
>+        (WebCore::setGPUByDisplayMask):
>+        (WebCore::GraphicsContext3D::GraphicsContext3D):
>+        (WebCore::GraphicsContext3D::screenDidChange):
>+        (WebCore::identifyAndSetCurrentGPU): Deleted.
>+        * platform/mac/PlatformScreenMac.mm:
>+        (WebCore::collectScreenProperties):
>+        (WebCore::primaryRegistryID):
>+        (WebCore::registryIDForDisplay):
>+        (WebCore::registryIDForDisplayMask):
>+        (WebCore::rendererIDForDisplayMask): Deleted.
>+        (WebCore::rendererIDForDisplay): Deleted.
>+        (WebCore::primaryRendererID): Deleted.
>+
> 2018-07-25  Justin Fan  <justin_fan@apple.com>
> 
>         Systems with no NSScreens hitting assertion in rendererIDForDisplay when creating WebGL context
>diff --git a/Source/WebCore/platform/PlatformScreen.h b/Source/WebCore/platform/PlatformScreen.h
>index fec77de6959adf4c52cb79724e53c7571e17d7dd..c71187aa9651a46292f58f49c21013e4c58da4eb 100644
>--- a/Source/WebCore/platform/PlatformScreen.h
>+++ b/Source/WebCore/platform/PlatformScreen.h
>@@ -58,6 +58,7 @@ class FloatSize;
> class Widget;
> 
> using PlatformDisplayID = uint32_t;
>+using PlatformRegistryID = int64_t;
> 
> int screenDepth(Widget*);
> int screenDepthPerComponent(Widget*);
>@@ -100,10 +101,15 @@ WEBCORE_EXPORT void setScreenProperties(const ScreenProperties&);
> 
> uint32_t primaryOpenGLDisplayMask();
> uint32_t displayMaskForDisplay(PlatformDisplayID);
>-int32_t rendererIDForDisplay(PlatformDisplayID);
>-int32_t primaryRendererID();
>+
>+#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101300
>+PlatformRegistryID primaryRegistryID();
>+PlatformRegistryID registryIDForDisplay(PlatformDisplayID);
>+PlatformRegistryID registryIDForDisplayMask(uint32_t);
> #endif
> 
>+#endif // !PLATFORM(MAC)
>+
> #if PLATFORM(IOS)
> 
> float screenPPIFactor();
>diff --git a/Source/WebCore/platform/ScreenProperties.h b/Source/WebCore/platform/ScreenProperties.h
>index 6ae628abac783543df348d00ed93c1299f65af1d..9ae778d69e27a843485feb783eaed50e68f65441 100644
>--- a/Source/WebCore/platform/ScreenProperties.h
>+++ b/Source/WebCore/platform/ScreenProperties.h
>@@ -46,7 +46,7 @@ struct ScreenData {
>     bool screenHasInvertedColors { false };
>     bool screenIsMonochrome { false };
>     uint32_t displayMask { 0 };
>-    int32_t rendererID { 0 };
>+    PlatformRegistryID registryID { 0 };
> 
>     enum EncodedColorSpaceDataType {
>         Null,
>@@ -94,7 +94,7 @@ std::optional<ScreenProperties> ScreenProperties::decode(Decoder& decoder)
> template<class Encoder>
> void ScreenData::encode(Encoder& encoder) const
> {
>-    encoder << screenAvailableRect << screenRect << screenDepth << screenDepthPerComponent << screenSupportsExtendedColor << screenHasInvertedColors << screenIsMonochrome << displayMask << rendererID;
>+    encoder << screenAvailableRect << screenRect << screenDepth << screenDepthPerComponent << screenSupportsExtendedColor << screenHasInvertedColors << screenIsMonochrome << displayMask << registryID;
> 
>     if (colorSpace) {
>         // Try to encode the name.
>@@ -163,9 +163,9 @@ std::optional<ScreenData> ScreenData::decode(Decoder& decoder)
>     if (!displayMask)
>         return std::nullopt;
> 
>-    std::optional<int32_t> rendererID;
>-    decoder >> rendererID;
>-    if (!rendererID)
>+    std::optional<PlatformRegistryID> registryID;
>+    decoder >> registryID;
>+    if (!registryID)
>         return std::nullopt;
>     
>     EncodedColorSpaceDataType dataType;
>@@ -203,7 +203,7 @@ std::optional<ScreenData> ScreenData::decode(Decoder& decoder)
>     }
>     }
> 
>-    return { { WTFMove(*screenAvailableRect), WTFMove(*screenRect), WTFMove(cgColorSpace), WTFMove(*screenDepth), WTFMove(*screenDepthPerComponent), WTFMove(*screenSupportsExtendedColor), WTFMove(*screenHasInvertedColors), WTFMove(*screenIsMonochrome), WTFMove(*displayMask), WTFMove(*rendererID) } };
>+    return { { WTFMove(*screenAvailableRect), WTFMove(*screenRect), WTFMove(cgColorSpace), WTFMove(*screenDepth), WTFMove(*screenDepthPerComponent), WTFMove(*screenSupportsExtendedColor), WTFMove(*screenHasInvertedColors), WTFMove(*screenIsMonochrome), WTFMove(*displayMask), WTFMove(*registryID) } };
> }
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/platform/graphics/cocoa/GraphicsContext3DCocoa.mm b/Source/WebCore/platform/graphics/cocoa/GraphicsContext3DCocoa.mm
>index 959919dba501a3c783c035b3aa0f0e250b642c24..a2bbaf3b4a747235fd1c331ded1e1aa21b8dfff7 100644
>--- a/Source/WebCore/platform/graphics/cocoa/GraphicsContext3DCocoa.mm
>+++ b/Source/WebCore/platform/graphics/cocoa/GraphicsContext3DCocoa.mm
>@@ -162,13 +162,15 @@ Ref<GraphicsContext3D> GraphicsContext3D::createShared(GraphicsContext3D& shared
> }
> 
> #if PLATFORM(MAC)
>-static void identifyAndSetCurrentGPU(PlatformGraphicsContext3D contextObj, CGLPixelFormatObj pixelFormatObj, GLint preferredRendererID)
>+
>+#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101300
>+static void setGPUByRegistryID(PlatformGraphicsContext3D contextObj, CGLPixelFormatObj pixelFormatObj, PlatformRegistryID preferredRegistryID)
> {
>-    // When the WebProcess does not have access to the WindowServer, there is no way for OpenGL to tell which GPU/renderer is connected to a display.
>-    // Find the virtual screen that corresponds to the preferred renderer.
>+    // 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.
>+    // On 10.13+, find the virtual screen that corresponds to the preferred GPU by its registryID.
>     // CGLSetVirtualScreen can then be used to tell OpenGL which GPU it should be using.
> 
>-    if (!contextObj || !preferredRendererID)
>+    if (!contextObj || !preferredRegistryID)
>         return;
> 
>     GLint virtualScreenCount = 0;
>@@ -178,16 +180,16 @@ static void identifyAndSetCurrentGPU(PlatformGraphicsContext3D contextObj, CGLPi
>     GLint firstAcceleratedScreen = -1;
> 
>     for (GLint virtualScreen = 0; virtualScreen < virtualScreenCount; ++virtualScreen) {
>-        GLint rendererID = 0;
>-        error = CGLDescribePixelFormat(pixelFormatObj, virtualScreen, kCGLPFARendererID, &rendererID);
>+        GLint displayMask = 0;
>+        error = CGLDescribePixelFormat(pixelFormatObj, virtualScreen, kCGLPFADisplayMask, &displayMask);
>         ASSERT(error == kCGLNoError);
>-        if (error != kCGLNoError)
>-            continue;
> 
>-        if (rendererID == preferredRendererID) {
>+        auto registryID = registryIDForDisplayMask(displayMask);
>+
>+        if (registryID == preferredRegistryID) {
>             error = CGLSetVirtualScreen(contextObj, virtualScreen);
>             ASSERT(error == kCGLNoError);
>-            LOG(WebGL, "Context (%p) set to GPU renderer (%d).", contextObj, rendererID);
>+            LOG(WebGL, "Context (%p) set to GPU with ID: (%lld).", contextObj, registryID);
>             return;
>         }
> 
>@@ -200,15 +202,46 @@ static void identifyAndSetCurrentGPU(PlatformGraphicsContext3D contextObj, CGLPi
>         }
>     }
> 
>-    // No renderer match found; set to first hardware-accelerated virtual screen.
>+    // No registryID match found; set to first hardware-accelerated virtual screen.
>     if (firstAcceleratedScreen >= 0) {
>         error = CGLSetVirtualScreen(contextObj, firstAcceleratedScreen);
>         ASSERT(error == kCGLNoError);
>-        LOG(WebGL, "Renderer (%d) not matched; Context (%p) set to virtual screen (%d).", preferredRendererID, contextObj, firstAcceleratedScreen);
>+        LOG(WebGL, "RegistryID (%lld) not matched; Context (%p) set to virtual screen (%d).", preferredRegistryID, contextObj, firstAcceleratedScreen);
>+    }
>+}
>+#else // __MAC_OS_X_VERSION_MIN_REQUIRED < 101300
>+static void setGPUByDisplayMask(PlatformGraphicsContext3D contextObj, CGLPixelFormatObj pixelFormatObj, uint32_t preferredDisplayMask)
>+{
>+    // A common case for multiple GPUs, external GPUs, is not supported before macOS 10.13.4.
>+    // In the rarer case where there are still multiple displays plugged into multiple GPUs, this should still work.
>+    // See code example at https://developer.apple.com/library/content/technotes/tn2229/_index.html#//apple_ref/doc/uid/DTS40008924-CH1-SUBSECTION7
>+    // FIXME: Window server is not blocked before 10.14. There might be a more straightforward way to detect the correct GPU.
>+
>+    if (!contextObj || !preferredDisplayMask)
>+        return;
>+
>+    GLint virtualScreenCount = 0;
>+    CGLError error = CGLDescribePixelFormat(pixelFormatObj, 0, kCGLPFAVirtualScreenCount, &virtualScreenCount);
>+    ASSERT(error == kCGLNoError);
>+
>+    for (GLint virtualScreen = 0; virtualScreen < virtualScreenCount; ++virtualScreen) {
>+        GLint displayMask = 0;
>+        error = CGLDescribePixelFormat(pixelFormatObj, virtualScreen, kCGLPFADisplayMask, &displayMask);
>+        ASSERT(error == kCGLNoError);
>+        if (error != kCGLNoError)
>+            continue;
>+
>+        if (displayMask & preferredDisplayMask) {
>+            error = CGLSetVirtualScreen(contextObj, virtualScreen);
>+            ASSERT(error == kCGLNoError);
>+            return;
>+        }
>     }
> }
> #endif
> 
>+#endif // !PLATFORM(MAC)
>+
> GraphicsContext3D::GraphicsContext3D(GraphicsContext3DAttributes attrs, HostWindow* hostWindow, GraphicsContext3D::RenderStyle, GraphicsContext3D* sharedContext)
>     : m_attrs(attrs)
> #if PLATFORM(IOS)
>@@ -278,12 +311,22 @@ GraphicsContext3D::GraphicsContext3D(GraphicsContext3DAttributes attrs, HostWind
>     CGLSetParameter(m_contextObj, kCGLCPAbortOnGPURestartStatusBlacklisted, &abortOnBlacklist);
>     
> #if PLATFORM(MAC)
>-    GLint rendererID = (hostWindow && hostWindow->displayID()) ? rendererIDForDisplay(hostWindow->displayID()) : primaryRendererID();
>-    identifyAndSetCurrentGPU(m_contextObj, pixelFormatObj, rendererID);
>+
>+#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101300
>+    auto registryID = (hostWindow && hostWindow->displayID()) ? registryIDForDisplay(hostWindow->displayID()) : primaryRegistryID();
>+    setGPUByRegistryID(m_contextObj, pixelFormatObj, registryID);
> #else
>-    UNUSED_PARAM(hostWindow);
>+    if (auto displayMask = primaryOpenGLDisplayMask()) {
>+        if (hostWindow && hostWindow->displayID())
>+            displayMask = displayMaskForDisplay(hostWindow->displayID());
>+        setGPUByDisplayMask(m_contextObj, pixelFormatObj, displayMask);
>+    }
> #endif
> 
>+#else
>+    UNUSED_PARAM(hostWindow);
>+#endif // !PLATFORM(MAC)
>+
>     CGLDestroyPixelFormat(pixelFormatObj);
>     
>     if (err != kCGLNoError || !m_contextObj) {
>@@ -603,10 +646,13 @@ void GraphicsContext3D::screenDidChange(PlatformDisplayID displayID)
> {
>     if (!m_contextObj)
>         return;
>-
>-    identifyAndSetCurrentGPU(m_contextObj, CGLGetPixelFormat(m_contextObj), rendererIDForDisplay(displayID));
>-}
>+#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101300
>+    setGPUByRegistryID(m_contextObj, CGLGetPixelFormat(m_contextObj), registryIDForDisplay(displayID));
>+#else
>+    setGPUByDisplayMask(m_contextObj, CGLGetPixelFormat(m_contextObj), displayMaskForDisplay(displayID));
> #endif
>+}
>+#endif // !PLATFORM(MAC)
> 
> }
> 
>diff --git a/Source/WebCore/platform/mac/PlatformScreenMac.mm b/Source/WebCore/platform/mac/PlatformScreenMac.mm
>index 147f26a21096245db73c0571c6234c2575f3dd16..745a6611c8ec929d630075168caa44fd48aad968 100644
>--- a/Source/WebCore/platform/mac/PlatformScreenMac.mm
>+++ b/Source/WebCore/platform/mac/PlatformScreenMac.mm
>@@ -107,26 +107,6 @@ static PlatformDisplayID& primaryScreenDisplayID()
>     return screenProperties().primaryDisplayID;
> }
> 
>-static GLint rendererIDForDisplayMask(GLuint displayMask)
>-{
>-    GLint numRenderers;
>-    CGLRendererInfoObj rendererInfo;
>-    CGLError error = CGLQueryRendererInfo(displayMask, &rendererInfo, &numRenderers);
>-    ASSERT(error == kCGLNoError);
>-
>-    GLint rendererID;
>-    error = CGLDescribeRenderer(rendererInfo, 0, kCGLRPRendererID, &rendererID);
>-    ASSERT(error == kCGLNoError);
>-
>-    // The 0th renderer should not be the software renderer.
>-    GLint isAccelerated;
>-    error = CGLDescribeRenderer(rendererInfo, 0, kCGLRPAccelerated, &isAccelerated);
>-    ASSERT(error == kCGLNoError);
>-    ASSERT(isAccelerated);
>-
>-    return rendererID;
>-}
>-
> ScreenProperties collectScreenProperties()
> {
>     ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
>@@ -147,9 +127,13 @@ ScreenProperties collectScreenProperties()
>         bool screenHasInvertedColors = CGDisplayUsesInvertedPolarity();
>         bool screenIsMonochrome = CGDisplayUsesForceToGray();
>         uint32_t displayMask = CGDisplayIDToOpenGLDisplayMask(displayID);
>-        GLint rendererID = rendererIDForDisplayMask(displayMask);
>+        PlatformRegistryID registryID = 0;
>+
>+#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101300
>+        registryID = registryIDForDisplayMask(displayMask);
>+#endif
> 
>-        screenProperties.screenDataMap.set(displayID, ScreenData { screenAvailableRect, screenRect, colorSpace, screenDepth, screenDepthPerComponent, screenSupportsExtendedColor, screenHasInvertedColors, screenIsMonochrome, displayMask, rendererID });
>+        screenProperties.screenDataMap.set(displayID, ScreenData { screenAvailableRect, screenRect, colorSpace, screenDepth, screenDepthPerComponent, screenSupportsExtendedColor, screenHasInvertedColors, screenIsMonochrome, displayMask, registryID });
> 
>         if (!screenProperties.primaryDisplayID)
>             screenProperties.primaryDisplayID = displayID;
>@@ -196,21 +180,47 @@ uint32_t displayMaskForDisplay(PlatformDisplayID displayID)
>     return 0;
> }
> 
>-GLint rendererIDForDisplay(PlatformDisplayID displayID)
>+#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101300
>+PlatformRegistryID primaryRegistryID()
>+{
>+    return registryIDForDisplay(screenProperties().primaryDisplayID);
>+}
>+
>+PlatformRegistryID registryIDForDisplay(PlatformDisplayID displayID)
> {
> #if ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
>     if (!screenProperties().screenDataMap.isEmpty())
>-        return screenData(displayID).rendererID;
>+        return screenData(displayID).registryID;
> #else
>-    return rendererIDForDisplayMask(CGDisplayIDToOpenGLDisplayMask(displayID));
>+    return registryIDForDisplayMask(CGDisplayIDToOpenGLDisplayMask(displayID));
> #endif
>     return 0;
> }
> 
>-GLint primaryRendererID()
>+PlatformRegistryID registryIDForDisplayMask(GLuint displayMask)
> {
>-    return rendererIDForDisplay(screenProperties().primaryDisplayID);
>+    GLint numRenderers;
>+    CGLRendererInfoObj rendererInfo;
>+    CGLError error = CGLQueryRendererInfo(displayMask, &rendererInfo, &numRenderers);
>+    ASSERT(error == kCGLNoError);
>+
>+    // The 0th renderer should not be the software renderer.
>+    GLint isAccelerated;
>+    error = CGLDescribeRenderer(rendererInfo, 0, kCGLRPAccelerated, &isAccelerated);
>+    ASSERT(error == kCGLNoError);
>+    ASSERT(isAccelerated);
>+
>+    GLint registryIDLow;
>+    GLint registryIDHigh;
>+
>+    error = CGLDescribeRenderer(rendererInfo, 0, kCGLRPRegistryIDLow, &registryIDLow);
>+    ASSERT(error == kCGLNoError);
>+    error = CGLDescribeRenderer(rendererInfo, 0, kCGLRPRegistryIDHigh, &registryIDHigh);
>+    ASSERT(error == kCGLNoError);
>+
>+    return (PlatformRegistryID) registryIDHigh << 32 | registryIDLow;
> }
>+#endif // !__MAC_OS_X_VERSION_MIN_REQUIRED >= 101300
> 
> static ScreenData getScreenProperties(Widget* widget)
> {

Actions: View | Formatted Diff | Diff

Attachments on bug 188072: 345861 | 345874 | 345879 | 345892 | 345933 | 345939 | 345958 | 345963 | 345982