Attachment 345861 Details for Bug 188072 – Patch

[patch] Patch

bug-188072-20180726132638.patch (text/plain), 12.57 KB, created by Justin Fan on 2018-07-26 13:26:39 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Justin Fan

Created: 2018-07-26 13:26:39 PDT

Size: 12.57 KB

patch

obsolete

>Subversion Revision: 234224
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 81c20c0363bd63df5bfddeb1ac7eadfabea8a2e0..dad7290a9c4b00b32d8c96a82ebf026e1b7dd78c 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,35 @@
>+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::identifyAndSetCurrentGPU):
>+        (WebCore::GraphicsContext3D::GraphicsContext3D):
>+        (WebCore::GraphicsContext3D::screenDidChange):
>+        * 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..6dcbff369e1e612118e6928ba92fb6dbcdea658a 100644
>--- a/Source/WebCore/platform/PlatformScreen.h
>+++ b/Source/WebCore/platform/PlatformScreen.h
>@@ -100,8 +100,9 @@ WEBCORE_EXPORT void setScreenProperties(const ScreenProperties&);
> 
> uint32_t primaryOpenGLDisplayMask();
> uint32_t displayMaskForDisplay(PlatformDisplayID);
>-int32_t rendererIDForDisplay(PlatformDisplayID);
>-int32_t primaryRendererID();
>+int64_t primaryRegistryID();
>+int64_t registryIDForDisplay(PlatformDisplayID);
>+int64_t registryIDForDisplayMask(uint32_t);
> #endif
> 
> #if PLATFORM(IOS)
>diff --git a/Source/WebCore/platform/ScreenProperties.h b/Source/WebCore/platform/ScreenProperties.h
>index 6ae628abac783543df348d00ed93c1299f65af1d..0866b1ba4ee736e2ebc316050c03de2816ab933e 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 };
>+    int64_t 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<int64_t> 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..889f404cfb7a8421c9f7db4b3573586d217dfb76 100644
>--- a/Source/WebCore/platform/graphics/cocoa/GraphicsContext3DCocoa.mm
>+++ b/Source/WebCore/platform/graphics/cocoa/GraphicsContext3DCocoa.mm
>@@ -162,13 +162,13 @@ Ref<GraphicsContext3D> GraphicsContext3D::createShared(GraphicsContext3D& shared
> }
> 
> #if PLATFORM(MAC)
>-static void identifyAndSetCurrentGPU(PlatformGraphicsContext3D contextObj, CGLPixelFormatObj pixelFormatObj, GLint preferredRendererID)
>+static void identifyAndSetCurrentGPU(PlatformGraphicsContext3D contextObj, CGLPixelFormatObj pixelFormatObj, int64_t 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.
>     // 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 +178,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) {
>+        int64_t 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;
>         }
> 
>@@ -204,7 +204,7 @@ static void identifyAndSetCurrentGPU(PlatformGraphicsContext3D contextObj, CGLPi
>     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);
>     }
> }
> #endif
>@@ -278,8 +278,8 @@ 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);
>+    int64_t registryID = (hostWindow && hostWindow->displayID()) ? registryIDForDisplay(hostWindow->displayID()) : primaryRegistryID();
>+    identifyAndSetCurrentGPU(m_contextObj, pixelFormatObj, registryID);
> #else
>     UNUSED_PARAM(hostWindow);
> #endif
>@@ -604,7 +604,7 @@ void GraphicsContext3D::screenDidChange(PlatformDisplayID displayID)
>     if (!m_contextObj)
>         return;
> 
>-    identifyAndSetCurrentGPU(m_contextObj, CGLGetPixelFormat(m_contextObj), rendererIDForDisplay(displayID));
>+    identifyAndSetCurrentGPU(m_contextObj, CGLGetPixelFormat(m_contextObj), registryIDForDisplay(displayID));
> }
> #endif
> 
>diff --git a/Source/WebCore/platform/mac/PlatformScreenMac.mm b/Source/WebCore/platform/mac/PlatformScreenMac.mm
>index 147f26a21096245db73c0571c6234c2575f3dd16..90ee919802b42e80fc267e4d4066fc74b94a4279 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,9 @@ ScreenProperties collectScreenProperties()
>         bool screenHasInvertedColors = CGDisplayUsesInvertedPolarity();
>         bool screenIsMonochrome = CGDisplayUsesForceToGray();
>         uint32_t displayMask = CGDisplayIDToOpenGLDisplayMask(displayID);
>-        GLint rendererID = rendererIDForDisplayMask(displayMask);
>+        int64_t registryID = registryIDForDisplayMask(displayMask);
> 
>-        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,20 +176,44 @@ uint32_t displayMaskForDisplay(PlatformDisplayID displayID)
>     return 0;
> }
> 
>-GLint rendererIDForDisplay(PlatformDisplayID displayID)
>+int64_t primaryRegistryID()
>+{
>+    return registryIDForDisplay(screenProperties().primaryDisplayID);
>+}
>+
>+int64_t 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()
>+int64_t 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 (int64_t) registryIDHigh << 32 | registryIDLow;
> }
> 
> static ScreenData getScreenProperties(Widget* widget)

Actions: View | Formatted Diff | Diff

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