cc: Toggle LCD text at raster time instead of record time.

cc/resources/resource_provider.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/resources/resource_provider.h"
 
 #include <algorithm>
 #include <limits>
 
 #include "base/containers/hash_tables.h"
@@ skipping 1082 matching lines @@
     : resource_provider_(resource_provider),
       resource_(resource_provider->LockForWrite(resource_id)) {
 }
 
 ResourceProvider::ScopedWriteLockGr::~ScopedWriteLockGr() {
   DCHECK(thread_checker_.CalledOnValidThread());
   resource_provider_->UnlockForWrite(resource_);
 }
 
 SkSurface* ResourceProvider::ScopedWriteLockGr::GetSkSurface(
-    bool use_distance_field_text) {
+    bool use_distance_field_text,
+    bool can_use_lcd_text) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(resource_->locked_for_write);
 
   // If the surface doesn't exist, or doesn't have the correct dff setting,
   // recreate the surface within the resource.

[reveman, 2014/11/11 21:16:49]

this comment needs an update.

[danakj, 2014/11/12 17:22:10]

Done.

-  if (!resource_->sk_surface ||
-      use_distance_field_text !=
-          resource_->sk_surface->props().isUseDistanceFieldFonts()) {
+  bool create_surface = !resource_->sk_surface;
+  if (!create_surface) {

[reveman, 2014/11/11 21:16:49]

this conditional seems overkill. Temporary variables for
surface_uses_distance_field_text and surface_can_use_lcd_text is nice but
create_surface is not needed imo.

[danakj, 2014/11/11 21:56:44]

It avoids 2 virtual function calls per resource in the common case though?

[reveman, 2014/11/13 17:23:15]

What are the virtual function calls we avoid here? Even if that's the case,
avoiding 2 virtual function calls per gpu rasterized tile seems like a premature
micro-optimization that I have a hard time to imagine makes a difference. I
prefer if we keep this as simple and clean as possible for now.

[danakj, 2014/11/13 17:30:32]

Mmh.. oh I was thinking of the CanUseLcdText function.. woops. OK.

[danakj, 2014/11/13 19:20:50]

OK I split this into a ShouldCreateSurface() function so we can return true
instead of setting create_surface=true in multiple places.

+    bool surface_uses_distance_field_text =
+        resource_->sk_surface->props().isUseDistanceFieldFonts();
+    if (use_distance_field_text != surface_uses_distance_field_text)
+      create_surface = true;
+    bool surface_can_use_lcd_text =
+        resource_->sk_surface->props().pixelGeometry() !=
+        kUnknown_SkPixelGeometry;
+    if (can_use_lcd_text != surface_can_use_lcd_text)
+      create_surface = true;
+  }
+  if (create_surface) {
     class GrContext* gr_context = resource_provider_->GrContext();
     // TODO(alokp): Implement TestContextProvider::GrContext().
     if (!gr_context)
       return nullptr;
 
     resource_provider_->LazyAllocate(resource_);
 
     GrBackendTextureDesc desc;
     desc.fFlags = kRenderTarget_GrBackendTextureFlag;
     desc.fWidth = resource_->size.width();
     desc.fHeight = resource_->size.height();
     desc.fConfig = ToGrPixelConfig(resource_->format);
     desc.fOrigin = kTopLeft_GrSurfaceOrigin;
     desc.fTextureHandle = resource_->gl_id;
     skia::RefPtr<GrTexture> gr_texture =
         skia::AdoptRef(gr_context->wrapBackendTexture(desc));
     if (!gr_texture)
       return nullptr;
-    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(), text_render_mode));
+    uint32_t flags = use_distance_field_text
+                         ? SkSurfaceProps::kUseDistanceFieldFonts_Flag
+                         : 0;
+    if (can_use_lcd_text) {
+      // LegacyFontHost will get LCD text and skia figures out what type to use.
+      SkSurfaceProps surface_props(flags,
+                                   SkSurfaceProps::kLegacyFontHost_InitType);
+      resource_->sk_surface = skia::AdoptRef(SkSurface::NewRenderTargetDirect(
+          gr_texture->asRenderTarget(), &surface_props));
+    } else {
+      // Use unknown pixel geometry to disable LCD text.
+      SkSurfaceProps surface_props(flags, kUnknown_SkPixelGeometry);
+      resource_->sk_surface = skia::AdoptRef(SkSurface::NewRenderTargetDirect(

[reveman, 2014/11/11 21:16:49]

Please avoid duplication of this line. I had to spend some time to determine
that it is the same as above. Maybe make this more consistent with your change
to RWP.

[danakj, 2014/11/11 21:56:44]

OK. I had it that way first but it looked really ugly with the flags here. I'll
try again. 

[danakj, 2014/11/12 17:22:10]

Done.

+          gr_texture->asRenderTarget(), &surface_props));
+    }
   }
   return resource_->sk_surface.get();
 }
 
 ResourceProvider::SynchronousFence::SynchronousFence(
     gpu::gles2::GLES2Interface* gl)
     : gl_(gl), has_synchronized_(true) {
 }
 
 ResourceProvider::SynchronousFence::~SynchronousFence() {
@@ skipping 979 matching lines @@
   ContextProvider* context_provider = output_surface_->context_provider();
   return context_provider ? context_provider->ContextGL() : NULL;
 }
 
 class GrContext* ResourceProvider::GrContext() const {
   ContextProvider* context_provider = output_surface_->context_provider();
   return context_provider ? context_provider->GrContext() : NULL;
 }
 
 }  // namespace cc