Add FontRenderParamsQuery.

ui/gfx/font_render_params.h

 // Copyright 2014 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.
 
 #ifndef UI_GFX_FONT_RENDER_PARAMS_H_
 #define UI_GFX_FONT_RENDER_PARAMS_H_
 
 #include <string>
 #include <vector>
 
@@ skipping 36 matching lines @@
   bool autohinter;
 
   // Should embedded bitmaps in fonts should be used?
   bool use_bitmaps;
 
   // Hinting level.
   Hinting hinting;
 
   // 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();
+// 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();
 
-// Returns the system's default parameters for WebKit font rendering.
-// TODO(derat): Rename to GetDefaultFontRenderParamsForWebContents().
-GFX_EXPORT const FontRenderParams& GetDefaultWebKitFontRenderParams();
+  bool is_empty() const {
+    return families.empty() && pixel_size <= 0 && point_size <= 0 && style < 0;
+  }
 
-// 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,
+  // 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
 
 #endif  // UI_GFX_FONT_RENDER_PARAMS_H_