[Cairo] Review of text API in new virtualized font interface

Carl Worth cworth at east.isi.edu
Tue Oct 21 11:47:46 PDT 2003


On Oct 20, graydon hoare wrote:
 > ok. I've changed this to do what Xft does (call FcUtf8ToUcs4 in a
 > loop) and also moved the cairo_ft_font_t to a private type, as carl
 > suggested. now there's just cairo_font_t in the public header, and the
 > ft-specific functions fail when passed something of the wrong
 > type. updated patch attached. anything else?

Hi graydon,

I'm sorry I haven't given in-depth feedback on your patch sooner.

One first note is that I don't want to commit anything to cairo.h that
does not yet have a working implementation, (looks like that includes
text_path, glyph_path, and the Win32 stuff right now). It would be OK
to commit commented-out declarations to cement API decisions that have
been made.

As for the implementation, I'm not too picky as long as it works. If
there are any organizational/style issues we can take that offline or
just work with it in CVS.

But I do want to make sure we have good consensus on the public API
before we go ahead. I've gone back and re-read the "Text APIs round 2"
thread. There was a lot of discussion there, but things didn't always
come to solid conclusions. So that left some things up in the air.

With the background of that discussion, and with this concrete patch
in hand, I think we should be able to nail down some decisions in
short order. I'll comment on specific portions of the patch below. In
each case, I've tried to make concrete proposals based on past
discussion.

I'm truly clueless when it comes to text/fonts, so any good ideas
below come from others in past or recent discussions. I'm also not
trying to lay down absolute decrees. I am interested in feedback, but
only with concrete proposals for changes. We've had lots of general
discussion already, and I'm more interested now in getting some code
committed.

	+typedef unsigned long cairo_glyph_index_t;

This typedef seems gratuitous. Why not just use "unsigned long" in the
struct below.

	+typedef struct cairo_glyph {
	+  cairo_font_t         *font;
	+  cairo_glyph_index_t  index;
	+  double               x;
	+  double               y;
	+} cairo_glyph_t;

There was a fair amount of discussion around eliminating the
cairo_font_t field from this structure, but Keith put forth
correctness and efficiency concerns. Near the end of the discussion,
Keith and Owen came up with the idea of adding optional
cairo_text_begin and cairo_text_end calls for merging text
operations. Keith, with an eye toward those calls, we can get away
with dropping the font field from the glyph structure, correct? Unless
Keith objects, I propose dropping this field.

	+typedef enum cairo_font_weight {
	+  CAIRO_FONT_WEIGHT_PLAIN,
	+  CAIRO_FONT_WEIGHT_BOLD
	+} cairo_font_weight_t;
	+  
	+typedef enum cairo_font_slant {
	+  CAIRO_FONT_SLANT_PLAIN,
	+  CAIRO_FONT_SLANT_ITALIC
	+} cairo_font_slant_t;

Why don't we follow the naming convention from CSS2 here. We can even
throw in the CSS2 weight values if we want, though I don't know if
that's really important:

typedef enum cairo_font_weight {
   CAIRO_FONT_WEIGHT_NORMAL = 400,
   CAIRO_FONT_WEIGHT_BOLD = 700
} cairo_font_weight_t;
        
typedef enum cairo_font_style {
    CAIRO_FONT_STYLE_NORMAL,
    CAIRO_FONT_STYLE_ITALIC,
    CAIRO_FONT_STYLE_OBLIQUE
} cairo_font_style_t;

	+extern void __external_linkage
	+cairo_select_font (cairo_t              *ct, 
	+                  char                 *family, 
	+                  double               scale,
	+                  cairo_font_slant_t   slant, 
	+                  cairo_font_weight_t  weight);

Of course, rename slant to style here as well. I think we're probably
fine ignoring variant and stretch from CSS2 since the idea here is to
make a simple text-selection API, not necessarily a complete one.

The scale parameter here is a new addition since the original "Text
APIs round 2" proposal. I'm not clear on the intended semantics of
this argument. Then again, that same proposal didn't have any
mechanism in the toy API for specifying a font size.

I would prefer to eliminate this argument from select_font. Instead I
think select_font should act in a PostScript-like fashion, installing
a font that has a line height of 1 user-space unit. Then, we can add
two functions to allow the user to transform the current font:

void
cairo_scale_font (cairo_t *cr, double scale);

void
cairo_transform_font (cairo_t *cr, cairo_matrix_t *matrix);

	+extern void __external_linkage
	+cairo_text_extents (cairo_t                *ct,
	+                   const unsigned char    *utf8,
	+                   double                 *x, 
	+                   double                 *y,
	+                   double                 *width, 
	+                   double                 *height,
	+                   double                 *dx, 
	+                   double                 *dy);

This function (and glyph_extents) have too many parameters. Let's add
a struct to simplify things. We'll also change to some more meaningful
names. All values here are in user-space units:

typedef struct {
    double left_side_bearing;
    double right_side_bearing;
    double ascent;
    double descent;
    double x_advance;
    double y_advance;
} cairo_text_extents_t;

void
cairo_text_extents (cairo_t *cr,
		    const unsigned char *utf8,
		    cairo_text_extents_t *extents);

void
cairo_glyph_extents (cairo_t *cr,
		     cairo_glyph_t *glyphs, int num_glyphs,
		     cairo_text_extents_t *extents);

Let's also convenience functions for getting the x_advance value:

double
cairo_text_x_advance (cairo_t *cr, const unsigned char *utf8);

double
cairo_glyph_x_advance (cairo_t *cr, cairo_glyph_t *glyphs, int num_glyphs)

	+extern cairo_font_t * __external_linkage
	+cairo_font_create (cairo_t              *ct, 
	+                  char                 *family, 
	+                  double               scale,
	+                  cairo_font_slant_t   slant, 
	+                  cairo_font_weight_t  weight);

I don't think we need this function at all. There's not much that
could be done with the returned cairo_font_t except for passing it to
cairo_set_font. But that's exactly what cairo_select_font does
anyway. The user can also call cairo_current_font if a pointer to the
cairo_font_t is really needed. Let's drop this function to avoid
confusion.

	+extern char * __external_linkage
	+cairo_font_current_family (cairo_font_t *font);
	+
	+extern double __external_linkage
	+cairo_font_current_scale (cairo_font_t *font);
	+
	+extern cairo_font_slant_t __external_linkage
	+cairo_font_current_slant (cairo_font_t *font);
	+
	+extern cairo_font_weight_t __external_linkage
	+cairo_font_current_weight (cairo_font_t *font);

The return values of these functions are not necessarily well-defined
for all fonts. Let's drop these. (And if we didn't, they should be
renamed with "get" in place of "current".)

	 extern void __external_linkage
	+cairo_font_transform (cairo_font_t *font, 
	+                     double xx, double xy,
	+                     double yx, double yy);

This function should accept a "cairo_matrix_t *matrix" instead of 4
doubles.

There was quite a bit of discussion about whether fonts should be
resizable or fixed to a single-size at creation time. I think that the
user should be able to transform a font after creating it. We can be
lazy about resolving the font until it needs to be drawn. And, if
needed, we can freeze the font after it has been resolved. We can also
add cairo_font_freeze/cairo_font_thaw calls or similar if the user
needs to control this. So I propose keeping the cairo_font_transform
call.

I also propose adding a new convenience function:

void
cairo_font_scale (cairo_font_t *font, double scale).

	 extern void __external_linkage
	+cairo_font_set_transform (cairo_font_t *font, 
	+                         double xx, double xy,
	+                         double yx, double yy);
 
	 extern void __external_linkage
	+cairo_font_current_transform (cairo_font_t *font, 
	+                             double *xx, double *xy,
	+                             double *yx, double *yy);

These need to accept a "cairo_matrix_t *matrix" as well, of course.

I'm trying to make sense of the set_transform call. I suppose that at
creation time, any cairo_font_t has an identity transform? If not, I
can't see how set_transform could have meaningful semantics.

	+extern double __external_linkage
	+cairo_font_ascent (cairo_font_t *font);
	+
	+extern double __external_linkage
	+cairo_font_descent (cairo_font_t *font);
	+
	+extern double __external_linkage
	+cairo_font_height (cairo_font_t *font);
	+
	+extern double __external_linkage
	+cairo_font_max_advance_width (cairo_font_t *font);

Let's replace these 4 functions with a single function accepting a new
struct. We'll also add the maximum Y advance value. Plus, getting
these metrics will require access to the CTM, so the function must
accept a cairo_t.

typedef struct {
    double ascent;
    double descent;
    double height;
    double max_x_advance;
    double max_y_advance;
} cairo_font_extents_t;

void
cairo_current_font_extents (cairo_t *cr, cairo_font_extents_t *extents);

	+#ifdef WIN32
	...
	+#else  /* !defined(WIN32) */

No WIN32 support exists yet. This should be dropped from the patch.

	+/* Fontconfig/Freetype platform-specific font interface */

The freetype-specific portion of the API should be moved into a new
file cairo-ft.h.

	+extern cairo_font_t * __external_linkage
	+cairo_ft_font_create_scale (FcPattern *pattern, 
	+                           double scale);
	+
	+extern cairo_font_t * __external_linkage
	+cairo_ft_font_create_transform (FcPattern *pattern, 
	+                               cairo_matrix_t *transform);

Since we have cairo_font_scale and cairo_font_transform, I propose
dropping these two variants of cairo_ft_font_create. That reduces the
number of functions and also clarifies the ambiguous interaction
between scale/transform and any size information in the FcPattern.

That sums it up for my comments on the new text API. Hopefully we can
have this comitted soon.

-Carl




More information about the cairo mailing list