Add FontRenderParamsQuery.

ui/gfx/font_render_params.h

Index: ui/gfx/font_render_params.h
diff --git a/ui/gfx/font_render_params.h b/ui/gfx/font_render_params.h
index 1b49e04220d5d594e8ec6f0086c2a6311914de9e..99ba2b0ca1bbe90d469e859df16c48f9b8704243 100644
--- a/ui/gfx/font_render_params.h
+++ b/ui/gfx/font_render_params.h
@@ -54,26 +54,41 @@ struct GFX_EXPORT FontRenderParams {
 
   // Whether subpixel rendering should be used or not, and if so, the display's
   // subpixel order.
+  // TODO(derat): Remove this; we don't set it in the browser and mostly ignore

[msw, 2014/07/23 22:16:34]

I think you mean to put this with |subpixel_positioning|, not
|subpixel_rendering|, which seems far more important and widely used.

[Daniel Erat, 2014/07/23 22:47:17]

whoops, yeah. done

+  // it in Blink: http://crbug.com/396659
   SubpixelRendering subpixel_rendering;
 };
 
-// Returns the system's default parameters for font rendering.
-GFX_EXPORT const FontRenderParams& GetDefaultFontRenderParams();
-
-// Returns the system's default parameters for WebKit font rendering.
-// TODO(derat): Rename to GetDefaultFontRenderParamsForWebContents().
-GFX_EXPORT const FontRenderParams& GetDefaultWebKitFontRenderParams();
-
-// Returns the appropriate parameters for rendering the font described by the
-// passed-in-arguments, any of which may be NULL. If |family_out| is non-NULL,
-// it will be updated to contain the recommended font family from |family_list|.
-// |style| optionally points to a bit field of Font::FontStyle values.
-GFX_EXPORT FontRenderParams GetCustomFontRenderParams(
-    bool for_web_contents,
-    const std::vector<std::string>* family_list,
-    const int* pixel_size,
-    const int* point_size,
-    const int* style,
+// A query used to determine the appropriate FontRenderParams.

[Daniel Erat, 2014/07/23 20:40:34]

if we want to, this should also make it easy to cache FontRenderParams in the
linux implementation to avoid issuing a bunch of redundant fontconfig queries

[msw, 2014/07/23 22:54:31]

That'd be good, I think we do a bunch on startup...

+struct GFX_EXPORT FontRenderParamsQuery {
+  explicit FontRenderParamsQuery(bool for_web_contents);

[Daniel Erat, 2014/07/23 20:40:34]

the for_web_contents argument here is a bit strange (along with it not being
checked in is_empty()), but it allows inline calls to initialize
FontRenderParams static variables and should disappear after i do
http://crbug.com/396659

[msw, 2014/07/23 22:16:35]

Acknowledged.

+  ~FontRenderParamsQuery();
+
+  bool is_empty() const {
+    return families.empty() && pixel_size <= 0 && point_size <= 0 && style < 0;
+  }
+
+  // True if rendering text for the web.
+  // TODO(derat): Remove this once FontRenderParams::subpixel_rendering is gone:

[msw, 2014/07/23 22:16:35]

Do you mean |subpixel_positioning|?

[Daniel Erat, 2014/07/23 22:47:17]

sigh, yep. at least i am consistently wrong.

+  // http://crbug.com/396659
+  bool for_web_contents;
+
+  // Requested font families, or empty if unset.
+  std::vector<std::string> families;
+
+  // Font size in pixels or points, or 0 if unset.
+  int pixel_size;
+  int point_size;
+
+  // gfx::Font::FontStyle bit field, or -1 if unset.

[Daniel Erat, 2014/07/23 20:40:34]

i don't love the -1 default here, because someone may do this and then be
confused about why their style is ignored:

  FontRenderParamsQuery query;
  query.style |= Font::BOLD;
  FontRenderParams params = GetFontRenderParams(query, NULL);

other alternatives that i also don't love:

- initializing this to gfx::Font::NORMAL, which means that there's no way to
signal that we don't want to include a style in the fontconfig query

- adding a separate use_style member, which someone can forget to set

- turning this into a class, making this a private member, and adding a
set_style() method so it's impossible to do a bitwise or with the "unset" value

can you think of anything better?

[msw, 2014/07/23 22:16:35]

Hmmm, I agree it's not ideal, but the results of misuse should be apparent and
without really severe consequences. I think it's fine as-is, plus maybe adding a
gfx::Font enum -1 like NONE, UNKNOWN_STYLE, UNSET, or similar.

[Daniel Erat, 2014/07/23 22:47:17]

i'm a bit nervous about adding a -1 constant to gfx::Font, since some other
class that uses Font::Style may not handle NONE correctly and instead believe
that all of the style bits are set. i'd rather keep the -1 hack constrained to
FontRenderParams.

[msw, 2014/07/23 22:54:31]

Acknowledged.

+  int style;
+};
+
+// Returns the appropriate parameters for rendering the font described by
+// |query|. If |family_out| is non-NULL, it will be updated to contain the
+// recommended font family from |query.families|.
+GFX_EXPORT FontRenderParams GetFontRenderParams(
+    const FontRenderParamsQuery& query,
     std::string* family_out);

[msw, 2014/07/23 22:16:35]

Should |family_out| be a member of FontRenderParams?
(maybe done in a separate CL)

[Daniel Erat, 2014/07/23 22:47:18]

hmm, i don't think so. it's really not very similar to the other members of that
struct, and i mostly just put it here to avoid needing to do an extra fontconfig
query. this is probably something we should figure out if/when trying to unify
the fallback code between blink and the browser for http://crbug.com/395313.

[msw, 2014/07/23 22:54:31]

Acknowledged.

 
 }  // namespace gfx