Skip to content

Commit

Permalink
Revert "[BlinkGenPropertyTrees] Paint frame overlay in correct proper…
Browse files Browse the repository at this point in the history
…ty tree state"

This reverts commit c656dcb.

Reason for revert: crbug.com/945152

Original change's description:
> [BlinkGenPropertyTrees] Paint frame overlay in correct property tree state
> 
> Frame overlays should use the device emulation transform node if exists
> as the transform state.
> 
> Because the node is ready only after PrePaint, we need to let frame
> overlays update graphics layer status at the end of PrePaint, instead of
> previously unclear (maybe during layout, or paint etc.)
> 
> Bug: 934300
> Change-Id: Ic6e2fe5d29f444dba1a3bf206ffa1775df4adc3d
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1525199
> Reviewed-by: Philip Rogers <pdr@chromium.org>
> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#642184}

TBR=wangxianzhu@chromium.org,pdr@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 934300, 945152
Change-Id: Ie50c55532da20dbe44da94c90f93a3ed20bd2a6f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1536912
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#643731}
  • Loading branch information
wangxianzhu authored and Commit Bot committed Mar 24, 2019
1 parent d488b3f commit 24025d1
Show file tree
Hide file tree
Showing 47 changed files with 229 additions and 240 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -476,9 +476,9 @@ String WebDevToolsAgentImpl::EvaluateInOverlayForTesting(const String& script) {
return result;
}

void WebDevToolsAgentImpl::UpdateOverlaysPrePaint() {
void WebDevToolsAgentImpl::UpdateOverlays() {
for (auto& it : overlay_agents_)
it.value->UpdatePrePaint();
it.value->UpdateAllOverlayLifecyclePhases();
}

void WebDevToolsAgentImpl::PaintOverlays(GraphicsContext& context) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class CORE_EXPORT WebDevToolsAgentImpl final
void FlushProtocolNotifications();

bool HasOverlays() const { return !overlay_agents_.IsEmpty(); }
void UpdateOverlaysPrePaint();
void UpdateOverlays();
void PaintOverlays(GraphicsContext&); // For CompositeAfterPaint.

WebInputEventResult HandleInputEvent(const WebInputEvent&);
Expand Down
79 changes: 45 additions & 34 deletions third_party/blink/renderer/core/frame/frame_overlay.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,14 @@
#include "cc/layers/picture_layer.h"
#include "third_party/blink/renderer/core/frame/local_frame.h"
#include "third_party/blink/renderer/core/frame/visual_viewport.h"
#include "third_party/blink/renderer/core/layout/layout_view.h"
#include "third_party/blink/renderer/core/frame/web_frame_widget_base.h"
#include "third_party/blink/renderer/core/frame/web_local_frame_impl.h"
#include "third_party/blink/renderer/core/page/page.h"
#include "third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.h"
#include "third_party/blink/renderer/platform/graphics/graphics_context.h"
#include "third_party/blink/renderer/platform/graphics/graphics_layer.h"
#include "third_party/blink/renderer/platform/graphics/graphics_layer_client.h"
#include "third_party/blink/renderer/platform/graphics/paint/scoped_paint_chunk_properties.h"
#include "third_party/blink/renderer/platform/graphics/paint/paint_record_builder.h"

namespace blink {

Expand All @@ -52,23 +53,33 @@ FrameOverlay::FrameOverlay(LocalFrame* local_frame,
DCHECK(frame_);
}

void FrameOverlay::UpdatePrePaint() {
FrameOverlay::~FrameOverlay() {
if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled())
return;
if (!layer_)
return;
layer_->RemoveFromParent();
layer_ = nullptr;
}

void FrameOverlay::Update() {
if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled()) {
delegate_->Invalidate();
return;
}

auto* parent_layer = frame_->LocalFrameRoot()
.View()
->GetLayoutView()
->Compositor()
->PaintRootGraphicsLayer();
if (!parent_layer) {
layer_ = nullptr;
auto* local_root_frame_widget =
WebLocalFrameImpl::FromFrame(frame_)->LocalRootFrameWidget();
if (!local_root_frame_widget->IsAcceleratedCompositingActive())
return;
}

GraphicsLayer* parent_layer =
frame_->IsMainFrame()
? frame_->GetPage()->GetVisualViewport().ContainerLayer()
: local_root_frame_widget->RootGraphicsLayer();
if (!layer_) {
if (!parent_layer)
return;
layer_ = std::make_unique<GraphicsLayer>(*this);
layer_->SetDrawsContent(true);

Expand All @@ -81,14 +92,14 @@ void FrameOverlay::UpdatePrePaint() {
cc_layer->AddMainThreadScrollingReasons(
cc::MainThreadScrollingReason::kFrameOverlay);
}

layer_->SetLayerState(PropertyTreeState::Root(), IntPoint());
}

DCHECK(parent_layer);
if (layer_->Parent() != parent_layer)
parent_layer->AddChild(layer_.get());
layer_->SetLayerState(DefaultPropertyTreeState(), IntPoint());
layer_->SetSize(gfx::Size(Size()));
layer_->SetNeedsDisplay();
if (parent_layer)
parent_layer->SetPaintArtifactCompositorNeedsUpdate();
}

IntSize FrameOverlay::Size() const {
Expand All @@ -108,37 +119,37 @@ IntRect FrameOverlay::ComputeInterestRect(const GraphicsLayer* graphics_layer,
return IntRect(IntPoint(), Size());
}

void FrameOverlay::EnsureOverlayAttached() const {
DCHECK(layer_);
auto* local_root_frame_widget =
WebLocalFrameImpl::FromFrame(frame_)->LocalRootFrameWidget();
GraphicsLayer* parent_layer =
frame_->GetPage()->MainFrame()->IsLocalFrame()
? frame_->GetPage()->GetVisualViewport().ContainerLayer()
: local_root_frame_widget->RootGraphicsLayer();
DCHECK(parent_layer);
if (layer_->Parent() != parent_layer)
parent_layer->AddChild(layer_.get());
}

void FrameOverlay::PaintContents(const GraphicsLayer* graphics_layer,
GraphicsContext& context,
GraphicsContext& gc,
GraphicsLayerPaintingPhase phase,
const IntRect& interest_rect) const {
DCHECK(!RuntimeEnabledFeatures::CompositeAfterPaintEnabled());
DCHECK_EQ(graphics_layer, layer_.get());
DCHECK_EQ(DefaultPropertyTreeState(), layer_->GetPropertyTreeState());
Paint(context);
DCHECK(layer_);
EnsureOverlayAttached();
delegate_->PaintFrameOverlay(*this, gc, Size());
}

String FrameOverlay::DebugName(const GraphicsLayer*) const {
DCHECK(!RuntimeEnabledFeatures::CompositeAfterPaintEnabled());
return "Frame Overlay Content Layer";
}

void FrameOverlay::Paint(GraphicsContext& context) const {
ScopedPaintChunkProperties properties(context.GetPaintController(),
DefaultPropertyTreeState(), *this,
DisplayItem::kFrameOverlay);
void FrameOverlay::Paint(GraphicsContext& context) {
DCHECK(RuntimeEnabledFeatures::CompositeAfterPaintEnabled());
delegate_->PaintFrameOverlay(*this, context, Size());
}

PropertyTreeState FrameOverlay::DefaultPropertyTreeState() const {
auto state = PropertyTreeState::Root();
if (frame_->IsMainFrame()) {
if (const auto* device_emulation = frame_->GetPage()
->GetVisualViewport()
.GetDeviceEmulationTransformNode())
state.SetTransform(*device_emulation);
}
return state;
}

} // namespace blink
14 changes: 9 additions & 5 deletions third_party/blink/renderer/core/frame/frame_overlay.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class CORE_EXPORT FrameOverlay : public GraphicsLayerClient,
public:
virtual ~Delegate() = default;

// Paints frame overlay contents.
// Paints page overlay contents.
virtual void PaintFrameOverlay(const FrameOverlay&,
GraphicsContext&,
const IntSize& view_size) const = 0;
Expand All @@ -59,11 +59,12 @@ class CORE_EXPORT FrameOverlay : public GraphicsLayerClient,
};

FrameOverlay(LocalFrame*, std::unique_ptr<FrameOverlay::Delegate>);
~FrameOverlay() override;

void UpdatePrePaint();
void Update();

// For CompositeAfterPaint.
void Paint(GraphicsContext&) const;
void Paint(GraphicsContext&);

GraphicsLayer* GetGraphicsLayer() const {
DCHECK(!RuntimeEnabledFeatures::CompositeAfterPaintEnabled());
Expand All @@ -73,6 +74,11 @@ class CORE_EXPORT FrameOverlay : public GraphicsLayerClient,
// FrameOverlay is always the same size as the viewport.
IntSize Size() const;

// Ensure that |layer_| is attached to the root graphics layer. Updates
// to the frames compositing may remove the graphics layer at any
// point. This should be called before calling PaintContents.
void EnsureOverlayAttached() const;

const Delegate* GetDelegate() const { return delegate_.get(); }
const LocalFrame& Frame() const { return *frame_; }

Expand All @@ -90,8 +96,6 @@ class CORE_EXPORT FrameOverlay : public GraphicsLayerClient,
const IntRect& interest_rect) const override;
String DebugName(const GraphicsLayer*) const override;

PropertyTreeState DefaultPropertyTreeState() const;

private:
Persistent<LocalFrame> frame_;
std::unique_ptr<FrameOverlay::Delegate> delegate_;
Expand Down
61 changes: 6 additions & 55 deletions third_party/blink/renderer/core/frame/frame_overlay_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,17 @@
#include "third_party/blink/renderer/core/exported/web_view_impl.h"
#include "third_party/blink/renderer/core/frame/frame_test_helpers.h"
#include "third_party/blink/renderer/core/frame/local_frame_view.h"
#include "third_party/blink/renderer/core/frame/visual_viewport.h"
#include "third_party/blink/renderer/core/frame/web_local_frame_impl.h"
#include "third_party/blink/renderer/platform/graphics/color.h"
#include "third_party/blink/renderer/platform/graphics/graphics_context.h"
#include "third_party/blink/renderer/platform/graphics/paint/drawing_recorder.h"
#include "third_party/blink/renderer/platform/graphics/paint/paint_controller.h"
#include "third_party/blink/renderer/platform/graphics/paint/paint_controller_test.h"
#include "third_party/blink/renderer/platform/graphics/paint/paint_record_builder.h"
#include "third_party/blink/renderer/platform/testing/paint_test_configurations.h"
#include "third_party/skia/include/core/SkCanvas.h"

using testing::ElementsAre;
using testing::_;
using testing::AtLeast;
using testing::Property;

namespace blink {
Expand Down Expand Up @@ -86,9 +85,9 @@ INSTANTIATE_PAINT_TEST_SUITE_P(FrameOverlayTest);

TEST_P(FrameOverlayTest, AcceleratedCompositing) {
std::unique_ptr<FrameOverlay> frame_overlay = CreateSolidYellowOverlay();
frame_overlay->UpdatePrePaint();
EXPECT_EQ(PropertyTreeState::Root(),
frame_overlay->DefaultPropertyTreeState());
frame_overlay->Update();
GetWebView()->MainFrameWidget()->UpdateAllLifecyclePhases(
WebWidget::LifecycleUpdateReason::kTest);

// Ideally, we would get results from the compositor that showed that this
// page overlay actually winds up getting drawn on top of the rest.
Expand All @@ -104,62 +103,14 @@ TEST_P(FrameOverlayTest, AcceleratedCompositing) {
builder.EndRecording()->Playback(&canvas);
} else {
auto* graphics_layer = frame_overlay->GetGraphicsLayer();
EXPECT_EQ(PropertyTreeState::Root(),
graphics_layer->GetPropertyTreeState());
graphics_layer->Paint();
graphics_layer->CapturePaintRecord()->Playback(&canvas);
}
}

TEST_P(FrameOverlayTest, DeviceEmulationScale) {
GetWebView()->SetDeviceEmulationTransform(TransformationMatrix().Scale(1.5));
GetWebView()->MainFrameWidget()->UpdateAllLifecyclePhases(
WebWidget::LifecycleUpdateReason::kTest);

std::unique_ptr<FrameOverlay> frame_overlay = CreateSolidYellowOverlay();
frame_overlay->UpdatePrePaint();
auto* transform = GetWebView()
->MainFrameImpl()
->GetFrame()
->GetPage()
->GetVisualViewport()
.GetDeviceEmulationTransformNode();
EXPECT_EQ(TransformationMatrix().Scale(1.5), transform->Matrix());
const auto& state = frame_overlay->DefaultPropertyTreeState();
EXPECT_EQ(transform, &state.Transform());
EXPECT_EQ(&ClipPaintPropertyNode::Root(), &state.Clip());
EXPECT_EQ(&EffectPaintPropertyNode::Root(), &state.Effect());

auto check_paint_results = [&frame_overlay,
&state](PaintController& paint_controller) {
EXPECT_THAT(
paint_controller.GetDisplayItemList(),
ElementsAre(IsSameId(frame_overlay.get(), DisplayItem::kFrameOverlay)));
EXPECT_THAT(
paint_controller.PaintChunks(),
ElementsAre(IsPaintChunk(
0, 1, PaintChunk::Id(*frame_overlay, DisplayItem::kFrameOverlay),
state)));
};

if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled()) {
auto paint_controller =
std::make_unique<PaintController>(PaintController::kTransient);
GraphicsContext context(*paint_controller);
frame_overlay->Paint(context);
paint_controller->CommitNewDisplayItems();
check_paint_results(*paint_controller);
} else {
auto* graphics_layer = frame_overlay->GetGraphicsLayer();
EXPECT_EQ(state, graphics_layer->GetPropertyTreeState());
graphics_layer->Paint();
check_paint_results(graphics_layer->GetPaintController());
}
}

TEST_P(FrameOverlayTest, VisualRect) {
std::unique_ptr<FrameOverlay> frame_overlay = CreateSolidYellowOverlay();
frame_overlay->UpdatePrePaint();
frame_overlay->Update();
GetWebView()->MainFrameWidget()->UpdateAllLifecyclePhases(
WebWidget::LifecycleUpdateReason::kTest);
EXPECT_EQ(LayoutRect(0, 0, kViewportWidth, kViewportHeight),
Expand Down
23 changes: 18 additions & 5 deletions third_party/blink/renderer/core/frame/local_frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1737,17 +1737,30 @@ void LocalFrame::SetFrameColorOverlay(SkColor color) {

frame_color_overlay_ = std::make_unique<FrameOverlay>(
this, std::make_unique<FrameColorOverlay>(this, color));

// Update compositing which will create graphics layers so the page color
// update below will be able to attach to the root graphics layer.
if (View()) {
View()->UpdateLifecycleToCompositingCleanPlusScrolling();
frame_color_overlay_->Update();
}
}

void LocalFrame::UpdateFrameColorOverlayPrePaint() {
if (frame_color_overlay_)
frame_color_overlay_->UpdatePrePaint();
void LocalFrame::PaintFrameColorOverlay() {
DCHECK(!RuntimeEnabledFeatures::CompositeAfterPaintEnabled());
if (!frame_color_overlay_)
return;
frame_color_overlay_->Update();
if (frame_color_overlay_->GetGraphicsLayer())
frame_color_overlay_->GetGraphicsLayer()->Paint();
}

void LocalFrame::PaintFrameColorOverlay(GraphicsContext& context) {
DCHECK(RuntimeEnabledFeatures::CompositeAfterPaintEnabled());
if (frame_color_overlay_)
frame_color_overlay_->Paint(context);
if (!frame_color_overlay_)
return;
frame_color_overlay_->Update();
frame_color_overlay_->Paint(context);
}

void LocalFrame::ForciblyPurgeV8Memory() {
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/frame/local_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ class CORE_EXPORT LocalFrame final : public Frame,
// Overlays a color on top of this LocalFrameView if it is associated with
// a subframe. Should not have multiple consumers.
void SetSubframeColorOverlay(SkColor color);
void UpdateFrameColorOverlayPrePaint();
void PaintFrameColorOverlay();

// For CompositeAfterPaint.
void PaintFrameColorOverlay(GraphicsContext&);
Expand Down
Loading

0 comments on commit 24025d1

Please sign in to comment.