libvips / lua-vips

Lua binding for the libvips image processing library
MIT License
125 stars 10 forks source link

vips_text() execution time regression if autofit and fontfile #35

Closed keiviv closed 4 years ago

keiviv commented 4 years ago

Platform

W10 Vips 8.9.2

Bug Description

If vips_text() is called with width and height (autofit), plus fontfile is specified, it executes progressively slower if looped.

There are no memory leaks, crashes or any other abnormalities except the gradually increasing wall time. It does not matter if the fontfile is installed system wide or not, if it is in ttf or otf format.

Bug Reproduction

In any language just set w, h, fontfile (minimal reproducing combination), and measure the cycles in milliseconds. This bug is easy to miss during testing if short loops and measured in seconds. The ms amount is unimportant, it's the progression that measuring in ms makes apparent.

Bug Reproduction Code

local vips = require 'vips'
vips.cache_set_max(5) -- to not just load cache.
local socket = require'socket' -- for milliseconds gettime()

for i = 1, 10000 do
   local time_mark = socket.gettime()
   local text = tostring(i):sub(-1) -- last digit only - to be unique, but w/o progressing autofit calculations.
   vips.Image.text(text, {width = 100, height = 100, fontfile = 'your_fontfile.ttf', font = 'Your_Font'})
   print(math.floor((socket.gettime() - time_mark) * 100) / 100, i) -- .nn precision
end

Notes

My results (timing is of course system and font dependent) are:

Sec   Loop
-----------
0.1   Start
0.2   1200
0.3   2400
0.4   3900
0.5   5300
...   etc

The same exact code and font, but without the fontfile param is 0.1 sec at any point — for any loop length.

While this ms difference might seem small — it's the ratio and progression that's important. In actual application with more complex and possibly multiple text manipulations — it becomes most apparent. This constant time increase is cruel.

Temp Workaround

Install all used fonts system wide. DON'T use fontfile, just font.

P.S.

Took quite some time to hunt the bug down — as there was absolutely no logical sense to suspect fontfile at all. I literally checked everything else including under the bed before realizing the butler did it. Hopefully you can guess where he is.

jcupitt commented 4 years ago

You're right, it looks like font files are loaded again, even if they've been loaded before. I suppose I'd hoped pango would spot this. text probably needs to keep track of previously loaded fonts.

You can do this as a workaround:

local vips = require 'vips'
vips.cache_set_max(5) -- to not just load cache.
local socket = require 'socket' -- for milliseconds gettime()

-- load the font
vips.Image.text('hello', {fontfile = '/home/john/pics/MissFajardose-Regular.ttf', font = 'Miss Fajardose'})

for i = 1, 10000 do
   local time_mark = socket.gettime()
   local text = tostring(i):sub(-1) -- last digit only - to be unique, but w/o progressing autofit calculations.
   vips.Image.text(text, {width = 100, height = 100, font = 'Miss Fajardose'})
   print(math.floor((socket.gettime() - time_mark) * 100) / 100, i) -- .nn precision
end
jcupitt commented 4 years ago

Let's close this and fix the libvips issue.

keiviv commented 4 years ago

Thank you for the fix. This bug was 100% missable during tests — it only becomes apparent on long loops with more complex and unique (uncached) text strings. Otherwise there is just a tiny ms difference. And like you said — Pango should have handled this.

Glad Vips has one bug less, you rock <3.