Skip to content

Commit

Permalink
Don't use DF text during animations
Browse files Browse the repository at this point in the history
Previously, we enabled distance-field text during animations in
order to improve GPU raster performance. Now that GPU raster
handles animations the same way as SW - only rastering at specific
scales and interpolating, this optimization isn't necessary.

In certain cases, the optimization causes visual differences
between non-animating and animating content, so removing this
improves our correctness as well.

R=ajuma@chromium.org
BUG=668723
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2605273003
Cr-Commit-Position: refs/heads/master@{#441204}
  • Loading branch information
ericrk authored and Commit bot committed Jan 3, 2017
1 parent 1b13222 commit b04a4a6
Show file tree
Hide file tree
Showing 5 changed files with 1 addition and 32 deletions.
3 changes: 0 additions & 3 deletions cc/layers/picture_layer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -478,9 +478,6 @@ bool PictureLayerImpl::UpdateTiles() {
was_screen_space_transform_animating_ =
draw_properties().screen_space_transform_is_animating;

if (screen_space_transform_is_animating())
raster_source_->SetShouldAttemptToUseDistanceFieldText();

double current_frame_time_in_seconds =
(layer_tree_impl()->CurrentBeginFrameArgs().frame_time -
base::TimeTicks()).InSecondsF();
Expand Down
1 change: 0 additions & 1 deletion cc/layers/picture_layer_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2586,7 +2586,6 @@ TEST_F(PictureLayerImplTest, HighResTilingDuringAnimation) {
maximum_animation_scale,
starting_animation_scale, animating_transform);
EXPECT_BOTH_EQ(HighResTiling()->contents_scale_key(), 3.f);
EXPECT_BOTH_TRUE(GetRasterSource()->ShouldAttemptToUseDistanceFieldText());

// Further changes to scale during the animation should not cause a new
// high-res tiling to get created.
Expand Down
11 changes: 0 additions & 11 deletions cc/playback/raster_source.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ RasterSource::RasterSource(const RecordingSource* other, bool can_use_lcd_text)
clear_canvas_with_debug_color_(other->clear_canvas_with_debug_color_),
slow_down_raster_scale_factor_for_debug_(
other->slow_down_raster_scale_factor_for_debug_),
should_attempt_to_use_distance_field_text_(false),
image_decode_cache_(nullptr) {}

RasterSource::RasterSource(const RasterSource* other, bool can_use_lcd_text)
Expand All @@ -56,8 +55,6 @@ RasterSource::RasterSource(const RasterSource* other, bool can_use_lcd_text)
clear_canvas_with_debug_color_(other->clear_canvas_with_debug_color_),
slow_down_raster_scale_factor_for_debug_(
other->slow_down_raster_scale_factor_for_debug_),
should_attempt_to_use_distance_field_text_(
other->should_attempt_to_use_distance_field_text_),
image_decode_cache_(other->image_decode_cache_) {}

RasterSource::~RasterSource() {
Expand Down Expand Up @@ -277,14 +274,6 @@ gfx::Rect RasterSource::RecordedViewport() const {
return recorded_viewport_;
}

void RasterSource::SetShouldAttemptToUseDistanceFieldText() {
should_attempt_to_use_distance_field_text_ = true;
}

bool RasterSource::ShouldAttemptToUseDistanceFieldText() const {
return should_attempt_to_use_distance_field_text_;
}

void RasterSource::AsValueInto(base::trace_event::TracedValue* array) const {
if (display_list_.get())
TracedValue::AppendIDRef(display_list_.get(), array);
Expand Down
11 changes: 0 additions & 11 deletions cc/playback/raster_source.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,6 @@ class CC_EXPORT RasterSource : public base::RefCountedThreadSafe<RasterSource> {
// Valid rectangle in which everything is recorded and can be rastered from.
virtual gfx::Rect RecordedViewport() const;

// Informs the raster source that it should attempt to use distance field text
// during rasterization.
virtual void SetShouldAttemptToUseDistanceFieldText();

// Return true iff this raster source would benefit from using distance
// field text.
virtual bool ShouldAttemptToUseDistanceFieldText() const;

// Tracing functionality.
virtual void DidBeginTracing();
virtual void AsValueInto(base::trace_event::TracedValue* array) const;
Expand Down Expand Up @@ -153,9 +145,6 @@ class CC_EXPORT RasterSource : public base::RefCountedThreadSafe<RasterSource> {
const gfx::Size size_;
const bool clear_canvas_with_debug_color_;
const int slow_down_raster_scale_factor_for_debug_;
// TODO(enne/vmiura): this has a read/write race between raster and compositor
// threads with multi-threaded Ganesh. Make this const or remove it.
bool should_attempt_to_use_distance_field_text_;

// In practice, this is only set once before raster begins, so it's ok with
// respect to threading.
Expand Down
7 changes: 1 addition & 6 deletions cc/raster/gpu_raster_buffer_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -213,16 +213,11 @@ void GpuRasterBufferProvider::PlaybackOnWorkerThread(
gl->WaitSyncTokenCHROMIUM(sync_token.GetConstData());
}

// Turn on distance fields for layers that have ever animated.
bool use_distance_field_text =
use_distance_field_text_ ||
raster_source->ShouldAttemptToUseDistanceFieldText();

RasterizeSource(raster_source, resource_has_previous_content,
resource_lock->size(), raster_full_rect, raster_dirty_rect,
scales, playback_settings, worker_context_provider_,
resource_lock, async_worker_context_enabled_,
use_distance_field_text, msaa_sample_count_);
use_distance_field_text_, msaa_sample_count_);

const uint64_t fence_sync = gl->InsertFenceSyncCHROMIUM();

Expand Down

0 comments on commit b04a4a6

Please sign in to comment.