diff options
author | Christopher Cameron <ccameron@chromium.org> | 2016-02-09 16:35:18 -0800 |
---|---|---|
committer | Christopher Cameron <ccameron@chromium.org> | 2016-02-10 00:37:52 +0000 |
commit | 2e3c5710eea999e0c95bc65869163afaa4085a0b (patch) | |
tree | 01fed1bc9b7abba9a349bcc3c648552401cced54 /content | |
parent | 01820197acc97766ea0223cf29d95d886c322a3f (diff) | |
download | chromium_src-2e3c5710eea999e0c95bc65869163afaa4085a0b.zip chromium_src-2e3c5710eea999e0c95bc65869163afaa4085a0b.tar.gz chromium_src-2e3c5710eea999e0c95bc65869163afaa4085a0b.tar.bz2 |
Mac Overlays: Fix blank white hangs
The issue is as follows:
We rebuild the CALayerTree based on the previous frame's CALayerTree,
trying to change as few properties as possible (cause redundant updates
waste CPU time).
Among the things we assume is that the root of previous frame's
CALayerTree is attached to the ImageTransportSurfaceMac::ca_root_layer_.
So, if we re-use the previous frame's CALayerTree, we will not re-attach
it to ImageTransportSurfaceMac::ca_root_layer_.
There is a bug where, if ImageTransportSurfaceMac gets no
CALayerOverlays and no OverlayPlanes, it will just leave the previous
frame's CALayerTree hanging out in
ImageTransportSurfaceMac::current_ca_layer_tree_, but it will disconnect
it from ImageTransportSurfaceMac::ca_root_layer_. As a consequence, the
next frame that has a CALayerOverlay will go and build its tree off of
ImageTransportSurfaceMac::current_ca_layer_tree_, assuming that it is
still connected to ImageTransportSurfaceMac::ca_root_layer_, which it
isn't.
And then we never see any new frames. The same bug also exists for
ImageTransportSurfaceMac::current_partial_damage_tree_.
The fix is to blow away
ImageTransportSurfaceMac::current_partial_damage_tree_ and
ImageTransportSurfaceMac::current_ca_layer_tree when we get a blank
frame.
BUG=583805
Review URL: https://codereview.chromium.org/1684713003
Cr-Commit-Position: refs/heads/master@{#374542}
(cherry picked from commit 3756dcea2132ee864484660e521ed8149ff31763)
Mac: Add extra logging to diagnose white screens
Most of this (but not some of the TRACEs) are to be removed once
we figure out what's going wrong.
My goal with this is to give people who see this the ability to take
a trace that will let me know if we're hitting any of the paths which
can result in a white screen, or at least which display path we're
taking.
TBR=jbauman
BUG=583805
Review URL: https://codereview.chromium.org/1674223004
Cr-Commit-Position: refs/heads/master@{#374320}
(cherry picked from commit 94fabc0ac9ac0168d74e66c6ba30c72117586915)
Review URL: https://codereview.chromium.org/1682223003 .
Cr-Commit-Position: refs/branch-heads/2623@{#342}
Cr-Branched-From: 92d77538a86529ca35f9220bd3cd512cbea1f086-refs/heads/master@{#369907}
Diffstat (limited to 'content')
4 files changed, 34 insertions, 5 deletions
diff --git a/content/browser/gpu/gpu_process_host_ui_shim.cc b/content/browser/gpu/gpu_process_host_ui_shim.cc index 083c99d..f50db21 100644 --- a/content/browser/gpu/gpu_process_host_ui_shim.cc +++ b/content/browser/gpu/gpu_process_host_ui_shim.cc @@ -223,10 +223,12 @@ void GpuProcessHostUIShim::OnGraphicsInfoCollected( #if defined(OS_MACOSX) void GpuProcessHostUIShim::OnAcceleratedSurfaceBuffersSwapped( const GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params& params) { - TRACE_EVENT0("renderer", + TRACE_EVENT0("browser", "GpuProcessHostUIShim::OnAcceleratedSurfaceBuffersSwapped"); if (!ui::LatencyInfo::Verify(params.latency_info, "GpuHostMsg_AcceleratedSurfaceBuffersSwapped")) { + + TRACE_EVENT0("browser", "ui::LatencyInfo::Verify failed"); return; } @@ -257,6 +259,8 @@ void GpuProcessHostUIShim::OnAcceleratedSurfaceBuffersSwapped( params.size, params.scale_factor, &ack_params.vsync_timebase, &ack_params.vsync_interval); + } else { + TRACE_EVENT0("browser", "Skipping recycled surface frame"); } content::ImageTransportFactory::GetInstance()->OnGpuSwapBuffersCompleted( diff --git a/content/common/gpu/ca_layer_partial_damage_tree_mac.mm b/content/common/gpu/ca_layer_partial_damage_tree_mac.mm index ee94726..f427389 100644 --- a/content/common/gpu/ca_layer_partial_damage_tree_mac.mm +++ b/content/common/gpu/ca_layer_partial_damage_tree_mac.mm @@ -7,6 +7,7 @@ #include "base/command_line.h" #include "base/mac/scoped_nsobject.h" #include "base/mac/sdk_forward_declarations.h" +#include "base/trace_event/trace_event.h" #include "ui/base/ui_base_switches.h" #include "ui/gfx/transform.h" @@ -237,6 +238,11 @@ void CALayerPartialDamageTree::UpdateCALayers(CALayer* superlayer, [superlayer setSublayers:nil]; [superlayer addSublayer:root_plane_->ca_layer]; } + // Excessive logging to debug white screens (crbug.com/583805). + // TODO(ccameron): change this back to a DLOG. + if ([root_plane_->ca_layer superlayer] != superlayer) { + LOG(ERROR) << "CALayerPartialDamageTree root layer not attached to tree."; + } for (auto& plane : partial_damage_planes_) { if (!plane->ca_layer) { DCHECK(plane == partial_damage_planes_.back()); @@ -273,6 +279,7 @@ void CALayerPartialDamageTree::CommitCALayers( scoped_ptr<CALayerPartialDamageTree> old_tree, float scale_factor, const gfx::Rect& pixel_damage_rect) { + TRACE_EVENT0("gpu", "CALayerPartialDamageTree::CommitCALayers"); UpdateRootAndPartialDamagePlanes(std::move(old_tree), pixel_damage_rect); UpdateCALayers(superlayer, scale_factor); } diff --git a/content/common/gpu/ca_layer_tree_mac.mm b/content/common/gpu/ca_layer_tree_mac.mm index 0861ae6..8c7d938 100644 --- a/content/common/gpu/ca_layer_tree_mac.mm +++ b/content/common/gpu/ca_layer_tree_mac.mm @@ -6,6 +6,7 @@ #include "base/command_line.h" #include "base/mac/sdk_forward_declarations.h" +#include "base/trace_event/trace_event.h" #include "gpu/GLES2/gl2extchromium.h" #include "third_party/skia/include/core/SkColor.h" #include "ui/base/cocoa/animation_utils.h" @@ -28,8 +29,10 @@ bool CALayerTree::ScheduleCALayer( unsigned background_color, unsigned edge_aa_mask, float opacity) { + // Excessive logging to debug white screens (crbug.com/583805). + // TODO(ccameron): change this back to a DLOG. if (has_committed_) { - DLOG(ERROR) << "ScheduleCALayer called after CommitScheduledCALayers."; + LOG(ERROR) << "ScheduleCALayer called after CommitScheduledCALayers."; return false; } return root_layer_.AddContentLayer(is_clipped, clip_rect, sorting_context_id, @@ -40,6 +43,7 @@ bool CALayerTree::ScheduleCALayer( void CALayerTree::CommitScheduledCALayers(CALayer* superlayer, scoped_ptr<CALayerTree> old_tree, float scale_factor) { + TRACE_EVENT0("gpu", "CALayerTree::CommitScheduledCALayers"); RootLayer* old_root_layer = nullptr; if (old_tree) { DCHECK(old_tree->has_committed_); @@ -204,7 +208,9 @@ bool CALayerTree::RootLayer::AddContentLayer( current_layer.sorting_context_id == sorting_context_id && (current_layer.is_clipped != is_clipped || current_layer.clip_rect != clip_rect)) { - DLOG(ERROR) << "CALayer changed clip inside non-zero sorting context."; + // Excessive logging to debug white screens (crbug.com/583805). + // TODO(ccameron): change this back to a DLOG. + LOG(ERROR) << "CALayer changed clip inside non-zero sorting context."; return false; } if (!is_singleton_sorting_context && @@ -271,7 +277,11 @@ void CALayerTree::RootLayer::CommitToCA(CALayer* superlayer, [superlayer addSublayer:ca_layer]; [superlayer setBorderWidth:0]; } - DCHECK_EQ([ca_layer superlayer], superlayer); + // Excessive logging to debug white screens (crbug.com/583805). + // TODO(ccameron): change this back to a DCHECK. + if ([ca_layer superlayer] != superlayer) { + LOG(ERROR) << "CALayerTree root layer not attached to tree."; + } for (size_t i = 0; i < clip_and_sorting_layers.size(); ++i) { ClipAndSortingLayer* old_clip_and_sorting_layer = nullptr; @@ -299,7 +309,11 @@ void CALayerTree::ClipAndSortingLayer::CommitToCA( [ca_layer setAnchorPoint:CGPointZero]; [superlayer addSublayer:ca_layer]; } - DCHECK_EQ([ca_layer superlayer], superlayer); + // Excessive logging to debug white screens (crbug.com/583805). + // TODO(ccameron): change this back to a DCHECK. + if ([ca_layer superlayer] != superlayer) { + LOG(ERROR) << "CALayerTree root layer not attached to tree."; + } if (update_is_clipped) [ca_layer setMasksToBounds:is_clipped]; diff --git a/content/common/gpu/image_transport_surface_overlay_mac.mm b/content/common/gpu/image_transport_surface_overlay_mac.mm index 034dc0b..1b0e337 100644 --- a/content/common/gpu/image_transport_surface_overlay_mac.mm +++ b/content/common/gpu/image_transport_surface_overlay_mac.mm @@ -21,6 +21,7 @@ typedef void* GLeglImageOES; #endif #include "base/mac/scoped_cftyperef.h" +#include "base/trace_event/trace_event.h" #include "content/common/gpu/ca_layer_partial_damage_tree_mac.h" #include "content/common/gpu/ca_layer_tree_mac.h" #include "content/common/gpu/gpu_messages.h" @@ -287,7 +288,10 @@ void ImageTransportSurfaceOverlayMac::DisplayFirstPendingSwapImmediately() { current_partial_damage_tree_.swap(swap->partial_damage_tree); current_ca_layer_tree_.reset(); } else { + TRACE_EVENT0("gpu", "Blank frame: No overlays or CALayers"); [ca_root_layer_ setSublayers:nil]; + current_partial_damage_tree_.reset(); + current_ca_layer_tree_.reset(); } swap->ca_layer_tree.reset(); swap->partial_damage_tree.reset(); |