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

cc/resources/resource_provider.cc

Index: cc/resources/resource_provider.cc
diff --git a/cc/resources/resource_provider.cc b/cc/resources/resource_provider.cc
index 023a1cdbf068832875073a5601c29420043d74b5..a3ff949376dbe8bd327f4964880a7ecd3ab5100b 100644
--- a/cc/resources/resource_provider.cc
+++ b/cc/resources/resource_provider.cc
@@ -1100,15 +1100,26 @@ ResourceProvider::ScopedWriteLockGr::~ScopedWriteLockGr() {
 }
 
 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)
@@ -1127,11 +1138,21 @@ SkSurface* ResourceProvider::ScopedWriteLockGr::GetSkSurface(
         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();
 }