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

Libvips 8.13 and GIF output support #139

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

karlvr
Copy link
Contributor

@karlvr karlvr commented Aug 29, 2022

Upgrade to libvips 8.13.

A note about adding support for more operations... I have created a code generator in TypeScript to read the metadata about supported operations and arguments and to generate the JVips API automatically. You can see it at https://github.com/cactuslab/JVips/tree/generate. When you have a moment I'd love to see whether you'd like to integrate these ideas. I have made a few opinionated decisions that change the API, like using "apply" for the in-place modification vs returning a new Image.

Copy link
Contributor

@warrenseine warrenseine left a comment

Choose a reason for hiding this comment

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

LGTM, but I'll let @dbouron have a look.

--without-magick
--without-orc
--without-gsf
--without-rsvg
${LIBSPNG_FLAGS}
${LIBHEIF_FLAGS}
DEPENDS libjpeg libpng libspng giflib libwebp libimagequant lcms2 libheif tiff
DEPENDS libjpeg libpng libspng giflib cgif libwebp libimagequant lcms2 libheif tiff
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I see that giflib is a decoder and cgif an encoder. I'd like to make sure that we can decode/encode/decode .gif files without any issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@warrenseine I've enabled GIFs in all of the tests, so I think they are confirming that?

@warrenseine
Copy link
Contributor

Thanks for contributing these features upstream! I can see that your own fork has many more commits 🙂

The idea to generate the code is not new to the project, and we actually already generate the enums, but generating all the classes sounds even better. It is a breaking change however, so I don't think we can integrate it as is (even if I like the proposed API changes). We'll need to cherry-pick the ideas and integrate it in a non-breaking way, unless a new major version of libvips is released.

@karlvr
Copy link
Contributor Author

karlvr commented Sep 2, 2022

@warrenseine yes, it's a tricky one... there's inconsistency in the API at the moment I think as mentioned in other issues regarding do we return a new image vs update the current one. It's a particularly tricky transition due to no warning in Java when you ignore a return value... so null returning methods can't safely change to non-null returning... subtle bugs result.

Please feel free to lift anything from that branch. Or I'm happy to continue a discussion around a design for the naming that doesn't break backwards compatibility and make the changes myself. I'm keen to get our projects back on your releases!

@endlacer
Copy link

libvips is on version 8.16 on the meanwhile..

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.

3 participants