api: remove redundant ImageSpec from ImageCache (issue #4436)

src/iconvert/iconvert.cpp

@@ -364,8 +364,7 @@ convert_file(const std::string& in_filename, const std::string& out_filename)
         if (nsubimages > 1) {
             subimagespecs.resize(nsubimages);
             for (int i = 0; i < nsubimages; ++i) {
-                ImageSpec inspec = *imagecache->imagespec(ufilename, i, 0,
-                                                          true /*native*/);
+                ImageSpec inspec = *imagecache->imagespec(ufilename, i);
                 subimagespecs[i] = inspec;
                 adjust_spec(in.get(), out.get(), inspec, subimagespecs[i]);
             }

src/include/OpenImageIO/imagecache.h

@@ -724,42 +724,51 @@ class OIIO_API ImageCache {
                         int miplevel, ustring dataname, TypeDesc datatype,
                         void* data);
 
-    /// Copy the ImageSpec associated with the named image (the first
-    /// subimage & miplevel by default, or as set by `subimage` and
-    /// `miplevel`).
-    ///
+    /// Copy the ImageSpec that describes the named image file.
+    ///
+    /// Note that the spec returned describes the file as it exists in the
+    /// file, at the base (highest-resolution) MIP level of that subimage.
+    /// Certain aspects of the in-cache representation may differ from the
+    /// file (due to ImageCache implementation strategy or options like
+    /// `"forcefloat"` or `"autotile"`). If you really need to know the
+    /// in-cache data type, tile size, or how the resolution or tiling changes
+    /// on a particular MIP level, you should use `get_cache_dimensions()`.
     /// @param  filename
     ///             The name of the image, as a UTF-8 encoded ustring.
     /// @param  spec
     ///             ImageSpec into which will be copied the spec for the
     ///             requested image.
-    /// @param  subimage/miplevel
-    ///             The subimage and MIP level to query.
-    /// @param  native
-    ///             If `false` (the default), then the spec retrieved will
-    ///             accurately describe the image stored internally in the
-    ///             cache, whereas if `native` is `true`, the spec retrieved
-    ///             will reflect the contents of the original file.  These
-    ///             may differ due to use of certain ImageCache settings
-    ///             such as `"forcefloat"` or `"autotile"`.
+    /// @param  subimage
+    ///             The subimage to query.
     /// @returns
     ///             `true` upon success, `false` upon failure failure (such
     ///             as being unable to find, open, or read the file, or if
-    ///             it does not contain the designated subimage or MIP
-    ///             level).
-    bool get_imagespec(ustring filename, ImageSpec& spec, int subimage = 0,
-                       int miplevel = 0, bool native = false);
+    ///             it does not contain the designated subimage.
+    bool get_imagespec(ustring filename, ImageSpec& spec, int subimage = 0);
     /// A more efficient variety of `get_imagespec()` for cases where you
     /// can use an `ImageHandle*` to specify the image and optionally have a
     /// `Perthread*` for the calling thread.
     bool get_imagespec(ImageHandle* file, Perthread* thread_info,
-                       ImageSpec& spec, int subimage = 0, int miplevel = 0,
-                       bool native = false);
+                       ImageSpec& spec, int subimage = 0);
+
+    /// DEPRECATED old API. Note that the miplevel and native parameters are ignored:
+    /// it will always get the native spec of miplevel 0. We recommend switching to
+    /// the new API.
+    bool get_imagespec(ustring filename, ImageSpec& spec, int subimage,
+                       int miplevel, bool native = false)
+    {
+        return get_imagespec(filename, spec, subimage);
+    }
+    bool get_imagespec(ImageHandle* file, Perthread* thread_info,
+                       ImageSpec& spec, int subimage, int miplevel,
+                       bool native = false)
+    {
+        return get_imagespec(file, thread_info, spec, subimage);
+    }
 
-    /// Return a pointer to an ImageSpec associated with the named image
-    /// (the first subimage & MIP level by default, or as set by `subimage`
-    /// and `miplevel`) if the file is found and is an image format that can
-    /// be read, otherwise return `nullptr`.
+    /// Return a pointer to an ImageSpec that describes the named image file.
+    /// If the file is found and is an image format that can be read,
+    /// otherwise return `nullptr`.
     ///
     /// This method is much more efficient than `get_imagespec()`, since it
     /// just returns a pointer to the spec held internally by the ImageCache
@@ -768,29 +777,70 @@ class OIIO_API ImageCache {
     /// (even other threads) calls `invalidate()` on the file, or
     /// `invalidate_all()`, or destroys the ImageCache.
     ///
+    /// Note that the spec returned describes the file as it exists in the
+    /// file, at the base (highest-resolution) MIP level of that subimage.
+    /// Certain aspects of the in-cache representation may differ from the
+    /// file (due to ImageCache implementation strategy or options like
+    /// `"forcefloat"` or `"autotile"`). If you really need to know the
+    /// in-cache data type, tile size, or how the resolution or tiling changes
+    /// on a particular MIP level, you should use `get_cache_dimensions()`.
     /// @param  filename
     ///             The name of the image, as a UTF-8 encoded ustring.
-    /// @param  subimage/miplevel
-    ///             The subimage and MIP level to query.
-    /// @param  native
-    ///             If `false` (the default), then the spec retrieved will
-    ///             accurately describe the image stored internally in the
-    ///             cache, whereas if `native` is `true`, the spec retrieved
-    ///             will reflect the contents of the original file.  These
-    ///             may differ due to use of certain ImageCache settings
-    ///             such as `"forcefloat"` or `"autotile"`.
+    /// @param  subimage
+    ///             The subimage to query.
     /// @returns
     ///             A pointer to the spec, if the image is found and able to
     ///             be opened and read by an available image format plugin,
-    ///             and the designated subimage and MIP level exists.
-    const ImageSpec* imagespec(ustring filename, int subimage = 0,
-                               int miplevel = 0, bool native = false);
+    ///             and the designated subimage exists.
+    const ImageSpec* imagespec(ustring filename, int subimage = 0);
     /// A more efficient variety of `imagespec()` for cases where you can
     /// use an `ImageHandle*` to specify the image and optionally have a
     /// `Perthread*` for the calling thread.
     const ImageSpec* imagespec(ImageHandle* file, Perthread* thread_info,
-                               int subimage = 0, int miplevel = 0,
-                               bool native = false);
+                               int subimage = 0);
+
+    /// DEPRECATED old API. Note that the miplevel and native parameters are ignored:
+    /// it will always get the native spec of miplevel 0. We recommend switching to
+    /// the new API.
+    const ImageSpec* imagespec(ustring filename, int subimage, int miplevel,
+                               bool native = false)
+    {
+        return imagespec(filename, subimage);
+    }
+    const ImageSpec* imagespec(ImageHandle* file, Perthread* thread_info,
+                               int subimage, int miplevel, bool native = false)
+    {
+        return imagespec(file, thread_info, subimage);
+    }
+
+    /// Copy the image dimensions (x, y, z, width, height, depth, full*,
+    /// nchannels, format) and data types that describes the named image
+    /// cache file for the specified subimage and miplevel. It does *not*
+    /// copy arbitrary named metadata or channel names (thus, for an
+    /// `ImageSpec` with lots of metadata, it is much less expensive than
+    /// copying the whole thing with `operator=()`). The associated
+    /// metadata and channels names can be retrieved with `imagespec()`
+    /// or `get_imagespec`.
+    ///
+    /// @param  filename
+    ///             The name of the image, as a UTF-8 encoded ustring.
+    /// @param  spec
+    ///             ImageSpec into which will be copied the dimensions
+    ///             for the requested image.
+    /// @param  subimage/miplevel
+    ///             The subimage and mip level to query.
+    /// @returns
+    ///             `true` upon success, `false` upon failure failure (such
+    ///             as being unable to find, open, or read the file, or if
+    ///             it does not contain the designated subimage or mip level.
+    bool get_cache_dimensions(ustring filename, ImageSpec& spec,
+                              int subimage = 0, int miplevel = 0);
+    /// A more efficient variety of `get_cache_dimensions()` for cases where
+    /// you can use an `ImageHandle*` to specify the image and optionally
+    /// have a `Perthread*` for the calling thread.
+    bool get_cache_dimensions(ImageHandle* file, Perthread* thread_info,
+                              ImageSpec& spec, int subimage = 0,
+                              int miplevel = 0);
 
     /// Copy into `thumbnail` any associated thumbnail associated with this
     /// image (for the first subimage by default, or as set by `subimage`).

src/include/OpenImageIO/texture.h

@@ -1433,12 +1433,24 @@ class OIIO_API TextureSystem {
     ///             as being unable to find, open, or read the file, or if
     ///             it does not contain the designated subimage or MIP
     ///             level).
-    bool get_imagespec(ustring filename, int subimage, ImageSpec& spec);
+    bool get_imagespec(ustring filename, ImageSpec& spec, int subimage = 0);
     /// A more efficient variety of `get_imagespec()` for cases where you
     /// can use a `TextureHandle*` to specify the image and optionally have
     /// a `Perthread*` for the calling thread.
     bool get_imagespec(TextureHandle* texture_handle, Perthread* thread_info,
-                       int subimage, ImageSpec& spec);
+                       ImageSpec& spec, int subimage = 0);
+
+    /// DEPRECATED old API. Note that the spec and subimage parameters are
+    /// inverted. We recommend switching to the new API.
+    bool get_imagespec(ustring filename, int subimage, ImageSpec& spec)
+    {
+        return get_imagespec(filename, spec, subimage);
+    }
+    bool get_imagespec(TextureHandle* texture_handle, Perthread* thread_info,
+                       int subimage, ImageSpec& spec)
+    {
+        return get_imagespec(texture_handle, thread_info, spec, subimage);
+    }
 
     /// Return a pointer to an ImageSpec associated with the named texture
     /// if the file is found and is an image format that can be read,

src/libOpenImageIO/imagebuf.cpp

@@ -1076,9 +1076,11 @@ ImageBufImpl::init_spec(string_view filename, int subimage, int miplevel,
         m_imagecache->get_image_info(m_name, subimage, miplevel, s_fileformat,
                                      TypeString, &fmt);
         m_fileformat = ustring(fmt);
-        m_imagecache->get_imagespec(m_name, m_spec, subimage, miplevel);
-        m_imagecache->get_imagespec(m_name, m_nativespec, subimage, miplevel,
-                                    true);
+
+        m_imagecache->get_imagespec(m_name, m_nativespec, subimage);
+        m_spec = m_nativespec;
+        m_imagecache->get_cache_dimensions(m_name, m_spec, subimage, miplevel);
+
         m_xstride = m_spec.pixel_bytes();
         m_ystride = m_spec.scanline_bytes();
         m_zstride = clamped_mult64(m_ystride, (imagesize_t)m_spec.height);

src/libOpenImageIO/imagecache_test.cpp

@@ -300,28 +300,68 @@ test_imagespec()
         Strutil::print("imagespec() of non-existant file:\n  {}\n",
                        ic->geterror());
     }
-    {  // imagespec() for nonexistant file
-        const ImageSpec* spec = ic->imagespec(ustring("noexist.exr"));
-        OIIO_CHECK_ASSERT(spec == nullptr && ic->has_error());
-        Strutil::print("imagespec() of non-existant file:\n  {}\n",
-                       ic->geterror());
-    }
     {  // imagespec() for null handle
         const ImageSpec* spec = ic->imagespec(nullptr, nullptr);
         OIIO_CHECK_ASSERT(spec == nullptr && ic->has_error());
         Strutil::print("imagespec(handle) of non-existant file:\n  {}\n",
                        ic->geterror());
     }
     {  // imagespec() for out of range subimage
-        const ImageSpec* spec = ic->imagespec(checkertex, 10, 0);
+        const ImageSpec* spec = ic->imagespec(checkertex, 10);
         OIIO_CHECK_ASSERT(spec == nullptr && ic->has_error());
         Strutil::print("imagespec() out-of-range subimage:\n  {}\n",
                        ic->geterror());
     }
-    {  // imagespec() for out of range mip level
-        const ImageSpec* spec = ic->imagespec(checkertex, 0, 100);
-        OIIO_CHECK_ASSERT(spec == nullptr && ic->has_error());
-        Strutil::print("imagespec() out-of-range subimage:\n  {}\n",
+}
+
+
+
+static void
+test_get_cache_dimensions()
+{
+    Strutil::print("\nTesting cache dimensions retrieval\n");
+    auto ic = ImageCache::create();
+
+    {  // basic get_cache_dimensions()
+        ImageSpec spec;
+        OIIO_CHECK_ASSERT(ic->get_cache_dimensions(checkertex, spec));
+        OIIO_CHECK_EQUAL(spec.width, 256);
+    }
+    {  // basic get_cache_dimensions() with handle
+        auto hand = ic->get_image_handle(checkertex);
+        ImageSpec spec;
+        OIIO_CHECK_ASSERT(ic->get_cache_dimensions(hand, nullptr, spec));
+        OIIO_CHECK_EQUAL(spec.width, 256);
+    }
+
+    {  // get_cache_dimensions() for nonexistant file
+        ImageSpec spec;
+        OIIO_CHECK_FALSE(
+            ic->get_cache_dimensions(ustring("noexist.exr"), spec));
+        OIIO_CHECK_ASSERT(ic->has_error());
+        Strutil::print("get_cache_dimensions() of non-existant file:\n  {}\n",
+                       ic->geterror());
+    }
+    {  // get_cache_dimensions() for null handle
+        ImageSpec spec;
+        const bool valid = ic->get_cache_dimensions(nullptr, nullptr, spec);
+        OIIO_CHECK_ASSERT(!valid && ic->has_error());
+        Strutil::print(
+            "get_cache_dimensions(handle) of non-existant file:\n  {}\n",
+            ic->geterror());
+    }
+    {  // get_cache_dimensions() for out of range subimage
+        ImageSpec spec;
+        const bool valid = ic->get_cache_dimensions(checkertex, spec, 10);
+        OIIO_CHECK_ASSERT(!valid && ic->has_error());
+        Strutil::print("get_cache_dimensions() out-of-range subimage:\n  {}\n",
+                       ic->geterror());
+    }
+    {  // get_cache_dimensions() for out of range mip level
+        ImageSpec spec;
+        const bool valid = ic->get_cache_dimensions(checkertex, spec, 0, 100);
+        OIIO_CHECK_ASSERT(!valid && ic->has_error());
+        Strutil::print("get_cache_dimensions() out-of-range miplevel:\n  {}\n",
                        ic->geterror());
     }
 }
@@ -345,6 +385,7 @@ main(int /*argc*/, char* /*argv*/[])
     test_get_pixels_errors();
     test_custom_threadinfo();
     test_imagespec();
+    test_get_cache_dimensions();
 
     auto ic = ImageCache::create();
     Strutil::print("\n\n{}\n", ic->getstats(5));

src/libOpenImageIO/imagespeed_test.cpp

@@ -496,7 +496,7 @@ main(int argc, char** argv)
     imagesize_t maxpelchans = 0;
     for (auto&& fn : input_filename) {
         ImageSpec spec;
-        if (!imagecache->get_imagespec(fn, spec, 0, 0, true)) {
+        if (!imagecache->get_imagespec(fn, spec)) {
             std::cout << "File \"" << fn << "\" could not be opened.\n";
             return -1;
         }