Fix up vips_text()

Fixes two issues:

1. vips_text() in autofit mode could set the wrong DPI, since it set the
DPI in its own copy of the variable, but did not do a final update on
the DPI setting that FT uses for rendering.

2. vips_text() in autofit mode allocated a new context each time, rather
than reusing the context for that call. This caused a small memory leak.

See https://github.com/libvips/libvips/issues/1174
This commit is contained in:
John Cupitt 2018-12-05 14:24:26 +00:00
parent d4815e8b7e
commit ac4897abee
2 changed files with 40 additions and 35 deletions

View File

@ -1,5 +1,7 @@
21/11/18 started 8.7.2 21/11/18 started 8.7.2
- more info output for temp files to help diagnose problems - more info output for temp files to help diagnose problems
- vips_text() could set the wrong DPI
- vips_text() leaked in autofit mode
23/9/18 started 8.7.1 23/9/18 started 8.7.1
- update function list in docs [janko-m] - update function list in docs [janko-m]

View File

@ -22,6 +22,9 @@
* - better fitting of fonts with overhanging edges, thanks Adrià * - better fitting of fonts with overhanging edges, thanks Adrià
* 26/4/18 fangqiao * 26/4/18 fangqiao
* - add fontfile option * - add fontfile option
* 5/12/18
* - fitting mode could set wrong dpi
* - fitting mode leaked
*/ */
/* /*
@ -170,8 +173,6 @@ vips_text_get_extents( VipsText *text, VipsRect *extents )
pango_ft2_font_map_set_resolution( pango_ft2_font_map_set_resolution(
PANGO_FT2_FONT_MAP( vips_text_fontmap ), text->dpi, text->dpi ); PANGO_FT2_FONT_MAP( vips_text_fontmap ), text->dpi, text->dpi );
text->context = pango_font_map_create_context(
PANGO_FONT_MAP( vips_text_fontmap ) );
if( !(text->layout = text_layout_new( text->context, if( !(text->layout = text_layout_new( text->context,
text->text, text->font, text->text, text->font,
@ -180,17 +181,18 @@ vips_text_get_extents( VipsText *text, VipsRect *extents )
pango_layout_get_pixel_extents( text->layout, pango_layout_get_pixel_extents( text->layout,
&ink_rect, &logical_rect ); &ink_rect, &logical_rect );
extents->left = ink_rect.x; extents->left = ink_rect.x;
extents->top = ink_rect.y; extents->top = ink_rect.y;
extents->width = ink_rect.width; extents->width = ink_rect.width;
extents->height = ink_rect.height; extents->height = ink_rect.height;
#ifdef DEBUG #ifdef DEBUG
printf( "vips_text_get_extents: dpi = %d, " printf( "vips_text_get_extents: dpi = %d\n", text->dpi );
"left = %d, top = %d, width = %d, height = %d\n", printf( " ink left = %d, top = %d, width = %d, height = %d\n",
text->dpi, ink_rect.x, ink_rect.y, ink_rect.width, ink_rect.height );
extents->left, extents->top, extents->width, extents->height ); printf( " logical left = %d, top = %d, width = %d, height = %d\n",
logical_rect.x, logical_rect.y,
logical_rect.width, logical_rect.height );
#endif /*DEBUG*/ #endif /*DEBUG*/
return( 0 ); return( 0 );
@ -215,27 +217,32 @@ vips_text_rect_difference( VipsRect *target, VipsRect *extents )
/* Adjust text->dpi to try to fit to the bounding box. /* Adjust text->dpi to try to fit to the bounding box.
*/ */
static int static int
vips_text_autofit( VipsText *text, VipsRect *out_extents ) vips_text_autofit( VipsText *text )
{ {
VipsRect target; VipsRect target;
VipsRect extents; VipsRect extents;
VipsRect previous_extents;
int difference; int difference;
int previous_difference; int previous_difference;
int previous_dpi; int previous_dpi;
int lower_dpi; int lower_dpi;
int upper_dpi; int upper_dpi;
VipsRect lower_extents;
VipsRect upper_extents;
/* First, repeatedly double or halve dpi until we pass the correct /* First, repeatedly double or halve dpi until we pass the correct
* value. This will give us a lower and upper bound. * value. This will give us a lower and upper bound.
*/ */
target.left = 0;
target.top = 0;
target.width = text->width; target.width = text->width;
target.height = text->height; target.height = text->height;
previous_dpi = -1; previous_dpi = -1;
#ifdef DEBUG
printf( "vips_text_autofit: "
"target left = %d, top = %d, width = %d, height = %d\n",
target.left, target.top, target.width, target.height );
#endif /*DEBUG*/
for(;;) { for(;;) {
if( vips_text_get_extents( text, &extents ) ) if( vips_text_get_extents( text, &extents ) )
return( -1 ); return( -1 );
@ -246,7 +253,6 @@ vips_text_autofit( VipsText *text, VipsRect *out_extents )
if( previous_dpi == -1 ) { if( previous_dpi == -1 ) {
previous_dpi = text->dpi; previous_dpi = text->dpi;
previous_difference = difference; previous_difference = difference;
previous_extents = extents;
} }
/* Hit the size, or we straddle the target. /* Hit the size, or we straddle the target.
@ -256,7 +262,6 @@ vips_text_autofit( VipsText *text, VipsRect *out_extents )
break; break;
previous_difference = difference; previous_difference = difference;
previous_extents = extents;
previous_dpi = text->dpi; previous_dpi = text->dpi;
text->dpi = difference < 0 ? text->dpi * 2 : text->dpi / 2; text->dpi = difference < 0 ? text->dpi * 2 : text->dpi / 2;
@ -266,17 +271,18 @@ vips_text_autofit( VipsText *text, VipsRect *out_extents )
/* We've been coming down. /* We've been coming down.
*/ */
lower_dpi = text->dpi; lower_dpi = text->dpi;
lower_extents = extents;
upper_dpi = previous_dpi; upper_dpi = previous_dpi;
upper_extents = previous_extents;
} }
else { else {
lower_dpi = previous_dpi; lower_dpi = previous_dpi;
lower_extents = previous_extents;
upper_dpi = text->dpi; upper_dpi = text->dpi;
upper_extents = extents;
} }
#ifdef DEBUG
printf( "vips_text_autofit: lower dpi = %d, upper dpi = %d\n",
lower_dpi, upper_dpi );
#endif /*DEBUG*/
/* Refine lower and upper until they are almost touching. /* Refine lower and upper until they are almost touching.
*/ */
while( upper_dpi - lower_dpi > 1 && while( upper_dpi - lower_dpi > 1 &&
@ -288,31 +294,26 @@ vips_text_autofit( VipsText *text, VipsRect *out_extents )
target.top = extents.top; target.top = extents.top;
difference = vips_text_rect_difference( &target, &extents ); difference = vips_text_rect_difference( &target, &extents );
if( difference < 0 ) { if( difference < 0 )
lower_dpi = text->dpi; lower_dpi = text->dpi;
lower_extents = extents; else
}
else {
upper_dpi = text->dpi; upper_dpi = text->dpi;
upper_extents = extents;
}
} }
/* If we've hit the target exactly and quit the loop, diff will be 0 /* If we've hit the target exactly and quit the loop, diff will be 0
* and we can use upper. Otherwise we are straddling the target and we * and we can use upper. Otherwise we are straddling the target and we
* must take lower. * must take lower.
*/ */
if( difference == 0 ) { if( difference == 0 )
text->dpi = upper_dpi; text->dpi = upper_dpi;
*out_extents = upper_extents; else
}
else {
text->dpi = lower_dpi; text->dpi = lower_dpi;
*out_extents = lower_extents;
}
g_object_set( text, "autofit_dpi", text->dpi, NULL ); g_object_set( text, "autofit_dpi", text->dpi, NULL );
#ifdef DEBUG
printf( "vips_text_autofit: final dpi = %d\n", text->dpi );
#endif /*DEBUG*/
return( 0 ); return( 0 );
} }
@ -340,6 +341,9 @@ vips_text_build( VipsObject *object )
if( !vips_text_fontmap ) if( !vips_text_fontmap )
vips_text_fontmap = pango_ft2_font_map_new(); vips_text_fontmap = pango_ft2_font_map_new();
text->context = pango_font_map_create_context(
PANGO_FONT_MAP( vips_text_fontmap ) );
if( text->fontfile && if( text->fontfile &&
!FcConfigAppFontAddFile( NULL, !FcConfigAppFontAddFile( NULL,
(const FcChar8 *) text->fontfile ) ) { (const FcChar8 *) text->fontfile ) ) {
@ -354,15 +358,14 @@ vips_text_build( VipsObject *object )
*/ */
if( vips_object_argument_isset( object, "height" ) && if( vips_object_argument_isset( object, "height" ) &&
!vips_object_argument_isset( object, "dpi" ) ) { !vips_object_argument_isset( object, "dpi" ) ) {
if( vips_text_autofit( text, &extents ) ) if( vips_text_autofit( text ) )
return( -1 ); return( -1 );
} }
else
if( vips_text_get_extents( text, &extents ) )
return( -1 );
/* Can happen for "", for example. /* Layout. Can fail for "", for example.
*/ */
if( vips_text_get_extents( text, &extents ) )
return( -1 );
if( extents.width == 0 || if( extents.width == 0 ||
extents.height == 0 ) { extents.height == 0 ) {
vips_error( class->nickname, "%s", _( "no text to render" ) ); vips_error( class->nickname, "%s", _( "no text to render" ) );