Commit f7fd083d authored by epenner's avatar epenner Committed by Commit bot

Revert of Fix assorted issues with remote CoreAnimation (patchset #2 of...

Revert of Fix assorted issues with remote CoreAnimation (patchset #2 of https://codereview.chromium.org/516643002/)

Reason for revert:
Speculatively reverting to see if this helps the mac perf bots:
https://code.google.com/p/chromium/issues/detail?id=408673

Original issue's description:
> Fix assorted issues with remote CoreAnimation
>
> These issues came up while running for a few days with the
> flag --enable-remote-core-animation.
>
> Fix flashes of old frames by hooking up the DiscardBackbuffer (which
> happens when being made non-visible) to re-set the CAContext and
> CALayer (so the browser gets a new one with new content for the next
> frame).
>
> Add support for disabling vsync by using setNeedsDisplay to draw.
>
> Change the backpressure mechanism to rely on the browser to apply
> backpressure in its compositor via the cc:: output surface.
>
> Add a ScopedSetGLToRealGLApi structure to ensure that we are talking
> to the real GL API while in the CoreAnimation callback.
>
> BUG=312462
>
> Committed: https://chromium.googlesource.com/chromium/src/+/3b6aee8ed0393d852ed21fae78f539ffffe3e8f8

TBR=piman@chromium.org,ccameron@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=312462

Review URL: https://codereview.chromium.org/517733002

Cr-Commit-Position: refs/heads/master@{#292438}
parent 565a0064
......@@ -61,7 +61,6 @@ BrowserCompositorViewMacInternal::BrowserCompositorViewMacInternal()
native_widget_,
content::GetContextFactory(),
RenderWidgetResizeHelper::Get()->task_runner()));
compositor_->SetVisible(false);
}
BrowserCompositorViewMacInternal::~BrowserCompositorViewMacInternal() {
......@@ -82,7 +81,6 @@ void BrowserCompositorViewMacInternal::SetClient(
DCHECK(background_layer);
[flipped_layer_ setBounds:[background_layer bounds]];
[background_layer addSublayer:flipped_layer_];
compositor_->SetVisible(true);
}
void BrowserCompositorViewMacInternal::ResetClient() {
......@@ -100,7 +98,6 @@ void BrowserCompositorViewMacInternal::ResetClient() {
accelerated_output_surface_id_ = 0;
last_swap_size_dip_ = gfx::Size();
compositor_->SetVisible(false);
compositor_->SetScaleAndSize(1.0, gfx::Size(0, 0));
compositor_->SetRootLayer(NULL);
client_ = NULL;
......
......@@ -28,27 +28,18 @@ class CALayerStorageProvider
CGLContextObj context, GLuint texture,
gfx::Size pixel_size, float scale_factor) OVERRIDE;
virtual void FreeColorBufferStorage() OVERRIDE;
virtual void SwapBuffers(const gfx::Size& size, float scale_factor) OVERRIDE;
virtual void WillWriteToBackbuffer() OVERRIDE;
virtual void DiscardBackbuffer() OVERRIDE;
virtual void SwapBuffersAckedByBrowser() OVERRIDE;
virtual uint64 GetSurfaceHandle() const OVERRIDE;
virtual void WillSwapBuffers() OVERRIDE;
virtual void CanFreeSwappedBuffer() OVERRIDE;
// Interface to ImageTransportLayer:
CGLContextObj LayerShareGroupContext();
bool LayerCanDraw();
void LayerDoDraw();
void LayerResetStorageProvider();
private:
void DrawWithVsyncDisabled();
void SendPendingSwapToBrowserAfterFrameDrawn();
ImageTransportSurfaceFBO* transport_surface_;
// Used to determine if we should use setNeedsDisplay or setAsynchronous to
// animate.
const bool gpu_vsync_disabled_;
// Set when a new swap occurs, and un-set when |layer_| draws that frame.
bool has_pending_draw_;
......@@ -62,13 +53,11 @@ class CALayerStorageProvider
base::ScopedTypeRef<CGLContextObj> share_group_context_;
GLuint fbo_texture_;
gfx::Size fbo_pixel_size_;
float fbo_scale_factor_;
// The CALayer that the current frame is being drawn into.
base::scoped_nsobject<CAContext> context_;
base::scoped_nsobject<ImageTransportLayer> layer_;
base::WeakPtrFactory<CALayerStorageProvider> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(CALayerStorageProvider);
};
......
......@@ -4,13 +4,10 @@
#include "content/common/gpu/image_transport_surface_calayer_mac.h"
#include "base/command_line.h"
#include "base/mac/sdk_forward_declarations.h"
#include "content/common/gpu/surface_handle_types_mac.h"
#include "ui/base/cocoa/animation_utils.h"
#include "ui/gfx/geometry/size_conversions.h"
#include "ui/gl/gl_gl_api_implementation.h"
#include "ui/gl/gl_switches.h"
@interface ImageTransportLayer : CAOpenGLLayer {
content::CALayerStorageProvider* storageProvider_;
......@@ -29,8 +26,6 @@
}
- (void)resetStorageProvider {
if (storageProvider_)
storageProvider_->LayerResetStorageProvider();
storageProvider_ = NULL;
}
......@@ -65,10 +60,6 @@
pixelFormat:(CGLPixelFormatObj)pixelFormat
forLayerTime:(CFTimeInterval)timeInterval
displayTime:(const CVTimeStamp*)timeStamp {
// While in this callback, CoreAnimation has set |glContext| to be current.
// Ensure that the GL calls that we make are made against the native GL API.
gfx::ScopedSetGLToRealGLApi scoped_set_gl_api;
if (storageProvider_) {
storageProvider_->LayerDoDraw();
} else {
......@@ -88,13 +79,17 @@ namespace content {
CALayerStorageProvider::CALayerStorageProvider(
ImageTransportSurfaceFBO* transport_surface)
: transport_surface_(transport_surface),
gpu_vsync_disabled_(CommandLine::ForCurrentProcess()->HasSwitch(
switches::kDisableGpuVsync)),
has_pending_draw_(false),
can_draw_returned_false_count_(0),
fbo_texture_(0),
fbo_scale_factor_(1),
weak_factory_(this) {}
fbo_texture_(0) {
// Allocate a CAContext to use to transport the CALayer to the browser
// process.
base::scoped_nsobject<NSDictionary> dict([[NSDictionary alloc] init]);
CGSConnectionID connection_id = CGSMainConnectionID();
context_.reset([CAContext contextWithCGSConnection:connection_id
options:dict]);
[context_ retain];
}
CALayerStorageProvider::~CALayerStorageProvider() {
}
......@@ -131,12 +126,15 @@ bool CALayerStorageProvider::AllocateColorBufferStorage(
// Disable the fade-in animation as the layer is changed.
ScopedCAActionDisabler disabler;
// Set the parameters that will be used to allocate the CALayer to draw the
// texture into.
// Allocate a CALayer to draw texture into.
share_group_context_.reset(CGLRetainContext(context));
fbo_texture_ = texture;
fbo_pixel_size_ = pixel_size;
fbo_scale_factor_ = scale_factor;
layer_.reset([[ImageTransportLayer alloc] initWithStorageProvider:this]);
gfx::Size dip_size(gfx::ToFlooredSize(gfx::ScaleSize(
fbo_pixel_size_, 1.0f / scale_factor)));
[layer_ setContentsScale:scale_factor];
[layer_ setFrame:CGRectMake(0, 0, dip_size.width(), dip_size.height())];
return true;
}
......@@ -157,78 +155,25 @@ void CALayerStorageProvider::FreeColorBufferStorage() {
fbo_pixel_size_ = gfx::Size();
}
void CALayerStorageProvider::SwapBuffers(
const gfx::Size& size, float scale_factor) {
uint64 CALayerStorageProvider::GetSurfaceHandle() const {
return SurfaceHandleFromCAContextID([context_ contextId]);
}
void CALayerStorageProvider::WillSwapBuffers() {
DCHECK(!has_pending_draw_);
has_pending_draw_ = true;
// Allocate a CAContext to use to transport the CALayer to the browser
// process.
if (!context_) {
base::scoped_nsobject<NSDictionary> dict([[NSDictionary alloc] init]);
CGSConnectionID connection_id = CGSMainConnectionID();
context_.reset([CAContext contextWithCGSConnection:connection_id
options:dict]);
[context_ retain];
}
// Allocate a CALayer to use to draw the content.
if (!layer_) {
layer_.reset([[ImageTransportLayer alloc] initWithStorageProvider:this]);
gfx::Size dip_size(gfx::ToFlooredSize(gfx::ScaleSize(
fbo_pixel_size_, 1.0f / fbo_scale_factor_)));
[layer_ setContentsScale:fbo_scale_factor_];
[layer_ setFrame:CGRectMake(0, 0, dip_size.width(), dip_size.height())];
// Make the CALayer current to the CAContext and display its contents
// immediately.
// Don't add the layer to the CAContext until a SwapBuffers is going to be
// called, because the texture does not have any content until the
// SwapBuffers call is about to be made.
if ([context_ layer] != layer_.get())
[context_ setLayer:layer_];
}
// Tell CoreAnimation to draw our frame. We will send the IPC to the browser
// when CoreAnimation has drawn our frame.
if (gpu_vsync_disabled_) {
DrawWithVsyncDisabled();
} else {
if (![layer_ isAsynchronous])
[layer_ setAsynchronous:YES];
}
if (![layer_ isAsynchronous])
[layer_ setAsynchronous:YES];
}
void CALayerStorageProvider::DrawWithVsyncDisabled() {
DCHECK(has_pending_draw_);
[layer_ setNeedsDisplay];
// Sometimes, setNeedsDisplay calls are dropped on the floor. Make this not
// hang the renderer by re-issuing the call if the draw has not yet
// happened.
if (has_pending_draw_) {
// Delay sending another draw immediately to avoid starving the run loop.
base::MessageLoop::current()->PostDelayedTask(
FROM_HERE,
base::Bind(&CALayerStorageProvider::DrawWithVsyncDisabled,
weak_factory_.GetWeakPtr()),
base::TimeDelta::FromMilliseconds(5));
}
}
void CALayerStorageProvider::WillWriteToBackbuffer() {
// TODO(ccameron): The browser may need to continue issuing swaps even when
// they do not draw. In these cases it is necessary to either double-buffer
// the resulting texture, or to drop frames.
}
void CALayerStorageProvider::DiscardBackbuffer() {
// If this surface's backbuffer is discarded, it is because this surface has
// been made non-visible. Ensure that the previous contents are not briefly
// flashed when this is made visible by creating a new CALayer and CAContext
// at the next swap.
[layer_ resetStorageProvider];
layer_.reset();
context_.reset();
}
void CALayerStorageProvider::SwapBuffersAckedByBrowser() {
void CALayerStorageProvider::CanFreeSwappedBuffer() {
}
CGLContextObj CALayerStorageProvider::LayerShareGroupContext() {
......@@ -240,22 +185,20 @@ bool CALayerStorageProvider::LayerCanDraw() {
can_draw_returned_false_count_ = 0;
return true;
} else {
if ([layer_ isAsynchronous]) {
DCHECK(!gpu_vsync_disabled_);
// If we are in asynchronous mode, we will be getting callbacks at every
// vsync, asking us if we have anything to draw. If we get 30 of these in
// a row, ask that we stop getting these callback for now, so that we
// don't waste CPU cycles.
if (can_draw_returned_false_count_ == 30)
if (can_draw_returned_false_count_ == 30) {
if ([layer_ isAsynchronous])
[layer_ setAsynchronous:NO];
else
can_draw_returned_false_count_ += 1;
} else {
can_draw_returned_false_count_ += 1;
}
return false;
}
}
void CALayerStorageProvider::LayerDoDraw() {
DCHECK(has_pending_draw_);
has_pending_draw_ = false;
GLint viewport[4] = {0, 0, 0, 0};
glGetIntegerv(GL_VIEWPORT, viewport);
gfx::Size viewport_size(viewport[2], viewport[3]);
......@@ -290,25 +233,7 @@ void CALayerStorageProvider::LayerDoDraw() {
glDisable(GL_TEXTURE_RECTANGLE_ARB);
// Allow forward progress in the context now that the swap is complete.
DCHECK(has_pending_draw_);
SendPendingSwapToBrowserAfterFrameDrawn();
}
void CALayerStorageProvider::LayerResetStorageProvider() {
// If we are providing back-pressure by waiting for a draw, that draw will
// now never come, so release the pressure now.
SendPendingSwapToBrowserAfterFrameDrawn();
}
void CALayerStorageProvider::SendPendingSwapToBrowserAfterFrameDrawn() {
if (!has_pending_draw_)
return;
weak_factory_.InvalidateWeakPtrs();
has_pending_draw_ = false;
transport_surface_->SendSwapBuffers(
SurfaceHandleFromCAContextID([context_ contextId]),
fbo_pixel_size_,
fbo_scale_factor_);
transport_surface_->UnblockContextAfterPendingSwap();
}
} // namespace content
......@@ -41,20 +41,20 @@ class ImageTransportSurfaceFBO
// GL texture that was bound has already been deleted by the caller.
virtual void FreeColorBufferStorage() = 0;
// Swap buffers and return the handle for the surface to send to the browser
// process to display.
virtual void SwapBuffers(const gfx::Size& size, float scale_factor) = 0;
// Indicate that the backbuffer will be written to.
virtual void WillWriteToBackbuffer() = 0;
// Indicate that the backbuffer has been discarded and should not be seen
// again.
virtual void DiscardBackbuffer() = 0;
// Called once for every SwapBuffers call when the IPC for the present has
// been processed by the browser.
virtual void SwapBuffersAckedByBrowser() = 0;
// Retrieve the handle for the surface to send to the browser process to
// display.
virtual uint64 GetSurfaceHandle() const = 0;
// Called when a new frame has been rendered into the texture, and the
// browser is about to be sent the surface to display.
virtual void WillSwapBuffers() = 0;
// Called once for every WillSwapBuffers call when the buffer that was sent
// to the browser may be released by the GPU process (this may be because
// the browser is holding a reference, in which case this will come
// quickly, or it may be because the browser is done with the surface, in
// which case it will come much later).
virtual void CanFreeSwappedBuffer() = 0;
};
ImageTransportSurfaceFBO(GpuChannelManager* manager,
......@@ -78,9 +78,7 @@ class ImageTransportSurfaceFBO
virtual void SetFrontbufferAllocation(bool allocated) OVERRIDE;
// Called when the context may continue to make forward progress after a swap.
void SendSwapBuffers(uint64 surface_handle,
const gfx::Size pixel_size,
float scale_factor);
void UnblockContextAfterPendingSwap();
protected:
// ImageTransportSurface implementation
......@@ -122,8 +120,12 @@ class ImageTransportSurfaceFBO
// Whether or not we've successfully made the surface current once.
bool made_current_;
// Whether a SwapBuffers IPC needs to be sent to the browser.
bool is_swap_buffers_send_pending_;
// Whether a SwapBuffers is pending.
bool is_swap_buffers_pending_;
// Whether we unscheduled command buffer because of pending SwapBuffers.
bool did_unschedule_;
std::vector<ui::LatencyInfo> latency_info_;
scoped_ptr<ImageTransportHelper> helper_;
......
......@@ -28,7 +28,8 @@ ImageTransportSurfaceFBO::ImageTransportSurfaceFBO(
context_(NULL),
scale_factor_(1.f),
made_current_(false),
is_swap_buffers_send_pending_(false) {
is_swap_buffers_pending_(false),
did_unschedule_(false) {
if (ui::RemoteLayerAPISupported())
storage_provider_.reset(new CALayerStorageProvider(this));
else
......@@ -61,9 +62,17 @@ void ImageTransportSurfaceFBO::Destroy() {
}
bool ImageTransportSurfaceFBO::DeferDraws() {
storage_provider_->WillWriteToBackbuffer();
// We should not have a pending send when we are drawing the next frame.
DCHECK(!is_swap_buffers_send_pending_);
// The command buffer hit a draw/clear command that could clobber the
// IOSurface in use by an earlier SwapBuffers. If a Swap is pending, abort
// processing of the command by returning true and unschedule until the Swap
// Ack arrives.
if(did_unschedule_)
return true; // Still unscheduled, so just return true.
if (is_swap_buffers_pending_) {
did_unschedule_ = true;
helper_->SetScheduled(false);
return true;
}
return false;
}
......@@ -92,8 +101,6 @@ bool ImageTransportSurfaceFBO::SetBackbufferAllocation(bool allocation) {
return true;
backbuffer_suggested_allocation_ = allocation;
AdjustBufferAllocation();
if (!allocation)
storage_provider_->DiscardBackbuffer();
return true;
}
......@@ -123,22 +130,18 @@ bool ImageTransportSurfaceFBO::SwapBuffers() {
return true;
glFlush();
// It is the responsibility of the storage provider to send the swap IPC.
is_swap_buffers_send_pending_ = true;
storage_provider_->SwapBuffers(size_, scale_factor_);
return true;
}
void ImageTransportSurfaceFBO::SendSwapBuffers(uint64 surface_handle,
const gfx::Size pixel_size,
float scale_factor) {
GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params params;
params.surface_handle = surface_handle;
params.size = pixel_size;
params.scale_factor = scale_factor;
params.surface_handle = storage_provider_->GetSurfaceHandle();
params.size = GetSize();
params.scale_factor = scale_factor_;
params.latency_info.swap(latency_info_);
helper_->SendAcceleratedSurfaceBuffersSwapped(params);
is_swap_buffers_send_pending_ = false;
DCHECK(!is_swap_buffers_pending_);
is_swap_buffers_pending_ = true;
storage_provider_->WillSwapBuffers();
return true;
}
bool ImageTransportSurfaceFBO::PostSubBuffer(
......@@ -167,7 +170,16 @@ void* ImageTransportSurfaceFBO::GetDisplay() {
void ImageTransportSurfaceFBO::OnBufferPresented(
const AcceleratedSurfaceMsg_BufferPresented_Params& params) {
context_->share_group()->SetRendererID(params.renderer_id);
storage_provider_->SwapBuffersAckedByBrowser();
storage_provider_->CanFreeSwappedBuffer();
}
void ImageTransportSurfaceFBO::UnblockContextAfterPendingSwap() {
DCHECK(is_swap_buffers_pending_);
is_swap_buffers_pending_ = false;
if (did_unschedule_) {
did_unschedule_ = false;
helper_->SetScheduled(true);
}
}
void ImageTransportSurfaceFBO::OnResize(gfx::Size size,
......
......@@ -105,26 +105,21 @@ void IOSurfaceStorageProvider::FreeColorBufferStorage() {
io_surface_id_ = 0;
}
void IOSurfaceStorageProvider::SwapBuffers(
const gfx::Size& size, float scale_factor) {
uint64 IOSurfaceStorageProvider::GetSurfaceHandle() const {
return SurfaceHandleFromIOSurfaceID(io_surface_id_);
}
void IOSurfaceStorageProvider::WillSwapBuffers() {
// The browser compositor will throttle itself, so we are free to unblock the
// context immediately. Make sure that the browser is doing its throttling
// appropriately by ensuring that the previous swap was acknowledged before
// we get another swap.
DCHECK(pending_swapped_surfaces_.empty());
pending_swapped_surfaces_.push_back(io_surface_);
transport_surface_->SendSwapBuffers(
SurfaceHandleFromIOSurfaceID(io_surface_id_), size, scale_factor);
}
void IOSurfaceStorageProvider::WillWriteToBackbuffer() {
}
void IOSurfaceStorageProvider::DiscardBackbuffer() {
transport_surface_->UnblockContextAfterPendingSwap();
}
void IOSurfaceStorageProvider::SwapBuffersAckedByBrowser() {
void IOSurfaceStorageProvider::CanFreeSwappedBuffer() {
DCHECK(!pending_swapped_surfaces_.empty());
pending_swapped_surfaces_.pop_front();
}
......
......@@ -28,10 +28,9 @@ class IOSurfaceStorageProvider
CGLContextObj context, GLuint texture,
gfx::Size pixel_size, float scale_factor) OVERRIDE;
virtual void FreeColorBufferStorage() OVERRIDE;
virtual void SwapBuffers(const gfx::Size& size, float scale_factor) OVERRIDE;
virtual void WillWriteToBackbuffer() OVERRIDE;
virtual void DiscardBackbuffer() OVERRIDE;
virtual void SwapBuffersAckedByBrowser() OVERRIDE;
virtual uint64 GetSurfaceHandle() const OVERRIDE;
virtual void WillSwapBuffers() OVERRIDE;
virtual void CanFreeSwappedBuffer() OVERRIDE;
private:
ImageTransportSurfaceFBO* transport_surface_;
......
......@@ -276,10 +276,6 @@ void Compositor::SetBackgroundColor(SkColor color) {
ScheduleDraw();
}
void Compositor::SetVisible(bool visible) {
host_->SetVisible(visible);
}
scoped_refptr<CompositorVSyncManager> Compositor::vsync_manager() const {
return vsync_manager_;
}
......
......@@ -186,9 +186,6 @@ class COMPOSITOR_EXPORT Compositor
// the |root_layer|.
void SetBackgroundColor(SkColor color);
// Set the visibility of the underlying compositor.
void SetVisible(bool visible);
// Returns the widget for this compositor.
gfx::AcceleratedWidget widget() const { return widget_; }
......
......@@ -492,13 +492,4 @@ void VirtualGLApi::glFinishFn() {
GLApiBase::SignalFlush();
}
ScopedSetGLToRealGLApi::ScopedSetGLToRealGLApi()
: old_gl_api_(GetCurrentGLApi()) {
SetGLToRealGLApi();
}
ScopedSetGLToRealGLApi::~ScopedSetGLToRealGLApi() {
SetGLApi(old_gl_api_);
}
} // namespace gfx
......@@ -7,7 +7,6 @@
#include "base/compiler_specific.h"
#include "ui/gl/gl_bindings.h"
#include "ui/gl/gl_export.h"
namespace gpu {
namespace gles2 {
......@@ -118,15 +117,6 @@ private:
std::string extensions_;
};
class GL_EXPORT ScopedSetGLToRealGLApi {
public:
ScopedSetGLToRealGLApi();
~ScopedSetGLToRealGLApi();
private:
GLApi* old_gl_api_;
};
} // namespace gfx
#endif // UI_GL_GL_GL_API_IMPLEMENTATION_H_
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment