Skip to content

Commit

Permalink
Implement per-view invalidation
Browse files Browse the repository at this point in the history
Only paint views that were explicitly invalidated (i.e. SchedulePaint
was invoked on that given view) as opposed to painting all views that
intersect an invalid rectangle on the layer. The change is
implemented behind a feature flag that is disabled by default.

Bug: 974349
Change-Id: I6feedaa56bc0c451aacef7e2b9eb5edb757397dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1694261
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Annie Su <anniesu@google.com>
Cr-Commit-Position: refs/heads/master@{#684970}
  • Loading branch information
Annie Su authored and Commit Bot committed Aug 7, 2019
1 parent b196f07 commit 50eee02
Show file tree
Hide file tree
Showing 14 changed files with 191 additions and 65 deletions.
28 changes: 13 additions & 15 deletions ash/frame/non_client_frame_view_ash.cc
Original file line number Diff line number Diff line change
Expand Up @@ -398,26 +398,17 @@ gfx::Size NonClientFrameViewAsh::GetMaximumSize() const {
return gfx::Size(width, height);
}

void NonClientFrameViewAsh::SchedulePaintInRect(const gfx::Rect& r) {
// We may end up here before |header_view_| has been added to the Widget.
if (header_view_->GetWidget()) {
// The HeaderView is not a child of NonClientFrameViewAsh. Redirect the
// paint to HeaderView instead.
gfx::RectF to_paint(r);
views::View::ConvertRectToTarget(this, header_view_, &to_paint);
header_view_->SchedulePaintInRect(gfx::ToEnclosingRect(to_paint));
} else {
views::NonClientFrameView::SchedulePaintInRect(r);
}
}

void NonClientFrameViewAsh::SetVisible(bool visible) {
overlay_view_->SetVisible(visible);
views::View::SetVisible(visible);
// We need to re-layout so that client view will occupy entire window.
InvalidateLayout();
}

void NonClientFrameViewAsh::SetShouldPaintHeader(bool paint) {
header_view_->SetShouldPaintHeader(paint);
}

const views::View* NonClientFrameViewAsh::GetAvatarIconViewForTest() const {
return header_view_->avatar_icon();
}
Expand All @@ -430,8 +421,15 @@ SkColor NonClientFrameViewAsh::GetInactiveFrameColorForTest() const {
return frame_->GetNativeWindow()->GetProperty(kFrameInactiveColorKey);
}

void NonClientFrameViewAsh::SetShouldPaintHeader(bool paint) {
header_view_->SetShouldPaintHeader(paint);
void NonClientFrameViewAsh::OnDidSchedulePaint(const gfx::Rect& r) {
// We may end up here before |header_view_| has been added to the Widget.
if (header_view_->GetWidget()) {
// The HeaderView is not a child of NonClientFrameViewAsh. Redirect the
// paint to HeaderView instead.
gfx::RectF to_paint(r);
views::View::ConvertRectToTarget(this, header_view_, &to_paint);
header_view_->SchedulePaintInRect(gfx::ToEnclosingRect(to_paint));
}
}

////////////////////////////////////////////////////////////////////////////////
Expand Down
5 changes: 4 additions & 1 deletion ash/frame/non_client_frame_view_ash.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ class ASH_EXPORT NonClientFrameViewAsh : public views::NonClientFrameView {
const char* GetClassName() const override;
gfx::Size GetMinimumSize() const override;
gfx::Size GetMaximumSize() const override;
void SchedulePaintInRect(const gfx::Rect& r) override;
void SetVisible(bool visible) override;

// If |paint| is false, we should not paint the header. Used for overview mode
Expand All @@ -108,6 +107,10 @@ class ASH_EXPORT NonClientFrameViewAsh : public views::NonClientFrameView {

views::Widget* frame() { return frame_; }

protected:
// views::View:
void OnDidSchedulePaint(const gfx::Rect& r) override;

private:
class OverlayView;
friend class NonClientFrameViewAshTestWidgetDelegate;
Expand Down
2 changes: 1 addition & 1 deletion ui/views/animation/bounds_animator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class TestView : public View {
public:
TestView() = default;

void SchedulePaintInRect(const gfx::Rect& r) override {
void OnDidSchedulePaint(const gfx::Rect& r) override {
if (dirty_rect_.IsEmpty())
dirty_rect_ = r;
else
Expand Down
9 changes: 5 additions & 4 deletions ui/views/controls/button/label_button_label_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ class TestLabel : public LabelButtonLabel {
last_color_(last_color) {}

// LabelButtonLabel:
void SchedulePaintInRect(const gfx::Rect& r) override {
LabelButtonLabel::SchedulePaintInRect(r);
void OnDidSchedulePaint(const gfx::Rect& r) override {
LabelButtonLabel::OnDidSchedulePaint(r);
*last_color_ = GetEnabledColor();
}

Expand Down Expand Up @@ -76,8 +76,9 @@ class LabelButtonLabelTest : public ViewsTestBase {

// Test that LabelButtonLabel reacts properly to themed and overridden colors.
TEST_F(LabelButtonLabelTest, Colors) {
// The SchedulePaintInRect override won't be called while the base class is
// initialized. Not much we can do about that, so give it the first for free.
// The OnDidSchedulePaint() override won't be called while the base
// class is initialized. Not much we can do about that, so give it the first
// for free.
EXPECT_EQ(SK_ColorCYAN, last_color_); // Sanity check.

// At the same time we can check that changing the auto color readability
Expand Down
4 changes: 2 additions & 2 deletions ui/views/controls/label_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ class TestLabel : public Label {
}

// View:
void SchedulePaintInRect(const gfx::Rect& r) override {
void OnDidSchedulePaint(const gfx::Rect& r) override {
++schedule_paint_count_;
Label::SchedulePaintInRect(r);
Label::OnDidSchedulePaint(r);
}

private:
Expand Down
25 changes: 19 additions & 6 deletions ui/views/paint_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
// found in the LICENSE file.

#include "ui/views/paint_info.h"
#include "base/feature_list.h"
#include "ui/views/views_features.h"

namespace views {
namespace {
Expand Down Expand Up @@ -68,9 +70,10 @@ PaintInfo PaintInfo::CreateChildPaintInfo(const PaintInfo& parent_paint_info,
const gfx::Rect& bounds,
const gfx::Size& parent_size,
ScaleType scale_type,
bool is_layer) {
return PaintInfo(parent_paint_info, bounds, parent_size, scale_type,
is_layer);
bool is_layer,
bool needs_paint) {
return PaintInfo(parent_paint_info, bounds, parent_size, scale_type, is_layer,
needs_paint);
}

PaintInfo::~PaintInfo() = default;
Expand All @@ -79,13 +82,21 @@ bool PaintInfo::IsPixelCanvas() const {
return context().is_pixel_canvas();
}

bool PaintInfo::ShouldPaint() const {
if (base::FeatureList::IsEnabled(features::kEnableViewPaintOptimization))
return needs_paint_;

return context().IsRectInvalid(gfx::Rect(paint_recording_size()));
}

PaintInfo::PaintInfo(const PaintInfo& other)
: paint_recording_scale_x_(other.paint_recording_scale_x_),
paint_recording_scale_y_(other.paint_recording_scale_y_),
paint_recording_bounds_(other.paint_recording_bounds_),
offset_from_parent_(other.offset_from_parent_),
context_(other.context(), gfx::Vector2d()),
root_context_(nullptr) {}
root_context_(nullptr),
needs_paint_(false) {}

// The root layer should use the ScaleToEnclosingRect, the same logic that
// cc(chrome compositor) is using.
Expand All @@ -104,7 +115,8 @@ PaintInfo::PaintInfo(const PaintInfo& parent_paint_info,
const gfx::Rect& bounds,
const gfx::Size& parent_size,
ScaleType scale_type,
bool is_layer)
bool is_layer,
bool needs_paint)
: paint_recording_scale_x_(1.f),
paint_recording_scale_y_(1.f),
paint_recording_bounds_(
Expand All @@ -116,7 +128,8 @@ PaintInfo::PaintInfo(const PaintInfo& parent_paint_info,
paint_recording_bounds_.OffsetFromOrigin() -
parent_paint_info.paint_recording_bounds_.OffsetFromOrigin()),
context_(parent_paint_info.context(), offset_from_parent_),
root_context_(nullptr) {
root_context_(nullptr),
needs_paint_(needs_paint) {
if (IsPixelCanvas()) {
if (scale_type == ScaleType::kUniformScaling) {
paint_recording_scale_x_ = paint_recording_scale_y_ =
Expand Down
14 changes: 12 additions & 2 deletions ui/views/paint_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,19 @@ class VIEWS_EXPORT PaintInfo {
const gfx::Rect& bounds,
const gfx::Size& parent_size,
ScaleType scale_type,
bool is_layer);
bool is_layer,
bool needs_paint = false);

PaintInfo(const PaintInfo& other);
~PaintInfo();

// Returns true if all paint commands are recorded at pixel size.
bool IsPixelCanvas() const;

// Returns true if the View should be painted based on whether per-view
// invalidation is enabled or not.
bool ShouldPaint() const;

const ui::PaintContext& context() const {
return root_context_ ? *root_context_ : context_;
}
Expand Down Expand Up @@ -85,7 +90,8 @@ class VIEWS_EXPORT PaintInfo {
const gfx::Rect& bounds,
const gfx::Size& parent_size,
ScaleType scale_type,
bool is_layer);
bool is_layer,
bool needs_paint = false);

// Scales the |child_bounds| to its recording bounds based on the
// |context.device_scale_factor()|. The recording bounds are snapped to the
Expand All @@ -112,6 +118,10 @@ class VIEWS_EXPORT PaintInfo {
// Compositor PaintContext associated with the view this object belongs to.
ui::PaintContext context_;
const ui::PaintContext* root_context_;

// True if the individual View has been marked invalid for paint (i.e.
// SchedulePaint() was invoked on the View).
bool needs_paint_ = false;
};

} // namespace views
Expand Down
42 changes: 28 additions & 14 deletions ui/views/view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "base/command_line.h"
#include "base/containers/adapters.h"
#include "base/feature_list.h"
#include "base/logging.h"
#include "base/macros.h"
#include "base/stl_util.h"
Expand Down Expand Up @@ -52,6 +53,7 @@
#include "ui/views/metadata/metadata_impl_macros.h"
#include "ui/views/view_observer.h"
#include "ui/views/view_tracker.h"
#include "ui/views/views_features.h"
#include "ui/views/views_switches.h"
#include "ui/views/widget/native_widget_private.h"
#include "ui/views/widget/root_view.h"
Expand Down Expand Up @@ -894,16 +896,8 @@ void View::SchedulePaint() {
}

void View::SchedulePaintInRect(const gfx::Rect& rect) {
if (!visible_)
return;

if (layer()) {
layer()->SchedulePaint(rect);
} else if (parent_) {
// Translate the requested paint rect to the parent's coordinate system
// then pass this notification up to the parent.
parent_->SchedulePaintInRect(ConvertRectToParent(rect));
}
needs_paint_ = true;
SchedulePaintInRectImpl(rect);
}

void View::Paint(const PaintInfo& parent_paint_info) {
Expand All @@ -915,11 +909,17 @@ void View::Paint(const PaintInfo& parent_paint_info) {

PaintInfo paint_info = PaintInfo::CreateChildPaintInfo(
parent_paint_info, GetMirroredBounds(), parent_bounds.size(),
GetPaintScaleType(), !!layer());
GetPaintScaleType(), !!layer(), needs_paint_);

needs_paint_ = false;

const ui::PaintContext& context = paint_info.context();
bool is_invalidated = true;
if (paint_info.context().CanCheckInvalid()) {
if (paint_info.context().CanCheckInvalid() ||
base::FeatureList::IsEnabled(features::kEnableViewPaintOptimization)) {
// For View paint optimization, do not default to repainting every View in
// the View hierarchy if the invalidation rect is empty. Repainting does not
// depend on the invalidation rect for View paint optimization.
#if DCHECK_IS_ON()
if (!context.is_pixel_canvas()) {
gfx::Vector2d offset;
Expand All @@ -943,8 +943,7 @@ void View::Paint(const PaintInfo& parent_paint_info) {

// If the View wasn't invalidated, don't waste time painting it, the output
// would be culled.
is_invalidated =
context.IsRectInvalid(gfx::Rect(paint_info.paint_recording_size()));
is_invalidated = paint_info.ShouldPaint();
}

TRACE_EVENT1("views", "View::Paint", "class", GetClassName());
Expand Down Expand Up @@ -1651,6 +1650,8 @@ void View::RemovedFromWidget() {}

// Painting --------------------------------------------------------------------

void View::OnDidSchedulePaint(const gfx::Rect& rect) {}

void View::PaintChildren(const PaintInfo& paint_info) {
TRACE_EVENT1("views", "View::PaintChildren", "class", GetClassName());
RecursivePaintHelper(&View::Paint, paint_info);
Expand Down Expand Up @@ -2031,6 +2032,19 @@ void View::DragInfo::PossibleDrag(const gfx::Point& p) {

// Painting --------------------------------------------------------------------

void View::SchedulePaintInRectImpl(const gfx::Rect& rect) {
OnDidSchedulePaint(rect);
if (!visible_)
return;
if (layer()) {
layer()->SchedulePaint(rect);
} else if (parent_) {
// Translate the requested paint rect to the parent's coordinate system
// then pass this notification up to the parent.
parent_->SchedulePaintInRectImpl(ConvertRectToParent(rect));
}
}

void View::SchedulePaintBoundsChanged(bool size_changed) {
if (!visible_)
return;
Expand Down
23 changes: 19 additions & 4 deletions ui/views/view.h
Original file line number Diff line number Diff line change
Expand Up @@ -842,12 +842,9 @@ class VIEWS_EXPORT View : public ui::LayerDelegate,

// Mark all or part of the View's bounds as dirty (needing repaint).
// |r| is in the View's coordinates.
// Rectangle |r| should be in the view's coordinate system. The
// transformations are applied to it to convert it into the parent coordinate
// system before propagating SchedulePaint up the view hierarchy.
// TODO(beng): Make protected.
void SchedulePaint();
virtual void SchedulePaintInRect(const gfx::Rect& r);
void SchedulePaintInRect(const gfx::Rect& r);

// Called by the framework to paint a View. Performs translation and clipping
// for View coordinates and language direction as required, allows the View
Expand Down Expand Up @@ -1460,6 +1457,11 @@ class VIEWS_EXPORT View : public ui::LayerDelegate,

// Painting ------------------------------------------------------------------

// Override to control paint redirection or to provide a different Rectangle
// |r| to be repainted. This is a function with an empty implementation in
// view.cc and is purely intended for subclasses to override.
virtual void OnDidSchedulePaint(const gfx::Rect& r);

// Responsible for calling Paint() on child Views. Override to control the
// order child Views are painted.
virtual void PaintChildren(const PaintInfo& info);
Expand Down Expand Up @@ -1611,6 +1613,16 @@ class VIEWS_EXPORT View : public ui::LayerDelegate,

// Painting -----------------------------------------------------------------

// Responsible for propagating SchedulePaint() to the view's layer. If there
// is no associated layer, the requested paint rect is propagated up the
// view hierarchy by calling this function on the parent view. Rectangle |r|
// is in the view's coordinate system. The transformations are applied to it
// to convert it into the parent coordinate system before propagating
// SchedulePaint() up the view hierarchy. This function should NOT be directly
// called. Instead call SchedulePaint() or SchedulePaintInRect(), which will
// call into this as necessary.
void SchedulePaintInRectImpl(const gfx::Rect& r);

// Invoked before and after the bounds change to schedule painting the old and
// new bounds.
void SchedulePaintBoundsChanged(bool size_changed);
Expand Down Expand Up @@ -1963,6 +1975,9 @@ class VIEWS_EXPORT View : public ui::LayerDelegate,
// Cached output of painting to be reused in future frames until invalidated.
ui::PaintCache paint_cache_;

// Whether SchedulePaintInRect() was invoked on this View.
bool needs_paint_ = false;

// Native theme --------------------------------------------------------------

// A native theme for this view and its descendants. Typically null, in which
Expand Down

0 comments on commit 50eee02

Please sign in to comment.