[PATCH] glamor: fallback if font is too large for FBO size.

[Dave Airlie]

On 12 January 2016 at 16:15, Keith Packard <keithp at keithp.com> wrote:
> Dave Airlie <airlied at gmail.com> writes:
>
>> From: Dave Airlie <airlied at redhat.com>
>>
>> running xfontsel on haswell here, with a max texture size
>> of 8kx8k, one font wants 9711 height. This fallsback to
>> sw in this case.
>>
>> A proper solution probably involves using an array texture.
>>
>> Signed-off-by: Dave Airlie <airlied at redhat.com>
>> ---
>>  glamor/glamor_font.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/glamor/glamor_font.c b/glamor/glamor_font.c
>> index 6753d50..bda1b58 100644
>> --- a/glamor/glamor_font.c
>> +++ b/glamor/glamor_font.c
>> @@ -80,6 +80,10 @@ glamor_font_get(ScreenPtr screen, FontPtr font)
>>      overall_width = glyph_width_bytes * num_cols;
>>      overall_height = glyph_height * num_rows;
>>
>> +    if (overall_width > glamor_priv->max_fbo_size ||
>> +        overall_height > glamor_priv->max_fbo_size) {
>> +        return NULL;
>> +    }
>
> I wonder why glTexImage2D didn't return GL_OUT_OF_MEMORY for this case;
> we check for that. Maybe there's a different error we should be looking
> for?

from glTexImage2D man page:
       GL_INVALID_VALUE is generated if width is less than 0 or greater than
       GL_MAX_TEXTURE_SIZE.

Granted we should bail early since we know we are going to fail.

>
> In any case, bailing early seems like a fine plan, and this looks like
> the right solution
>
>>      bits = malloc(overall_width * overall_height);
>>      if (!bits)
>>          return NULL;
>
> I note that the case where GL_OUT_OF_MEMORY is returned fails to free
> the allocated bits (oops!). Here's a patch that just moves the free
> above the error return:

Reviewed-by: Dave Airlie <airlied at redhat.com> for your hunk to move bits.

Dave.
>
> diff --git a/glamor/glamor_font.c b/glamor/glamor_font.c
> index 6753d50..3925f2a 100644
> --- a/glamor/glamor_font.c
> +++ b/glamor/glamor_font.c
> @@ -132,11 +132,12 @@ glamor_font_get(ScreenPtr screen, FontPtr font)
>      glTexImage2D(GL_TEXTURE_2D, 0, GL_R8UI, overall_width, overall_height,
>                   0, GL_RED_INTEGER, GL_UNSIGNED_BYTE, bits);
>      glamor_priv->suppress_gl_out_of_memory_logging = false;
> -    if (glGetError() == GL_OUT_OF_MEMORY)
> -        return NULL;
>
>      free(bits);
>
> +    if (glGetError() == GL_OUT_OF_MEMORY)
> +        return NULL;
> +
>      glamor_font->realized = TRUE;
>
>      return glamor_font;
>
>
> --
> -keith