Title: [221872] trunk/Source/WebCore
Revision
221872
Author
[email protected]
Date
2017-09-11 12:39:46 -0700 (Mon, 11 Sep 2017)

Log Message

[WebGL macOS] No need to multisample when blitting into WebGLLayer
https://bugs.webkit.org/show_bug.cgi?id=176666
<rdar://problem/27774626>

Reviewed by Sam Weinig.

We were seeing performance profiles suggesting WebGL was
doing 8x MSAA, even though we explicitly set it to only
use 4 samples in the GLPixelFormatObj used to create
the WebGL CGLContextObj. However, that same CGLPixelFormatObj
was also used for the WebGLLayer's CGLContextObj, meaning the
blit of the WebGL FBO into the WebGLLayer's backing store was
multisampling as well -- so an extra 4 samples on top of the
original 4, making it look like we were doing 8x.

This was obviously unnecessary, since we already have the
multisampled FBO and just want to copy it, as is, into the layer.

Now, instead of copying the CGLPixelFormatObj, we create
a new one and copy most of the attributes, leaving out
the multisample flags (and the depth buffer, since we're
only doing 2d blits).

Covered by existing WebGL tests, since there should be no
visible change.

* platform/graphics/cocoa/WebGLLayer.mm:
(-[WebGLLayer copyCGLPixelFormatForDisplayMask:]): Create a new
CGLPixelFormatObj that copies most of the values from
the corresponding object on the WebGL's backing CGLContextObj.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (221871 => 221872)


--- trunk/Source/WebCore/ChangeLog	2017-09-11 19:26:00 UTC (rev 221871)
+++ trunk/Source/WebCore/ChangeLog	2017-09-11 19:39:46 UTC (rev 221872)
@@ -1,3 +1,36 @@
+2017-09-11  Dean Jackson  <[email protected]>
+
+        [WebGL macOS] No need to multisample when blitting into WebGLLayer
+        https://bugs.webkit.org/show_bug.cgi?id=176666
+        <rdar://problem/27774626>
+
+        Reviewed by Sam Weinig.
+
+        We were seeing performance profiles suggesting WebGL was
+        doing 8x MSAA, even though we explicitly set it to only
+        use 4 samples in the GLPixelFormatObj used to create
+        the WebGL CGLContextObj. However, that same CGLPixelFormatObj
+        was also used for the WebGLLayer's CGLContextObj, meaning the
+        blit of the WebGL FBO into the WebGLLayer's backing store was
+        multisampling as well -- so an extra 4 samples on top of the
+        original 4, making it look like we were doing 8x.
+
+        This was obviously unnecessary, since we already have the
+        multisampled FBO and just want to copy it, as is, into the layer.
+
+        Now, instead of copying the CGLPixelFormatObj, we create
+        a new one and copy most of the attributes, leaving out
+        the multisample flags (and the depth buffer, since we're
+        only doing 2d blits).
+
+        Covered by existing WebGL tests, since there should be no
+        visible change.
+
+        * platform/graphics/cocoa/WebGLLayer.mm:
+        (-[WebGLLayer copyCGLPixelFormatForDisplayMask:]): Create a new
+        CGLPixelFormatObj that copies most of the values from
+        the corresponding object on the WebGL's backing CGLContextObj.
+
 2017-09-11  Zan Dobersek  <[email protected]>
 
         [EME] ClearKey: implement CDMInstanceClearKey state modifiers, callback dispatches

Modified: trunk/Source/WebCore/platform/graphics/cocoa/WebGLLayer.mm (221871 => 221872)


--- trunk/Source/WebCore/platform/graphics/cocoa/WebGLLayer.mm	2017-09-11 19:26:00 UTC (rev 221871)
+++ trunk/Source/WebCore/platform/graphics/cocoa/WebGLLayer.mm	2017-09-11 19:39:46 UTC (rev 221872)
@@ -62,14 +62,48 @@
 #if !PLATFORM(IOS)
 -(CGLPixelFormatObj)copyCGLPixelFormatForDisplayMask:(uint32_t)mask
 {
-    // FIXME: The mask param tells you which display (on a multi-display system)
-    // is to be used. But since we are now getting the pixel format from the
-    // Canvas CGL context, we don't use it. This seems to do the right thing on
-    // one multi-display system. But there may be cases where this is not the case.
-    // If needed we will have to set the display mask in the Canvas CGLContext and
-    // make sure it matches.
+    // We're basically copying the pixel format object from the existing
+    // WebGL context, so we don't need to use the display mask.
     UNUSED_PARAM(mask);
-    return CGLRetainPixelFormat(CGLGetPixelFormat(_context->platformGraphicsContext3D()));
+
+    CGLPixelFormatObj webglPixelFormat = CGLGetPixelFormat(_context->platformGraphicsContext3D());
+
+    Vector<CGLPixelFormatAttribute> attribs;
+    GLint value;
+
+    CGLDescribePixelFormat(webglPixelFormat, 0, kCGLPFAColorSize, &value);
+    attribs.append(kCGLPFAColorSize);
+    attribs.append(static_cast<CGLPixelFormatAttribute>(value));
+
+    // We don't need to specify a depth size since we're only
+    // using this context as a 2d blit destination for the WebGL FBO.
+    attribs.append(kCGLPFADepthSize);
+    attribs.append(static_cast<CGLPixelFormatAttribute>(0));
+
+    CGLDescribePixelFormat(webglPixelFormat, 0, kCGLPFAAllowOfflineRenderers, &value);
+    if (value)
+        attribs.append(kCGLPFAAllowOfflineRenderers);
+
+    CGLDescribePixelFormat(webglPixelFormat, 0, kCGLPFAAccelerated, &value);
+    if (value)
+        attribs.append(kCGLPFAAccelerated);
+
+    CGLDescribePixelFormat(webglPixelFormat, 0, kCGLPFAOpenGLProfile, &value);
+    if (value) {
+        attribs.append(kCGLPFAOpenGLProfile);
+        attribs.append(static_cast<CGLPixelFormatAttribute>(value));
+    }
+
+    attribs.append(static_cast<CGLPixelFormatAttribute>(0));
+
+    CGLPixelFormatObj pixelFormat;
+    GLint numPixelFormats = 0;
+    CGLChoosePixelFormat(attribs.data(), &pixelFormat, &numPixelFormats);
+
+    ASSERT(pixelFormat);
+    ASSERT(numPixelFormats);
+
+    return pixelFormat;
 }
 
 -(CGLContextObj)copyCGLContextForPixelFormat:(CGLPixelFormatObj)pixelFormat
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to