IC and TS getattributetype() -- BREAKING CHANGE

ImageSpec and ParamValueList have had (since 2.1) a getattributetype()
method, which simply returns the TypeDesc of the named attribute, or
TypeUnknown if no such attribute exits. This goes along well with the
related getattribute() and attribute() family of methods.

The getattribute() method was not present in ImageCache and
TextureSystem, despite their having very similar
getattribute()/attribute() interfaces.

A recent issue made me realize that in the Python bindings, the IC and
TS bindings for `getattribute(name)` did not work properly, always
returning None, and *required* you to use the full
`getattribute(name,type)` variety where you have to supply -- and
already know -- the type. This isn't especially Pythonic, where you
should just be able to ask by name and get a polymorphic result. And
when I went to look at how to repair this, it was going to be very
awkward without getattributetype(), but trivial with it.

So this patch sets up getattributetype() for both IC and TS, and
exposes them to both C++ and Python APIs, and also fully fixes the
getattribute() so that it works properly with the name only.

Unfortunately, because getattributename is a new virtual method, it
changes the vtable, breaking the ABI, and thus cannot be backported to
any supported release that makes ABI compatibility promises.

Now, it is worth noting that OIIO 2.4 is currently in release
candidate form, but has not yet been released. It was scheduled for
this week.  We thus have a last-minute opportunity to squeeze this
into 2.4 rather than having this enhancement only in 2.5+, but at the
expense of delaying the release by a week or two and making an ABI
change later than we would ordinarily have allowed (but still ok to
do, no promises are final until we do a "final release").

So I ask interested parties now, should we:

(a) Put this in master only, i.e. for next year's 2.5 release, and go
    ahead and release 2.4 now as-is (and without any last-minute
    change, potential instabilities, or ABI changes since it went into
    beta).

(b) Delay 2.4 final release until Oct 1, have this very minor ABI
    break versus the beta, and squeeze this into 2.4 so everybody will
    have it this year.

What say you?

src/include/OpenImageIO/imagecache.h

@@ -18,6 +18,9 @@
 // Is the close() method present?
 #define OIIO_IMAGECACHE_SUPPORTS_CLOSE 1
 
+// Is the getattributetype() method present? (Added in 2.5)
+#define OIIO_IMAGECACHE_SUPPORTS_GETATTRIBUTETYPE 1
+
 // Does invalidate() support the optional `force` flag?
 #define OIIO_IMAGECACHE_INVALIDATE_FORCE 1
 
@@ -395,6 +398,12 @@ class OIIO_API ImageCache {
     /// as a `std::string`.
     virtual bool getattribute (string_view name, std::string &val) const = 0;
 
+    /// If the named attribute is known, return its data type. If no such
+    /// attribute exists, return `TypeUnknown`.
+    ///
+    /// This was added in version 2.5.
+    virtual TypeDesc getattributetype(string_view name) const = 0;
+
     /// @}
 
 

src/include/OpenImageIO/texture.h

@@ -21,6 +21,9 @@
 // features are supported.
 #define OIIO_TEXTURESYSTEM_SUPPORTS_CLOSE 1
 
+// Is the getattributetype() method present? (Added in 2.5)
+#define OIIO_TEXTURESYSTEM_SUPPORTS_GETATTRIBUTETYPE 1
+
 #define OIIO_TEXTURESYSTEM_SUPPORTS_STOCHASTIC 1
 
 #ifndef INCLUDED_IMATHVEC_H
@@ -715,6 +718,12 @@ class OIIO_API TextureSystem {
     /// as a `std::string`.
     virtual bool getattribute(string_view name, std::string& val) const = 0;
 
+    /// If the named attribute is known, return its data type. If no such
+    /// attribute exists, return `TypeUnknown`.
+    ///
+    /// This was added in version 2.5.
+    virtual TypeDesc getattributetype (string_view name) const = 0;
+
     /// @}
 
     /// @{

src/libtexture/imagecache.cpp

@@ -2241,6 +2241,78 @@ ImageCacheImpl::attribute(string_view name, TypeDesc type, const void* val)
 
 
 
+TypeDesc
+ImageCacheImpl::getattributetype(string_view name) const
+{
+    static std::unordered_map<std::string, TypeDesc> attr_types {
+        { "max_open_files", TypeInt },
+        { "max_memory_MB", TypeFloat },
+        { "statistics:level", TypeInt },
+        { "max_errors_per_file", TypeInt },
+        { "autotile", TypeInt },
+        { "autoscanline", TypeInt },
+        { "automip", TypeInt },
+        { "forcefloat", TypeInt },
+        { "accept_untiled", TypeInt },
+        { "accept_unmipped", TypeInt },
+        { "deduplicate", TypeInt },
+        { "unassociatedalpha", TypeInt },
+        { "trust_file_extensions", TypeInt },
+        { "failure_retries", TypeInt },
+        { "total_files", TypeInt },
+        { "max_mip_res", TypeInt },
+        { "searchpath", TypeString },
+        { "plugin_searchpath", TypeString },
+        { "worldtocommon", TypeMatrix },
+        { "commontoworld", TypeMatrix },
+        { "latlong_up", TypeString },
+        { "substitute_image", TypeString },
+        { "stat:cache_memory_used", TypeInt64 },
+        { "stat:tiles_created", TypeInt },
+        { "stat:tiles_current", TypeInt },
+        { "stat:tiles_peak", TypeInt },
+        { "stat:open_files_created", TypeInt },
+        { "stat:open_files_current", TypeInt },
+        { "stat:open_files_peak", TypeInt },
+        { "stat:find_tile_calls", TypeInt64 },
+        { "stat:find_tile_microcache_misses", TypeInt64 },
+        { "stat:find_tile_cache_misses", TypeInt },
+        { "stat:files_totalsize", TypeInt64 },
+        { "stat:image_size", TypeInt64 },
+        { "stat:file_size", TypeInt64 },
+        { "stat:bytes_read", TypeInt64 },
+        { "stat:unique_files", TypeInt },
+        { "stat:fileio_time", TypeFloat },
+        { "stat:fileopen_time", TypeFloat },
+        { "stat:file_locking_time", TypeFloat },
+        { "stat:tile_locking_time", TypeFloat },
+        { "stat:find_file_time", TypeFloat },
+        { "stat:find_tile_time", TypeFloat },
+        { "stat:texture_queries", TypeInt64 },
+        { "stat:texture3d_queries", TypeInt64 },
+        { "stat:environment_queries", TypeInt64 },
+        { "stat:getimageinfo_queries", TypeInt64 },
+        { "stat:gettextureinfo_queries", TypeInt64 },
+    };
+
+    // For all the easy cases, if the attribute is in the table and has a
+    // simple type, use that.
+    const auto found = attr_types.find(name);
+    if (found != attr_types.end())
+        return found->second;
+
+    // The cases that don't fit in the tabke
+    if (name == "all_filenames") {
+        // all_filenames is an array, but the length depends on the actual
+        // number of files we've encountered.
+        return TypeDesc(TypeDesc::STRING, int(m_files.size()));
+    }
+
+    return TypeUnknown;
+}
+
+
+
 bool
 ImageCacheImpl::getattribute(string_view name, TypeDesc type, void* val) const
 {

src/libtexture/imagecache_pvt.h

@@ -808,6 +808,8 @@ class ImageCacheImpl final : public ImageCache {
         return attribute(name, TypeDesc::STRING, &s);
     }
 
+    TypeDesc getattributetype(string_view name) const override;
+
     bool getattribute(string_view name, TypeDesc type,
                       void* val) const override;
     bool getattribute(string_view name, int& val) const override

src/libtexture/texture_pvt.h

@@ -61,6 +61,8 @@ class TextureSystemImpl final : public TextureSystem {
         return attribute(name, TypeDesc::STRING, &s);
     }
 
+    TypeDesc getattributetype(string_view name) const override;
+
     bool getattribute(string_view name, TypeDesc type,
                       void* val) const override;
     bool getattribute(string_view name, int& val) const override

src/libtexture/texturesys.cpp

@@ -485,6 +485,37 @@ TextureSystemImpl::attribute(string_view name, TypeDesc type, const void* val)
 
 
 
+TypeDesc
+TextureSystemImpl::getattributetype(string_view name) const
+{
+    // clang-format off
+    static std::unordered_map<std::string, TypeDesc> attr_types {
+        { "worldtocommon", TypeMatrix },
+        { "commontoworld", TypeMatrix },
+        { "gray_to_rgb", TypeInt },
+        { "grey_to_rgb", TypeInt },
+        { "flip_t", TypeInt },
+        { "max_tile_channels", TypeInt },
+        { "stochastic", TypeInt },
+    };
+    // clang-format on
+
+    // For all the easy cases, if the attribute is in the table and has a
+    // simple type, use that.
+    const auto found = attr_types.find(name);
+    if (found != attr_types.end())
+        return found->second;
+
+    // Maybe it's an ImageCache attribute
+    TypeDesc ict = m_imagecache->getattributetype(name);
+    if (ict != TypeUnknown)
+        return ict;
+
+    return TypeUnknown;
+}
+
+
+
 bool
 TextureSystemImpl::getattribute(string_view name, TypeDesc type,
                                 void* val) const

src/python/py_imagecache.cpp

@@ -103,9 +103,17 @@ declare_imagecache(py::module& m)
             "getattribute",
             [](const ImageCacheWrap& ic, const std::string& name,
                TypeDesc type) {
+                if (type == TypeUnknown)
+                    type = ic.m_cache->getattributetype(name);
                 return getattribute_typed(*ic.m_cache, name, type);
             },
             "name"_a, "type"_a = TypeUnknown)
+        .def(
+            "getattributetype",
+            [](const ImageCacheWrap& ic, const std::string& name) {
+                return ic.m_cache->getattributetype(name);
+            },
+            "name"_a)
         .def("resolve_filename",
              [](ImageCacheWrap& ic, const std::string& filename) {
                  py::gil_scoped_release gil;

src/python/py_texturesys.cpp

@@ -205,10 +205,18 @@ declare_texturesystem(py::module& m)
                  if (ts.m_texsys)
                      attribute_typed(*ts.m_texsys, name, type, obj);
              })
+        .def(
+            "getattributetype",
+            [](const TextureSystemWrap& ts, const std::string& name) {
+                return ts.m_texsys->getattributetype(name);
+            },
+            "name"_a)
         .def(
             "getattribute",
             [](const TextureSystemWrap& ts, const std::string& name,
                TypeDesc type) {
+                if (type == TypeUnknown)
+                    type = ts.m_texsys->getattributetype(name);
                 return getattribute_typed(*ts.m_texsys, name, type);
             },
             "name"_a, "type"_a = TypeUnknown)

testsuite/python-imagecache/ref/out.txt

@@ -1,6 +1,14 @@
-stat:cache_memory_used 0
-stat:image_size 4036864
-total_files 2
-all_filenames ('../common/grid.tif', '../common/tahoe-tiny.tif')
+full getattribute stat:cache_memory_used 0
+full getattribute stat:image_size 4036864
+full getattribute total_files 2
+full getattribute all_filenames ('../common/grid.tif', '../common/tahoe-tiny.tif')
+getattributetype stat:cache_memory_used int64
+getattributetype stat:image_size int64
+getattributetype total_files int
+getattributetype all_filenames string[2]
+untyped getattribute stat:cache_memory_used 0
+untyped getattribute stat:image_size 4036864
+untyped getattribute total_files 2
+untyped getattribute all_filenames ('../common/grid.tif', '../common/tahoe-tiny.tif')
 
 Done.

testsuite/python-imagecache/src/test_imagecache.py

@@ -18,11 +18,24 @@
     ib = oiio.ImageBuf("../common/tahoe-tiny.tif")
     ib = oiio.ImageBuf("../common/grid.tif")
 
-    print ("stat:cache_memory_used", ic.getattribute("stat:cache_memory_used", 'int64'))
-    print ("stat:image_size", ic.getattribute("stat:image_size", 'int64'))
+    # Test getattribute(name, type) with the full type specified
+    print ("full getattribute stat:cache_memory_used", ic.getattribute("stat:cache_memory_used", 'int64'))
+    print ("full getattribute stat:image_size", ic.getattribute("stat:image_size", 'int64'))
     total_files = ic.getattribute("total_files", 'int')
-    print ("total_files", ic.getattribute("total_files", 'int'))
-    print ("all_filenames", ic.getattribute("all_filenames", 'string[{}]'.format(total_files)))
+    print ("full getattribute total_files", ic.getattribute("total_files", 'int'))
+    print ("full getattribute all_filenames", ic.getattribute("all_filenames", 'string[{}]'.format(total_files)))
+
+    # Test getattributetype() to retrieve the type of an attribute
+    print ("getattributetype stat:cache_memory_used", ic.getattributetype("stat:cache_memory_used"))
+    print ("getattributetype stat:image_size", ic.getattributetype("stat:image_size"))
+    print ("getattributetype total_files", ic.getattributetype("total_files"))
+    print ("getattributetype all_filenames", ic.getattributetype("all_filenames"))
+
+    # Test getattribute(name) with the type inferred from the attribute
+    print ("untyped getattribute stat:cache_memory_used", ic.getattribute("stat:cache_memory_used"))
+    print ("untyped getattribute stat:image_size", ic.getattribute("stat:image_size"))
+    print ("untyped getattribute total_files", ic.getattribute("total_files"))
+    print ("untyped getattribute all_filenames", ic.getattribute("all_filenames"))
 
     print ("\nDone.")
 except Exception as detail:

testsuite/python-texturesys/ref/out-windows.txt

@@ -14,4 +14,12 @@ default-missingcolor = (0.0, 0.0, 0.0, 0.0)
 top mip pixel differences when streaming = 0
 
 udim file.<UDIM>.tx -> 2x4 ['.\\file.1001.tx', '.\\file.1002.tx', '.\\file.1011.tx', '.\\file.1012.tx', '', '', '', '.\\file.1032.tx']
+getattributetype stat:image_size int64
+getattributetype total_files int
+getattributetype max_memory_MB float
+getattributetype worldtocommon matrix
+untyped getattribute stat:image_size 1048575
+untyped getattribute total_files 3
+untyped getattribute max_memory_MB 1024.0
+untyped getattribute worldtocommon (1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0)
 Done.

testsuite/python-texturesys/ref/out.txt

@@ -14,4 +14,12 @@ default-missingcolor = (0.0, 0.0, 0.0, 0.0)
 top mip pixel differences when streaming = 0
 
 udim file.<UDIM>.tx -> 2x4 ['./file.1001.tx', './file.1002.tx', './file.1011.tx', './file.1012.tx', '', '', '', './file.1032.tx']
+getattributetype stat:image_size int64
+getattributetype total_files int
+getattributetype max_memory_MB float
+getattributetype worldtocommon matrix
+untyped getattribute stat:image_size 1048575
+untyped getattribute total_files 3
+untyped getattribute max_memory_MB 1024.0
+untyped getattribute worldtocommon (1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0)
 Done.

testsuite/python-texturesys/src/test_texture_sys.py

@@ -15,6 +15,10 @@
 texture_opt.swrap = oiio.Wrap.Periodic
 texture_opt.twrap = oiio.Wrap.Periodic
 
+
+# Test getattribute
+
+
 print ("checker middle, top mip, channels: 1 =", texture_sys.texture(checker, texture_opt, 0.5, 0.5, 0, 0, 0, 0, 1))
 print ("checker middle, top mip, channels: 2 =", texture_sys.texture(checker, texture_opt, 0.5, 0.5, 0, 0, 0, 0, 2))
 print ("checker middle, top mip, channels: 3 =", texture_sys.texture(checker, texture_opt, 0.5, 0.5, 0, 0, 0, 0, 3))
@@ -57,4 +61,16 @@
 print("udim {} -> {}x{} {}".format(udname, utiles, vtiles, tilenames))
 
 
+# Test getattribute() and getattributetype()
+print ("getattributetype stat:image_size", texture_sys.getattributetype("stat:image_size"))
+print ("getattributetype total_files", texture_sys.getattributetype("total_files"))
+print ("getattributetype max_memory_MB", texture_sys.getattributetype("max_memory_MB"))
+print ("getattributetype worldtocommon", texture_sys.getattributetype("worldtocommon"))
+
+# Test getattribute(name) with the type inferred from the attribute
+print ("untyped getattribute stat:image_size", texture_sys.getattribute("stat:image_size"))
+print ("untyped getattribute total_files", texture_sys.getattribute("total_files"))
+print ("untyped getattribute max_memory_MB", texture_sys.getattribute("max_memory_MB"))
+print ("untyped getattribute worldtocommon", texture_sys.getattribute("worldtocommon"))
+
 print("Done.")