[PATCH] glamor: store old fonts in double width textures.

[Keith Packard]

Dave Airlie <airlied at gmail.com> writes:

> From: Dave Airlie <airlied at redhat.com>
>
> There is a problem with some fonts that the height necessary
> to store the font is greater than the max texture size, which
> causes a fallback to occur. We can avoid this by storing two
> macro columns side-by-side in the texture and adjusting
> the calculations to suit.
>
> This fixes
> xfd -fn -*-*-*-*-*-*-*-*-*-*-*-*-*-*
> falling back here, when it picks
> -arabic-newspaper-medium-r-normal--32-246-100-100-p-137-iso10646-1
>
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
>  glamor/glamor_font.c | 21 +++++++++++++++++----
>  glamor/glamor_font.h |  2 +-
>  glamor/glamor_text.c | 10 +++++++---
>  3 files changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/glamor/glamor_font.c b/glamor/glamor_font.c
> index bda1b58..3c95324 100644
> --- a/glamor/glamor_font.c
> +++ b/glamor/glamor_font.c
> @@ -77,11 +77,18 @@ glamor_font_get(ScreenPtr screen, FontPtr font)
>      glamor_font->glyph_width_bytes = glyph_width_bytes;
>      glamor_font->glyph_height = glyph_height;
>  
> -    overall_width = glyph_width_bytes * num_cols;
> -    overall_height = glyph_height * num_rows;
> +    /*
> +     * Layout the font two blocks of columns wide.
> +     * This avoids a problem with some fonts that are too high to fit.
> +     */
> +    glamor_font->row_width = glyph_width_bytes * num_cols;
> +
> +    overall_width = glamor_font->row_width * ((num_rows > 1) ? 2 : 1);
> +    overall_height = glyph_height * ((num_rows + 1) / 2);

Cute math here, but I think it'd be clearer to just make the two cases separate:

        if (num_rows > 1) {
                overall_width = glamor_font->row_width * 2;
                overall_height = glyph_height * ((num_rows + 1) / 2);
        } else {
                overall_width = glamor_font->row_width;
                overall_height = glyph_height;
        }

Feel free to ignore this suggestion; it does make the code slower (!)

> -
> +            Bool    second_row = false;
>              x += ci->metrics.characterWidth;
>  
>              if (sixteen) {
> @@ -153,8 +153,10 @@ glamor_text(DrawablePtr drawable, GCPtr gc,
>                      row = chars[0];
>                      col = chars[1];
>                  }
> -                if (FONTLASTROW(font) != 0)
> -                    ty = (row - firstRow) * glyph_spacing_y;
> +                if (FONTLASTROW(font) != 0) {
> +                    ty = ((row - firstRow) / 2) * glyph_spacing_y;
> +                    second_row = (row - firstRow) & 1;
> +                }
>                  else
>                      col += row << 8;
>              } else {
> @@ -165,6 +167,8 @@ glamor_text(DrawablePtr drawable, GCPtr gc,
>              }
>  
>              tx = (col - firstCol) * glyph_spacing_x;
> +            /* adjust for second row layout */
> +            tx += second_row * glamor_font->row_width * 8;

I'm not OK using a Bool for multiplication here. Just declare it as an
int and initialize to zero and we'll be good.

Otherwise, this patch lgtm.

Reviewed-by: Keith Packard <keithp at keithp.com>

(and here I expected to see the multi-texture patch coming!)

-- 
-keith
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 810 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20160111/13c9941f/attachment.sig>