Skip to content

Conversation

@kleisauke
Copy link
Member

See commit cb58d7d for rationale.

History:
Commit 1214f94 moved this flag to the base class, and commit 53db48d later removed it, likely under the assumption that the _source class still had this flag set.

Targets the 8.17 branch.

@kleisauke
Copy link
Member Author

Found by this (not yet committed) change in NetVips:

--- a/samples/NetVips.Samples/Samples/GenerateImageClass.cs
+++ b/samples/NetVips.Samples/Samples/GenerateImageClass.cs
@@ -156,7 +156,8 @@ private string GenerateFunction(string operationName, string indent = "
         bool mutable = false, IReadOnlyList<Introspect.Argument> outParameters = null)
     {
         using var op = Operation.NewFromName(operationName);
-        if ((op.GetFlags() & Enums.OperationFlags.DEPRECATED) != 0)
+        var flags = op.GetFlags();
+        if ((flags & Enums.OperationFlags.DEPRECATED) != 0)
         {
             throw new ArgumentException($"No such operator. Operator \"{operationName}\" is deprecated");
         }
@@ -172,8 +173,8 @@ private string GenerateFunction(string operationName, string indent = "
         // we are only interested in non-deprecated args
         var optionalInput = intro.OptionalInput
             .Where(arg => (arg.Value.Flags & Enums.ArgumentFlags.DEPRECATED) == 0)
-            // Drop "revalidate" option from source and buffer loaders as they are already uncached
-            .Where(arg => arg.Key != "revalidate" || (firstArgType != GValue.SourceType && firstArgType != GValue.BlobType))
+            // Drop "revalidate" option from buffer loaders and operations marked "nocache" as they are already uncached
+            .Where(arg => arg.Key != "revalidate" || ((flags & Enums.OperationFlags.NOCACHE) == 0 && firstArgType != GValue.BlobType))
             .Select(x => x.Value)
             .ToArray();
         var optionalOutput = intro.OptionalOutput

@jcupitt
Copy link
Member

jcupitt commented Aug 21, 2025

That's annoying. How about adding this to operation.c:

static void
vips_operation_summary_class(VipsObjectClass *object_class, VipsBuf *buf)
{
    VipsOperationClass *operation_class = VIPS_OPERATION_CLASS(object_class);

    VIPS_OBJECT_CLASS(vips_operation_parent_class)
        ->summary_class(object_class, buf);
 
    GType gtype = G_OBJECT_CLASS_TYPE(operation_class);
    if (!G_TYPE_IS_ABSTRACT(gtype) &&
        operation_class->flags & VIPS_OPERATION_NOCACHE)
        vips_buf_appendf(buf, ", nocache");
}

Now vips -l displays eg.:

$ vips -l | grep _source
          VipsForeignLoadCsvSource (csvload_source), load csv, nocache, priority=0, untrusted, is_a_source, get_flags, header, load
...

And we can test that every source loader is tagged correctly with:

$ vips -l | grep _source | grep -v nocache
          VipsForeignLoadOpenslideSource (openslideload_source), load source with OpenSlide, priority=100, untrusted, is_a_source, get_flags, get_flags_filename, header, load
$ 

(before this PR, of course hehe)

@kleisauke
Copy link
Member Author

Good idea! Let's do that as a follow-up.

@jcupitt
Copy link
Member

jcupitt commented Aug 23, 2025

What should we do about your netvips commit? Do you think revalidate should move out of the base class?

@kleisauke
Copy link
Member Author

I'll apply that NetVips commit when regenerating the methods for libvips 8.18.

I'm not sure there's much benefit to moving the revalidate flag to VipsOperation, it would likely create unnecessary noise in downstream bindings. The only use case I can see is benchmarking libvips, but in that case it's probably better to disable the cache globally rather than individually for each operation.

@jcupitt jcupitt merged commit 122648d into libvips:8.17 Aug 25, 2025
6 checks passed
@jcupitt
Copy link
Member

jcupitt commented Aug 25, 2025

OK, let's merge then!

@kleisauke kleisauke deleted the 8.17-openslideload-source-nocache branch August 25, 2025 13:22
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