RenderTextHarfBuzz: Set font render parameters in font data functions

ui/gfx/render_text.cc

Index: ui/gfx/render_text.cc
diff --git a/ui/gfx/render_text.cc b/ui/gfx/render_text.cc
index 807f8cd12ebe8d5b2579c25df00b65947789bc13..615c7aa9aaa98a1ac20cbb19319ffc0e75058baf 100644
--- a/ui/gfx/render_text.cc
+++ b/ui/gfx/render_text.cc
@@ -211,14 +211,11 @@ void SkiaTextRenderer::SetDrawLooper(SkDrawLooper* draw_looper) {
   paint_.setLooper(draw_looper);
 }
 
-void SkiaTextRenderer::SetFontRenderParams(const FontRenderParams& params,
+void SkiaTextRenderer::SetFontRenderParams(FontRenderParams params,

[Daniel Erat, 2014/08/17 01:53:35]

i'd prefer that you left this as a const reference; it feels strange to me to
change the public interface to do a copy to accommodate a corner case of the
implementation.

i'd also still prefer that the
disable-subpixel-rendering-for-transparent-backgrounds logic only lived in one
place. regarding the earlier discussion: i meant that ApplyRenderParams() could
also take a background_is_transparent argument. is there a reason that that
wouldn't work?

void SetFontRenderParams(const FontRenderParams& params,
                         bool background_is_transparent) {
  ApplyRenderParams(params, background_is_transparent, &paint_);
}

void ApplyRenderParams(const FontRenderParams& params,
                       bool background_is_transparent,
                       SkPaint* paint) {
  ...
  paint->setLCDRenderText(!background_is_transparent &&
      params.subpixel_rendering != FontRenderParams::SUBPIXEL_RENDERING_NONE);
}

and then pass background_is_transparent through CreateHarfBuzzFont() as well, if
that doesn't seem too gross.

(i take it that there's no way to determine from the SkPaint that the
background's going to be transparent at this point, right?)

[ckocagil, 2014/08/18 20:01:22]

Done!

                                            bool background_is_transparent) {
-  paint_.setAntiAlias(params.antialiasing);
-  paint_.setLCDRenderText(!background_is_transparent &&
-      params.subpixel_rendering != FontRenderParams::SUBPIXEL_RENDERING_NONE);
-  paint_.setSubpixelText(params.subpixel_positioning);
-  paint_.setAutohinted(params.autohinter);
-  paint_.setHinting(FontRenderParamsHintingToSkPaintHinting(params.hinting));
+  if (background_is_transparent)
+    params.subpixel_rendering = FontRenderParams::SUBPIXEL_RENDERING_NONE;
+  ApplyRenderParams(params, &paint_);
 }
 
 void SkiaTextRenderer::SetTypeface(SkTypeface* typeface) {
@@ -416,6 +413,15 @@ skia::RefPtr<SkTypeface> CreateSkiaTypeface(const std::string& family,
   return skia::AdoptRef(SkTypeface::CreateFromName(family.c_str(), skia_style));
 }
 
+void ApplyRenderParams(const FontRenderParams& params, SkPaint* paint) {
+  paint->setAntiAlias(params.antialiasing);
+  paint->setLCDRenderText(
+      params.subpixel_rendering != FontRenderParams::SUBPIXEL_RENDERING_NONE);
+  paint->setSubpixelText(params.subpixel_positioning);
+  paint->setAutohinted(params.autohinter);
+  paint->setHinting(FontRenderParamsHintingToSkPaintHinting(params.hinting));
+}
+
 }  // namespace internal
 
 RenderText::~RenderText() {