api: imagebuf.h add deprecation warnings to deprecated things (AcademySoftwareFoundation#4341)

* Add deprecation warnings to the varieties of ImageBuf constructor and
`reset()` that don't take subimage and miplevel parameters. The
equivalent is to just pass `0` for both of those parameters. The warned
constructors have been marked as deprecated since OIIO 2.2.
* The misspelled `ImageBuf::make_writeable()` has been given deprecation
warnings. Since OIIO 2.2, the correct spelling is `make_writable`.
* The `ImageBuf::error()` method that uses printf-style formatting now
has deprecation warnings. Use `ImageBuf::errorfmt()` instead.
* The `ImageBuf::interppixel_NDC_full()` method, which has been marked
as deprecated since OIIO 1.5, now has deprecation warnings. Use
`interppixel_NDC()` instead.

Undeprecate the name+spec varieties of constructors. They were
documented as deprecated but warnings were never added. In doing this
set of changes, I realized that they were used for something useful
after all -- it makes it easy to remember the name of a new buffer
you're making not from a file, so you can later `write()` it without
needing to know the filename at that time. I also added a new
`set_name()` method to do it directly for times when you don't want to
rely on the constructor or want to change the name.

By the way, I'm being careful also to convert all the warned or marked
deprecated functions to inline, if at all possible, so that in the
future removing them will not be an ABI break.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Scott Wilson <scott@propersquid.com>

Author: lgritz

docs/Deprecations-3.0.md

@@ -67,19 +67,27 @@ long-deprecated API facets. This document lists the deprecations and removals.
   severefmt, messagefmt, debugfmt, respectively, which all use the std::format
   notation.
 
+## imagebuf.h
+
+* Add deprecation warnings to the varieties of ImageBuf constructor and
+  `reset()` that don't take subimage and miplevel parameters, which have been
+  marked as deprecated since OIIO 2.2. The equivalent is to just pass `0` for
+  both of those parameters.
+* The misspelled `ImageBuf::make_writeable()` has been given deprecation
+  warnings. Since OIIO 2.2, we have used the correct spelling,
+  `make_writable`.
+* The `ImageBuf::error()` method that uses printf-style formatting conventions
+  now has deprecation warnings. Use `ImageBuf::errorfmt()` instead.
+* The `ImageBuf::interppixel_NDC_full()` method, which has been marked as
+  deprecated since OIIO 1.5, now has deprecation warnings. Use
+  `interppixel_NDC()` instead.
+
 ## missingmath.h
 
 * This header has been removed entirely. It has was originally needed for
   pre-C++11 MSVS, but has been unused since OIIO 2.0 (other than transitively
   including fmath.h).
 
-## parallel.h
-
-* Removed several varieties of `parallel_for` functions where the task
-  functions took a thread ID argument in addition to the range, which have
-  been considered deprecated since OIIO 2.3. Please use task functions that do
-  not take a thread ID parameter.
-
 ## paramlist.h
 
 * Removed some ParamValue constructor variants that have been deprecated since
@@ -88,6 +96,13 @@ long-deprecated API facets. This document lists the deprecations and removals.
   behavior, please use the newer variety of constructors that instead use a
   "strong" type where you pass `ParamValue:Copy(bool)`.
 
+## parallel.h
+
+* Removed several varieties of `parallel_for` functions where the task
+  functions took a thread ID argument in addition to the range, which have
+  been considered deprecated since OIIO 2.3. Please use task functions that do
+  not take a thread ID parameter.
+
 ## strutil.h
 
 * Added deprecation warnings to all the old (printf-convention) string

src/idiff/idiff.cpp

@@ -126,7 +126,7 @@ read_input(const std::string& filename, ImageBuf& img, ImageCache* cache,
         && img.miplevel() == miplevel)
         return true;
 
-    img.reset(filename, cache);
+    img.reset(filename, 0, 0, cache);
     if (img.read(subimage, miplevel, false, TypeFloat))
         return true;
 

src/include/OpenImageIO/imagebuf.h

@@ -190,9 +190,6 @@ class OIIO_API ImageBuf {
                       const ImageSpec* config      = nullptr,
                       Filesystem::IOProxy* ioproxy = nullptr);
 
-    // Deprecated synonym for `ImageBuf(name, 0, 0, imagecache, nullptr)`.
-    ImageBuf(string_view name, ImageCache* imagecache);
-
     /// Construct a writable ImageBuf with the given specification
     /// (including resolution, data type, metadata, etc.). The ImageBuf will
     /// allocate and own its own pixel memory and will free that memory
@@ -219,10 +216,14 @@ class OIIO_API ImageBuf {
     explicit ImageBuf(const ImageSpec& spec,
                       InitializePixels zero = InitializePixels::Yes);
 
-    // Deprecated/useless synonym for `ImageBuf(spec,zero)` but also gives
-    // it an internal name.
+    // Synonym for `ImageBuf(spec,zero)` but also gives it an internal name
+    // that will be used if write() is called with an empty filename.
     ImageBuf(string_view name, const ImageSpec& spec,
-             InitializePixels zero = InitializePixels::Yes);
+             InitializePixels zero = InitializePixels::Yes)
+        : ImageBuf(spec, zero)
+    {
+        set_name(name);
+    }
 
     /// Construct a writable ImageBuf that "wraps" existing pixel memory
     /// owned by the calling application. The ImageBuf does not own the
@@ -250,10 +251,6 @@ class OIIO_API ImageBuf {
     ImageBuf(const ImageSpec& spec, void* buffer, stride_t xstride = AutoStride,
              stride_t ystride = AutoStride, stride_t zstride = AutoStride);
 
-    // Deprecated/useless synonym for `ImageBuf(spec,buffer)` but also gives
-    // it an internal name.
-    ImageBuf(string_view name, const ImageSpec& spec, void* buffer);
-
     /// Construct a copy of an ImageBuf.
     ImageBuf(const ImageBuf& src);
 
@@ -269,9 +266,6 @@ class OIIO_API ImageBuf {
     /// `IBStorage::UNINITIALIZED`).
     void reset() { clear(); }
 
-    // Deprecated/useless synonym for `reset(name, 0, 0, imagecache, nullptr)`
-    void reset(string_view name, ImageCache* imagecache);
-
     /// Destroy any previous contents of the ImageBuf and re-initialize it
     /// as if newly constructed with the same arguments, as a read-only
     /// representation of an existing image file.
@@ -294,10 +288,13 @@ class OIIO_API ImageBuf {
     void reset(const ImageSpec& spec,
                InitializePixels zero = InitializePixels::Yes);
 
-    // Deprecated/useless synonym for `reset(spec, zero)` and also give it an
-    // internal name.
+    /// Synonym for `reset(spec, zero)` and also give it an internal name.
     void reset(string_view name, const ImageSpec& spec,
-               InitializePixels zero = InitializePixels::Yes);
+               InitializePixels zero = InitializePixels::Yes)
+    {
+        reset(spec, zero);
+        set_name(name);
+    }
 
     /// Destroy any previous contents of the ImageBuf and re-initialize it
     /// as if newly constructed with the same arguments, to "wrap" existing
@@ -323,10 +320,6 @@ class OIIO_API ImageBuf {
     ///             necessary), `false` if something went horribly wrong.
     bool make_writable(bool keep_cache_type = false);
 
-    // DEPRECATED(2.2): This is an alternate, and less common, spelling.
-    // Let's standardize on "writable". We will eventually remove this.
-    bool make_writeable(bool keep_cache_type = false);
-
     /// @}
 
 
@@ -712,13 +705,6 @@ class OIIO_API ImageBuf {
     void interppixel_NDC(float s, float t, float* pixel,
                          WrapMode wrap = WrapBlack) const;
 
-#ifndef DOXYGEN_SHOULD_SKIP_THIS
-    // DEPRECATED (1.5) synonym for interppixel_NDC.
-    OIIO_DEPRECATED("use interppixel_NDC (1.5)")
-    void interppixel_NDC_full(float s, float t, float* pixel,
-                              WrapMode wrap = WrapBlack) const;
-#endif
-
     /// Bicubic interpolation at pixel coordinates (x,y).
     void interppixel_bicubic(float x, float y, float* pixel,
                              WrapMode wrap = WrapBlack) const;
@@ -867,6 +853,10 @@ class OIIO_API ImageBuf {
     /// Return the name of the buffer as a ustring.
     ustring uname(void) const;
 
+    /// Set the name of the ImageBuf, will be used later as default
+    /// filename if write() is called with an empty filename.
+    void set_name(string_view name);
+
     /// Return the name of the image file format of the file this ImageBuf
     /// refers to (for example `"openexr"`).  Returns an empty string for an
     /// ImageBuf that was not constructed as a direct reference to a file.
@@ -1063,17 +1053,6 @@ class OIIO_API ImageBuf {
         error(Strutil::fmt::format(fmt, args...));
     }
 
-    /// Error reporting for ImageBuf: call this with Strutil::format
-    /// formatting conventions.  It is not necessary to have the error
-    /// message contain a trailing newline. Beware, this is in transition,
-    /// is currently printf-like but will someday change to python-like!
-    template<typename... Args>
-    OIIO_FORMAT_DEPRECATED void error(const char* fmt,
-                                      const Args&... args) const
-    {
-        error(Strutil::format(fmt, args...));
-    }
-
     /// Returns `true` if the ImageBuf has had an error and has an error
     /// message ready to retrieve via `geterror()`.
     bool has_error(void) const;
@@ -1168,6 +1147,54 @@ class OIIO_API ImageBuf {
     /// Return the name corresponding to the wrap mode.
     static ustring wrapmode_name(WrapMode wrap);
 
+#if !defined(OIIO_DOXYGEN) && !defined(OIIO_INTERNAL)
+    // Deprecated things -- might be removed at any time
+
+    OIIO_DEPRECATED("Use `ImageBuf(name, 0, 0, imagecache, nullptr)` (2.2)")
+    ImageBuf(string_view name, ImageCache* imagecache)
+        : ImageBuf(name, 0, 0, imagecache)
+    {
+    }
+
+    OIIO_DEPRECATED(
+        "The name parameter is not used, use `ImageBuf(spec,buffer)` (2.2)")
+    ImageBuf(string_view name, const ImageSpec& spec, void* buffer)
+        : ImageBuf(spec, buffer)
+    {
+    }
+
+    OIIO_DEPRECATED("Use `reset(name, 0, 0, imagecache)` (2.2)")
+    void reset(string_view name, ImageCache* imagecache)
+    {
+        reset(name, 0, 0, imagecache);
+    }
+
+    OIIO_DEPRECATED("Use make_writable (2.2)")
+    bool make_writeable(bool keep_cache_type = false)
+    {
+        return make_writable(keep_cache_type);
+    }
+
+    OIIO_DEPRECATED("use interppixel_NDC (1.5)")
+    void interppixel_NDC_full(float s, float t, float* pixel,
+                              WrapMode wrap = WrapBlack) const
+    {
+        const ImageSpec& spec(this->spec());
+        interppixel(static_cast<float>(spec.full_x)
+                        + s * static_cast<float>(spec.full_width),
+                    static_cast<float>(spec.full_y)
+                        + t * static_cast<float>(spec.full_height),
+                    pixel, wrap);
+    }
+
+    template<typename... Args>
+    OIIO_DEPRECATED("Use errorfmt")
+    void error(const char* fmt, const Args&... args) const
+    {
+        error(Strutil::old::format(fmt, args...));
+    }
+#endif
+
     friend class IteratorBase;
 
     /// Base class for Iterator and ConstIterator -- this contains all the
@@ -1616,11 +1643,6 @@ class OIIO_API ImageBuf {
                        int& tilexend, bool& haderr, bool exists,
                        WrapMode wrap) const;
 
-    // DEPRECATED(2.4)
-    const void* retile(int x, int y, int z, pvt::ImageCacheTile*& tile,
-                       int& tilexbegin, int& tileybegin, int& tilezbegin,
-                       int& tilexend, bool exists, WrapMode wrap) const;
-
     const void* blackpixel() const;
 
     // Given x,y,z known to be outside the pixel data range, and a wrap

src/iv/ivimage.cpp

@@ -75,9 +75,9 @@ IvImage::read_iv(int subimage, int miplevel, bool force, TypeDesc format,
                                        progress_callback_data);
 
     if (m_image_valid && secondary_data && spec().format == TypeDesc::UINT8) {
-        m_corrected_image.reset("", ImageSpec(spec().width, spec().height,
-                                              std::min(spec().nchannels, 4),
-                                              spec().format));
+        m_corrected_image.reset(ImageSpec(spec().width, spec().height,
+                                          std::min(spec().nchannels, 4),
+                                          spec().format));
     } else {
         m_corrected_image.clear();
     }