Discuss about other bindings implementation

[dbouron]

Hello,

The goal is to implement missing bindings. This non exhaustive list aims to report the axis of improvement, conventions and technical choices for the project. I will update this task list once we agree on a solution:

Propositions

1. Naming convention (see also: https://github.com/criteo/JVips/issues/40)

   • variable, method and function names in camel case
   • class names in pascal case

2. Stateless VipsImage Actually, VipsImage object is a stateful object. It has a mutable state when some methods are invoked (e.g.: resize(), pad(), crop(), ...). The .NET wrapper has a different approach. Every image method returns a new image. Should we keep the actual implementation or move to a stateless VipsImage?

3. How to handle the optional input arguments of some vips operation in java?

   • create a OptionalArguments class with default values and primitive type. Use builder pattern.
   • add non primitive type (e.g.: Integer) but nullable and append only non-nullable parameter to vips_operation (possible [non-]primitive type mixing in the method signature -> ugly 🤮)
   • use default value annotation

4. How to handle optional output in java?

   • return a Result struct
   • pass a ResultRef struct as parameter which wraps and packages required fields into a ByteBuffer. Thus we share a memory space which can be read/written by both C and Java code (not thread safe ⛐)

5. max() is a kind of union result using optional output It returns either a single maximum or a array of maximum values (with the coordinates in both cases)

   • split this in function into two functions: max1() and max() (some function actually do this in libvips)

Status

• [x] Naming convention
• [ ] VipsImage statelessness
• [x] Optional arguments
• [ ] Optional results
• [x] max variadic result

[warrenseine]

My take:

1. Ok
2. Yes. We probably need copy-on-write for non-mutating functions, but that's just an optimisation.
3. The idiomatic pattern in Java is to overload a method and call the original by passing the default value. Do you see any problem with this approach?
4. Optional results is a really weird pattern. Do you have an example in Vips API which returns "optional output"?
5. From what I read, max() always returns an array. Its size varies depending on the size argument. I don't see why we should make a specific case here. Let's do exactly what the C function does.

[dbouron]

3. Ok, so what about using the "default annotation"? Ideally, it would be worth to have named parameters in Java 4 . Sure:
   • https://github.com/libvips/libvips/blob/981d5c4b1695de0e984bb1ef7472b35759b36c8d/libvips/conversion/autorot.c#L237
   • https://github.com/libvips/libvips/blob/10c4831a709063189d67b7d6076b1b26bf62b2b2/libvips/iofuncs/system.c#L329
4. According to the doc:

   You can read out the position of the maximum with x and y . You can read out arrays of the values and positions of the top size maxima with out_array , x_array and y_array . These values are returned sorted from largest to smallest.

If size is not specified or equals to 1, output values are stored in x and y, in x/y_array otherwise

[warrenseine]

3. This seems to require preprocessing, as annotations in Java can't inject arguments. That's complex, adds a new dependency, and isn't idiomatic anyway. I wouldn't recommend it.

4. Ok, that's an unusual pattern. I don't think we'll get anything better than a ResultRef, but none of these approaches seem idiomatic.

5. In that case, maybe an Either-like structure would be a good pattern?

[dbouron]

3. You're right, patching the lack of named parameter with annotation is not the best approach.
4. Ok, let's s do that. It may be a source of leak with forgotten references which are not garbage collected.
5. +1

[karlvr]

Pardon me for jumping on in on this issue, but this has piqued my interest. I've built an experimental TypeScript program to parse the output of vips -l and vips <op> and to generate Java native stubs and JNI code using the g_blah approach from https://www.libvips.org/API/current/binding.html and it appears promising.

This is an example of the first function I've tried generating:

JNIEXPORT void JNICALL
Java_com_criteo_vips_VipsImage_embed(JNIEnv *env, jobject in, jint x, jint y, jint width, jint height, jobject options)
{
        GValue gvalue = { 0 };

        VipsOperation *op = vips_operation_new("embed");

        // in
        g_value_init( &gvalue, VIPS_TYPE_IMAGE );
        g_value_set_object(&gvalue, (VipsImage *) (*env)->GetLongField(env, in, handle_fid));
        g_object_set_property(G_OBJECT(op), "in", &gvalue);
        g_value_unset(&gvalue);

        // x
        g_value_init(&gvalue, G_TYPE_INT );
        g_value_set_int(&gvalue, x);
        g_object_set_property(G_OBJECT(op), "x", &gvalue);
        g_value_unset(&gvalue);

        // y
        g_value_init(&gvalue, G_TYPE_INT );
        g_value_set_int(&gvalue, y);
        g_object_set_property(G_OBJECT(op), "y", &gvalue);
        g_value_unset(&gvalue);

        // width
        g_value_init(&gvalue, G_TYPE_INT );
        g_value_set_int(&gvalue, width);
        g_object_set_property(G_OBJECT(op), "width", &gvalue);
        g_value_unset(&gvalue);

        // height
        g_value_init(&gvalue, G_TYPE_INT );
        g_value_set_int(&gvalue, height);
        g_object_set_property(G_OBJECT(op), "height", &gvalue);
        g_value_unset(&gvalue);

        // Operation
        VipsOperation *new_op;
        if (!(new_op = vips_cache_operation_build(op))) {
                g_object_unref(op);
                vips_error_exit(NULL); 
        }
        g_object_unref(op);
        op = new_op;

        // Image result
        g_value_init(&gvalue, VIPS_TYPE_IMAGE);
        g_object_get_property(G_OBJECT(op), "out", &gvalue);
        VipsImage *_out = VIPS_IMAGE(g_value_get_object(&gvalue));
        g_object_ref(_out); 
        g_value_unset(&gvalue);
        g_object_unref((VipsImage *) (*env)->GetLongField(env, in, handle_fid));
        (*env)->SetLongField(env, in, handle_fid, (jlong) _out);

        // Free the operation
        vips_object_unref_outputs(VIPS_OBJECT(op)); 
        g_object_unref(op);

}

I'd love to collaborate with you both on this if it piques your interest too. At the moment I think it's quite likely that we can automatically generate most of the implementation.

[karlvr]

A complete thumbnail_image:

JNIEXPORT void JNICALL
Java_com_criteo_vips_VipsImage_thumbnailImageNative(JNIEnv *env, jobject in, jint width, jobject options)
{
        GValue gvalue = { 0 };

        VipsOperation *op = vips_operation_new("thumbnail_image");

        // in
        if (in != NULL) {
                g_value_init(&gvalue, VIPS_TYPE_IMAGE);
                g_value_set_object(&gvalue, (VipsImage *) (*env)->GetLongField(env, in, handle_fid));
                g_object_set_property(G_OBJECT(op), "in", &gvalue);
                g_value_unset(&gvalue);
        }

        // width
        g_value_init(&gvalue, G_TYPE_INT);
        g_value_set_int(&gvalue, width);
        g_object_set_property(G_OBJECT(op), "width", &gvalue);
        g_value_unset(&gvalue);

        // Optionals
        if (options != NULL) {
                jclass optionsCls = (*env)->GetObjectClass(env, options);

                // height
                jfieldID heightFid = (*env)->GetFieldID(env, optionsCls, "height", "Ljava/lang/Integer;");
                jobject heightObjectValue = (*env)->GetObjectField(env, options, heightFid);
                if (heightObjectValue != NULL) {
                        jint height = (*env)->CallIntMethod(env, heightObjectValue, intValue_mid);
                        g_value_init(&gvalue, G_TYPE_INT);
                        g_value_set_int(&gvalue, height);
                        g_object_set_property(G_OBJECT(op), "height", &gvalue);
                        g_value_unset(&gvalue);
                }

                // size
                jfieldID sizeFid = (*env)->GetFieldID(env, optionsCls, "size", "Lcom/criteo/vips/enums/VipsSize;");
                jobject size = (*env)->GetObjectField(env, options, sizeFid);
                if (size != NULL) {
                        jclass sizeCls = (*env)->GetObjectClass(env, size);
                        jfieldID sizeValueFid = (*env)->GetFieldID(env, sizeCls, "value", "I");
                        jint sizeValue = (*env)->GetIntField(env, size, sizeValueFid);
                        g_value_init(&gvalue, G_TYPE_INT);
                        g_value_set_int(&gvalue, sizeValue);
                        g_object_set_property(G_OBJECT(op), "size", &gvalue);
                        g_value_unset(&gvalue);
                }

                // no-rotate
                jfieldID noRotateFid = (*env)->GetFieldID(env, optionsCls, "noRotate", "Ljava/lang/Boolean;");
                jobject noRotateObjectValue = (*env)->GetObjectField(env, options, noRotateFid);
                if (noRotateObjectValue != NULL) {
                        jboolean noRotate = (*env)->CallBooleanMethod(env, noRotateObjectValue, booleanValue_mid);
                        g_value_init(&gvalue, G_TYPE_BOOLEAN);
                        g_value_set_boolean(&gvalue, noRotate);
                        g_object_set_property(G_OBJECT(op), "no-rotate", &gvalue);
                        g_value_unset(&gvalue);
                }

                // crop
                jfieldID cropFid = (*env)->GetFieldID(env, optionsCls, "crop", "Lcom/criteo/vips/enums/VipsInteresting;");
                jobject crop = (*env)->GetObjectField(env, options, cropFid);
                if (crop != NULL) {
                        jclass cropCls = (*env)->GetObjectClass(env, crop);
                        jfieldID cropValueFid = (*env)->GetFieldID(env, cropCls, "value", "I");
                        jint cropValue = (*env)->GetIntField(env, crop, cropValueFid);
                        g_value_init(&gvalue, G_TYPE_INT);
                        g_value_set_int(&gvalue, cropValue);
                        g_object_set_property(G_OBJECT(op), "crop", &gvalue);
                        g_value_unset(&gvalue);
                }

                // linear
                jfieldID linearFid = (*env)->GetFieldID(env, optionsCls, "linear", "Ljava/lang/Boolean;");
                jobject linearObjectValue = (*env)->GetObjectField(env, options, linearFid);
                if (linearObjectValue != NULL) {
                        jboolean linear = (*env)->CallBooleanMethod(env, linearObjectValue, booleanValue_mid);
                        g_value_init(&gvalue, G_TYPE_BOOLEAN);
                        g_value_set_boolean(&gvalue, linear);
                        g_object_set_property(G_OBJECT(op), "linear", &gvalue);
                        g_value_unset(&gvalue);
                }

                // import-profile
                jfieldID importProfileFid = (*env)->GetFieldID(env, optionsCls, "importProfile", "Ljava/lang/String;");
                jstring importProfile = (jstring) (*env)->GetObjectField(env, options, importProfileFid);
                if (importProfile != NULL) {
                        const char *importProfileChars = (*env)->GetStringUTFChars(env, importProfile, NULL);
                        g_value_init(&gvalue, G_TYPE_STRING);
                        g_value_set_string(&gvalue, importProfileChars);
                        (*env)->ReleaseStringUTFChars(env, importProfile, importProfileChars);
                        g_object_set_property(G_OBJECT(op), "import-profile", &gvalue);
                        g_value_unset(&gvalue);
                }

                // export-profile
                jfieldID exportProfileFid = (*env)->GetFieldID(env, optionsCls, "exportProfile", "Ljava/lang/String;");
                jstring exportProfile = (jstring) (*env)->GetObjectField(env, options, exportProfileFid);
                if (exportProfile != NULL) {
                        const char *exportProfileChars = (*env)->GetStringUTFChars(env, exportProfile, NULL);
                        g_value_init(&gvalue, G_TYPE_STRING);
                        g_value_set_string(&gvalue, exportProfileChars);
                        (*env)->ReleaseStringUTFChars(env, exportProfile, exportProfileChars);
                        g_object_set_property(G_OBJECT(op), "export-profile", &gvalue);
                        g_value_unset(&gvalue);
                }

                // intent
                jfieldID intentFid = (*env)->GetFieldID(env, optionsCls, "intent", "Lcom/criteo/vips/enums/VipsIntent;");
                jobject intent = (*env)->GetObjectField(env, options, intentFid);
                if (intent != NULL) {
                        jclass intentCls = (*env)->GetObjectClass(env, intent);
                        jfieldID intentValueFid = (*env)->GetFieldID(env, intentCls, "value", "I");
                        jint intentValue = (*env)->GetIntField(env, intent, intentValueFid);
                        g_value_init(&gvalue, G_TYPE_INT);
                        g_value_set_int(&gvalue, intentValue);
                        g_object_set_property(G_OBJECT(op), "intent", &gvalue);
                        g_value_unset(&gvalue);
                }

        }
        // Operation
        VipsOperation *new_op;
        if (!(new_op = vips_cache_operation_build(op))) {
                g_object_unref(op);
                throwVipsException(env, "thumbnail_image failed");
                return;
        }
        g_object_unref(op);
        op = new_op;

        // Mutating image result
        g_value_init(&gvalue, VIPS_TYPE_IMAGE);
        g_object_get_property(G_OBJECT(op), "out", &gvalue);
        VipsImage *_out = VIPS_IMAGE(g_value_get_object(&gvalue));
        g_object_ref(_out); 
        g_value_unset(&gvalue);
        g_object_unref((VipsImage *) (*env)->GetLongField(env, in, handle_fid));
        (*env)->SetLongField(env, in, handle_fid, (jlong) _out);

        // Free the operation
        vips_object_unref_outputs(VIPS_OBJECT(op)); 
        g_object_unref(op);

}

[lopcode]

In case it's of interest to anyone in this thread - I recently shipped JVM libvips bindings that use the libvips GObject and operations API to generate ~100% operation coverage, using JDK 22's "FFM" feature: https://github.com/lopcode/vips-ffm

I mention because there don't appear to have been any changes to this repo for a couple of years. But please feel free to delete this comment if the project is still active or you don't feel it's appropriate.

Happy to answer any questions over there, and take feedback from users. And thanks for your efforts with JVips over the years.