Commit 756a4197 authored by Christopher Cameron's avatar Christopher Cameron Committed by Commit Bot

Disable ui::Compositor recycling for SkiaRenderer

The ui::Compositor recycling path involves resizing the surface to be
0x0 and ignoring in-flight swaps. This is delicate logic that probably
doesn't work with SkiaRenderer. There are two DCHECKs that are hit when
this is enabled that aren't when it isn't.

ui::Compositor recycling was designed to work around poor design
decision that dates back at least 8 years. This is the design where the
GLRenderer and all its state (shader cache, etc), is a per-compositor
object, and there are compositors for every tab (historically) and
every window (currently).

A concrete example of where this causes problems: If the user presses a
key in the omnibox, then before any content can be rendered, a whole
bunch of shaders have to be compiled, because it's a new compositor.
If the GLRenderer were a global object shared by all compositors, this
wouldn't happen.

With SkiaRenderer, we happen to have sort-of solved this problem, by
moving all of the GLRenderer state (DirectRenderer state, to be more
accurate) into a single GrContext that happens to be shared
between all compositors.

So, rather than try to understand exactly how the delicate dance of
ui::Compositor recycling breaks on SkiaRenderer, just disable it.

Bug: 1135663, 1135399, 1135386
Change-Id: I0d5bf93aaf4584ef26af0e38da40deec0ef9bb16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2454089Reviewed-by: default avatarPeng Huang <penghuang@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814515}
parent b9be8cb0
......@@ -243,7 +243,6 @@ void BrowserCompositorMac::TransitionToState(State new_state) {
if (state_ == HasOwnCompositor) {
recyclable_compositor_->widget()->ResetNSView();
recyclable_compositor_->compositor()->SetRootLayer(nullptr);
recyclable_compositor_->InvalidateSurface();
ui::RecyclableCompositorMacFactory::Get()->RecycleCompositor(
std::move(recyclable_compositor_));
}
......
......@@ -130,6 +130,8 @@ void RecyclableCompositorMacFactory::RecycleCompositor(
if (recycling_disabled_)
return;
// Invalidate the surface before suspending it.
compositor->InvalidateSurface();
compositor->accelerated_widget_mac_->SetSuspended(true);
// Make this RecyclableCompositorMac recyclable for future instances.
......@@ -154,7 +156,10 @@ void RecyclableCompositorMacFactory::RecycleCompositor(
}
RecyclableCompositorMacFactory::RecyclableCompositorMacFactory()
: weak_factory_(this) {}
: weak_factory_(this) {
if (features::IsUsingSkiaRenderer())
recycling_disabled_ = true;
}
RecyclableCompositorMacFactory::~RecyclableCompositorMacFactory() = default;
......
......@@ -43,12 +43,13 @@ class COMPOSITOR_EXPORT RecyclableCompositorMac
void UpdateSurface(const gfx::Size& size_pixels,
float scale_factor,
const gfx::DisplayColorSpaces& display_color_spaces);
// Invalidate the compositor's surface information.
void InvalidateSurface();
private:
friend class RecyclableCompositorMacFactory;
// Invalidate the compositor's surface information.
void InvalidateSurface();
// ui::CompositorObserver implementation:
void OnCompositingDidCommit(ui::Compositor* compositor) override;
......
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