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/layers | |
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/layers')
-rw-r--r-- | cc/layers/content_layer.cc | 14 | ||||
-rw-r--r-- | cc/layers/content_layer.h | 3 | ||||
-rw-r--r-- | cc/layers/content_layer_client.h | 4 | ||||
-rw-r--r-- | cc/layers/picture_image_layer.h | 1 | ||||
-rw-r--r-- | cc/layers/picture_layer.cc | 29 | ||||
-rw-r--r-- | cc/layers/picture_layer.h | 4 | ||||
-rw-r--r-- | cc/layers/picture_layer_impl_unittest.cc | 6 | ||||
-rw-r--r-- | cc/layers/picture_layer_unittest.cc | 1 |
8 files changed, 43 insertions, 19 deletions
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; }; }; |