Skip to content
Permalink
Browse files
Lots of missing frames in YouTube360 when fullscreen on MacBook
https://bugs.webkit.org/show_bug.cgi?id=177903
<rdar://problem/33273300>

Reviewed by Sam Weinig.

Source/WebCore:

Our compositing path for WebGL on macOS was too slow, requiring a copy
of the framebuffer into another GL context. Replace this by having
WebGL render into a texture that is backed by an IOSurface, and then
set the WebGLLayer to use the IOSurface as contents.

Covered by the existing WebGL tests.

* platform/graphics/GraphicsContext3D.h:
(WebCore::GraphicsContext3D::platformTexture const): We no longer use the
framebuffer object outside the class, so change this to return the GL texture
that the framebuffer is rendering in to. It was kind-of strange that it was
named this way originally.
Also make endPaint available on macOS, and add the definitions for
createIOSurfaceBackingStore and updateFramebufferTextureBackingStoreFromLayer.

* platform/graphics/cocoa/GraphicsContext3DCocoa.mm:
(WebCore::GraphicsContext3D::GraphicsContext3D): Now that we're using an IOSurface,
we're binding to a new attachment point, GL_TEXTURE_RECTANGLE.
(WebCore::GraphicsContext3D::endPaint): This is now being called on macOS and iOS,
so add a comment that explains the extra work that iOS needs to do. At some future
point it would be nice to make this slightly cleaner, so that iOS and macOS are
more similar.
(WebCore::GraphicsContext3D::allocateIOSurfaceBackingStore): New function that calls
into the corresponding WebGLLayer function.
(WebCore::GraphicsContext3D::updateFramebufferTextureBackingStoreFromLayer): Ditto.

* platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:
(WebCore::wipeAlphaChannelFromPixels): Both readPixels and drawing a WebGL context
into another buffer need to fill out the alpha channel if this context was
created without one, otherwise the IOSurface backing store will happily provide
what might be non-zero values.
(WebCore::GraphicsContext3D::readPixelsAndConvertToBGRAIfNecessary): Call the helper above.
(WebCore::GraphicsContext3D::reshapeFBOs): Add more code to call into the macOS-specific
function to use an IOSurface as the framebuffer texture.
(WebCore::GraphicsContext3D::readPixels): Call the helper above.

* platform/graphics/ca/cocoa/PlatformCALayerCocoa.mm:
(PlatformCALayerCocoa::copyContentsFromLayer): Replace the use of the
deprecated setContentsChanged with reloadValueForKeyPath.

* platform/graphics/cocoa/WebGLLayer.h: The macOS implementation now
inherits from CALayer directly rather than CAOpenGLLayer. It also adds
a few member variables to handle the IOSurfaces used for triple buffering.

* platform/graphics/cocoa/WebGLLayer.mm:
(-[WebGLLayer initWithGraphicsContext3D:]): If we were created without an
alpha channel, tell CA that we're an opaque layer. Also set the layer's transform
to identity, so that it calls into the code below to flip the contents.
(-[WebGLLayer setTransform:]): Because an IOSurface is used for the layer contents,
we don't get a chance to flip the drawing the way we do via the drawInContext delegate.
Instead we have to apply a scale(1, -1) transform on top of the layer transform to
make sure the layer is rendered right-way up.
(-[WebGLLayer setAnchorPoint:]): Ditto, except we have to assume the anchor point is
at the bottom of the layer, so flip the Y value.
(-[WebGLLayer display]): Swap between the drawing buffer and the contents buffer, and
then get a new buffer ready for display.
(createAppropriateIOSurface): Helper.
(-[WebGLLayer allocateIOSurfaceBackingStoreWithSize:usingAlpha:]): Initializes the
IOSurfaces used for drawing buffers.
(-[WebGLLayer bindFramebufferToNextAvailableSurface]): Take the next available IOSurface and
make it the drawing buffer (binding in to WebGL at the same time).
(-[WebGLLayer copyCGLPixelFormatForDisplayMask:]): Deleted.
(-[WebGLLayer copyCGLContextForPixelFormat:]): Deleted.
(-[WebGLLayer drawInCGLContext:pixelFormat:forLayerTime:displayTime:]): Deleted.

* platform/graphics/mac/WebLayer.mm: Remove the definition of reloadValueForKeyPath.

Source/WebCore/PAL:

Add reloadValueForKeyPath to replace setContentsChanged on CALayer.

* pal/spi/cocoa/QuartzCoreSPI.h:

Canonical link: https://commits.webkit.org/194233@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@222961 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
grorg committed Oct 6, 2017
1 parent e8faeff commit 56835752f27454558546eaf02ba1f614cbd75f28
@@ -1513,3 +1513,5 @@ imported/w3c/web-platform-tests/css/css-pseudo-4/first-letter-002.html [ ImageOn
imported/w3c/web-platform-tests/css/css-pseudo-4/first-letter-003.html [ ImageOnlyFailure ]

webkit.org/b/177799 accessibility/table-detection.html [ Pass Failure ]

webkit.org/b/177997 webgl/1.0.2/conformance/textures/copy-tex-image-2d-formats.html [ Pass Failure ]
@@ -1,3 +1,78 @@
2017-10-05 Dean Jackson <dino@apple.com>

Lots of missing frames in YouTube360 when fullscreen on MacBook
https://bugs.webkit.org/show_bug.cgi?id=177903
<rdar://problem/33273300>

Reviewed by Sam Weinig.

Our compositing path for WebGL on macOS was too slow, requiring a copy
of the framebuffer into another GL context. Replace this by having
WebGL render into a texture that is backed by an IOSurface, and then
set the WebGLLayer to use the IOSurface as contents.

Covered by the existing WebGL tests.

* platform/graphics/GraphicsContext3D.h:
(WebCore::GraphicsContext3D::platformTexture const): We no longer use the
framebuffer object outside the class, so change this to return the GL texture
that the framebuffer is rendering in to. It was kind-of strange that it was
named this way originally.
Also make endPaint available on macOS, and add the definitions for
createIOSurfaceBackingStore and updateFramebufferTextureBackingStoreFromLayer.

* platform/graphics/cocoa/GraphicsContext3DCocoa.mm:
(WebCore::GraphicsContext3D::GraphicsContext3D): Now that we're using an IOSurface,
we're binding to a new attachment point, GL_TEXTURE_RECTANGLE.
(WebCore::GraphicsContext3D::endPaint): This is now being called on macOS and iOS,
so add a comment that explains the extra work that iOS needs to do. At some future
point it would be nice to make this slightly cleaner, so that iOS and macOS are
more similar.
(WebCore::GraphicsContext3D::allocateIOSurfaceBackingStore): New function that calls
into the corresponding WebGLLayer function.
(WebCore::GraphicsContext3D::updateFramebufferTextureBackingStoreFromLayer): Ditto.

* platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:
(WebCore::wipeAlphaChannelFromPixels): Both readPixels and drawing a WebGL context
into another buffer need to fill out the alpha channel if this context was
created without one, otherwise the IOSurface backing store will happily provide
what might be non-zero values.
(WebCore::GraphicsContext3D::readPixelsAndConvertToBGRAIfNecessary): Call the helper above.
(WebCore::GraphicsContext3D::reshapeFBOs): Add more code to call into the macOS-specific
function to use an IOSurface as the framebuffer texture.
(WebCore::GraphicsContext3D::readPixels): Call the helper above.

* platform/graphics/ca/cocoa/PlatformCALayerCocoa.mm:
(PlatformCALayerCocoa::copyContentsFromLayer): Replace the use of the
deprecated setContentsChanged with reloadValueForKeyPath.

* platform/graphics/cocoa/WebGLLayer.h: The macOS implementation now
inherits from CALayer directly rather than CAOpenGLLayer. It also adds
a few member variables to handle the IOSurfaces used for triple buffering.

* platform/graphics/cocoa/WebGLLayer.mm:
(-[WebGLLayer initWithGraphicsContext3D:]): If we were created without an
alpha channel, tell CA that we're an opaque layer. Also set the layer's transform
to identity, so that it calls into the code below to flip the contents.
(-[WebGLLayer setTransform:]): Because an IOSurface is used for the layer contents,
we don't get a chance to flip the drawing the way we do via the drawInContext delegate.
Instead we have to apply a scale(1, -1) transform on top of the layer transform to
make sure the layer is rendered right-way up.
(-[WebGLLayer setAnchorPoint:]): Ditto, except we have to assume the anchor point is
at the bottom of the layer, so flip the Y value.
(-[WebGLLayer display]): Swap between the drawing buffer and the contents buffer, and
then get a new buffer ready for display.
(createAppropriateIOSurface): Helper.
(-[WebGLLayer allocateIOSurfaceBackingStoreWithSize:usingAlpha:]): Initializes the
IOSurfaces used for drawing buffers.
(-[WebGLLayer bindFramebufferToNextAvailableSurface]): Take the next available IOSurface and
make it the drawing buffer (binding in to WebGL at the same time).
(-[WebGLLayer copyCGLPixelFormatForDisplayMask:]): Deleted.
(-[WebGLLayer copyCGLContextForPixelFormat:]): Deleted.
(-[WebGLLayer drawInCGLContext:pixelFormat:forLayerTime:displayTime:]): Deleted.

* platform/graphics/mac/WebLayer.mm: Remove the definition of reloadValueForKeyPath.

2017-10-05 Frederic Wang <fwang@igalia.com>

Update Source/ThirdParty/woff2 to 22c256bc457777744ba14b7325a6e8e0e7dec91c
@@ -1,3 +1,15 @@
2017-10-05 Dean Jackson <dino@apple.com>

Lots of missing frames in YouTube360 when fullscreen on MacBook
https://bugs.webkit.org/show_bug.cgi?id=177903
<rdar://problem/33273300>

Reviewed by Sam Weinig.

Add reloadValueForKeyPath to replace setContentsChanged on CALayer.

* pal/spi/cocoa/QuartzCoreSPI.h:

2017-10-05 Commit Queue <commit-queue@webkit.org>

Unreviewed, rolling out r222951 and r222952.
@@ -91,7 +91,7 @@ typedef struct _CARenderContext CARenderContext;
- (void)setContextId:(uint32_t)contextID;
- (CGSize)size;
- (void *)regionBeingDrawn;
- (void)setContentsChanged;
- (void)reloadValueForKeyPath:(NSString *)keyPath;
@property BOOL allowsGroupBlending;
@property BOOL canDrawConcurrently;
@property BOOL contentsOpaque;
@@ -747,7 +747,7 @@ class GraphicsContext3D : public RefCounted<GraphicsContext3D> {

#if PLATFORM(COCOA)
PlatformGraphicsContext3D platformGraphicsContext3D() const { return m_contextObj; }
Platform3DObject platformTexture() const { return m_fbo; }
Platform3DObject platformTexture() const { return m_texture; }
CALayer* platformLayer() const { return reinterpret_cast<CALayer*>(m_webGLLayer.get()); }
#else
PlatformGraphicsContext3D platformGraphicsContext3D();
@@ -1147,12 +1147,16 @@ class GraphicsContext3D : public RefCounted<GraphicsContext3D> {
RefPtr<ImageData> paintRenderingResultsToImageData();
bool paintCompositedResultsToCanvas(ImageBuffer*);

#if PLATFORM(IOS)
#if PLATFORM(COCOA)
void endPaint();
#endif

#if PLATFORM(MAC)
void allocateIOSurfaceBackingStore(IntSize);
void updateFramebufferTextureBackingStoreFromLayer();
void updateCGLContext();
#endif

void setContextVisibility(bool);

GraphicsContext3DPowerPreference powerPreferenceUsedForCreation() const { return m_powerPreferenceUsedForCreation; }
@@ -411,7 +411,7 @@ - (void)setOwner:(PlatformCALayer*)owner
if ([m_layer contents] != [caLayer contents])
[m_layer setContents:[caLayer contents]];
else
[m_layer setContentsChanged];
[m_layer reloadValueForKeyPath:@"contents"];
END_BLOCK_OBJC_EXCEPTIONS
}

@@ -479,28 +479,30 @@ static void setPixelFormat(Vector<CGLPixelFormatAttribute>& attribs, int colorBi
::glEnable(GL_MULTISAMPLE);
#endif

// Create the texture that will be used for the framebuffer.
#if PLATFORM(IOS)
::glGenRenderbuffers(1, &m_texture);
#else
// create a texture to render into
::glGenTextures(1, &m_texture);
::glBindTexture(GL_TEXTURE_2D, m_texture);
::glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
::glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
::glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
::glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
::glBindTexture(GL_TEXTURE_2D, 0);
// We bind to GL_TEXTURE_RECTANGLE_EXT rather than TEXTURE_2D because
// that's what is required for a texture backed by IOSurface.
::glBindTexture(GL_TEXTURE_RECTANGLE_EXT, m_texture);
::glTexParameteri(GL_TEXTURE_RECTANGLE_EXT, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
::glTexParameteri(GL_TEXTURE_RECTANGLE_EXT, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
::glTexParameteri(GL_TEXTURE_RECTANGLE_EXT, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
::glTexParameteri(GL_TEXTURE_RECTANGLE_EXT, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
::glBindTexture(GL_TEXTURE_RECTANGLE_EXT, 0);
#endif

// create an FBO
// Create the framebuffer object.
::glGenFramebuffersEXT(1, &m_fbo);
::glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, m_fbo);

m_state.boundFBO = m_fbo;
if (!m_attrs.antialias && (m_attrs.stencil || m_attrs.depth))
::glGenRenderbuffersEXT(1, &m_depthStencilBuffer);

// create an multisample FBO
// If necessary, create another framebuffer for the multisample results.
if (m_attrs.antialias) {
::glGenFramebuffersEXT(1, &m_multisampleFBO);
::glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, m_multisampleFBO);
@@ -654,20 +656,35 @@ static void setPixelFormat(Vector<CGLPixelFormatAttribute>& attribs, int colorBi
#endif
}

#if PLATFORM(IOS)
void GraphicsContext3D::endPaint()
{
makeContextCurrent();
if (m_attrs.antialias)
resolveMultisamplingIfNecessary();
#if PLATFORM(IOS)
// This is the place where we actually push our current rendering
// results to the compositor on iOS. On macOS it comes from the
// calling function, which is inside WebGLLayer.
::glFlush();
::glBindRenderbuffer(GL_RENDERBUFFER, m_texture);
[static_cast<EAGLContext*>(m_contextObj) presentRenderbuffer:GL_RENDERBUFFER];
[EAGLContext setCurrentContext:nil];
}
#endif
}

#if PLATFORM(MAC)
void GraphicsContext3D::allocateIOSurfaceBackingStore(IntSize size)
{
LOG(WebGL, "GraphicsContext3D::allocateIOSurfaceBackingStore at %d x %d. (%p)", size.width(), size.height(), this);
[m_webGLLayer allocateIOSurfaceBackingStoreWithSize:size usingAlpha:m_attrs.alpha];
}

void GraphicsContext3D::updateFramebufferTextureBackingStoreFromLayer()
{
LOG(WebGL, "GraphicsContext3D::updateFramebufferTextureBackingStoreFromLayer(). (%p)", this);
[m_webGLLayer bindFramebufferToNextAvailableSurface];
}

void GraphicsContext3D::updateCGLContext()
{
if (!m_contextObj)
@@ -25,21 +25,30 @@

#pragma once

#import "IOSurface.h"
#import "IntSize.h"
#import <QuartzCore/QuartzCore.h>

namespace WebCore {
class GraphicsLayer;
class GraphicsContext3D;
}

#if PLATFORM(IOS)
@interface WebGLLayer : CAEAGLLayer
#if PLATFORM(MAC)
@interface WebGLLayer : CALayer
#else
@interface WebGLLayer : CAOpenGLLayer
@interface WebGLLayer : CAEAGLLayer
#endif
{
WebCore::GraphicsContext3D* _context;
float _devicePixelRatio;
#if PLATFORM(MAC)
std::unique_ptr<WebCore::IOSurface> _contentsBuffer;
std::unique_ptr<WebCore::IOSurface> _drawingBuffer;
std::unique_ptr<WebCore::IOSurface> _spareBuffer;
WebCore::IntSize _bufferSize;
BOOL _usingAlpha;
#endif
}

@property (nonatomic) WebCore::GraphicsContext3D* context;
@@ -48,5 +57,10 @@ class GraphicsContext3D;

- (CGImageRef)copyImageSnapshotWithColorSpace:(CGColorSpaceRef)colorSpace;

#if PLATFORM(MAC)
- (void)allocateIOSurfaceBackingStoreWithSize:(WebCore::IntSize)size usingAlpha:(BOOL)usingAlpha;
- (void)bindFramebufferToNextAvailableSurface;
#endif

@end

0 comments on commit 5683575

Please sign in to comment.