diff options
author | jyasskin <jyasskin@chromium.org> | 2014-11-17 17:46:35 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2014-11-18 01:46:55 +0000 |
commit | 777195d442721886dba367a28ca74e45cdc09edc (patch) | |
tree | d95e67805d76df6d7d68c5bb3e97f41dee3addb3 /cc | |
parent | 1ae48cc1358ff6367a0645f6027f3303266c21d7 (diff) | |
download | chromium_src-777195d442721886dba367a28ca74e45cdc09edc.zip chromium_src-777195d442721886dba367a28ca74e45cdc09edc.tar.gz chromium_src-777195d442721886dba367a28ca74e45cdc09edc.tar.bz2 |
Revert of cc: Toggle LCD text at raster time instead of record time. (patchset #21 id:460001 of https://codereview.chromium.org/684543006/)
Reason for revert:
Broke several MSan bots: http://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20MSan%20Browser%20%281%29/builds/2669,
http://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20MSan%20Browser%20%282%29/builds/2648,
and
http://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20MSan%20Browser%20%283%29/builds/2677
show traces like:
==9== WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x7ffe6939a3ce in
cc::PicturePile::UpdateAndExpandInvalidation(cc::ContentLayerClient*,
cc::Region*, unsigned int, bool, bool, bool, gfx::Size const&, gfx::Rect const&,
int, cc::Picture::RecordingMode) cc/resources/picture_pile.cc:215:7
#1 0x7ffe692dc068 in cc::PictureLayer::Update(cc::ResourceUpdateQueue*,
cc::OcclusionTracker<cc::Layer> const*) cc/layers/picture_layer.cc:133:14
#2 0x7ffe69424ea5 in
cc::LayerTreeHost::PaintLayerContents(cc::RenderSurfaceLayerList const&,
cc::ResourceUpdateQueue*, bool*, bool*) cc/trees/layer_tree_host.cc:1049:29
#3 0x7ffe6942343d in cc::LayerTreeHost::UpdateLayers(cc::Layer*,
cc::ResourceUpdateQueue*) cc/trees/layer_tree_host.cc:886:3
#4 0x7ffe6942260c in
cc::LayerTreeHost::UpdateLayers(cc::ResourceUpdateQueue*)
cc/trees/layer_tree_host.cc:758:17
#5 0x7ffe694aa463 in
cc::ThreadProxy::BeginMainFrame(scoped_ptr<cc::ThreadProxy::BeginMainFrameAndCommitState,
base::DefaultDeleter<cc::ThreadProxy::BeginMainFrameAndCommitState> >)
cc/trees/thread_proxy.cc:779:18
#6 0x7ffe694b6cc9 in Run base/bind_internal.h:190:12
#7 0x7ffe694b6cc9 in base::internal::InvokeHelper<true, void,
base::internal::RunnableAdapter<void
(cc::ThreadProxy::*)(scoped_ptr<cc::ThreadProxy::BeginMainFrameAndCommitState,
base::DefaultDeleter<cc::ThreadProxy::BeginMainFrameAndCommitState> >)>, void
(base::WeakPtr<cc::ThreadProxy> const&,
scoped_ptr<cc::ThreadProxy::BeginMainFrameAndCommitState,
base::DefaultDeleter<cc::ThreadProxy::BeginMainFrameAndCommitState>
)>::MakeItSo(base::internal::RunnableAdapter<void
(cc::ThreadProxy::*)(scoped_ptr<cc::ThreadProxy::BeginMainFrameAndCommitState,
base::DefaultDeleter<cc::ThreadProxy::BeginMainFrameAndCommitState> >)>,
base::WeakPtr<cc::ThreadProxy> const&,
scoped_ptr<cc::ThreadProxy::BeginMainFrameAndCommitState,
base::DefaultDeleter<cc::ThreadProxy::BeginMainFrameAndCommitState> >)
base/bind_internal.h:909:0
#8 0x7ffe694b6a55 in base::internal::Invoker<2,
base::internal::BindState<base::internal::RunnableAdapter<void
(cc::ThreadProxy::*)(scoped_ptr<cc::ThreadProxy::BeginMainFrameAndCommitState,
base::DefaultDeleter<cc::ThreadProxy::BeginMainFrameAndCommitState> >)>, void
(cc::ThreadProxy*, scoped_ptr<cc::ThreadProxy::BeginMainFrameAndCommitState,
base::DefaultDeleter<cc::ThreadProxy::BeginMainFrameAndCommitState> >), void
(base::WeakPtr<cc::ThreadProxy>,
base::internal::PassedWrapper<scoped_ptr<cc::ThreadProxy::BeginMainFrameAndCommitState,
base::DefaultDeleter<cc::ThreadProxy::BeginMainFrameAndCommitState> > >)>, void
(cc::ThreadProxy*, scoped_ptr<cc::ThreadProxy::BeginMainFrameAndCommitState,
base::DefaultDeleter<cc::ThreadProxy::BeginMainFrameAndCommitState>
)>::Run(base::internal::BindStateBase*) base/bind_internal.h:1248:12
#9 0x7ffe5eb6ec33 in Run base/callback.h:401:12
#10 0x7ffe5eb6ec33 in base::debug::TaskAnnotator::RunTask(char const*, char
const*, base::PendingTask const&) base/debug/task_annotator.cc:63:0
#11 0x7ffe5eaa968e in base::MessageLoop::RunTask(base::PendingTask const&)
base/message_loop/message_loop.cc:448:3
#12 0x7ffe5eaaa4ea in DeferOrRunPendingTask
base/message_loop/message_loop.cc:458:5
#13 0x7ffe5eaaa4ea in base::MessageLoop::DoWork()
base/message_loop/message_loop.cc:567:0
#14 0x7ffe5eab1132 in
base::MessagePumpDefault::Run(base::MessagePump::Delegate*)
base/message_loop/message_pump_default.cc:32:21
#15 0x7ffe5eadcc3f in base::RunLoop::Run() base/run_loop.cc:55:3
#16 0x7ffe5eaa7d29 in base::MessageLoop::Run()
base/message_loop/message_loop.cc:310:3
#17 0x7ffe6acdf3c8 in content::RendererMain(content::MainFunctionParams
const&) content/renderer/renderer_main.cc:235:7
#18 0x7ffe6a6f4b00 in content::RunZygote(content::MainFunctionParams const&,
content::ContentMainDelegate*) content/app/content_main_runner.cc:347:14
#19 0x7ffe6a6f5e8a in
content::RunNamedProcessTypeMain(std::__1::basic_string<char,
std::__1::char_traits<char>, std::__1::allocator<char> > const&,
content::MainFunctionParams const&, content::ContentMainDelegate*)
content/app/content_main_runner.cc:431:12
#20 0x7ffe6a6f7307 in content::ContentMainRunnerImpl::Run()
content/app/content_main_runner.cc:789:12
#21 0x7ffe6a6f3d35 in content::ContentMain(content::ContentMainParams
const&) content/app/content_main.cc:19:15
#22 0x7ffe6883fc4c in content::LaunchTests(content::TestLauncherDelegate*,
int, int, char**) content/public/test/test_launcher.cc:474:12
#23 0x7ffe5e9addea in LaunchChromeTests(int, ChromeTestSuiteRunner*, int,
char**) chrome/test/base/chrome_test_launcher.cc:125:10
#24 0x7ffe5d9b8870 in main chrome/test/base/browser_tests_main.cc:21:10
#25 0x7ffe5180d76c in __libc_start_main
/build/buildd/eglibc-2.15/csu/libc-start.c:226:0
#26 0x7ffe5bd34c28 in _start ??:0:0
SUMMARY: MemorySanitizer: use-of-uninitialized-value ??:0 ??
See http://www.chromium.org/developers/testing/memorysanitizer to test locally under msan.
Original issue's description:
> cc: Toggle LCD text at raster time instead of record time.
>
> Always tell blink that it can use LCD text (in future this can stop
> being passed at all). And at raster time:
>
> - If LCD text is allowed initialize SkSurfaceProps with the LegacyHost
> parameter which will cause skia to pick an LCD text format matching
> current behaviour.
> - If LCD is not allowed initialize SkSurfaceProps with an "unknown
> pixelformat" causing LCD text to not be used.
>
> This also removes the need for SK_SUPPORT_LEGACY_TEXTRENDERMODE from
> resource_provider.cc.
>
> R=enne,reveman
> BUG=430617
>
> Committed: https://crrev.com/ad672f645e82ca677978b097a170c908c614d184
> Cr-Commit-Position: refs/heads/master@{#304486}
TBR=enne@chromium.org,reed@google.com,reveman@chromium.org,sky@chromium.org,vmpstr@chromium.org,danakj@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=430617
Review URL: https://codereview.chromium.org/733233003
Cr-Commit-Position: refs/heads/master@{#304538}
Diffstat (limited to 'cc')
36 files changed, 251 insertions, 278 deletions
diff --git a/cc/blink/web_content_layer_impl.cc b/cc/blink/web_content_layer_impl.cc index d0a4cca..6dfdad9 100644 --- a/cc/blink/web_content_layer_impl.cc +++ b/cc/blink/web_content_layer_impl.cc @@ -19,12 +19,13 @@ using cc::PictureLayer; namespace cc_blink { WebContentLayerImpl::WebContentLayerImpl(blink::WebContentLayerClient* client) - : client_(client) { + : client_(client), ignore_lcd_text_change_(false) { if (WebLayerImpl::UsingPictureLayer()) layer_ = make_scoped_ptr(new WebLayerImpl(PictureLayer::Create(this))); else layer_ = make_scoped_ptr(new WebLayerImpl(ContentLayer::Create(this))); layer_->layer()->SetIsDrawable(true); + can_use_lcd_text_ = layer_->layer()->can_use_lcd_text(); } WebContentLayerImpl::~WebContentLayerImpl() { @@ -53,16 +54,30 @@ void WebContentLayerImpl::PaintContents( if (!client_) return; - // TODO(danakj): Stop passing this to blink it should always use LCD when it - // wants to. crbug.com/430617 - bool can_use_lcd_text = true; client_->paintContents( - canvas, clip, can_use_lcd_text, + canvas, + clip, + can_use_lcd_text_, graphics_context_status == ContentLayerClient::GRAPHICS_CONTEXT_ENABLED ? blink::WebContentLayerClient::GraphicsContextEnabled : blink::WebContentLayerClient::GraphicsContextDisabled); } +void WebContentLayerImpl::DidChangeLayerCanUseLCDText() { + // It is important to make this comparison because the LCD text status + // here can get out of sync with that in the layer. + if (can_use_lcd_text_ == layer_->layer()->can_use_lcd_text()) + return; + + // LCD text cannot be enabled once disabled. + if (layer_->layer()->can_use_lcd_text() && ignore_lcd_text_change_) + return; + + can_use_lcd_text_ = layer_->layer()->can_use_lcd_text(); + ignore_lcd_text_change_ = true; + layer_->invalidate(); +} + bool WebContentLayerImpl::FillsBoundsCompletely() const { return false; } diff --git a/cc/blink/web_content_layer_impl.h b/cc/blink/web_content_layer_impl.h index 2966bd4..b9bb871 100644 --- a/cc/blink/web_content_layer_impl.h +++ b/cc/blink/web_content_layer_impl.h @@ -40,6 +40,7 @@ class WebContentLayerImpl : public blink::WebContentLayer, const gfx::Rect& clip, ContentLayerClient::GraphicsContextStatus graphics_context_status) override; + void DidChangeLayerCanUseLCDText() override; bool FillsBoundsCompletely() const override; scoped_ptr<WebLayerImpl> layer_; diff --git a/cc/layers/content_layer.cc b/cc/layers/content_layer.cc index 53c7120..88031e4 100644 --- a/cc/layers/content_layer.cc +++ b/cc/layers/content_layer.cc @@ -36,7 +36,9 @@ scoped_refptr<ContentLayer> ContentLayer::Create(ContentLayerClient* client) { } ContentLayer::ContentLayer(ContentLayerClient* client) - : TiledLayer(), client_(client) { + : TiledLayer(), + client_(client), + can_use_lcd_text_last_frame_(can_use_lcd_text()) { } ContentLayer::~ContentLayer() {} @@ -72,6 +74,7 @@ bool ContentLayer::Update(ResourceUpdateQueue* queue, true); CreateUpdaterIfNeeded(); + UpdateCanUseLCDText(); } bool updated = TiledLayer::Update(queue, occlusion); @@ -115,6 +118,15 @@ void ContentLayer::SetContentsOpaque(bool opaque) { updater_->SetOpaque(opaque); } +void ContentLayer::UpdateCanUseLCDText() { + if (can_use_lcd_text_last_frame_ == can_use_lcd_text()) + return; + + can_use_lcd_text_last_frame_ = can_use_lcd_text(); + if (client_) + client_->DidChangeLayerCanUseLCDText(); +} + bool ContentLayer::SupportsLCDText() const { return true; } diff --git a/cc/layers/content_layer.h b/cc/layers/content_layer.h index 22dd8fd..fe0bd37 100644 --- a/cc/layers/content_layer.h +++ b/cc/layers/content_layer.h @@ -65,8 +65,11 @@ class CC_EXPORT ContentLayer : public TiledLayer { // TiledLayer implementation. void CreateUpdaterIfNeeded() override; + void UpdateCanUseLCDText(); + ContentLayerClient* client_; scoped_refptr<ContentLayerUpdater> updater_; + bool can_use_lcd_text_last_frame_; DISALLOW_COPY_AND_ASSIGN(ContentLayer); }; diff --git a/cc/layers/content_layer_client.h b/cc/layers/content_layer_client.h index ea22e97..0140513 100644 --- a/cc/layers/content_layer_client.h +++ b/cc/layers/content_layer_client.h @@ -27,6 +27,10 @@ class CC_EXPORT ContentLayerClient { const gfx::Rect& clip, GraphicsContextStatus gc_status) = 0; + // Called by the content layer during the update phase. + // If the client paints LCD text, it may want to invalidate the layer. + virtual void DidChangeLayerCanUseLCDText() = 0; + // If true the layer may skip clearing the background before rasterizing, // because it will cover any uncleared data with content. virtual bool FillsBoundsCompletely() const = 0; diff --git a/cc/layers/picture_image_layer.h b/cc/layers/picture_image_layer.h index 3f8d68d..3575d88 100644 --- a/cc/layers/picture_image_layer.h +++ b/cc/layers/picture_image_layer.h @@ -27,6 +27,7 @@ class CC_EXPORT PictureImageLayer : public PictureLayer, ContentLayerClient { SkCanvas* canvas, const gfx::Rect& clip, ContentLayerClient::GraphicsContextStatus gc_status) override; + void DidChangeLayerCanUseLCDText() override {} bool FillsBoundsCompletely() const override; protected: diff --git a/cc/layers/picture_layer.cc b/cc/layers/picture_layer.cc index 29190a05..dbfa63c 100644 --- a/cc/layers/picture_layer.cc +++ b/cc/layers/picture_layer.cc @@ -23,7 +23,7 @@ PictureLayer::PictureLayer(ContentLayerClient* client) recording_source_(new PicturePile), instrumentation_object_tracker_(id()), update_source_frame_number_(-1), - can_use_lcd_text_for_update_(true) { + can_use_lcd_text_last_frame_(can_use_lcd_text()) { } PictureLayer::PictureLayer(ContentLayerClient* client, @@ -95,14 +95,18 @@ bool PictureLayer::Update(ResourceUpdateQueue* queue, update_source_frame_number_ = layer_tree_host()->source_frame_number(); bool updated = Layer::Update(queue, occlusion); - bool can_use_lcd_text_changed = UpdateCanUseLCDText(); + { + base::AutoReset<bool> ignore_set_needs_commit(&ignore_set_needs_commit_, + true); + UpdateCanUseLCDText(); + } gfx::Rect visible_layer_rect = gfx::ScaleToEnclosingRect( visible_content_rect(), 1.f / contents_scale_x()); gfx::Size layer_size = paint_properties().bounds; if (last_updated_visible_content_rect_ == visible_content_rect() && - recording_source_->GetSize() == layer_size && !can_use_lcd_text_changed && + recording_source_->GetSize() == layer_size && pending_invalidation_.IsEmpty()) { // Only early out if the visible content rect of this layer hasn't changed. return updated; @@ -132,9 +136,9 @@ bool PictureLayer::Update(ResourceUpdateQueue* queue, DCHECK(client_); updated |= recording_source_->UpdateAndExpandInvalidation( client_, &recording_invalidation_, SafeOpaqueBackgroundColor(), - contents_opaque(), client_->FillsBoundsCompletely(), - can_use_lcd_text_for_update_, layer_size, visible_layer_rect, - update_source_frame_number_, Picture::RECORD_NORMALLY); + contents_opaque(), client_->FillsBoundsCompletely(), layer_size, + visible_layer_rect, update_source_frame_number_, + Picture::RECORD_NORMALLY); last_updated_visible_content_rect_ = visible_content_rect(); if (updated) { @@ -156,14 +160,13 @@ bool PictureLayer::SupportsLCDText() const { return true; } -bool PictureLayer::UpdateCanUseLCDText() { - if (!can_use_lcd_text_for_update_) - return false; // Don't allow the LCD text state to change once disabled. - if (can_use_lcd_text_for_update_ == can_use_lcd_text()) - return false; +void PictureLayer::UpdateCanUseLCDText() { + if (can_use_lcd_text_last_frame_ == can_use_lcd_text()) + return; - can_use_lcd_text_for_update_ = can_use_lcd_text(); - return true; + can_use_lcd_text_last_frame_ = can_use_lcd_text(); + if (client_) + client_->DidChangeLayerCanUseLCDText(); } skia::RefPtr<SkPicture> PictureLayer::GetPicture() const { diff --git a/cc/layers/picture_layer.h b/cc/layers/picture_layer.h index 4d84371..ebd6427 100644 --- a/cc/layers/picture_layer.h +++ b/cc/layers/picture_layer.h @@ -50,7 +50,7 @@ class CC_EXPORT PictureLayer : public Layer { ~PictureLayer() override; bool HasDrawableContent() const override; - bool UpdateCanUseLCDText(); + void UpdateCanUseLCDText(); private: ContentLayerClient* client_; @@ -64,7 +64,7 @@ class CC_EXPORT PictureLayer : public Layer { gfx::Rect last_updated_visible_content_rect_; int update_source_frame_number_; - bool can_use_lcd_text_for_update_; + bool can_use_lcd_text_last_frame_; DISALLOW_COPY_AND_ASSIGN(PictureLayer); }; diff --git a/cc/layers/picture_layer_impl_unittest.cc b/cc/layers/picture_layer_impl_unittest.cc index 4d9c52f..a717ade 100644 --- a/cc/layers/picture_layer_impl_unittest.cc +++ b/cc/layers/picture_layer_impl_unittest.cc @@ -4504,7 +4504,7 @@ void PictureLayerImplTest::TestQuadsForSolidColor(bool test_for_solid) { Region invalidation(layer_rect); recording_source->UpdateAndExpandInvalidation( - &client, &invalidation, SK_ColorWHITE, false, false, false, layer_bounds, + &client, &invalidation, SK_ColorWHITE, false, false, layer_bounds, layer_rect, frame_number++, Picture::RECORD_NORMALLY); scoped_refptr<RasterSource> pending_raster_source = @@ -4571,7 +4571,7 @@ TEST_F(PictureLayerImplTest, NonSolidToSolidNoTilings) { Region invalidation1(layer_rect); recording_source->UpdateAndExpandInvalidation( - &client, &invalidation1, SK_ColorWHITE, false, false, false, layer_bounds, + &client, &invalidation1, SK_ColorWHITE, false, false, layer_bounds, layer_rect, frame_number++, Picture::RECORD_NORMALLY); scoped_refptr<RasterSource> raster_source1 = @@ -4589,7 +4589,7 @@ TEST_F(PictureLayerImplTest, NonSolidToSolidNoTilings) { Region invalidation2(layer_rect); recording_source->UpdateAndExpandInvalidation( - &client, &invalidation2, SK_ColorWHITE, false, false, false, layer_bounds, + &client, &invalidation2, SK_ColorWHITE, false, false, layer_bounds, layer_rect, frame_number++, Picture::RECORD_NORMALLY); scoped_refptr<RasterSource> raster_source2 = diff --git a/cc/layers/picture_layer_unittest.cc b/cc/layers/picture_layer_unittest.cc index 3f81ef8..7873b48 100644 --- a/cc/layers/picture_layer_unittest.cc +++ b/cc/layers/picture_layer_unittest.cc @@ -24,6 +24,7 @@ class MockContentLayerClient : public ContentLayerClient { SkCanvas* canvas, const gfx::Rect& clip, ContentLayerClient::GraphicsContextStatus gc_status) override {} + void DidChangeLayerCanUseLCDText() override {} bool FillsBoundsCompletely() const override { return false; }; }; diff --git a/cc/resources/gpu_raster_worker_pool.cc b/cc/resources/gpu_raster_worker_pool.cc index a304a4a..c913970 100644 --- a/cc/resources/gpu_raster_worker_pool.cc +++ b/cc/resources/gpu_raster_worker_pool.cc @@ -41,8 +41,7 @@ class RasterBufferImpl : public RasterBuffer { bool use_distance_field_text = use_distance_field_text_ || raster_source->ShouldAttemptToUseDistanceFieldText(); - SkSurface* sk_surface = lock_.GetSkSurface(use_distance_field_text, - raster_source->CanUseLCDText()); + SkSurface* sk_surface = lock_.GetSkSurface(use_distance_field_text); if (!sk_surface) return; diff --git a/cc/resources/picture_pile.cc b/cc/resources/picture_pile.cc index 4daa2c3..ab138ca 100644 --- a/cc/resources/picture_pile.cc +++ b/cc/resources/picture_pile.cc @@ -193,7 +193,6 @@ bool PicturePile::UpdateAndExpandInvalidation( SkColor background_color, bool contents_opaque, bool contents_fill_bounds_completely, - bool can_use_lcd_text, const gfx::Size& layer_size, const gfx::Rect& visible_layer_rect, int frame_number, @@ -201,25 +200,15 @@ bool PicturePile::UpdateAndExpandInvalidation( background_color_ = background_color; contents_opaque_ = contents_opaque; contents_fill_bounds_completely_ = contents_fill_bounds_completely; - bool can_use_lcd_text_changed = can_use_lcd_text_ != can_use_lcd_text; - can_use_lcd_text_ = can_use_lcd_text; bool updated = false; - Region synthetic_invalidation; + Region resize_invalidation; gfx::Size old_tiling_size = GetSize(); if (old_tiling_size != layer_size) { tiling_.SetTilingSize(layer_size); updated = true; } - if (can_use_lcd_text_changed) { - // When LCD text is enabled/disabled, we must drop any raster tiles for - // the pile, so they can be recreated in a manner consistent with the new - // setting. We do this with |synthetic_invalidation| since we don't need to - // do a new recording, just invalidate rastered content. - synthetic_invalidation.Union(gfx::Rect(GetSize())); - updated = true; - } gfx::Rect interest_rect = visible_layer_rect; interest_rect.Inset(-pixel_record_distance_, -pixel_record_distance_); @@ -383,11 +372,11 @@ bool PicturePile::UpdateAndExpandInvalidation( exposed_top, exposed_left_until - exposed_left, exposed_bottom - exposed_top); - synthetic_invalidation.Union(left_rect); - synthetic_invalidation.Union(right_rect); - synthetic_invalidation.Union(top_rect); - synthetic_invalidation.Union(bottom_rect); - synthetic_invalidation.Union(exposed_rect); + resize_invalidation.Union(left_rect); + resize_invalidation.Union(right_rect); + resize_invalidation.Union(top_rect); + resize_invalidation.Union(bottom_rect); + resize_invalidation.Union(exposed_rect); } if (min_toss_y < tiling_.num_tiles_y()) { // The same thing occurs here as in the case above, but the invalidation @@ -419,11 +408,11 @@ bool PicturePile::UpdateAndExpandInvalidation( exposed_top, exposed_right - exposed_left, exposed_top_until - exposed_top); - synthetic_invalidation.Union(left_rect); - synthetic_invalidation.Union(right_rect); - synthetic_invalidation.Union(top_rect); - synthetic_invalidation.Union(bottom_rect); - synthetic_invalidation.Union(exposed_rect); + resize_invalidation.Union(left_rect); + resize_invalidation.Union(right_rect); + resize_invalidation.Union(top_rect); + resize_invalidation.Union(bottom_rect); + resize_invalidation.Union(exposed_rect); } } @@ -485,7 +474,7 @@ bool PicturePile::UpdateAndExpandInvalidation( invalidation->Union(invalidation_expanded_to_full_tiles); } - invalidation->Union(synthetic_invalidation); + invalidation->Union(resize_invalidation); // Make a list of all invalid tiles; we will attempt to // cluster these into multiple invalidation regions. diff --git a/cc/resources/picture_pile.h b/cc/resources/picture_pile.h index 767a6c2..1f41d01 100644 --- a/cc/resources/picture_pile.h +++ b/cc/resources/picture_pile.h @@ -28,7 +28,6 @@ class CC_EXPORT PicturePile : public RecordingSource { SkColor background_color, bool contents_opaque, bool contents_fill_bounds_completely, - bool can_use_lcd_text, const gfx::Size& layer_size, const gfx::Rect& visible_layer_rect, int frame_number, @@ -102,7 +101,6 @@ class CC_EXPORT PicturePile : public RecordingSource { int slow_down_raster_scale_factor_for_debug_; bool contents_opaque_; bool contents_fill_bounds_completely_; - bool can_use_lcd_text_; bool clear_canvas_with_debug_color_; // A hint about whether there are any recordings. This may be a false // positive. diff --git a/cc/resources/picture_pile_impl.cc b/cc/resources/picture_pile_impl.cc index ee0ad47..7ec9252 100644 --- a/cc/resources/picture_pile_impl.cc +++ b/cc/resources/picture_pile_impl.cc @@ -29,7 +29,6 @@ PicturePileImpl::PicturePileImpl() : background_color_(SK_ColorTRANSPARENT), contents_opaque_(false), contents_fill_bounds_completely_(false), - can_use_lcd_text_(false), is_solid_color_(false), solid_color_(SK_ColorTRANSPARENT), has_any_recordings_(false), @@ -46,7 +45,6 @@ PicturePileImpl::PicturePileImpl(const PicturePile* other) background_color_(other->background_color_), contents_opaque_(other->contents_opaque_), contents_fill_bounds_completely_(other->contents_fill_bounds_completely_), - can_use_lcd_text_(other->can_use_lcd_text_), is_solid_color_(other->is_solid_color_), solid_color_(other->solid_color_), recorded_viewport_(other->recorded_viewport_), @@ -429,10 +427,6 @@ bool PicturePileImpl::IsMask() const { return is_mask_; } -bool PicturePileImpl::CanUseLCDText() const { - return can_use_lcd_text_; -} - PicturePileImpl::PixelRefIterator::PixelRefIterator( const gfx::Rect& content_rect, float contents_scale, diff --git a/cc/resources/picture_pile_impl.h b/cc/resources/picture_pile_impl.h index 0160ec2..9f5b5f6 100644 --- a/cc/resources/picture_pile_impl.h +++ b/cc/resources/picture_pile_impl.h @@ -60,7 +60,6 @@ class CC_EXPORT PicturePileImpl : public RasterSource { SkColor GetSolidColor() const override; bool HasRecordings() const override; bool IsMask() const override; - bool CanUseLCDText() const override; // Tracing functionality. void DidBeginTracing() override; @@ -111,7 +110,6 @@ class CC_EXPORT PicturePileImpl : public RasterSource { SkColor background_color_; bool contents_opaque_; bool contents_fill_bounds_completely_; - bool can_use_lcd_text_; bool is_solid_color_; SkColor solid_color_; gfx::Rect recorded_viewport_; diff --git a/cc/resources/picture_pile_unittest.cc b/cc/resources/picture_pile_unittest.cc index 55e432e..bfb4692 100644 --- a/cc/resources/picture_pile_unittest.cc +++ b/cc/resources/picture_pile_unittest.cc @@ -45,7 +45,7 @@ class PicturePileTestBase { frame_number_++; return pile_.UpdateAndExpandInvalidation( &client_, invalidation, background_color_, contents_opaque_, false, - false, layer_size, visible_layer_rect, frame_number_, + layer_size, visible_layer_rect, frame_number_, Picture::RECORD_NORMALLY); } diff --git a/cc/resources/raster_source.h b/cc/resources/raster_source.h index bcc28d1..f7ea271 100644 --- a/cc/resources/raster_source.h +++ b/cc/resources/raster_source.h @@ -93,9 +93,6 @@ class CC_EXPORT RasterSource : public base::RefCountedThreadSafe<RasterSource> { // TODO(vmpstr): This should be a layer property. virtual bool IsMask() const = 0; - // Return true if LCD anti-aliasing may be used when rastering text. - virtual bool CanUseLCDText() const = 0; - protected: friend class base::RefCountedThreadSafe<RasterSource>; diff --git a/cc/resources/raster_worker_pool.cc b/cc/resources/raster_worker_pool.cc index 4511f9d..ca26edd 100644 --- a/cc/resources/raster_worker_pool.cc +++ b/cc/resources/raster_worker_pool.cc @@ -228,26 +228,28 @@ void RasterWorkerPool::PlaybackToMemory(void* memory, SkColorType buffer_color_type = ResourceFormatToSkColorType(format); bool needs_copy = buffer_color_type != info.colorType(); - // Use unknown pixel geometry to disable LCD text. - SkSurfaceProps surface_props(0, kUnknown_SkPixelGeometry); - if (raster_source->CanUseLCDText()) { - // LegacyFontHost will get LCD text and skia figures out what type to use. - surface_props = SkSurfaceProps(SkSurfaceProps::kLegacyFontHost_InitType); - } + // TODO(danakj): Make a SkSurfaceProps with an SkPixelGeometry to enable or + // disable LCD text. + // TODO(danakj): Disable LCD text on Mac during layout tests: + // https://cs.chromium.org#chromium/src/third_party/WebKit/Source/platform/fonts/mac/FontPlatformDataMac.mm&l=55 + // TODO(danakj): On Windows when LCD text is disabled, ask skia to draw LCD + // text offscreen and downsample it to AA text. + // https://cs.chromium.org#chromium/src/third_party/WebKit/Source/platform/fonts/win/FontPlatformDataWin.cpp&l=86 + SkSurfaceProps* surface_props = nullptr; if (!stride) stride = info.minRowBytes(); if (!needs_copy) { skia::RefPtr<SkSurface> surface = skia::AdoptRef( - SkSurface::NewRasterDirect(info, memory, stride, &surface_props)); + SkSurface::NewRasterDirect(info, memory, stride, surface_props)); skia::RefPtr<SkCanvas> canvas = skia::SharePtr(surface->getCanvas()); raster_source->PlaybackToCanvas(canvas.get(), rect, scale); return; } skia::RefPtr<SkSurface> surface = - skia::AdoptRef(SkSurface::NewRaster(info, &surface_props)); + skia::AdoptRef(SkSurface::NewRaster(info, surface_props)); skia::RefPtr<SkCanvas> canvas = skia::SharePtr(surface->getCanvas()); raster_source->PlaybackToCanvas(canvas.get(), rect, scale); diff --git a/cc/resources/recording_source.h b/cc/resources/recording_source.h index ecb64fa..d2b0893 100644 --- a/cc/resources/recording_source.h +++ b/cc/resources/recording_source.h @@ -31,7 +31,6 @@ class CC_EXPORT RecordingSource { SkColor background_color, bool contents_opaque, bool contents_fill_bounds_completely, - bool can_use_lcd_text, const gfx::Size& layer_size, const gfx::Rect& visible_layer_rect, int frame_number, diff --git a/cc/resources/resource_provider.cc b/cc/resources/resource_provider.cc index 4939c48..023a1cd 100644 --- a/cc/resources/resource_provider.cc +++ b/cc/resources/resource_provider.cc @@ -1100,15 +1100,15 @@ ResourceProvider::ScopedWriteLockGr::~ScopedWriteLockGr() { } SkSurface* ResourceProvider::ScopedWriteLockGr::GetSkSurface( - bool use_distance_field_text, - bool can_use_lcd_text) { + bool use_distance_field_text) { DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(resource_->locked_for_write); - bool create_surface = - !resource_->sk_surface.get() || - !SurfaceHasMatchingProperties(use_distance_field_text, can_use_lcd_text); - if (create_surface) { + // If the surface doesn't exist, or doesn't have the correct dff setting, + // recreate the surface within the resource. + if (!resource_->sk_surface || + use_distance_field_text != + resource_->sk_surface->props().isUseDistanceFieldFonts()) { class GrContext* gr_context = resource_provider_->GrContext(); // TODO(alokp): Implement TestContextProvider::GrContext(). if (!gr_context) @@ -1127,34 +1127,15 @@ SkSurface* ResourceProvider::ScopedWriteLockGr::GetSkSurface( skia::AdoptRef(gr_context->wrapBackendTexture(desc)); if (!gr_texture) return nullptr; - uint32_t flags = use_distance_field_text - ? SkSurfaceProps::kUseDistanceFieldFonts_Flag - : 0; - // Use unknown pixel geometry to disable LCD text. - SkSurfaceProps surface_props(flags, kUnknown_SkPixelGeometry); - if (can_use_lcd_text) { - // LegacyFontHost will get LCD text and skia figures out what type to use. - surface_props = - SkSurfaceProps(flags, SkSurfaceProps::kLegacyFontHost_InitType); - } + SkSurface::TextRenderMode text_render_mode = + use_distance_field_text ? SkSurface::kDistanceField_TextRenderMode + : SkSurface::kStandard_TextRenderMode; resource_->sk_surface = skia::AdoptRef(SkSurface::NewRenderTargetDirect( - gr_texture->asRenderTarget(), &surface_props)); + gr_texture->asRenderTarget(), text_render_mode)); } return resource_->sk_surface.get(); } -bool ResourceProvider::ScopedWriteLockGr::SurfaceHasMatchingProperties( - bool use_distance_field_text, - bool can_use_lcd_text) const { - const SkSurface* surface = resource_->sk_surface.get(); - bool surface_uses_distance_field_text = - surface->props().isUseDistanceFieldFonts(); - bool surface_can_use_lcd_text = - surface->props().pixelGeometry() != kUnknown_SkPixelGeometry; - return use_distance_field_text == surface_uses_distance_field_text && - can_use_lcd_text == surface_can_use_lcd_text; -} - ResourceProvider::SynchronousFence::SynchronousFence( gpu::gles2::GLES2Interface* gl) : gl_(gl), has_synchronized_(true) { diff --git a/cc/resources/resource_provider.h b/cc/resources/resource_provider.h index c2f349f..40103f6b 100644 --- a/cc/resources/resource_provider.h +++ b/cc/resources/resource_provider.h @@ -335,13 +335,9 @@ class CC_EXPORT ResourceProvider { ResourceProvider::ResourceId resource_id); ~ScopedWriteLockGr(); - SkSurface* GetSkSurface(bool use_distance_field_text, - bool can_use_lcd_text); + SkSurface* GetSkSurface(bool use_distance_field_text); private: - bool SurfaceHasMatchingProperties(bool use_distance_field_text, - bool can_use_lcd_text) const; - ResourceProvider* resource_provider_; ResourceProvider::Resource* resource_; base::ThreadChecker thread_checker_; diff --git a/cc/test/fake_content_layer_client.h b/cc/test/fake_content_layer_client.h index 141bc97..7a7cc44 100644 --- a/cc/test/fake_content_layer_client.h +++ b/cc/test/fake_content_layer_client.h @@ -31,6 +31,7 @@ class FakeContentLayerClient : public ContentLayerClient { SkCanvas* canvas, const gfx::Rect& rect, ContentLayerClient::GraphicsContextStatus gc_status) override; + void DidChangeLayerCanUseLCDText() override {} bool FillsBoundsCompletely() const override; void set_fill_with_nonsolid_color(bool nonsolid) { diff --git a/cc/test/layer_tree_host_common_test.cc b/cc/test/layer_tree_host_common_test.cc index 8441657..33e0caf 100644 --- a/cc/test/layer_tree_host_common_test.cc +++ b/cc/test/layer_tree_host_common_test.cc @@ -59,8 +59,7 @@ void LayerTreeHostCommonTestBase::ExecuteCalculateDrawProperties( float device_scale_factor, float page_scale_factor, Layer* page_scale_application_layer, - bool can_use_lcd_text, - bool layers_always_allowed_lcd_text) { + bool can_use_lcd_text) { EXPECT_TRUE(page_scale_application_layer || (page_scale_factor == 1.f)); gfx::Transform identity_matrix; gfx::Size device_viewport_size = @@ -78,7 +77,6 @@ void LayerTreeHostCommonTestBase::ExecuteCalculateDrawProperties( inputs.page_scale_factor = page_scale_factor; inputs.page_scale_application_layer = page_scale_application_layer; inputs.can_use_lcd_text = can_use_lcd_text; - inputs.layers_always_allowed_lcd_text = layers_always_allowed_lcd_text; inputs.can_adjust_raster_scales = true; LayerTreeHostCommon::CalculateDrawProperties(&inputs); } @@ -88,8 +86,7 @@ void LayerTreeHostCommonTestBase::ExecuteCalculateDrawProperties( float device_scale_factor, float page_scale_factor, LayerImpl* page_scale_application_layer, - bool can_use_lcd_text, - bool layers_always_allowed_lcd_text) { + bool can_use_lcd_text) { gfx::Transform identity_matrix; gfx::Size device_viewport_size = gfx::Size(root_layer->bounds().width() * device_scale_factor, @@ -106,7 +103,6 @@ void LayerTreeHostCommonTestBase::ExecuteCalculateDrawProperties( inputs.page_scale_factor = page_scale_factor; inputs.page_scale_application_layer = page_scale_application_layer; inputs.can_use_lcd_text = can_use_lcd_text; - inputs.layers_always_allowed_lcd_text = layers_always_allowed_lcd_text; inputs.can_adjust_raster_scales = true; ++render_surface_layer_list_count_; diff --git a/cc/test/layer_tree_host_common_test.h b/cc/test/layer_tree_host_common_test.h index 456183f..38b57d4 100644 --- a/cc/test/layer_tree_host_common_test.h +++ b/cc/test/layer_tree_host_common_test.h @@ -68,29 +68,30 @@ class LayerTreeHostCommonTestBase { float device_scale_factor, float page_scale_factor, Layer* page_scale_application_layer, - bool can_use_lcd_text, - bool layers_always_allowed_lcd_text); + bool can_use_lcd_text); void ExecuteCalculateDrawProperties(LayerImpl* root_layer, float device_scale_factor, float page_scale_factor, LayerImpl* page_scale_application_layer, - bool can_use_lcd_text, - bool layers_always_allowed_lcd_text); + bool can_use_lcd_text); template <class LayerType> void ExecuteCalculateDrawProperties(LayerType* root_layer) { LayerType* page_scale_application_layer = NULL; - ExecuteCalculateDrawProperties(root_layer, 1.f, 1.f, - page_scale_application_layer, false, false); + ExecuteCalculateDrawProperties( + root_layer, 1.f, 1.f, page_scale_application_layer, false); } template <class LayerType> void ExecuteCalculateDrawProperties(LayerType* root_layer, float device_scale_factor) { LayerType* page_scale_application_layer = NULL; - ExecuteCalculateDrawProperties(root_layer, device_scale_factor, 1.f, - page_scale_application_layer, false, false); + ExecuteCalculateDrawProperties(root_layer, + device_scale_factor, + 1.f, + page_scale_application_layer, + false); } template <class LayerType> @@ -98,9 +99,11 @@ class LayerTreeHostCommonTestBase { float device_scale_factor, float page_scale_factor, LayerType* page_scale_application_layer) { - ExecuteCalculateDrawProperties(root_layer, device_scale_factor, + ExecuteCalculateDrawProperties(root_layer, + device_scale_factor, page_scale_factor, - page_scale_application_layer, false, false); + page_scale_application_layer, + false); } RenderSurfaceLayerList* render_surface_layer_list() const { diff --git a/cc/test/solid_color_content_layer_client.h b/cc/test/solid_color_content_layer_client.h index 7a962cf..a851b72 100644 --- a/cc/test/solid_color_content_layer_client.h +++ b/cc/test/solid_color_content_layer_client.h @@ -16,6 +16,7 @@ class SolidColorContentLayerClient : public ContentLayerClient { explicit SolidColorContentLayerClient(SkColor color) : color_(color) {} // ContentLayerClient implementation. + void DidChangeLayerCanUseLCDText() override {} void PaintContents( SkCanvas* canvas, const gfx::Rect& rect, diff --git a/cc/trees/layer_tree_host.cc b/cc/trees/layer_tree_host.cc index 808bf21..f85d4e7 100644 --- a/cc/trees/layer_tree_host.cc +++ b/cc/trees/layer_tree_host.cc @@ -845,12 +845,17 @@ bool LayerTreeHost::UpdateLayers(Layer* root_layer, // Change this if this information is required. int render_surface_layer_list_id = 0; LayerTreeHostCommon::CalcDrawPropsMainInputs inputs( - root_layer, device_viewport_size(), gfx::Transform(), - device_scale_factor_, page_scale_factor_, page_scale_layer, - GetRendererCapabilities().max_texture_size, settings_.can_use_lcd_text, - settings_.layers_always_allowed_lcd_text, + root_layer, + device_viewport_size(), + gfx::Transform(), + device_scale_factor_, + page_scale_factor_, + page_scale_layer, + GetRendererCapabilities().max_texture_size, + settings_.can_use_lcd_text, can_render_to_separate_surface, - settings_.layer_transforms_should_scale_layer_contents, &update_list, + settings_.layer_transforms_should_scale_layer_contents, + &update_list, render_surface_layer_list_id); LayerTreeHostCommon::CalculateDrawProperties(&inputs); diff --git a/cc/trees/layer_tree_host_common.cc b/cc/trees/layer_tree_host_common.cc index 818abf5..0ff6608 100644 --- a/cc/trees/layer_tree_host_common.cc +++ b/cc/trees/layer_tree_host_common.cc @@ -1269,7 +1269,6 @@ struct SubtreeGlobals { const LayerType* page_scale_application_layer; bool can_adjust_raster_scales; bool can_render_to_separate_surface; - bool layers_always_allowed_lcd_text; }; template<typename LayerType> @@ -1792,15 +1791,13 @@ static void CalculateDrawPropertiesInternal( // causes jank. bool adjust_text_aa = !animating_opacity_to_screen && !animating_transform_to_screen; - bool layer_can_use_lcd_text = true; - if (!globals.layers_always_allowed_lcd_text) { - // To avoid color fringing, LCD text should only be used on opaque layers - // with just integral translation. - layer_can_use_lcd_text = data_from_ancestor.subtree_can_use_lcd_text && - accumulated_draw_opacity == 1.f && - layer_draw_properties.target_space_transform - .IsIdentityOrIntegerTranslation(); - } + // To avoid color fringing, LCD text should only be used on opaque layers with + // just integral translation. + bool layer_can_use_lcd_text = + data_from_ancestor.subtree_can_use_lcd_text && + accumulated_draw_opacity == 1.f && + layer_draw_properties.target_space_transform. + IsIdentityOrIntegerTranslation(); gfx::Rect content_rect(layer->content_bounds()); @@ -2377,8 +2374,6 @@ static void ProcessCalcDrawPropsInputs( globals->can_render_to_separate_surface = inputs.can_render_to_separate_surface; globals->can_adjust_raster_scales = inputs.can_adjust_raster_scales; - globals->layers_always_allowed_lcd_text = - inputs.layers_always_allowed_lcd_text; data_for_recursion->parent_matrix = scaled_device_transform; data_for_recursion->full_hierarchy_matrix = identity_matrix; diff --git a/cc/trees/layer_tree_host_common.h b/cc/trees/layer_tree_host_common.h index 349b341..089fe29 100644 --- a/cc/trees/layer_tree_host_common.h +++ b/cc/trees/layer_tree_host_common.h @@ -40,7 +40,6 @@ class CC_EXPORT LayerTreeHostCommon { const LayerType* page_scale_application_layer, int max_texture_size, bool can_use_lcd_text, - bool layers_always_allowed_lcd_text, bool can_render_to_separate_surface, bool can_adjust_raster_scales, RenderSurfaceLayerListType* render_surface_layer_list, @@ -53,7 +52,6 @@ class CC_EXPORT LayerTreeHostCommon { page_scale_application_layer(page_scale_application_layer), max_texture_size(max_texture_size), can_use_lcd_text(can_use_lcd_text), - layers_always_allowed_lcd_text(layers_always_allowed_lcd_text), can_render_to_separate_surface(can_render_to_separate_surface), can_adjust_raster_scales(can_adjust_raster_scales), render_surface_layer_list(render_surface_layer_list), @@ -68,7 +66,6 @@ class CC_EXPORT LayerTreeHostCommon { const LayerType* page_scale_application_layer; int max_texture_size; bool can_use_lcd_text; - bool layers_always_allowed_lcd_text; bool can_render_to_separate_surface; bool can_adjust_raster_scales; RenderSurfaceLayerListType* render_surface_layer_list; @@ -225,7 +222,6 @@ LayerTreeHostCommon::CalcDrawPropsInputsForTesting<LayerType, NULL, std::numeric_limits<int>::max() / 2, false, - false, true, false, render_surface_layer_list, @@ -250,7 +246,6 @@ LayerTreeHostCommon::CalcDrawPropsInputsForTesting<LayerType, NULL, std::numeric_limits<int>::max() / 2, false, - false, true, false, render_surface_layer_list, diff --git a/cc/trees/layer_tree_host_common_perftest.cc b/cc/trees/layer_tree_host_common_perftest.cc index 39bcbe6..12e7a5e 100644 --- a/cc/trees/layer_tree_host_common_perftest.cc +++ b/cc/trees/layer_tree_host_common_perftest.cc @@ -93,17 +93,19 @@ class CalcDrawPropsMainTest : public LayerTreeHostCommonPerfTest { RenderSurfaceLayerList update_list; LayerTreeHostCommon::CalcDrawPropsMainInputs inputs( layer_tree_host()->root_layer(), - layer_tree_host()->device_viewport_size(), gfx::Transform(), + layer_tree_host()->device_viewport_size(), + gfx::Transform(), layer_tree_host()->device_scale_factor(), layer_tree_host()->page_scale_factor(), - layer_tree_host()->page_scale_layer(), max_texture_size, + layer_tree_host()->page_scale_layer(), + max_texture_size, layer_tree_host()->settings().can_use_lcd_text, - layer_tree_host()->settings().layers_always_allowed_lcd_text, can_render_to_separate_surface, layer_tree_host() ->settings() .layer_transforms_should_scale_layer_contents, - &update_list, 0); + &update_list, + 0); LayerTreeHostCommon::CalculateDrawProperties(&inputs); timer_.NextLap(); @@ -145,15 +147,18 @@ class CalcDrawPropsImplTest : public LayerTreeHostCommonPerfTest { LayerTreeHostImpl* host_impl) { LayerImplList update_list; LayerTreeHostCommon::CalcDrawPropsImplInputs inputs( - active_tree->root_layer(), active_tree->DrawViewportSize(), - host_impl->DrawTransform(), active_tree->device_scale_factor(), + active_tree->root_layer(), + active_tree->DrawViewportSize(), + host_impl->DrawTransform(), + active_tree->device_scale_factor(), active_tree->total_page_scale_factor(), - active_tree->InnerViewportContainerLayer(), max_texture_size, + active_tree->InnerViewportContainerLayer(), + max_texture_size, host_impl->settings().can_use_lcd_text, - host_impl->settings().layers_always_allowed_lcd_text, can_render_to_separate_surface, host_impl->settings().layer_transforms_should_scale_layer_contents, - &update_list, 0); + &update_list, + 0); LayerTreeHostCommon::CalculateDrawProperties(&inputs); } }; diff --git a/cc/trees/layer_tree_host_common_unittest.cc b/cc/trees/layer_tree_host_common_unittest.cc index 1a2c0a0..b40f66db 100644 --- a/cc/trees/layer_tree_host_common_unittest.cc +++ b/cc/trees/layer_tree_host_common_unittest.cc @@ -59,6 +59,7 @@ class MockContentLayerClient : public ContentLayerClient { SkCanvas* canvas, const gfx::Rect& clip, ContentLayerClient::GraphicsContextStatus gc_status) override {} + void DidChangeLayerCanUseLCDText() override {} bool FillsBoundsCompletely() const override { return false; } }; @@ -5643,14 +5644,13 @@ TEST_F(LayerTreeHostCommonTest, OpacityAnimatingOnPendingTree) { ASSERT_EQ(2u, root->render_surface()->layer_list().size()); } -using LCDTextTestParam = std::tr1::tuple<bool, bool, bool>; +typedef std::tr1::tuple<bool, bool> LCDTextTestParam; class LCDTextTest : public LayerTreeHostCommonTestBase, public testing::TestWithParam<LCDTextTestParam> { protected: virtual void SetUp() { can_use_lcd_text_ = std::tr1::get<0>(GetParam()); - layers_always_allowed_lcd_text_ = std::tr1::get<1>(GetParam()); root_ = Layer::Create(); child_ = Layer::Create(); @@ -5681,14 +5681,13 @@ class LCDTextTest true, false); - child_->SetForceRenderSurface(std::tr1::get<2>(GetParam())); + child_->SetForceRenderSurface(std::tr1::get<1>(GetParam())); host_ = CreateFakeLayerTreeHost(); host_->SetRootLayer(root_); } bool can_use_lcd_text_; - bool layers_always_allowed_lcd_text_; scoped_ptr<FakeLayerTreeHost> host_; scoped_refptr<Layer> root_; scoped_refptr<Layer> child_; @@ -5696,115 +5695,108 @@ class LCDTextTest }; TEST_P(LCDTextTest, CanUseLCDText) { - bool expect_lcd_text = can_use_lcd_text_ || layers_always_allowed_lcd_text_; - bool expect_not_lcd_text = layers_always_allowed_lcd_text_; - // Case 1: Identity transform. gfx::Transform identity_matrix; - ExecuteCalculateDrawProperties(root_.get(), 1.f, 1.f, NULL, can_use_lcd_text_, - layers_always_allowed_lcd_text_); - EXPECT_EQ(expect_lcd_text, root_->can_use_lcd_text()); - EXPECT_EQ(expect_lcd_text, child_->can_use_lcd_text()); - EXPECT_EQ(expect_lcd_text, grand_child_->can_use_lcd_text()); + ExecuteCalculateDrawProperties( + root_.get(), 1.f, 1.f, NULL, can_use_lcd_text_); + EXPECT_EQ(can_use_lcd_text_, root_->can_use_lcd_text()); + EXPECT_EQ(can_use_lcd_text_, child_->can_use_lcd_text()); + EXPECT_EQ(can_use_lcd_text_, grand_child_->can_use_lcd_text()); // Case 2: Integral translation. gfx::Transform integral_translation; integral_translation.Translate(1.0, 2.0); child_->SetTransform(integral_translation); - ExecuteCalculateDrawProperties(root_.get(), 1.f, 1.f, NULL, can_use_lcd_text_, - layers_always_allowed_lcd_text_); - EXPECT_EQ(expect_lcd_text, root_->can_use_lcd_text()); - EXPECT_EQ(expect_lcd_text, child_->can_use_lcd_text()); - EXPECT_EQ(expect_lcd_text, grand_child_->can_use_lcd_text()); + ExecuteCalculateDrawProperties( + root_.get(), 1.f, 1.f, NULL, can_use_lcd_text_); + EXPECT_EQ(can_use_lcd_text_, root_->can_use_lcd_text()); + EXPECT_EQ(can_use_lcd_text_, child_->can_use_lcd_text()); + EXPECT_EQ(can_use_lcd_text_, grand_child_->can_use_lcd_text()); // Case 3: Non-integral translation. gfx::Transform non_integral_translation; non_integral_translation.Translate(1.5, 2.5); child_->SetTransform(non_integral_translation); - ExecuteCalculateDrawProperties(root_.get(), 1.f, 1.f, NULL, can_use_lcd_text_, - layers_always_allowed_lcd_text_); - EXPECT_EQ(expect_lcd_text, root_->can_use_lcd_text()); - EXPECT_EQ(expect_not_lcd_text, child_->can_use_lcd_text()); - EXPECT_EQ(expect_not_lcd_text, grand_child_->can_use_lcd_text()); + ExecuteCalculateDrawProperties( + root_.get(), 1.f, 1.f, NULL, can_use_lcd_text_); + EXPECT_EQ(can_use_lcd_text_, root_->can_use_lcd_text()); + EXPECT_FALSE(child_->can_use_lcd_text()); + EXPECT_FALSE(grand_child_->can_use_lcd_text()); // Case 4: Rotation. gfx::Transform rotation; rotation.Rotate(10.0); child_->SetTransform(rotation); - ExecuteCalculateDrawProperties(root_.get(), 1.f, 1.f, NULL, can_use_lcd_text_, - layers_always_allowed_lcd_text_); - EXPECT_EQ(expect_lcd_text, root_->can_use_lcd_text()); - EXPECT_EQ(expect_not_lcd_text, child_->can_use_lcd_text()); - EXPECT_EQ(expect_not_lcd_text, grand_child_->can_use_lcd_text()); + ExecuteCalculateDrawProperties( + root_.get(), 1.f, 1.f, NULL, can_use_lcd_text_); + EXPECT_EQ(can_use_lcd_text_, root_->can_use_lcd_text()); + EXPECT_FALSE(child_->can_use_lcd_text()); + EXPECT_FALSE(grand_child_->can_use_lcd_text()); // Case 5: Scale. gfx::Transform scale; scale.Scale(2.0, 2.0); child_->SetTransform(scale); - ExecuteCalculateDrawProperties(root_.get(), 1.f, 1.f, NULL, can_use_lcd_text_, - layers_always_allowed_lcd_text_); - EXPECT_EQ(expect_lcd_text, root_->can_use_lcd_text()); - EXPECT_EQ(expect_not_lcd_text, child_->can_use_lcd_text()); - EXPECT_EQ(expect_not_lcd_text, grand_child_->can_use_lcd_text()); + ExecuteCalculateDrawProperties( + root_.get(), 1.f, 1.f, NULL, can_use_lcd_text_); + EXPECT_EQ(can_use_lcd_text_, root_->can_use_lcd_text()); + EXPECT_FALSE(child_->can_use_lcd_text()); + EXPECT_FALSE(grand_child_->can_use_lcd_text()); // Case 6: Skew. gfx::Transform skew; skew.SkewX(10.0); child_->SetTransform(skew); - ExecuteCalculateDrawProperties(root_.get(), 1.f, 1.f, NULL, can_use_lcd_text_, - layers_always_allowed_lcd_text_); - EXPECT_EQ(expect_lcd_text, root_->can_use_lcd_text()); - EXPECT_EQ(expect_not_lcd_text, child_->can_use_lcd_text()); - EXPECT_EQ(expect_not_lcd_text, grand_child_->can_use_lcd_text()); + ExecuteCalculateDrawProperties( + root_.get(), 1.f, 1.f, NULL, can_use_lcd_text_); + EXPECT_EQ(can_use_lcd_text_, root_->can_use_lcd_text()); + EXPECT_FALSE(child_->can_use_lcd_text()); + EXPECT_FALSE(grand_child_->can_use_lcd_text()); // Case 7: Translucent. child_->SetTransform(identity_matrix); child_->SetOpacity(0.5f); - ExecuteCalculateDrawProperties(root_.get(), 1.f, 1.f, NULL, can_use_lcd_text_, - layers_always_allowed_lcd_text_); - EXPECT_EQ(expect_lcd_text, root_->can_use_lcd_text()); - EXPECT_EQ(expect_not_lcd_text, child_->can_use_lcd_text()); - EXPECT_EQ(expect_not_lcd_text, grand_child_->can_use_lcd_text()); + ExecuteCalculateDrawProperties( + root_.get(), 1.f, 1.f, NULL, can_use_lcd_text_); + EXPECT_EQ(can_use_lcd_text_, root_->can_use_lcd_text()); + EXPECT_FALSE(child_->can_use_lcd_text()); + EXPECT_FALSE(grand_child_->can_use_lcd_text()); // Case 8: Sanity check: restore transform and opacity. child_->SetTransform(identity_matrix); child_->SetOpacity(1.f); - ExecuteCalculateDrawProperties(root_.get(), 1.f, 1.f, NULL, can_use_lcd_text_, - layers_always_allowed_lcd_text_); - EXPECT_EQ(expect_lcd_text, root_->can_use_lcd_text()); - EXPECT_EQ(expect_lcd_text, child_->can_use_lcd_text()); - EXPECT_EQ(expect_lcd_text, grand_child_->can_use_lcd_text()); + ExecuteCalculateDrawProperties( + root_.get(), 1.f, 1.f, NULL, can_use_lcd_text_); + EXPECT_EQ(can_use_lcd_text_, root_->can_use_lcd_text()); + EXPECT_EQ(can_use_lcd_text_, child_->can_use_lcd_text()); + EXPECT_EQ(can_use_lcd_text_, grand_child_->can_use_lcd_text()); } TEST_P(LCDTextTest, CanUseLCDTextWithAnimation) { - bool expect_lcd_text = can_use_lcd_text_ || layers_always_allowed_lcd_text_; - // Sanity check: Make sure can_use_lcd_text_ is set on each node. - ExecuteCalculateDrawProperties(root_.get(), 1.f, 1.f, NULL, can_use_lcd_text_, - layers_always_allowed_lcd_text_); - EXPECT_EQ(expect_lcd_text, root_->can_use_lcd_text()); - EXPECT_EQ(expect_lcd_text, child_->can_use_lcd_text()); - EXPECT_EQ(expect_lcd_text, grand_child_->can_use_lcd_text()); + ExecuteCalculateDrawProperties( + root_.get(), 1.f, 1.f, NULL, can_use_lcd_text_); + EXPECT_EQ(can_use_lcd_text_, root_->can_use_lcd_text()); + EXPECT_EQ(can_use_lcd_text_, child_->can_use_lcd_text()); + EXPECT_EQ(can_use_lcd_text_, grand_child_->can_use_lcd_text()); // Add opacity animation. child_->SetOpacity(0.9f); AddOpacityTransitionToController( child_->layer_animation_controller(), 10.0, 0.9f, 0.1f, false); - ExecuteCalculateDrawProperties(root_.get(), 1.f, 1.f, NULL, can_use_lcd_text_, - layers_always_allowed_lcd_text_); + ExecuteCalculateDrawProperties( + root_.get(), 1.f, 1.f, NULL, can_use_lcd_text_); // Text AA should not be adjusted while animation is active. // Make sure LCD text AA setting remains unchanged. - EXPECT_EQ(expect_lcd_text, root_->can_use_lcd_text()); - EXPECT_EQ(expect_lcd_text, child_->can_use_lcd_text()); - EXPECT_EQ(expect_lcd_text, grand_child_->can_use_lcd_text()); + EXPECT_EQ(can_use_lcd_text_, root_->can_use_lcd_text()); + EXPECT_EQ(can_use_lcd_text_, child_->can_use_lcd_text()); + EXPECT_EQ(can_use_lcd_text_, grand_child_->can_use_lcd_text()); } INSTANTIATE_TEST_CASE_P(LayerTreeHostCommonTest, LCDTextTest, - testing::Combine(testing::Bool(), - testing::Bool(), - testing::Bool())); + testing::Combine(testing::Bool(), testing::Bool())); TEST_F(LayerTreeHostCommonTest, SubtreeHidden_SingleLayer) { FakeImplProxy proxy; diff --git a/cc/trees/layer_tree_host_pixeltest_masks.cc b/cc/trees/layer_tree_host_pixeltest_masks.cc index 4d0717d..f4f174c 100644 --- a/cc/trees/layer_tree_host_pixeltest_masks.cc +++ b/cc/trees/layer_tree_host_pixeltest_masks.cc @@ -24,6 +24,8 @@ class MaskContentLayerClient : public ContentLayerClient { explicit MaskContentLayerClient(const gfx::Size& bounds) : bounds_(bounds) {} ~MaskContentLayerClient() override {} + void DidChangeLayerCanUseLCDText() override {} + bool FillsBoundsCompletely() const override { return false; } void PaintContents( diff --git a/cc/trees/layer_tree_host_pixeltest_on_demand_raster.cc b/cc/trees/layer_tree_host_pixeltest_on_demand_raster.cc index e7dd149..d703e22 100644 --- a/cc/trees/layer_tree_host_pixeltest_on_demand_raster.cc +++ b/cc/trees/layer_tree_host_pixeltest_on_demand_raster.cc @@ -59,6 +59,8 @@ class BlueYellowLayerClient : public ContentLayerClient { explicit BlueYellowLayerClient(gfx::Rect layer_rect) : layer_rect_(layer_rect) {} + void DidChangeLayerCanUseLCDText() override {} + bool FillsBoundsCompletely() const override { return false; } void PaintContents( diff --git a/cc/trees/layer_tree_host_unittest.cc b/cc/trees/layer_tree_host_unittest.cc index b78dd5c..d9f7b83a 100644 --- a/cc/trees/layer_tree_host_unittest.cc +++ b/cc/trees/layer_tree_host_unittest.cc @@ -1114,6 +1114,7 @@ class TestOpacityChangeLayerDelegate : public ContentLayerClient { if (test_layer_) test_layer_->SetOpacity(0.f); } + void DidChangeLayerCanUseLCDText() override {} bool FillsBoundsCompletely() const override { return false; } private: @@ -2307,41 +2308,46 @@ class LayerTreeHostTestShutdownWithOnlySomeResourcesEvicted SINGLE_AND_MULTI_THREAD_NOIMPL_TEST_F( LayerTreeHostTestShutdownWithOnlySomeResourcesEvicted); -class LayerTreeHostTestLCDChange : public LayerTreeHostTest { +class LayerTreeHostTestLCDNotification : public LayerTreeHostTest { public: - class PaintClient : public FakeContentLayerClient { + class NotificationClient : public ContentLayerClient { public: - PaintClient() : paint_count_(0) {} + NotificationClient() + : layer_(0), paint_count_(0), lcd_notification_count_(0) {} + void set_layer(Layer* layer) { layer_ = layer; } int paint_count() const { return paint_count_; } + int lcd_notification_count() const { return lcd_notification_count_; } void PaintContents( SkCanvas* canvas, const gfx::Rect& clip, ContentLayerClient::GraphicsContextStatus gc_status) override { - FakeContentLayerClient::PaintContents(canvas, clip, gc_status); ++paint_count_; } - + void DidChangeLayerCanUseLCDText() override { + ++lcd_notification_count_; + layer_->SetNeedsDisplay(); + } bool FillsBoundsCompletely() const override { return false; } private: + Layer* layer_; int paint_count_; + int lcd_notification_count_; }; void SetupTree() override { - num_tiles_rastered_ = 0; - scoped_refptr<Layer> root_layer; if (layer_tree_host()->settings().impl_side_painting) root_layer = PictureLayer::Create(&client_); else root_layer = ContentLayer::Create(&client_); - client_.set_fill_with_nonsolid_color(true); root_layer->SetIsDrawable(true); - root_layer->SetBounds(gfx::Size(10, 10)); + root_layer->SetBounds(gfx::Size(1, 1)); layer_tree_host()->SetRootLayer(root_layer); + client_.set_layer(root_layer.get()); // The expecations are based on the assumption that the default // LCD settings are: @@ -2352,76 +2358,49 @@ class LayerTreeHostTestLCDChange : public LayerTreeHostTest { } void BeginTest() override { PostSetNeedsCommitToMainThread(); } + void AfterTest() override {} - void DidCommitAndDrawFrame() override { + void DidCommit() override { switch (layer_tree_host()->source_frame_number()) { case 1: - // The first update consists of a paint of the whole layer. + // The first update consists of one LCD notification and one paint. + EXPECT_EQ(1, client_.lcd_notification_count()); EXPECT_EQ(1, client_.paint_count()); // LCD text must have been enabled on the layer. EXPECT_TRUE(layer_tree_host()->root_layer()->can_use_lcd_text()); PostSetNeedsCommitToMainThread(); break; case 2: - // Since nothing changed on layer, there should be no paint. + // Since nothing changed on layer, there should be no notification + // or paint on the second update. + EXPECT_EQ(1, client_.lcd_notification_count()); EXPECT_EQ(1, client_.paint_count()); // LCD text must not have changed. EXPECT_TRUE(layer_tree_host()->root_layer()->can_use_lcd_text()); - // Change layer opacity that should trigger lcd change. + // Change layer opacity that should trigger lcd notification. layer_tree_host()->root_layer()->SetOpacity(.5f); + // No need to request a commit - setting opacity will do it. break; - case 3: - // LCD text doesn't require re-recording, so no painting should occur. - EXPECT_EQ(1, client_.paint_count()); + default: + // Verify that there is no extra commit due to layer invalidation. + EXPECT_EQ(3, layer_tree_host()->source_frame_number()); + // LCD notification count should have incremented due to + // change in layer opacity. + EXPECT_EQ(2, client_.lcd_notification_count()); + // Paint count should be incremented due to invalidation. + EXPECT_EQ(2, client_.paint_count()); // LCD text must have been disabled on the layer due to opacity. EXPECT_FALSE(layer_tree_host()->root_layer()->can_use_lcd_text()); - // Change layer opacity that should not trigger lcd change. - layer_tree_host()->root_layer()->SetOpacity(1.f); - break; - case 4: - // LCD text doesn't require re-recording, so no painting should occur. - EXPECT_EQ(1, client_.paint_count()); - // Even though LCD text could be allowed. - EXPECT_TRUE(layer_tree_host()->root_layer()->can_use_lcd_text()); EndTest(); break; } } - void NotifyTileStateChangedOnThread(LayerTreeHostImpl* host_impl, - const Tile* tile) override { - ++num_tiles_rastered_; - } - - void DrawLayersOnThread(LayerTreeHostImpl* host_impl) override { - switch (host_impl->active_tree()->source_frame_number()) { - case 0: - // The first draw. - EXPECT_EQ(1, num_tiles_rastered_); - break; - case 1: - // Nothing changed on the layer. - EXPECT_EQ(1, num_tiles_rastered_); - break; - case 2: - // LCD text was disabled, it should be re-rastered with LCD text off. - EXPECT_EQ(2, num_tiles_rastered_); - break; - case 3: - // LCD text was enabled but it's sticky and stays off. - EXPECT_EQ(2, num_tiles_rastered_); - break; - } - } - - void AfterTest() override {} - private: - PaintClient client_; - int num_tiles_rastered_; + NotificationClient client_; }; -SINGLE_AND_MULTI_THREAD_IMPL_TEST_F(LayerTreeHostTestLCDChange); +SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestLCDNotification); // Verify that the BeginFrame notification is used to initiate rendering. class LayerTreeHostTestBeginFrameNotification : public LayerTreeHostTest { @@ -2592,6 +2571,8 @@ class LayerTreeHostTestChangeLayerPropertiesInPaintContents layer_->SetBounds(gfx::Size(2, 2)); } + void DidChangeLayerCanUseLCDText() override {} + bool FillsBoundsCompletely() const override { return false; } private: diff --git a/cc/trees/layer_tree_impl.cc b/cc/trees/layer_tree_impl.cc index c005a18..6b2b37f 100644 --- a/cc/trees/layer_tree_impl.cc +++ b/cc/trees/layer_tree_impl.cc @@ -490,14 +490,18 @@ bool LayerTreeImpl::UpdateDrawProperties() { ++render_surface_layer_list_id_; LayerTreeHostCommon::CalcDrawPropsImplInputs inputs( - root_layer(), DrawViewportSize(), - layer_tree_host_impl_->DrawTransform(), device_scale_factor(), - total_page_scale_factor(), page_scale_layer, - resource_provider()->max_texture_size(), settings().can_use_lcd_text, - settings().layers_always_allowed_lcd_text, + root_layer(), + DrawViewportSize(), + layer_tree_host_impl_->DrawTransform(), + device_scale_factor(), + total_page_scale_factor(), + page_scale_layer, + resource_provider()->max_texture_size(), + settings().can_use_lcd_text, can_render_to_separate_surface, settings().layer_transforms_should_scale_layer_contents, - &render_surface_layer_list_, render_surface_layer_list_id_); + &render_surface_layer_list_, + render_surface_layer_list_id_); LayerTreeHostCommon::CalculateDrawProperties(&inputs); } diff --git a/cc/trees/layer_tree_settings.cc b/cc/trees/layer_tree_settings.cc index 14c1def..48c4133 100644 --- a/cc/trees/layer_tree_settings.cc +++ b/cc/trees/layer_tree_settings.cc @@ -43,7 +43,6 @@ LayerTreeSettings::LayerTreeSettings() timeout_and_draw_when_animation_checkerboards(true), maximum_number_of_failed_draws_before_draw_is_forced_(3), layer_transforms_should_scale_layer_contents(false), - layers_always_allowed_lcd_text(false), minimum_contents_scale(0.0625f), low_res_contents_scale_factor(0.25f), top_controls_height(0.f), diff --git a/cc/trees/layer_tree_settings.h b/cc/trees/layer_tree_settings.h index 69ac995..3097e59 100644 --- a/cc/trees/layer_tree_settings.h +++ b/cc/trees/layer_tree_settings.h @@ -54,7 +54,6 @@ class CC_EXPORT LayerTreeSettings { bool timeout_and_draw_when_animation_checkerboards; int maximum_number_of_failed_draws_before_draw_is_forced_; bool layer_transforms_should_scale_layer_contents; - bool layers_always_allowed_lcd_text; float minimum_contents_scale; float low_res_contents_scale_factor; float top_controls_height; |