Add resvg #2

RReverser · 2023-01-14T16:07:33Z

Adds resvg as an alternative to rsvg. Latest resvg is only available when built from source.

PR 3125 would resolve this in a better way with clang-format.

kleisauke · 2023-01-18T13:51:23Z

meson.build

+    # It's annoying that I have to spell out the `include_directories` here, but without it Meson can build
+    # but `compile_commands.json` doesn't have the -I$PREFIX/include, making code analysis tools & VSCode fail.
+    libvips_deps += declare_dependency(dependencies: [libresvg_dep], include_directories: [include_directories(get_option('prefix') / get_option('includedir'))], link_args: ['-ldl'])


Hmm, weird. I assume this is on Windows or macOS (due to the use of -ldl)? Perhaps compiling with CFLAGS="-I$PREFIX/include" would be an alternative way to fix this?

Alternatively, we could add dependency() construction and generate the resvg.pc manually, similar to what we do with PDFium, see: https://github.com/libvips/libvips#pdfium.

and generate the resvg.pc manually

Is it worth it / is the current implementation that bad?

And no, I haven't tried to build it on Windows / macOS, it was either on Linux or Emscripten that I needed that.

Is it worth it / is the current implementation that bad?

I'm not sure, I think this is also fine. I'm only a bit worried that I broke VSCode and code analysis tools with commit 349a7b7, which avoids this declare_dependency() construction altogether and just uses cc.find_library() instead.

Might be worth checking compile_commands.json for presence of this -I. I think the problem was that otherwise Meson would pass include directory via environment variable, which obviously wouldn't be reflected in compile_commands.json explicit flags.

OTOH if you don't use autocomplete or code analysis tools, maybe it doesn't matter much. For me it was particularly useful for debugging while working on the integration though.

I see, I'll investigate.

I could fix the compilation database (compile_commands.json) with commit cbc753c.

Tested with (in combination with CLion):

Details

Build and install resvg with:

$ git clone https://github.com/RazrFalcon/resvg $ cd resvg/c-api/ $ cargo build --release $ sudo cp ../target/release/libresvg.so /usr/lib64/ $ sudo cp resvg.h /usr/include/

Build and install libvips with:

$ CC="gcc" CXX="g++" meson setup build --prefix=/usr --buildtype=release -Ddeprecated=false -Drsvg=disabled -Dmodules=disabled $ meson compile -Cbuild $ sudo meson install -Cbuild

Note: CC="gcc" CXX="g++" is due to broken compiler detection in CLion for ccache.

Though, that installs the libraries in /usr, which may not be desirable.

Note that I still don't see the -I/usr/include in the compilation database, so I suspect that this issue may also occur when installing the libraries in a non-standard prefix.

Details

$ grep -o '\-I/usr/include/\S*' build/compile_commands.json | sort --unique -I/usr/include/blkid -I/usr/include/cairo -I/usr/include/cfitsio -I/usr/include/freetype2 -I/usr/include/fribidi -I/usr/include/glib-2.0 -I/usr/include/harfbuzz -I/usr/include/ImageMagick-6 -I/usr/include/Imath -I/usr/include/libgsf-1 -I/usr/include/libmount -I/usr/include/libpng16 -I/usr/include/libvmaf -I/usr/include/libxml2 -I/usr/include/nifti -I/usr/include/OpenEXR -I/usr/include/openjpeg-2.5 -I/usr/include/openslide -I/usr/include/pango-1.0 -I/usr/include/pixman-1 -I/usr/include/poppler -I/usr/include/poppler/glib -I/usr/include/rav1e -I/usr/include/svt-av1 -I/usr/include/sysprof-4 -I/usr/include/webp

kleisauke · 2023-01-18T18:38:10Z

Using this test script:

import Vips from './lib/node-es6/vips.mjs';

const vips = await Vips({
  dynamicLibraries: ['../vips-resvg.wasm']
});

// Image source: https://upload.wikimedia.org/wikipedia/commons/0/0f/Complex_sin_imag_01_Pengo.svg
const im = vips.Image.newFromFile('Complex_sin_imag_01_Pengo.svg', {
  access: vips.Access.sequential
});
im.writeToFile('x.png');

I see:

$ node svg-to-png.mjs 
$ vipsheader x.png
x.png: 768x576 uchar, 4 bands, srgb, pngload
$ vips copy Complex_sin_imag_01_Pengo.svg x2.png
$ vipsheader x2.png
x2.png: 576x432 uchar, 4 bands, srgb, pngload
$ grep "<svg" Complex_sin_imag_01_Pengo.svg
<svg width="576pt" height="432pt" viewBox="0 0 576 432"

So, somehow this implementation produces an image of 768x576 where the librsvg implementation produces an image of 576x432.

Perhaps some scaling issue? I confirm that replacing pt with px resolves this.

RReverser · 2023-01-18T18:44:25Z

where the librsvg implementation produces an image of 576x432

Hm, you have a working Wasm build of librsvg?

Anyway, I believe you need to set explicit density if you don't want libraries to use their defaults. (unlike px, pt is a relative unit that depends on the configured DPI)

kleisauke · 2023-01-18T18:51:50Z

Hm, you have a working Wasm build of librsvg?

No, the reference librsvg image was produced on my Fedora PC with the vips utility. I think compiling librsvg to Wasm would be a huge amount of work. 😅

Anyway, I believe you need to set explicit density if you don't want libraries to use their defaults.

Indeed, it could be some sort of DPI issue. Perhaps that svg->cairo_scale thing is not needed in resvg. I'll investigate.

RReverser · 2023-01-18T18:54:55Z

Indeed, it could be some sort of DPI issue.

I'm not sure it's an issue (as in, a bug). It could be just that the vips utility sets a different DPI. Quick calculation (768/576*72 = 96 vs 576/576*72=72) suggests that Wasm binding / libresvg simply uses 96 dpi, whereas vips utility uses 72 dpi.

But either one is valid result in absence of explicit DPI. The sharp tests pass because it sets density explicitly.

kleisauke · 2023-01-18T19:06:42Z

The vips utility is just a thin wrapper around the libvips library, so it should use a DPI of 72.

libvips/libvips/foreign/svgload.c

Line 781 in 08dac03

0.001, 100000.0, 72.0 );

Perhaps resvg is not taking resvg_options_set_dpi() into account during resvg_get_image_size()?:

libvips/libvips/foreign/svgload.c

Line 546 in 08dac03

resvg_options_set_dpi(svg->options, 72.0);

libvips/libvips/foreign/svgload.c

Line 515 in 08dac03

resvg_size size = resvg_get_image_size(svg->tree);

I'm not sure if the sharp testsuite has SVG fixtures that uses anything other than px dimensions, so I'm not sure it would catch such issues.

RReverser · 2023-01-18T19:14:35Z

I'm not sure if the sharp testsuite has SVG fixtures that uses anything other than px dimensions, so I'm not sure it would catch such issues.

It does have tests for the same SVG at different DPIs added in the same commit as density option itself, so it generally should cover it. lovell/sharp@cf7664a

RReverser · 2023-01-18T19:18:36Z

Perhaps resvg is not taking resvg_options_set_dpi() into account during resvg_get_image_size()?:

Ah could be. Since this command is just for the header, I suppose it doesn't get to the actual rendering, whereas sharp's tests do render to raster before comparing, which is scaled correctly.

resvg uses a DPI of 96 by default, libvips defaults to 72. Call `resvg_options_set_dpi()` before parsing the tree with either `resvg_parse_tree_from_file()` or `resvg_parse_tree_from_data()` to ensure that `resvg_get_image_size()` returns the correct dimensions.

kleisauke · 2023-01-19T15:54:39Z

Fixed with commit f4115ac. I now see (using the image and commands mentioned in #2 (comment)):

resvg	librsvg

🎉

kleisauke

Thank you!

Co-authored-by: Kleis Auke Wolthuizen <github@kleisauke.nl>

kleisauke · 2023-01-24T13:26:41Z

Cherry-picked as commit a7a8a7d in the wasm-vips-8.14 branch (containing the Wasm specific patches on top of libvips 8.14.x).
libvips/libvips@8.14...kleisauke:libvips:wasm-vips-8.14

Co-authored-by: Kleis Auke Wolthuizen <github@kleisauke.nl>

Add resvg

08dac03

RReverser mentioned this pull request Jan 17, 2023

Add SVG support via resvg kleisauke/wasm-vips#40

Merged

kleisauke added 5 commits January 17, 2023 18:17

Revert whitespace-only changes

0b75df2

Adapt to libvips style

d15c1ed

PR 3125 would resolve this in a better way with clang-format.

Fix build with librsvg

bc9dd66

Build a dynamically loadable module for resvg

349a7b7

Simplify conditional directive

f00e7b1

kleisauke reviewed Jan 18, 2023

View reviewed changes

kleisauke mentioned this pull request Jan 23, 2023

test: allow svgload suite to be run with resvg libvips/libvips#3307

Merged

kleisauke added 5 commits January 23, 2023 15:14

s/libresvg/resvg/g

d70f038

Adapt to libvips style

385117c

Add missing resvg_dep.found()

e15de4c

Merge branch 'wasm-vips'

fec32aa

Update after cbc753c

b5c4b25

kleisauke approved these changes Jan 24, 2023

View reviewed changes

kleisauke merged commit 95c6dff into kleisauke:wasm-vips Jan 24, 2023

kleisauke added a commit to RReverser/wasm-vips that referenced this pull request Jan 24, 2023

Update after merge of kleisauke/libvips#2

3168aa3

kleisauke added a commit that referenced this pull request Jan 24, 2023

Add SVG load support via resvg (#2)

a7a8a7d

Co-authored-by: Kleis Auke Wolthuizen <github@kleisauke.nl>

kleisauke added a commit that referenced this pull request Mar 21, 2023

Add SVG load support via resvg (#2)

f8da874

Co-authored-by: Kleis Auke Wolthuizen <github@kleisauke.nl>

kleisauke added a commit that referenced this pull request Jul 5, 2023

Add SVG load support via resvg (#2)

cdd9d71

Co-authored-by: Kleis Auke Wolthuizen <github@kleisauke.nl>

kleisauke added a commit that referenced this pull request Jul 16, 2023

Add SVG load support via resvg (#2)

284de6a

Co-authored-by: Kleis Auke Wolthuizen <github@kleisauke.nl>

kleisauke added a commit that referenced this pull request Jul 16, 2023

Add SVG load support via resvg (#2)

220fcf9

Co-authored-by: Kleis Auke Wolthuizen <github@kleisauke.nl>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add resvg #2

Add resvg #2

RReverser commented Jan 14, 2023

kleisauke Jan 18, 2023 •

edited

Loading

kleisauke Jan 18, 2023

RReverser Jan 18, 2023

RReverser Jan 18, 2023

kleisauke Jan 18, 2023

RReverser Jan 18, 2023

kleisauke Jan 18, 2023

kleisauke Jan 24, 2023 •

edited

Loading

kleisauke commented Jan 18, 2023

RReverser commented Jan 18, 2023 •

edited

Loading

kleisauke commented Jan 18, 2023

RReverser commented Jan 18, 2023

kleisauke commented Jan 18, 2023

RReverser commented Jan 18, 2023

RReverser commented Jan 18, 2023

kleisauke commented Jan 19, 2023

kleisauke left a comment

kleisauke commented Jan 24, 2023

Add resvg #2

Add resvg #2

Conversation

RReverser commented Jan 14, 2023

kleisauke Jan 18, 2023 • edited Loading

Choose a reason for hiding this comment

kleisauke Jan 18, 2023

Choose a reason for hiding this comment

RReverser Jan 18, 2023

Choose a reason for hiding this comment

RReverser Jan 18, 2023

Choose a reason for hiding this comment

kleisauke Jan 18, 2023

Choose a reason for hiding this comment

RReverser Jan 18, 2023

Choose a reason for hiding this comment

kleisauke Jan 18, 2023

Choose a reason for hiding this comment

kleisauke Jan 24, 2023 • edited Loading

Choose a reason for hiding this comment

kleisauke commented Jan 18, 2023

RReverser commented Jan 18, 2023 • edited Loading

kleisauke commented Jan 18, 2023

RReverser commented Jan 18, 2023

kleisauke commented Jan 18, 2023

RReverser commented Jan 18, 2023

RReverser commented Jan 18, 2023

kleisauke commented Jan 19, 2023

kleisauke left a comment

Choose a reason for hiding this comment

kleisauke commented Jan 24, 2023

kleisauke Jan 18, 2023 •

edited

Loading

kleisauke Jan 24, 2023 •

edited

Loading

RReverser commented Jan 18, 2023 •

edited

Loading