WebKit Bugzilla
Attachment 345933 Details for
Bug 188072
: Match GraphicsContext3D with correct virtual screen using registryID
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-188072-20180727121646.patch (text/plain), 15.81 KB, created by
Justin Fan
on 2018-07-27 12:16:47 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Justin Fan
Created:
2018-07-27 12:16:47 PDT
Size:
15.81 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..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..b5b77ab9aea741f308836fc55426ba4e6f9d5569 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,44 @@ 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) >+{ >+ // 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 >+ >+ 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 +309,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 +644,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, ®istryIDLow); >+ ASSERT(error == kCGLNoError); >+ error = CGLDescribeRenderer(rendererInfo, 0, kCGLRPRegistryIDHigh, ®istryIDHigh); >+ ASSERT(error == kCGLNoError); >+ >+ return (PlatformRegistryID) registryIDHigh << 32 | registryIDLow; > } >+#endif // !__MAC_OS_X_VERSION_MIN_REQUIRED >= 101300 > > static ScreenData getScreenProperties(Widget* widget) > {
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 188072
:
345861
|
345874
|
345879
|
345892
|
345933
|
345939
|
345958
|
345963
|
345982