Skip to content

Commit

Permalink
Refactor cc painted scrollbar
Browse files Browse the repository at this point in the history
- Remove unused cc/layers/scrollbar_theme_painter.h
- Add comments in cc::Scrollbar
- Remove rect parameter from cc::Scrollbar::PaintPart() because the
  rect can be easily calculated by blink, given that the coordinate
  space of the canvas is clearly defined
- Improve performance of blink::ScrollbarLayerDelegate::HasTickMarks
  (implementation of cc::Scrollbar::HasTickMarks()). The old
  implementation queried all tick marks and see if it was empty, which
  may be slow when there are a lot of tick marks (e.g. when searching
  in a huge document).
- Add blink::ScrollbarTheme::PaintTrackAndButtonsForCompositor() to
  avoid exposing too much details of ScrollbarTheme to outside.

This is a preparation for CompositeAfterPaint composited scrollbar
implementation.

Change-Id: Ibf8c7a90d80ddf972f215495d0fd89470fa2deb4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1834286
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702201}
  • Loading branch information
wangxianzhu authored and Commit Bot committed Oct 2, 2019
1 parent 5e38ff2 commit 7920cf5
Show file tree
Hide file tree
Showing 28 changed files with 233 additions and 258 deletions.
1 change: 0 additions & 1 deletion build/check_gn_headers_whitelist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ cc/input/event_listener_properties.h
cc/input/scrollbar.h
cc/input/scroller_size_metrics.h
cc/layers/performance_properties.h
cc/layers/scrollbar_theme_painter.h
cc/output/bsp_compare_result.h
cc/resources/release_callback_impl.h
cc/resources/return_callback.h
Expand Down
18 changes: 14 additions & 4 deletions cc/input/scrollbar.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ static constexpr int kDefaultWinScrollbarThickness = 17;
namespace cc {

enum ScrollbarOrientation { HORIZONTAL, VERTICAL };
enum ScrollDirection { SCROLL_BACKWARD, SCROLL_FORWARD };

enum ScrollbarPart {
THUMB,
Expand All @@ -55,15 +54,26 @@ class Scrollbar {
virtual bool SupportsDragSnapBack() const = 0;
virtual int ThumbThickness() const = 0;
virtual int ThumbLength() const = 0;

// Returns the track rect relative to the scrollbar's origin.
virtual gfx::Rect TrackRect() const = 0;
// Returns the back button rect relative to the scrollbar's origin.
virtual gfx::Rect BackButtonRect() const = 0;
// Returns the forward button rect relative to the scrollbar's origin.
virtual gfx::Rect ForwardButtonRect() const = 0;

virtual float ThumbOpacity() const = 0;
virtual bool HasTickmarks() const = 0;

// Whether we need to repaint the part. Only THUMB and TRACK are supported.
// For TRACK, the return value means that the track, any buttons or tickmarks
// need repaint.
virtual bool NeedsPaintPart(ScrollbarPart part) const = 0;
virtual void PaintPart(PaintCanvas* canvas,
ScrollbarPart part,
const gfx::Rect& content_rect) = 0;
// Paints the part. Only THUMB, TRACK and TICKMARKS are supported. When TRACK
// is specified, track, buttons and tickmarks will be painted. The canvas's
// coordinate space is relative to the part's origin.
virtual void PaintPart(PaintCanvas* canvas, ScrollbarPart part) = 0;

virtual bool UsesNinePatchThumbResource() const = 0;
virtual gfx::Size NinePatchThumbCanvasSize() const = 0;
virtual gfx::Rect NinePatchThumbAperture() const = 0;
Expand Down
25 changes: 6 additions & 19 deletions cc/layers/painted_overlay_scrollbar_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ PaintedOverlayScrollbarLayer::PaintedOverlayScrollbarLayer(
scroll_element_id_(scroll_element_id),
thumb_thickness_(scrollbar_->ThumbThickness()),
thumb_length_(scrollbar_->ThumbLength()) {
DCHECK(scrollbar_->HasThumb());
DCHECK(scrollbar_->IsOverlay());
DCHECK(scrollbar_->UsesNinePatchThumbResource());
SetIsScrollbar(true);
}
Expand Down Expand Up @@ -117,9 +119,6 @@ bool PaintedOverlayScrollbarLayer::Update() {
bool updated = false;
updated |= Layer::Update();

DCHECK(scrollbar_->HasThumb());
DCHECK(scrollbar_->IsOverlay());
DCHECK(scrollbar_->UsesNinePatchThumbResource());
updated |= UpdateProperty(scrollbar_->TrackRect(), &track_rect_);
updated |= UpdateProperty(scrollbar_->Location(), &location_);
updated |= UpdateProperty(scrollbar_->ThumbThickness(), &thumb_thickness_);
Expand All @@ -143,15 +142,9 @@ bool PaintedOverlayScrollbarLayer::PaintThumbIfNeeded() {
SkBitmap skbitmap;
skbitmap.allocN32Pixels(paint_rect.width(), paint_rect.height());
SkiaPaintCanvas canvas(skbitmap);
canvas.clear(SK_ColorTRANSPARENT);

SkRect content_skrect = RectToSkRect(paint_rect);
PaintFlags flags;
flags.setAntiAlias(false);
flags.setBlendMode(SkBlendMode::kClear);
canvas.drawRect(content_skrect, flags);
canvas.clipRect(content_skrect);

scrollbar_->PaintPart(&canvas, THUMB, paint_rect);
scrollbar_->PaintPart(&canvas, THUMB);
// Make sure that the pixels are no longer mutable to unavoid unnecessary
// allocation and copying.
skbitmap.setImmutable();
Expand Down Expand Up @@ -183,15 +176,9 @@ bool PaintedOverlayScrollbarLayer::PaintTickmarks() {
SkBitmap skbitmap;
skbitmap.allocN32Pixels(paint_rect.width(), paint_rect.height());
SkiaPaintCanvas canvas(skbitmap);
canvas.clear(SK_ColorTRANSPARENT);

SkRect content_skrect = RectToSkRect(paint_rect);
PaintFlags flags;
flags.setAntiAlias(false);
flags.setBlendMode(SkBlendMode::kClear);
canvas.drawRect(content_skrect, flags);
canvas.clipRect(content_skrect);

scrollbar_->PaintPart(&canvas, TICKMARKS, paint_rect);
scrollbar_->PaintPart(&canvas, TICKMARKS);
// Make sure that the pixels are no longer mutable to unavoid unnecessary
// allocation and copying.
skbitmap.setImmutable();
Expand Down
1 change: 0 additions & 1 deletion cc/layers/painted_overlay_scrollbar_layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include "cc/input/scrollbar.h"
#include "cc/layers/layer.h"
#include "cc/layers/scrollbar_layer_interface.h"
#include "cc/layers/scrollbar_theme_painter.h"
#include "cc/resources/scoped_ui_resource.h"

namespace cc {
Expand Down
4 changes: 1 addition & 3 deletions cc/layers/painted_overlay_scrollbar_layer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ class MockScrollbar : public FakeScrollbar {
public:
MockScrollbar() : FakeScrollbar(true, true, true) {}

void PaintPart(PaintCanvas* canvas,
ScrollbarPart part,
const gfx::Rect& content_rect) override {
void PaintPart(PaintCanvas* canvas, ScrollbarPart part) override {
if (part == TICKMARKS)
paint_tickmarks_called_ = true;
}
Expand Down
7 changes: 1 addition & 6 deletions cc/layers/painted_scrollbar_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -263,13 +263,8 @@ UIResourceBitmap PaintedScrollbarLayer::RasterizeScrollbarPart(
float scale_y =
content_rect.height() / static_cast<float>(layer_rect.height());
canvas.scale(SkFloatToScalar(scale_x), SkFloatToScalar(scale_y));
// TODO(pdr): Scrollbars are painted with an offset (see Scrollbar::PaintPart)
// and the canvas is translated so that scrollbars are drawn at the origin.
// Refactor this code to not use an offset at all so Scrollbar::PaintPart
// paints at the origin and no translation is needed below.
canvas.translate(-layer_rect.x(), -layer_rect.y());

scrollbar_->PaintPart(&canvas, part, layer_rect);
scrollbar_->PaintPart(&canvas, part);
// Make sure that the pixels are no longer mutable to unavoid unnecessary
// allocation and copying.
skbitmap.setImmutable();
Expand Down
1 change: 0 additions & 1 deletion cc/layers/painted_scrollbar_layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include "cc/input/scrollbar.h"
#include "cc/layers/layer.h"
#include "cc/layers/scrollbar_layer_interface.h"
#include "cc/layers/scrollbar_theme_painter.h"
#include "cc/resources/scoped_ui_resource.h"

namespace cc {
Expand Down
26 changes: 13 additions & 13 deletions cc/layers/painted_scrollbar_layer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ namespace {

class MockScrollbar : public FakeScrollbar {
public:
MockScrollbar() : FakeScrollbar(true, true, true) {}
MOCK_METHOD3(PaintPart,
void(PaintCanvas* canvas,
ScrollbarPart part,
const gfx::Rect& content_rect));
MockScrollbar()
: FakeScrollbar(/*paint*/ true,
/*has_thumb*/ true,
/*is_overlay*/ false) {}
MOCK_METHOD2(PaintPart, void(PaintCanvas* canvas, ScrollbarPart part));
};

TEST(PaintedScrollbarLayerTest, NeedsPaint) {
Expand All @@ -52,28 +52,28 @@ TEST(PaintedScrollbarLayerTest, NeedsPaint) {
// yet been initialized.
scrollbar->set_needs_paint_thumb(false);
scrollbar->set_needs_paint_track(false);
EXPECT_CALL(*scrollbar, PaintPart(_, THUMB, _)).Times(1);
EXPECT_CALL(*scrollbar, PaintPart(_, TRACK, _)).Times(1);
EXPECT_CALL(*scrollbar, PaintPart(_, THUMB)).Times(1);
EXPECT_CALL(*scrollbar, PaintPart(_, TRACK)).Times(1);
scrollbar_layer->Update();
Mock::VerifyAndClearExpectations(scrollbar);

// The next update will paint nothing because the first update caused a paint.
EXPECT_CALL(*scrollbar, PaintPart(_, THUMB, _)).Times(0);
EXPECT_CALL(*scrollbar, PaintPart(_, TRACK, _)).Times(0);
EXPECT_CALL(*scrollbar, PaintPart(_, THUMB)).Times(0);
EXPECT_CALL(*scrollbar, PaintPart(_, TRACK)).Times(0);
scrollbar_layer->Update();
Mock::VerifyAndClearExpectations(scrollbar);

// Enable the thumb.
EXPECT_CALL(*scrollbar, PaintPart(_, THUMB, _)).Times(1);
EXPECT_CALL(*scrollbar, PaintPart(_, TRACK, _)).Times(0);
EXPECT_CALL(*scrollbar, PaintPart(_, THUMB)).Times(1);
EXPECT_CALL(*scrollbar, PaintPart(_, TRACK)).Times(0);
scrollbar->set_needs_paint_thumb(true);
scrollbar->set_needs_paint_track(false);
scrollbar_layer->Update();
Mock::VerifyAndClearExpectations(scrollbar);

// Enable the track.
EXPECT_CALL(*scrollbar, PaintPart(_, THUMB, _)).Times(0);
EXPECT_CALL(*scrollbar, PaintPart(_, TRACK, _)).Times(1);
EXPECT_CALL(*scrollbar, PaintPart(_, THUMB)).Times(0);
EXPECT_CALL(*scrollbar, PaintPart(_, TRACK)).Times(1);
scrollbar->set_needs_paint_thumb(false);
scrollbar->set_needs_paint_track(true);
scrollbar_layer->Update();
Expand Down
6 changes: 5 additions & 1 deletion cc/layers/scrollbar_layer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,9 @@ TEST_F(ScrollbarLayerTest, RepaintOverlayWhenResourceDisposed) {

class FakeNinePatchScrollbar : public FakeScrollbar {
public:
FakeNinePatchScrollbar()
: FakeScrollbar(/*paint*/ true, /*has_thumb*/ true, /*is_overlay*/ true) {
}
bool UsesNinePatchThumbResource() const override { return true; }
};

Expand Down Expand Up @@ -1354,7 +1357,8 @@ class ScaledScrollbarLayerTestScaledRasterization : public ScrollbarLayerTest {
scrollbar_layer->SetBounds(scrollbar_rect.size());
scrollbar_layer->SetPosition(gfx::PointF(scrollbar_rect.origin()));
scrollbar_layer->fake_scrollbar()->set_location(scrollbar_rect.origin());
scrollbar_layer->fake_scrollbar()->set_track_rect(scrollbar_rect);
scrollbar_layer->fake_scrollbar()->set_track_rect(
gfx::Rect(scrollbar_rect.size()));

layer_tree_host_->SetViewportRectAndScale(
layer_tree_host_->device_viewport_rect(), test_scale,
Expand Down
44 changes: 0 additions & 44 deletions cc/layers/scrollbar_theme_painter.h

This file was deleted.

30 changes: 21 additions & 9 deletions cc/test/fake_scrollbar.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,7 @@ bool FakeScrollbar::HasTickmarks() const {
return has_tickmarks_;
}

void FakeScrollbar::PaintPart(PaintCanvas* canvas,
ScrollbarPart part,
const gfx::Rect& content_rect) {
void FakeScrollbar::PaintPart(PaintCanvas* canvas, ScrollbarPart part) {
if (!paint_)
return;

Expand All @@ -100,12 +98,7 @@ void FakeScrollbar::PaintPart(PaintCanvas* canvas,
flags.setAntiAlias(false);
flags.setColor(paint_fill_color());
flags.setStyle(PaintFlags::kFill_Style);

// Emulate the how the real scrollbar works by using scrollbar's rect for
// TRACK and the given content_rect for the THUMB
SkRect rect = part == TRACK ? RectToSkRect(TrackRect())
: RectToSkRect(content_rect);
canvas->drawRect(rect, flags);
canvas->drawRect(RectToSkRect(GetPartRect(part)), flags);
}

bool FakeScrollbar::UsesNinePatchThumbResource() const {
Expand All @@ -120,4 +113,23 @@ gfx::Rect FakeScrollbar::NinePatchThumbAperture() const {
return gfx::Rect();
}

gfx::Rect FakeScrollbar::GetPartRect(ScrollbarPart part) const {
switch (part) {
case THUMB:
if (UsesNinePatchThumbResource())
return gfx::Rect(NinePatchThumbCanvasSize());
if (Orientation() == VERTICAL)
return gfx::Rect(ThumbThickness(), ThumbLength());
return gfx::Rect(ThumbLength(), ThumbThickness());
case TRACK:
return gfx::UnionRects(
TrackRect(), gfx::UnionRects(BackButtonRect(), ForwardButtonRect()));
case TICKMARKS:
return gfx::Rect(TrackRect().size());
default:
NOTREACHED();
return gfx::Rect();
}
}

} // namespace cc
6 changes: 3 additions & 3 deletions cc/test/fake_scrollbar.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,7 @@ class FakeScrollbar : public Scrollbar {
float ThumbOpacity() const override;
bool NeedsPaintPart(ScrollbarPart part) const override;
bool HasTickmarks() const override;
void PaintPart(PaintCanvas* canvas,
ScrollbarPart part,
const gfx::Rect& content_rect) override;
void PaintPart(PaintCanvas* canvas, ScrollbarPart part) override;
bool UsesNinePatchThumbResource() const override;
gfx::Size NinePatchThumbCanvasSize() const override;
gfx::Rect NinePatchThumbAperture() const override;
Expand All @@ -65,6 +63,8 @@ class FakeScrollbar : public Scrollbar {
}
void set_has_tickmarks(bool has_tickmarks) { has_tickmarks_ = has_tickmarks; }

gfx::Rect GetPartRect(ScrollbarPart part) const;

private:
bool paint_;
bool has_thumb_;
Expand Down
Loading

0 comments on commit 7920cf5

Please sign in to comment.