The layout test fast/canvas/webgl/read-pixels-test.html is timing out.
authorpvollan@apple.com <pvollan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Mar 2018 23:39:40 +0000 (23:39 +0000)
committerpvollan@apple.com <pvollan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Mar 2018 23:39:40 +0000 (23:39 +0000)
https://bugs.webkit.org/show_bug.cgi?id=183923
<rdar://problem/38756869>

Reviewed by Brent Fulgham.

Source/WebCore:

The test is timing out when we do not interact directly with the WindowServer, causing
OpenGL to fall back to software rendering. In this mode, any call to CGLChoosePixelFormat
requesting an accelerated pixel format will fail because it cannot determine which GPU is
connected to the display.

OpenGL treats all GPUs as if they were offline when used in a process (like the WebContent
process) that does not directly control the display.

We can get correct behavior if we tell OpenGL which GPU is currently connected to the
display, and if we instruct CGLChoosePixelFormat to create an offline renderer pixel format
by including the 'kCGLPFAAllowOfflineRenderers' flag in its arguments.

We can use CGLSetVirtualScreen with an OpenGL display mask that tells the OpenGL framework
which GPU it should use.

See https://developer.apple.com/library/content/technotes/tn2229/_index.html#//apple_ref/doc/uid/DTS40008924-CH1-SUBSECTION7
for details on how the virtual screen is found from the OpenGL display mask.

No new tests, covered by existing tests.

* WebCore.xcodeproj/project.pbxproj:
* platform/graphics/GraphicsContext3D.h:
* platform/graphics/cocoa/GraphicsContext3DCocoa.mm:
(WebCore::setPixelFormat):
(WebCore::identifyAndSetCurrentGPU):
(WebCore::GraphicsContext3D::GraphicsContext3D):
(WebCore::GraphicsContext3D::setOpenGLDisplayMask):
(WebCore::GraphicsContext3D::allowOfflineRenderers):

Source/WebKit:

Send OpenGL display mask to the WebContent process when the display ID is changing.

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::windowScreenDidChange):
* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/WebPage.messages.in:
* WebProcess/WebPage/mac/WebPageMac.mm:
(WebKit::WebPage::openGLDisplayMaskChanged):

Source/WTF:

Add compile guard for blocking of the WindowServer in the WebProcess.

* wtf/FeatureDefines.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@230015 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WTF/ChangeLog
Source/WTF/wtf/FeatureDefines.h
Source/WebCore/ChangeLog
Source/WebCore/WebCore.xcodeproj/project.pbxproj
Source/WebCore/platform/graphics/GraphicsContext3D.h
Source/WebCore/platform/graphics/cocoa/GraphicsContext3DCocoa.mm
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/WebProcess/WebPage/WebPage.h
Source/WebKit/WebProcess/WebPage/WebPage.messages.in
Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm

index fdd29ef..2f4c459 100644 (file)
@@ -1,3 +1,15 @@
+2018-03-27  Per Arne Vollan  <pvollan@apple.com>
+
+        The layout test fast/canvas/webgl/read-pixels-test.html is timing out.
+        https://bugs.webkit.org/show_bug.cgi?id=183923
+        <rdar://problem/38756869>
+
+        Reviewed by Brent Fulgham.
+
+        Add compile guard for blocking of the WindowServer in the WebProcess.
+
+        * wtf/FeatureDefines.h:
+
 2018-03-26  Tim Horton  <timothy_horton@apple.com>
 
         Add and adopt HAVE(CORE_ANIMATION_RENDER_SERVER)
index be4a9d7..b3976e7 100644 (file)
@@ -231,6 +231,10 @@ the public iOS SDK. See <https://webkit.org/b/179167>. */
 #define ENABLE_MAC_GESTURE_EVENTS 1
 #endif
 
+#if !defined(ENABLE_WEBPROCESS_WINDOWSERVER_BLOCKING)
+#define ENABLE_WEBPROCESS_WINDOWSERVER_BLOCKING 0
+#endif
+
 #endif /* PLATFORM(MAC) */
 
 #if PLATFORM(COCOA)
index da73211..87cdcfd 100644 (file)
@@ -1,3 +1,40 @@
+2018-03-27  Per Arne Vollan  <pvollan@apple.com>
+
+        The layout test fast/canvas/webgl/read-pixels-test.html is timing out.
+        https://bugs.webkit.org/show_bug.cgi?id=183923
+        <rdar://problem/38756869>
+
+        Reviewed by Brent Fulgham.
+
+        The test is timing out when we do not interact directly with the WindowServer, causing
+        OpenGL to fall back to software rendering. In this mode, any call to CGLChoosePixelFormat
+        requesting an accelerated pixel format will fail because it cannot determine which GPU is
+        connected to the display.
+
+        OpenGL treats all GPUs as if they were offline when used in a process (like the WebContent
+        process) that does not directly control the display.
+
+        We can get correct behavior if we tell OpenGL which GPU is currently connected to the
+        display, and if we instruct CGLChoosePixelFormat to create an offline renderer pixel format
+        by including the 'kCGLPFAAllowOfflineRenderers' flag in its arguments.
+
+        We can use CGLSetVirtualScreen with an OpenGL display mask that tells the OpenGL framework
+        which GPU it should use.
+
+        See https://developer.apple.com/library/content/technotes/tn2229/_index.html#//apple_ref/doc/uid/DTS40008924-CH1-SUBSECTION7
+        for details on how the virtual screen is found from the OpenGL display mask.
+
+        No new tests, covered by existing tests.
+
+        * WebCore.xcodeproj/project.pbxproj:
+        * platform/graphics/GraphicsContext3D.h:
+        * platform/graphics/cocoa/GraphicsContext3DCocoa.mm:
+        (WebCore::setPixelFormat):
+        (WebCore::identifyAndSetCurrentGPU):
+        (WebCore::GraphicsContext3D::GraphicsContext3D):
+        (WebCore::GraphicsContext3D::setOpenGLDisplayMask):
+        (WebCore::GraphicsContext3D::allowOfflineRenderers):
+
 2018-03-27  Jiewen Tan  <jiewen_tan@apple.com>
 
         [WebAuthN] Implement authenticatorGetAssertion
index 36fb81d..f899f09 100644 (file)
                46EFAF121E5FB9F100E7F34B /* LowPowerModeNotifier.h in Headers */ = {isa = PBXBuildFile; fileRef = 46EFAF101E5FB9E100E7F34B /* LowPowerModeNotifier.h */; settings = {ATTRIBUTES = (Private, ); }; };
                46FCB6181A70820E00C5A21E /* DiagnosticLoggingKeys.h in Headers */ = {isa = PBXBuildFile; fileRef = CD37B37515C1A7E1006DC898 /* DiagnosticLoggingKeys.h */; settings = {ATTRIBUTES = (Private, ); }; };
                490707E61219C04300D90E51 /* ANGLEWebKitBridge.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 490707E41219C04300D90E51 /* ANGLEWebKitBridge.cpp */; };
-               490707E71219C04300D90E51 /* ANGLEWebKitBridge.h in Headers */ = {isa = PBXBuildFile; fileRef = 490707E51219C04300D90E51 /* ANGLEWebKitBridge.h */; };
+               490707E71219C04300D90E51 /* ANGLEWebKitBridge.h in Headers */ = {isa = PBXBuildFile; fileRef = 490707E51219C04300D90E51 /* ANGLEWebKitBridge.h */; settings = {ATTRIBUTES = (Private, ); }; };
                49291E4B134172C800E753DE /* ImageRenderingMode.h in Headers */ = {isa = PBXBuildFile; fileRef = 49291E4A134172C800E753DE /* ImageRenderingMode.h */; };
                493E5E0912D6420500020081 /* PlatformCALayerClient.h in Headers */ = {isa = PBXBuildFile; fileRef = 493E5E0812D6420500020081 /* PlatformCALayerClient.h */; settings = {ATTRIBUTES = (Private, ); }; };
                4945BFD413CF809000CC3B38 /* TransformState.h in Headers */ = {isa = PBXBuildFile; fileRef = 4945BFD213CF809000CC3B38 /* TransformState.h */; };
                49C7B9E31042D32F0009D447 /* WebGLShader.h in Headers */ = {isa = PBXBuildFile; fileRef = 49C7B9C31042D32F0009D447 /* WebGLShader.h */; };
                49C7B9E51042D32F0009D447 /* WebGLTexture.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 49C7B9C51042D32F0009D447 /* WebGLTexture.cpp */; };
                49C7B9E61042D32F0009D447 /* WebGLTexture.h in Headers */ = {isa = PBXBuildFile; fileRef = 49C7B9C61042D32F0009D447 /* WebGLTexture.h */; };
-               49C7B9FC1042D3650009D447 /* GraphicsContext3D.h in Headers */ = {isa = PBXBuildFile; fileRef = 49C7B9FB1042D3650009D447 /* GraphicsContext3D.h */; };
+               49C7B9FC1042D3650009D447 /* GraphicsContext3D.h in Headers */ = {isa = PBXBuildFile; fileRef = 49C7B9FB1042D3650009D447 /* GraphicsContext3D.h */; settings = {ATTRIBUTES = (Private, ); }; };
                49D5DC2C0F423A73008F20FD /* Matrix3DTransformOperation.h in Headers */ = {isa = PBXBuildFile; fileRef = 49D5DC280F423A73008F20FD /* Matrix3DTransformOperation.h */; settings = {ATTRIBUTES = (Private, ); }; };
                49D5DC2E0F423A73008F20FD /* PerspectiveTransformOperation.h in Headers */ = {isa = PBXBuildFile; fileRef = 49D5DC2A0F423A73008F20FD /* PerspectiveTransformOperation.h */; settings = {ATTRIBUTES = (Private, ); }; };
                49E911C40EF86D47009D0CAF /* TransformationMatrix.h in Headers */ = {isa = PBXBuildFile; fileRef = 49E911B40EF86D47009D0CAF /* TransformationMatrix.h */; settings = {ATTRIBUTES = (Private, ); }; };
index 8944c77..bd51d0e 100644 (file)
@@ -1282,6 +1282,10 @@ public:
     GC3Denum currentBoundTarget() const { return m_state.currentBoundTarget(); }
     unsigned textureSeed(GC3Duint texture) { return m_state.textureSeedCount.count(texture); }
 
+#if PLATFORM(MAC) && ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
+    WEBCORE_EXPORT static void setOpenGLDisplayMask(CGOpenGLDisplayMask);
+#endif
+
 private:
     GraphicsContext3D(GraphicsContext3DAttributes, HostWindow*, RenderStyle = RenderOffscreen, GraphicsContext3D* sharedContext = nullptr);
 
@@ -1315,6 +1319,10 @@ private:
     void resolveMultisamplingIfNecessary(const IntRect& = IntRect());
     void attachDepthAndStencilBufferIfNeeded(GLuint internalDepthStencilFormat, int width, int height);
 
+#if PLATFORM(COCOA)
+    bool allowOfflineRenderers() const;
+#endif
+
     int m_currentWidth { 0 };
     int m_currentHeight { 0 };
 
@@ -1493,6 +1501,9 @@ private:
     Platform3DObject m_vao { 0 };
 #endif
 
+#if PLATFORM(MAC) && ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
+    static std::optional<CGOpenGLDisplayMask> m_displayMask;
+#endif
 };
 
 } // namespace WebCore
index 58ad85f..e76425e 100644 (file)
@@ -64,6 +64,10 @@ namespace WebCore {
 
 static const unsigned statusCheckThreshold = 5;
 
+#if PLATFORM(MAC) && ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
+std::optional<CGOpenGLDisplayMask> GraphicsContext3D::m_displayMask;
+#endif
+
 #if HAVE(APPLE_GRAPHICS_CONTROL)
 
 enum {
@@ -316,7 +320,7 @@ public:
 
 #if USE(OPENGL)
 
-static void setPixelFormat(Vector<CGLPixelFormatAttribute>& attribs, int colorBits, int depthBits, bool accelerated, bool supersample, bool closest, bool antialias, bool useGLES3)
+static void setPixelFormat(Vector<CGLPixelFormatAttribute>& attribs, int colorBits, int depthBits, bool accelerated, bool supersample, bool closest, bool antialias, bool useGLES3, bool allowOfflineRenderers)
 {
     attribs.clear();
     
@@ -329,10 +333,8 @@ static void setPixelFormat(Vector<CGLPixelFormatAttribute>& attribs, int colorBi
     // allowing us to request the integrated graphics on a dual GPU
     // system, and not force the discrete GPU.
     // See https://developer.apple.com/library/mac/technotes/tn2229/_index.html
-#if HAVE(APPLE_GRAPHICS_CONTROL)
-    if (hasMuxableGPU())
+    if (allowOfflineRenderers)
         attribs.append(kCGLPFAAllowOfflineRenderers);
-#endif
 
     if (accelerated)
         attribs.append(kCGLPFAAccelerated);
@@ -397,6 +399,31 @@ Ref<GraphicsContext3D> GraphicsContext3D::createShared(GraphicsContext3D& shared
     return context;
 }
 
+#if PLATFORM(MAC) && ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
+static void identifyAndSetCurrentGPU(CGLPixelFormatObj pixelFormatObj, int numPixelFormats, CGOpenGLDisplayMask displayMaskOpenGL, PlatformGraphicsContext3D contextObj)
+{
+    // 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.
+    // CGLSetVirtualScreen can be used to tell OpenGL which GPU it should be using.
+    // See code example at https://developer.apple.com/library/content/technotes/tn2229/_index.html#//apple_ref/doc/uid/DTS40008924-CH1-SUBSECTION7
+    
+    if (!displayMaskOpenGL || !contextObj)
+        return;
+
+    for (int virtualScreen = 0; virtualScreen < numPixelFormats; ++virtualScreen) {
+        GLint displayMask = 0;
+        CGLError error = CGLDescribePixelFormat(pixelFormatObj, virtualScreen, kCGLPFADisplayMask, &displayMask);
+        ASSERT(error == kCGLNoError);
+        if (error != kCGLNoError)
+            continue;
+        if (displayMask & displayMaskOpenGL) {
+            error = CGLSetVirtualScreen(contextObj, virtualScreen);
+            ASSERT(error == kCGLNoError);
+            break;
+        }
+    }
+}
+#endif
+
 GraphicsContext3D::GraphicsContext3D(GraphicsContext3DAttributes attrs, HostWindow*, GraphicsContext3D::RenderStyle, GraphicsContext3D* sharedContext)
     : m_attrs(attrs)
 #if PLATFORM(IOS)
@@ -438,19 +465,19 @@ GraphicsContext3D::GraphicsContext3D(GraphicsContext3DAttributes attrs, HostWind
     m_powerPreferenceUsedForCreation = GraphicsContext3DPowerPreference::Default;
 #endif
 
-    setPixelFormat(attribs, 32, 32, !attrs.forceSoftwareRenderer, true, false, useMultisampling, attrs.useGLES3);
+    setPixelFormat(attribs, 32, 32, !attrs.forceSoftwareRenderer, true, false, useMultisampling, attrs.useGLES3, allowOfflineRenderers());
     CGLChoosePixelFormat(attribs.data(), &pixelFormatObj, &numPixelFormats);
 
     if (!numPixelFormats) {
-        setPixelFormat(attribs, 32, 32, !attrs.forceSoftwareRenderer, false, false, useMultisampling, attrs.useGLES3);
+        setPixelFormat(attribs, 32, 32, !attrs.forceSoftwareRenderer, false, false, useMultisampling, attrs.useGLES3, allowOfflineRenderers());
         CGLChoosePixelFormat(attribs.data(), &pixelFormatObj, &numPixelFormats);
 
         if (!numPixelFormats) {
-            setPixelFormat(attribs, 32, 16, !attrs.forceSoftwareRenderer, false, false, useMultisampling, attrs.useGLES3);
+            setPixelFormat(attribs, 32, 16, !attrs.forceSoftwareRenderer, false, false, useMultisampling, attrs.useGLES3, allowOfflineRenderers());
             CGLChoosePixelFormat(attribs.data(), &pixelFormatObj, &numPixelFormats);
 
             if (!attrs.forceSoftwareRenderer && !numPixelFormats) {
-                setPixelFormat(attribs, 32, 16, false, false, true, false, attrs.useGLES3);
+                setPixelFormat(attribs, 32, 16, false, false, true, false, attrs.useGLES3, allowOfflineRenderers());
                 CGLChoosePixelFormat(attribs.data(), &pixelFormatObj, &numPixelFormats);
                 useMultisampling = false;
             }
@@ -463,6 +490,12 @@ GraphicsContext3D::GraphicsContext3D(GraphicsContext3DAttributes attrs, HostWind
     CGLError err = CGLCreateContext(pixelFormatObj, sharedContext ? sharedContext->m_contextObj : nullptr, &m_contextObj);
     GLint abortOnBlacklist = 0;
     CGLSetParameter(m_contextObj, kCGLCPAbortOnGPURestartStatusBlacklisted, &abortOnBlacklist);
+    
+#if PLATFORM(MAC) && ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
+    if (m_displayMask.has_value())
+        identifyAndSetCurrentGPU(pixelFormatObj, numPixelFormats, m_displayMask.value(), m_contextObj);
+#endif
+
     CGLDestroyPixelFormat(pixelFormatObj);
     
     if (err != kCGLNoError || !m_contextObj) {
@@ -756,6 +789,34 @@ void GraphicsContext3D::simulateContextChanged()
     manager().updateAllContexts();
 }
 
+#if PLATFORM(MAC) && ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
+void GraphicsContext3D::setOpenGLDisplayMask(CGOpenGLDisplayMask displayMask)
+{
+    m_displayMask = displayMask;
+}
+#endif
+
+bool GraphicsContext3D::allowOfflineRenderers() const
+{
+#if PLATFORM(MAC) && ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
+    // When WindowServer access is blocked in the WebProcess, there is no way
+    // for OpenGL to decide which GPU is connected to a display (online/offline).
+    // OpenGL will then consider all GPUs, or renderers, as offline, which means
+    // all offline renderers need to be considered when finding a pixel format.
+    // In WebKit legacy, there will still be a WindowServer connection, and
+    // m_displayMask will not be set in this case.
+    if (m_displayMask.has_value())
+        return true;
+#endif
+        
+#if HAVE(APPLE_GRAPHICS_CONTROL)
+    if (hasMuxableGPU())
+        return true;
+#endif
+    
+    return false;
+}
+
 }
 
 #endif // ENABLE(GRAPHICS_CONTEXT_3D)
index 5fc9349..d5bf094 100644 (file)
@@ -1,3 +1,20 @@
+2018-03-27  Per Arne Vollan  <pvollan@apple.com>
+
+        The layout test fast/canvas/webgl/read-pixels-test.html is timing out.
+        https://bugs.webkit.org/show_bug.cgi?id=183923
+        <rdar://problem/38756869>
+
+        Reviewed by Brent Fulgham.
+
+        Send OpenGL display mask to the WebContent process when the display ID is changing.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::windowScreenDidChange):
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/WebPage.messages.in:
+        * WebProcess/WebPage/mac/WebPageMac.mm:
+        (WebKit::WebPage::openGLDisplayMaskChanged):
+
 2018-03-27  Youenn Fablet  <youenn@apple.com>
 
         Move request checking out of PingLoad for future reuse in NetworkLoad
index 18a10e7..169f654 100644 (file)
@@ -2646,6 +2646,11 @@ void WebPageProxy::windowScreenDidChange(PlatformDisplayID displayID)
         return;
 
     m_process->send(Messages::WebPage::WindowScreenDidChange(displayID), m_pageID);
+
+#if PLATFORM(MAC) && ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
+    auto currentDisplaymask = CGDisplayIDToOpenGLDisplayMask(displayID);
+    m_process->send(Messages::WebPage::OpenGLDisplayMaskChanged(currentDisplaymask), m_pageID);
+#endif
 }
 
 float WebPageProxy::deviceScaleFactor() const
index 0744a2f..2251e71 100644 (file)
@@ -1064,6 +1064,10 @@ public:
     void didFinishLoadingApplicationManifest(uint64_t, const std::optional<WebCore::ApplicationManifest>&);
 #endif
 
+#if PLATFORM(MAC) && ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
+    void openGLDisplayMaskChanged(uint32_t displayMask);
+#endif
+
 private:
     WebPage(uint64_t pageID, WebPageCreationParameters&&);
 
index 5a7aaf4..161e0fd 100644 (file)
@@ -508,4 +508,8 @@ messages -> WebPage LegacyReceiver {
 #if ENABLE(APPLICATION_MANIFEST)
     GetApplicationManifest(WebKit::CallbackID callbackID)
 #endif
+
+#if PLATFORM(MAC) && ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
+    OpenGLDisplayMaskChanged(uint32_t displayMask)
+#endif
 }
index 3ddbd73..b1a77e0 100644 (file)
@@ -66,6 +66,7 @@
 #import <WebCore/FrameLoader.h>
 #import <WebCore/FrameView.h>
 #import <WebCore/GraphicsContext.h>
+#import <WebCore/GraphicsContext3D.h>
 #import <WebCore/HTMLConverter.h>
 #import <WebCore/HTMLPlugInImageElement.h>
 #import <WebCore/HitTestResult.h>
@@ -1190,6 +1191,12 @@ void WebPage::setShouldPlayToPlaybackTarget(uint64_t contextId, bool shouldPlay)
 }
 #endif
 
+#if ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
+void WebPage::openGLDisplayMaskChanged(uint32_t displayMask)
+{
+    GraphicsContext3D::setOpenGLDisplayMask(displayMask);
+}
+#endif
 
 } // namespace WebKit