Skip to content

Commit

Permalink
Move inspector layer view update after LayerTreeHost::UpdateLayers
Browse files Browse the repository at this point in the history
The inspector uses layer and cc property tree information to update the
layer view via LayerTreeDidChange. Previously, this was called after
paint but before the property trees had updated to-screen transforms
(calculated in draw_property_utils::UpdatePropertyTrees) which resulted
in incorrect layer positions. This patch moves the LayerTreeDidChange
notification after LayerTreeHost::UpdateLayers which is the first point
where cc property trees have correct to-screen transforms. This fixes
the inspector layer positioning.

Bug: 977578
Change-Id: Ie033508eb2985b3bce8c8494558a4788b9c65fa2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1726874
Commit-Queue: Philip Rogers <pdr@chromium.org>
Reviewed-by: Chris Harrelson OOO <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#685866}
  • Loading branch information
progers authored and Commit Bot committed Aug 10, 2019
1 parent 96aee58 commit 2e66be7
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 5 deletions.
3 changes: 3 additions & 0 deletions third_party/blink/renderer/core/exported/web_view_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@
#include "third_party/blink/renderer/core/paint/first_meaningful_paint_detector.h"
#include "third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h"
#include "third_party/blink/renderer/core/paint/paint_timing_detector.h"
#include "third_party/blink/renderer/core/probe/core_probes.h"
#include "third_party/blink/renderer/core/scroll/scrollbar_theme.h"
#include "third_party/blink/renderer/core/timing/dom_window_performance.h"
#include "third_party/blink/renderer/core/timing/window_performance.h"
Expand Down Expand Up @@ -1534,6 +1535,8 @@ void WebViewImpl::EndUpdateLayers() {
MainFrameImpl()->GetFrame()->View()->EnsureUkmAggregator().RecordSample(
LocalFrameUkmAggregator::kUpdateLayers,
update_layers_start_time_.value(), base::TimeTicks::Now());
if (RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled())
probe::LayerTreeDidChange(MainFrameImpl()->GetFrame());
}
update_layers_start_time_.reset();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
#include "third_party/blink/renderer/core/page/pointer_lock_controller.h"
#include "third_party/blink/renderer/core/page/validation_message_client.h"
#include "third_party/blink/renderer/core/paint/compositing/paint_layer_compositor.h"
#include "third_party/blink/renderer/core/probe/core_probes.h"
#include "third_party/blink/renderer/platform/graphics/color.h"
#include "third_party/blink/renderer/platform/keyboard_codes.h"

Expand Down Expand Up @@ -317,6 +318,8 @@ void WebFrameWidgetImpl::EndUpdateLayers() {
LocalRootImpl()->GetFrame()->View()->EnsureUkmAggregator().RecordSample(
LocalFrameUkmAggregator::kUpdateLayers,
update_layers_start_time_.value(), base::TimeTicks::Now());
if (RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled())
probe::LayerTreeDidChange(LocalRootImpl()->GetFrame());
}
update_layers_start_time_.reset();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,10 @@ Response InspectorLayerTreeAgent::enable() {

if (RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled() ||
RuntimeEnabledFeatures::CompositeAfterPaintEnabled()) {
if (document->Lifecycle().GetState() >= DocumentLifecycle::kPaintClean)
if (document->Lifecycle().GetState() >= DocumentLifecycle::kPaintClean) {
LayerTreePainted();
LayerTreeDidChange();
}
} else if (document->Lifecycle().GetState() >=
DocumentLifecycle::kCompositingClean) {
LayerTreeDidChange();
Expand All @@ -290,8 +292,6 @@ Response InspectorLayerTreeAgent::disable() {
}

void InspectorLayerTreeAgent::LayerTreeDidChange() {
DCHECK(!RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled() &&
!RuntimeEnabledFeatures::CompositeAfterPaintEnabled());
GetFrontend()->layerTreeDidChange(BuildLayerTree());
}

Expand Down Expand Up @@ -320,8 +320,6 @@ void InspectorLayerTreeAgent::LayerTreePainted() {
DCHECK(RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled() ||
RuntimeEnabledFeatures::CompositeAfterPaintEnabled());

GetFrontend()->layerTreeDidChange(BuildLayerTree());

for (const auto& layer :
inspected_frames_->Root()->View()->RootCcLayer()->children()) {
if (!layer->update_rect().IsEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Layers after update in Layers3DView

Pass: Layer quads are unchanged by no-op update

Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

(async function() {
TestRunner.addResult(`Layers after update in Layers3DView\n`);
await TestRunner.loadModule('layers_test_runner');

await TestRunner.loadHTML(`
<style>
#layer {
will-change: transform;
width: 300px;
height: 300px;
transform: translate(100px, 100px);
}
</style>
<div id="layer"></div>
`);

TestRunner.showPanel('layers');
await LayersTestRunner.requestLayers();
var layer = LayersTestRunner.findLayerByNodeIdAttribute('layer');
initialQuads = layer.quad().toString();

// Updating layers should not produce invalid layer to-screen transforms
// (see: https://crbug.com/977578). Backface visibility is changed, rather
// than just adjusting the transform, to ensure fast-path optimizations do not
// prevent a full layer tree update.
await LayersTestRunner.evaluateAndWaitForTreeChange(
'layer.style.backfaceVisibility = "hidden";');

layer = LayersTestRunner.findLayerByNodeIdAttribute('layer');
if (initialQuads === layer.quad().toString())
TestRunner.addResult('Pass: Layer quads are unchanged by no-op update');
else
TestRunner.addResult('Fail: Layer quads are changed by no-op update');

TestRunner.completeTest();
})();

0 comments on commit 2e66be7

Please sign in to comment.