Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a function to do in-memory conversion #59

Merged
merged 4 commits into from
Nov 12, 2023

Conversation

linkmauve
Copy link
Contributor

This PR is best viewed commit per commit.

The goal is to make vtracer more usable as a library, in order to do so I have moved out path handling and clap parsing from the Config struct. This makes it much better than going through a PNG file, especially when the application already has pixels in memory.

A future commit can make clap and image optional for building the library (in the latter case the convert_image_to_svg() function won’t be available).

This is a public API change, because convert_image_to_svg() now takes the input and output paths as arguments, whereas before they were passed in the Config struct, so a minor version bump will have to follow for the next release.

@tyt2y3
Copy link
Member

tyt2y3 commented Oct 22, 2023

Thank you for your contribution. I don't mind a rewrite of the cli at all.

As per the other issue, do you think it's also needed to add dylib to Cargo.toml?

@linkmauve
Copy link
Contributor Author

I’m not sure the right one should be dylib either, most Rust projects want to statically link their libraries instead of having to carry a .so around. For Python it’s the opposite, you do need a .so. A good solution would be to split this crate, into a library crate, a cli tool crate, a python binding crate, so the user could install whichever they need. This would also have the benefit of letting the library remove its dependency on clap, which isn’t needed for anything but the cli version.

@tyt2y3
Copy link
Member

tyt2y3 commented Oct 24, 2023

Looking at the docs https://doc.rust-lang.org/beta/reference/linkage.html again I think adding rlib is the correct option.

This lets us decouple file reading/writing from the actual conversion.
Instead the input_path and output_path have to be passed to
convert_image_to_svg() manually.
@linkmauve
Copy link
Contributor Author

linkmauve commented Oct 29, 2023

Thanks for the rlib fix, this indeed makes this library usable as a Rust library again!

I’ve pushed a new version, rebased on top of master, with the Python binding fixed to use the new API (the Python API stays identical).

This lets the user convert an image from memory, without encoding it to
PNG, writing it to a file, then reopening this file and decoding it
using the image crate.

It also allows the user to not write a SVG directly to a file.
@tyt2y3
Copy link
Member

tyt2y3 commented Oct 29, 2023

Thank you for the update. Allow me a bit of time to go through this )

Copy link
Member

@tyt2y3 tyt2y3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it looks pretty good. Only blocked by me to actually try this out.

Copy link
Member

@tyt2y3 tyt2y3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you

@tyt2y3 tyt2y3 merged commit 37a2570 into visioncortex:master Nov 12, 2023
13 checks passed
@tyt2y3
Copy link
Member

tyt2y3 commented Nov 21, 2023

I published on https://crates.io/crates/vtracer/0.6.3

@linkmauve linkmauve deleted the in-memory-conversion branch December 9, 2023 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants