Skip to content

Conversation

@versi-tech
Copy link
Contributor

There are some operations like findTrim or project providing multiple outputs as a result. Current implementation lacks API exposing it to the client.

Similarly to other high-level language bindings to libvips - this PR suggests dealing with multiple outputs via generic key-value structure - Map from JVM.

As a result - VImage operations providing multiple outputs return Map<String, VipsOption> instead of just a first output value. I've decided to suggest usings maps instead of simpler structures like list due to 2 reasons: consistency with other bindings & efficiency of seeking the results (direct access).

Copy link
Owner

@lopcode lopcode left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

This looks like a great improvement on the face of it. There are a couple of details like the generic option I want to have a think about, but I'll review and maybe tweak in the next day or two.

@versi-tech
Copy link
Contributor Author

Thanks for the PR!

This looks like a great improvement on the face of it. There are a couple of details like the generic option I want to have a think about, but I'll review and maybe tweak in the next day or two.

Thanks! Yeah - generic option approach was simply my first idea, but I guess there might be other ways. Thanks for looking into it - just FIY - I'm open to work on adjustments as per your suggestions on my own as well.

@lopcode lopcode force-pushed the feature/multi_output_results_handling branch from 89813eb to 06f1c00 Compare July 9, 2025 20:35
@lopcode
Copy link
Owner

lopcode commented Jul 9, 2025

Playing around with this, this week. I think we can keep VipsOption generics-free, plus Palantir's JavaPoet supports record types so we can maybe wrap multiple required outputs in to a helper record with nicely named properties.

@lopcode lopcode force-pushed the feature/multi_output_results_handling branch from 06f1c00 to 8df9f5e Compare July 12, 2025 23:39
@lopcode lopcode changed the title Multi-output results handling via Map added Add support for multi-output operations Jul 13, 2025
@lopcode lopcode force-pushed the feature/multi_output_results_handling branch from 8df9f5e to e458300 Compare July 13, 2025 21:43
@lopcode lopcode enabled auto-merge (squash) July 13, 2025 21:45
@lopcode lopcode merged commit a2019bd into lopcode:main Jul 13, 2025
7 checks passed
@lopcode
Copy link
Owner

lopcode commented Jul 18, 2025

Ended up going for the record based approach - the required outputs are known at generation time, so we can make a container record for the outputs including their original types, and parameter names according to the docs.

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