Expose vips_text to create an image containing rendered text #512 #3252

brahima · 2022-06-10T15:28:40Z

Hello @lovell ,

This a PR regarding the vips_text feature (#512)

Some notes about this PR :

I started from your last proposal API
- What @rorz did helped me getting started and i removed all the color related stuffs but integrated the rgba option
- maybe the spacing parameter should be renamed as lineSpacing ?
I did not take into consideration the autofit_dpi parameter exported by libvips. Should i ?
Regarding the tests
- I have all my new tests passing but some existing ones are failing (even on your main branch). I tried with libvips installed via brew and recompiled locally with no success.
- I added a font (Bitstream Vera from the Gnome Project) for my tests.
  - What do you think of the copyright/license ? If not ok, do you think of another font that could be embedded ? Is it in the right place (fixtures) ?
- I was not able to run the memory-leak test also (no valgrind in mac) and i struggled a lot to install kcachegrind (macport, compile ... seems to be a known problem on last MacOs versions) with no success.

Thanks for your insights on my modifications and any advice about my macos related problems regarding the tests.

Regards,
Brahim

brahima · 2022-06-10T23:14:30Z

Hi again @lovell,

I installed a fresh ubuntu image via docker with libvips dependencies, node 16.15, valgrind ... and now have only one error remaining on the existing tests (even on main branch). The error occurs in test/util.js (Vendor -> Contains expected attributes) :

1) Utilities
       Vendor
         Contains expected attributes:

      AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

true !== false

      + expected - actual

      -true
      +false
      
      at Context.<anonymous> (test/unit/util.js:143:14)
      at processImmediate (node:internal/timers:466:21)

The sharp.vendor.installed variable in my case contains this (the installed array is empty) :
{ current: 'linux-x64', installed: [] }

Any clue about that ?

Also i have some of my own tests failing right now ... i'm having a look.

Thanks for your help

brahima · 2022-06-11T18:22:54Z

Hello @lovell,

This is me again. I now understand why i have failures on my own (new) tests. Generated image based on text can be quite different depending on the OS and font combination. That's why i introduced a specific font file for the tests ... but that's not that easy :-)

I did some tests using different vips command to see the difference between MacOs / Linux (Windows is not even in the equation). Here's only two of them :

vips text /tmp/text_macos.png "<span>This is a text to justify</span>" --font 'Arial 100px' --align centre --width 300

vips text /tmp/text_macos.png "<span>This is a text to justify</span>" --font 'Bitstream Vera Sans Bold Oblique 100px' --align centre --width 300

That makes me think that the Bitstream Vera font is not a good fit for my unit tests.

Identifying a font that can give the « same » results across the different Oses and keeping the fixtures.assertSimilar() in the unit tests may not be a viable option.

Maybe @jcupitt can give some insights about this ?

Thanks in advance for your help regarding this issue.

PS : I still have the vendor unit test failing locally

Best regards,
Brahim

jcupitt · 2022-06-11T19:12:01Z

Hi @brahima

Text rendering is really complex, and tiny changes to your system can completely alter the bitmap you get. You can't expect to get the same result from ubuntu 21.04 and 21.10, for example, never mind between macos and linux. You can't even expect two users both on ubuntu 22.04 to get the same result.

I would just check that the generated text image has a not-crazy width and height, has one band, and that the image average is between 0 and 255.

Here's how libvips tests text:

https://github.com/libvips/libvips/blob/master/test/test-suite/test_create.py#L403-L418

brahima · 2022-06-12T22:05:25Z

Hi @jcupitt and thanks for your feedback, makes sense.
Just after sending my last message, i had the idea to check the unit test of libvips.

Also, @lovell, after playing around with my version and testing, i think that having default values for width (0), height (0) and dpi (72) is not a good idea and makes the default behavior strange and different from vips (unless making some adjustments)

For exemple, for the following commands (vips default behavior vs. sharp) :

vips text /tmp/text_vips.png 'Hello, world!' --width 500 --height 500

Here, since we set the dpi to 72 by default ... it is not computed by libvips based on width and height.

I see 2 options :

remove these default values and let libvips « do its work »
add some rules (ex : if width and height are set => do not set default value for dpi ...). However, the default value has no real interest then.

I do think that the 1st option is better but would like to have your point of view on this.

Also, here after getting the image from libvips, even if i set the image type manually to VIPS_INTERPRETATION_B_W, i always get a 3 channel image in sharp when testing (despite the image.bands() being equals to 1). I expected to have only one channel image with its space being 'b-w' and not 'srgb' when the rgba option is set to false.

Sorry for all my questions and hope i made my self clear :-)

Regards,
Brahim

brahima · 2022-06-13T04:33:33Z

Hello,

I just re-pushed with updated tests :

i removed the font
kept the default values with some more checks (height and dpi input are mutually exclusive)

I'll wait for your review before doing any further changes.

Regards,
Brahim

lovell · 2022-06-21T07:44:58Z

@brahima Thank you very much for the PR, I plan to review this next week.

brahima · 2022-06-23T21:46:43Z

Hello @lovell,

Thanks in advance for your time. I stay tuned :-)

kantorcodes · 2022-07-06T07:17:11Z

@brahima Thank you very much for the PR, I plan to review this next week.

Any updates on this one? Would love to utilize this feature.

brahima · 2022-07-06T20:15:46Z

Hi @lovell,

Just rebased my last work on main branch.

lovell

Thank you very much for taking the time to work on this feature, it's looking really good, only a couple of small inline comments.

The tests appear to be failing on Linux due to a lack of fonts in the CI environment. Whilst adding fonts should be straightforward, it highlights that we need to do something (code? docs?) to catch this problem and aid Linux users. I imagine it'll be common in FaaS environments such as AWS Lambda that lack control over the OS, or minimal containers that don't install fonts by default.

I'm going to switch the base branch to the eagle branch for inclusion in sharp v0.31.0, so you may need to rebase as there have been a couple of changes relating to the forthcoming libvips v8.13.0 upgrade.

Thanks again, this is an oft-requested feature and will be greatly appreciated by many.

src/common.cc

lib/constructor.js

lovell · 2022-07-08T13:56:58Z

src/common.cc

+        }
+        image = VImage::new_memory().text(const_cast<char *>(descriptor->textValue.data()), textOptions);
+        image.get_image()->Type = image.guess_interpretation();
+        imageType = ImageType::RAW;


Is it worth reading out the autofit_dpi / autofitDpi value and adding it to the info object of the response? It might be useful for someone to know at what DPI a font was rendered.

Are you talking about the info object exposed via output options (toFile / toBuffer ...) ?

Also, libvips seems to set the autofit_dpi calculated on the options object
However, the VOption class have only setters and no getters (with a private Pair structure).
Any hints on how to access the computed option value ?

Thanks

You can set a reference to the variable to assign the value to, eg.

->set("autofit_dpi", &output_dpi)

And it'll assign the value on return.

Thanks @jcupitt, ok makes sense. Way easier than what i was thinking about :-)

@lovell Just pushed a commit with the autofitDdpi parameter added to the info output object.
Still need to add some lines to the documentation but waiting to know if the value is exposed at the right place.
Thanks

src/common.cc

brahima · 2022-07-15T17:46:47Z

Hello @lovell and @kleisauke and thanks for the time you took reviewing my PR.

Thank you very much for taking the time to work on this feature, it's looking really good, only a couple of small inline comments.

You're welcome :-)

The tests appear to be failing on Linux due to a lack of fonts in the CI environment. Whilst adding fonts should be straightforward, it highlights that we need to do something (code? docs?) to catch this problem and aid Linux users. I imagine it'll be common in FaaS environments such as AWS Lambda that lack control over the OS, or minimal containers that don't install fonts by default.

About adding the fonts for the CI env, do you have specific packages in mind ?

Regarding font problem detection :

what should be the better place for the doc ? constructor page or installation page
for the code part, aren't the fontconfig/pango errors sufficient enough ? Do you want sharp to wrap them in its own specific error messages ?

Fontconfig error: Cannot load default config file: No such file: (null)
(process:626): Pango-CRITICAL **: 17:59:42.755: pango_font_describe: assertion 'font != NULL' failed

I'm going to switch the base branch to the eagle branch for inclusion in sharp v0.31.0, so you may need to rebase as there have been a couple of changes relating to the forthcoming libvips v8.13.0 upgrade.

Ok, done :-)

Thanks again, this is an oft-requested feature and will be greatly appreciated by many.

lovell · 2022-07-17T21:12:04Z

Thanks for the updates, it looks like there are a couple of linting errors.

~~For the Linux-based CI environments, we can probably use the Open Sans typeface as that's widely available (add open-sans-fonts package on Centos, font-opensans on Alpine etc.)~~

I'm happy to write the documentation for the use of fonts after we merge this PR. There have been a few questions in the past about SVG font rendering too, so it would be good to get it all down in one place.

lovell · 2022-07-18T13:21:52Z

Commit 254944f on the eagle branch ensures the Noto Sans font is available in all the CI environments, which should allow the generic "sans" font name to be used in tests.

brahima · 2022-07-18T16:30:31Z

Thanks for the updates, it looks like there are a couple of linting errors.

@lovell My bad ... should be fixed now.

coveralls · 2022-07-18T19:20:37Z

Coverage remained the same at 100.0% when pulling 3fa968f on brahima:vips_text_512 into 76c4c51 on lovell:eagle.

brahima · 2022-07-21T11:56:02Z

@lovell Hi,
Any feedback regarding the autofitDpi exposed via the info object ?

lovell

Thank you very much for all the updates, a couple of very minor inline comments and we're good to go.

src/common.h

docs/api-constructor.md

lovell · 2022-07-23T15:32:28Z

I've added some documentation about font discovery via commit 3e327a5

brahima · 2022-07-24T20:34:42Z

@lovell also added 2 lines to api-output documentation regarding autofit dpi.

Hope we're good to go but do not hesitate if you have any feedback.

lovell · 2022-07-25T10:32:53Z

Thank you very much for working on and persisting with this PR, it will be included in v0.31.0.

brahima force-pushed the vips_text_512 branch 2 times, most recently from 287208e to e743c26 Compare June 13, 2022 04:30

brahima force-pushed the vips_text_512 branch from e743c26 to 689d86b Compare June 13, 2022 12:54

brahima force-pushed the vips_text_512 branch from 689d86b to 5a8f488 Compare July 6, 2022 20:14

lovell requested changes Jul 8, 2022

View reviewed changes

lovell changed the base branch from main to eagle July 8, 2022 14:04

kleisauke reviewed Jul 9, 2022

View reviewed changes

src/common.cc Outdated Show resolved Hide resolved

src/common.cc Outdated Show resolved Hide resolved

brahima force-pushed the vips_text_512 branch from 5a8f488 to 0fa6531 Compare July 15, 2022 17:11

brahima force-pushed the vips_text_512 branch from 5454feb to 63d25f9 Compare July 18, 2022 16:15

lovell requested changes Jul 22, 2022

View reviewed changes

src/common.h Outdated Show resolved Hide resolved

src/common.h Outdated Show resolved Hide resolved

docs/api-constructor.md Outdated Show resolved Hide resolved

brahima added 4 commits July 24, 2022 12:32

Expose vips_text to create an image containing rendered text lovell#512

5d65903

add text api examples

76bb64a

Comments resolve for libvips text api

ab0ef7e

add autofitDdpi to info

bfe99b9

brahima added 3 commits July 24, 2022 12:32

fix lint errors

7236ce4

100% code coverage

c6906cb

Comments resolve for textAutofitDpi and examples

ae26840

brahima force-pushed the vips_text_512 branch from 5f95c81 to ae26840 Compare July 24, 2022 12:36

textAutofitDpi output documentation

3fa968f

lovell approved these changes Jul 25, 2022

View reviewed changes

lovell merged commit ea7cf2a into lovell:eagle Jul 25, 2022

lovell added this to the v0.31.0 milestone Jul 25, 2022

lovell added a commit that referenced this pull request Jul 25, 2022

Docs: changelog and credit for #3252

dd56a96

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose vips_text to create an image containing rendered text #512 #3252

Expose vips_text to create an image containing rendered text #512 #3252

brahima commented Jun 10, 2022 •

edited

Loading

brahima commented Jun 10, 2022

brahima commented Jun 11, 2022

jcupitt commented Jun 11, 2022 •

edited

Loading

brahima commented Jun 12, 2022 •

edited

Loading

brahima commented Jun 13, 2022

lovell commented Jun 21, 2022

brahima commented Jun 23, 2022

kantorcodes commented Jul 6, 2022

brahima commented Jul 6, 2022

lovell left a comment

lovell Jul 8, 2022

brahima Jul 15, 2022

jcupitt Jul 15, 2022

brahima Jul 15, 2022

brahima Jul 16, 2022

brahima commented Jul 15, 2022

lovell commented Jul 17, 2022 •

edited

Loading

lovell commented Jul 18, 2022

brahima commented Jul 18, 2022 •

edited

Loading

coveralls commented Jul 18, 2022 •

edited

Loading

brahima commented Jul 21, 2022

lovell left a comment

lovell commented Jul 23, 2022

brahima commented Jul 24, 2022

lovell commented Jul 25, 2022

Expose vips_text to create an image containing rendered text #512 #3252

Expose vips_text to create an image containing rendered text #512 #3252

Conversation

brahima commented Jun 10, 2022 • edited Loading

brahima commented Jun 10, 2022

brahima commented Jun 11, 2022

jcupitt commented Jun 11, 2022 • edited Loading

brahima commented Jun 12, 2022 • edited Loading

brahima commented Jun 13, 2022

lovell commented Jun 21, 2022

brahima commented Jun 23, 2022

kantorcodes commented Jul 6, 2022

brahima commented Jul 6, 2022

lovell left a comment

Choose a reason for hiding this comment

lovell Jul 8, 2022

Choose a reason for hiding this comment

brahima Jul 15, 2022

Choose a reason for hiding this comment

jcupitt Jul 15, 2022

Choose a reason for hiding this comment

brahima Jul 15, 2022

Choose a reason for hiding this comment

brahima Jul 16, 2022

Choose a reason for hiding this comment

brahima commented Jul 15, 2022

lovell commented Jul 17, 2022 • edited Loading

lovell commented Jul 18, 2022

brahima commented Jul 18, 2022 • edited Loading

coveralls commented Jul 18, 2022 • edited Loading

brahima commented Jul 21, 2022

lovell left a comment

Choose a reason for hiding this comment

lovell commented Jul 23, 2022

brahima commented Jul 24, 2022

lovell commented Jul 25, 2022

brahima commented Jun 10, 2022 •

edited

Loading

jcupitt commented Jun 11, 2022 •

edited

Loading

brahima commented Jun 12, 2022 •

edited

Loading

lovell commented Jul 17, 2022 •

edited

Loading

brahima commented Jul 18, 2022 •

edited

Loading

coveralls commented Jul 18, 2022 •

edited

Loading