Start transition from tinyformat to fmt/std::format -- multi-stage (#…

…2076)

For many years we have used Chris Foster's magnificent "tinyformat"
package to do a lot of the heavy lifting for our type-safe string
formatting with printf-like notation, which we use for Strutil::format,
nearly every error reporting facility in the major classes, etc. Thanks,
Chris!

The C++ committee has finally caught up to Chris, advancing drafts of a
"string formatting" proposal that now seems very likely to be part of
C++20. The best implementation at this point is the {fmt} package.

Unfortunately, {fmt} and the C++ draft papers differ from tinyformat in
that rather than traditional C printf notation for formatting, they use
a notation very similar to Python string formatting, e.g.,

    format ("%d %.3f", 1, 23.4)  -->  format ("{} {.3f}", 1, 23.4)

This compatibility break is one reason I've stayed away from {fmt},
despite that (no slight to Chris) it's at least twice twice the speed of
tinyformat.

But knowing that this is almost certainly the shape of the future C++
standard, I am confident that the best long-term path is to migrate to
its conventions. And it will be nice to reuse the same cognitive
machinery whether we're programming in C++ or Python, and get the speed
boost.

But how to do this migration without breaking every string format
operation in OIIO, and additionally in all the packages that call OIIO
and use any of its string formatting or error reporting facilities (and
indeed, we use it extensively in OSL)? This PR is the first step of
implementing the transition, which will occur over a couple major
releases. Here we go:

1. Introduce Strutil::sprintf(), ustring::sprintf(), and various
   errorf(), warningf(), etc., functions as needed that now and forever
   will use the printf notation (and for now are still implemented
   primarily with tinyformat, but that's not a detail you need to worry
   about).

   Neary all internal uses of the format() commands within OIIO internals
   have switched to sprintf().

   Calling apps that make use of OIIO's string formatting can therefore
   also change their calls from format() to sprintf() and everything
   will continue working for the forseeable future.

2. Introduce, in a namespace, Strutil::fmt::format() and certain
   fmterror(), etc., that use the NEW Python-like notation, implemented
   underneath with fmt::format().

   Calling apps may start using these now if they prefer the Python
   notation...

   But BEWARE! I have discovered that the fmt library does not correctly
   implement the promised locale-inedependent formatting for floating
   point values (it does for integers), and so could cause problems for
   those of you who might have apps running on platforms wth certain
   locales that, for example, use ',' as the decimal (sheesh). This is a
   known problem with fmt, will be fixed soon, and as soon as it does,
   I'll update our embedded copy of fmt and give you the all clear to go
   nuts with the new python style formatting directives.

3. For OIIO 2.0, Strutil::format, ustring::format, and the various error()
   functions will behave identically to sprintf (C printf notation), but
   for OIIO 2.1, I am planning on having them all switch to to the new
   python (a.k.a. C++20-ish std::format()) format designations.

So, the upshot is that NOTHING should break for your existing code's
format()/error() calls when switching from OIIO 1.x to 2.0.
(Proof: OSL master compiles against it and passes all tests, without a
single change.)

But after you upgrade to 2.0 and before 2.1 is released (I'm assuming in
the latter half of 2019 at the earliest), you'll want to either switch
all those calls to Strutil::sprintf()/errorf() and stick to the
printf-style formatting *OR* change them to
Strutil::fmt::format()/fmterror() using the new formatting specs.
Pretty much all of the OIIO internals (error calls, etc.) have been
changed to use the explicit printf-like forms. So they seem safe.

After 2.1, then, the old names format()/error() will be the new behavior
and you can switch back to those if you want. And by the time C++20
comes out, we'll all be on the same page, and some day OIIO won't even
need to use the fmt library because it can rely on std::format.

There you go. Here is the PR that implements the first phase.

We have put the relevant parts of fmt library in
include/OpenImageIO/fmt.  We are currently using commit 01640f44, with
one fix by me on top of it that fixes a Windows compile issue. We will
update the version of fmt as fixes and improvements come.

Sorry for such a weird change in the middle of the beta period. But
sometimes a looming deadline of a code freeze and major release (after
which I wouldn't want to introduce this kind of disrupction) really
makes one focus about doing it right now instead of having to wait
potentially an additional year.

LICENSE-THIRD-PARTY.md

@@ -414,5 +414,33 @@ SOFTWARE.
 
 -------------------------------------------------------------------------
 
+{fmt} library - https://github.com/fmtlib/fmt
+
+Copyright (c) 2012 - 2016, Victor Zverovich
+
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions are met:
+
+1. Redistributions of source code must retain the above copyright notice, this
+   list of conditions and the following disclaimer.
+2. Redistributions in binary form must reproduce the above copyright notice,
+   this list of conditions and the following disclaimer in the documentation
+   and/or other materials provided with the distribution.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
+ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
+ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+-------------------------------------------------------------------------
+
 If we have left anything out, it is unintentional. Please let us know.
 

src/cineon.imageio/cineoninput.cpp

@@ -138,38 +138,38 @@ CineonInput::open(const std::string& name, ImageSpec& newspec)
         switch (m_cin.header.ImageDescriptor(i)) {
         case cineon::kGrayscale:
             if (++gscount > 1) {
-                std::string ch = Strutil::format("I%d", gscount);
+                std::string ch = Strutil::sprintf("I%d", gscount);
                 m_spec.channelnames.push_back(ch);
             } else
                 m_spec.channelnames.emplace_back("I");
             break;
         case cineon::kPrintingDensityRed:
         case cineon::kRec709Red:
             if (++gscount > 1) {
-                std::string ch = Strutil::format("R%d", rcount);
+                std::string ch = Strutil::sprintf("R%d", rcount);
                 m_spec.channelnames.push_back(ch);
             } else
                 m_spec.channelnames.emplace_back("R");
             break;
         case cineon::kPrintingDensityGreen:
         case cineon::kRec709Green:
             if (++gcount > 1) {
-                std::string ch = Strutil::format("G%d", gcount);
+                std::string ch = Strutil::sprintf("G%d", gcount);
                 m_spec.channelnames.push_back(ch);
             } else
                 m_spec.channelnames.emplace_back("G");
             break;
         case cineon::kPrintingDensityBlue:
         case cineon::kRec709Blue:
             if (++bcount > 1) {
-                std::string ch = Strutil::format("B%d", bcount);
+                std::string ch = Strutil::sprintf("B%d", bcount);
                 m_spec.channelnames.push_back(ch);
             } else
                 m_spec.channelnames.emplace_back("B");
             break;
         default:
-            std::string ch = Strutil::format("channel%d",
-                                             m_spec.channelnames.size());
+            std::string ch = Strutil::sprintf("channel%d",
+                                              m_spec.channelnames.size());
             m_spec.channelnames.push_back(ch);
             break;
         }
@@ -209,7 +209,7 @@ CineonInput::open(const std::string& name, ImageSpec& newspec)
         if (!isinf(m_cin.header.Gamma()) && m_cin.header.Gamma() != 0.0f)
             // actual gamma value is read later on
             m_spec.attribute("oiio:ColorSpace",
-                             Strutil::format("GammaCorrected%.2g", g));
+                             Strutil::sprintf("GammaCorrected%.2g", g));
         break;
     }
 
@@ -226,8 +226,8 @@ CineonInput::open(const std::string& name, ImageSpec& newspec)
         // libcineon's date/time format is pretty close to OIIO's (libcineon
         // uses %Y:%m:%d:%H:%M:%S%Z)
         m_spec.attribute("DateTime",
-                         Strutil::format("%s %s", m_cin.header.creationDate,
-                                         m_cin.header.creationTime));
+                         Strutil::sprintf("%s %s", m_cin.header.creationDate,
+                                          m_cin.header.creationTime));
         // FIXME: do something about the time zone
     }
 
@@ -357,8 +357,8 @@ CineonInput::open(const std::string& name, ImageSpec& newspec)
         // libcineon's date/time format is pretty close to OIIO's (libcineon
         // uses %Y:%m:%d:%H:%M:%S%Z)
         m_spec.attribute("DateTime",
-                         Strutil::format("%s %s", m_cin.header.sourceDate,
-                                         m_cin.header.sourceTime));
+                         Strutil::sprintf("%s %s", m_cin.header.sourceDate,
+                                          m_cin.header.sourceTime));
         // FIXME: do something about the time zone
     }
     m_cin.header.FilmEdgeCode(buf);

src/cmake/compiler.cmake

@@ -24,7 +24,8 @@ option (CLANG_TIDY_FIX "Have clang-tidy fix source" OFF)
 set (CLANG_FORMAT_INCLUDES "src/*.h" "src/*.cpp"
     CACHE STRING "Glob patterns to include for clang-format")
     # Eventually: want this to be: "src/*.h;src/*.cpp"
-set (CLANG_FORMAT_EXCLUDES "*pugixml*" "*SHA1*" "*/farmhash.cpp" "*/tinyformat.h"
+set (CLANG_FORMAT_EXCLUDES "src/include/OpenImageIO/fmt/*.h"
+                           "*pugixml*" "*SHA1*" "*/farmhash.cpp" "*/tinyformat.h"
                            "src/dpx.imageio/libdpx/*"
                            "src/cineon.imageio/libcineon/*"
                            "src/dds.imageio/squish/*"

src/dicom.imageio/dicominput.cpp

@@ -292,7 +292,7 @@ DICOMInput::read_metadata()
             std::string tagname = tag.getTagName();
             if (ignore_tags.find(tagname) != ignore_tags.end())
                 continue;
-            std::string name = Strutil::format("dicom:%s", tag.getTagName());
+            std::string name = Strutil::sprintf("dicom:%s", tag.getTagName());
             DcmEVR evr       = tag.getEVR();
             // VR codes explained:
             // http://dicom.nema.org/Dicom/2013/output/chtml/part05/sect_6.2.html

src/doc/imageioapi.tex

@@ -390,6 +390,12 @@ \section{Efficient unique strings: {\cf ustring}}
 a {\cf string_view}.  All of these are extremely inexpensive.
 \apiend
 
+\apiitem{static ustring ustring::{\ce sprintf} (const char* fmt, ...)}
+Create a formatted \ustring using the C printf formatting notation
+(e.g., \qkw{%d %s}, akin to C {\cf sprintf()}, except that this one
+uses variadic templates and is type-safe).
+\apiend
+
 
 
 \section{Helper: \ROI}

src/doc/oiiointro.tex

@@ -251,6 +251,8 @@ \section{Acknowledgments}
   UIUC license (compatible with BSD) \\ \url{llvm.org}
 \item {\cf FindOpenVDB.cmake} \copyright\ 2015 Blender Foundation, BSD license.
 \item {\cf FindTBB.cmake} \copyright\ 2015 Justus Calvin, MIT license.
+\item {\cf fmt} library \copyright\ Victor Zverovich. BSD 2-clause license. \\
+    \url{https://github.com/fmtlib/fmt}
 \end{itemize}
 
 \noindent \product Has the following build-time dependencies (using

src/doc/openimageio.tex

@@ -96,7 +96,7 @@
  \bigskip \\
 }
 \date{{\large
-Date: 7 Nov 2018
+Date: 23 Nov 2018
 %\\ (with corrections, 19 Mar 2018)
 }}
 

src/dpx.imageio/dpxinput.cpp

@@ -298,7 +298,7 @@ DPXInput::seek_subimage(int subimage, int miplevel)
     default: {
         for (int i = 0; i < m_dpx.header.ImageElementComponentCount(subimage);
              i++) {
-            std::string ch = Strutil::format("channel%d", i);
+            std::string ch = Strutil::sprintf("channel%d", i);
             m_spec.channelnames.push_back(ch);
         }
     }
@@ -331,7 +331,7 @@ DPXInput::seek_subimage(int subimage, int miplevel)
         if (!isnan(m_dpx.header.Gamma()) && m_dpx.header.Gamma() != 0) {
             float g = float(m_dpx.header.Gamma());
             m_spec.attribute("oiio:ColorSpace",
-                             Strutil::format("GammaCorrected%.2g", g));
+                             Strutil::sprintf("GammaCorrected%.2g", g));
             m_spec.attribute("oiio:Gamma", g);
             break;
         }
@@ -543,7 +543,7 @@ DPXInput::seek_subimage(int subimage, int miplevel)
         // don't set the attribute at all
         break;
     default:
-        tmpstr = Strutil::format("Undefined %d", (int)m_dpx.header.Signal());
+        tmpstr = Strutil::sprintf("Undefined %d", (int)m_dpx.header.Signal());
         break;
     }
     if (!tmpstr.empty())

src/field3d.imageio/field3dinput.cpp

@@ -224,9 +224,9 @@ Field3DInput::read_one_layer(FieldRes::Ptr field, layerrecord& lay,
         lay.unique_name = lay.name;
     else
         lay.unique_name
-            = duplicates ? Strutil::format("%s.%u:%s", lay.name, duplicates + 1,
-                                           lay.attribute)
-                         : Strutil::format("%s:%s", lay.name, lay.attribute);
+            = duplicates ? Strutil::sprintf("%s.%u:%s", lay.name,
+                                            duplicates + 1, lay.attribute)
+                         : Strutil::sprintf("%s:%s", lay.name, lay.attribute);
 
     lay.spec        = ImageSpec();  // Clear everything with default constructor
     lay.spec.format = lay.datatype;

src/field3d.imageio/field3doutput.cpp

@@ -115,8 +115,8 @@ OIIO_EXPORT int field3d_imageio_version = OIIO_PLUGIN_VERSION;
 OIIO_EXPORT const char*
 field3d_imageio_library_version()
 {
-    return ustring::format("Field3d %d.%d.%d", FIELD3D_MAJOR_VER,
-                           FIELD3D_MINOR_VER, FIELD3D_MICRO_VER)
+    return ustring::sprintf("Field3d %d.%d.%d", FIELD3D_MAJOR_VER,
+                            FIELD3D_MINOR_VER, FIELD3D_MICRO_VER)
         .c_str();
 }
 

src/fits.imageio/fitsinput.cpp

@@ -404,18 +404,18 @@ FitsInput::convert_date(const std::string& date)
     std::string ndate;
     if (date[4] == '-') {
         // YYYY-MM-DDThh:mm:ss convention is used since 1 January 2000
-        ndate = Strutil::format("%04u:%02u:%02u", atoi(&date[0]),
-                                atoi(&date[5]), atoi(&date[8]));
+        ndate = Strutil::sprintf("%04u:%02u:%02u", atoi(&date[0]),
+                                 atoi(&date[5]), atoi(&date[8]));
         if (date.size() >= 11 && date[10] == 'T')
-            ndate += Strutil::format(" %02u:%02u:%02u", atoi(&date[11]),
-                                     atoi(&date[14]), atoi(&date[17]));
+            ndate += Strutil::sprintf(" %02u:%02u:%02u", atoi(&date[11]),
+                                      atoi(&date[14]), atoi(&date[17]));
         return ndate;
     }
 
     if (date[2] == '/') {
         // DD/MM/YY convention was used before 1 January 2000
-        ndate = Strutil::format("19%02u:%02u:%02u 00:00:00", atoi(&date[6]),
-                                atoi(&date[3]), atoi(&date[0]));
+        ndate = Strutil::sprintf("19%02u:%02u:%02u 00:00:00", atoi(&date[6]),
+                                 atoi(&date[3]), atoi(&date[0]));
         return ndate;
     }
     // unrecognized format

src/fits.imageio/fitsoutput.cpp

@@ -221,10 +221,10 @@ FitsOutput::create_fits_header(void)
         // to Date format before adding it to the FITS file
         if (keyname == "DateTime") {
             keyname = "Date";
-            value   = Strutil::format("%04u-%02u-%02uT%02u:%02u:%02u",
-                                    atoi(&value[0]), atoi(&value[5]),
-                                    atoi(&value[8]), atoi(&value[11]),
-                                    atoi(&value[14]), atoi(&value[17]));
+            value   = Strutil::sprintf("%04u-%02u-%02uT%02u:%02u:%02u",
+                                     atoi(&value[0]), atoi(&value[5]),
+                                     atoi(&value[8]), atoi(&value[11]),
+                                     atoi(&value[14]), atoi(&value[17]));
         }
 
         header += create_card(keyname, value);

src/hdr.imageio/hdrinput.cpp

@@ -164,7 +164,7 @@ HdrInput::seek_subimage(int subimage, int miplevel)
             m_spec.attribute("oiio:ColorSpace", "linear");
         else
             m_spec.attribute("oiio:ColorSpace",
-                             Strutil::format("GammaCorrected%.2g", g));
+                             Strutil::sprintf("GammaCorrected%.2g", g));
     } else {
         // Presume linear color space
         m_spec.attribute("oiio:ColorSpace", "linear");

src/iconvert/iconvert.cpp

@@ -345,8 +345,8 @@ convert_file(const std::string& in_filename, const std::string& out_filename)
         std::string err = geterror();
         std::cerr << "iconvert ERROR: "
                   << (err.length() ? err
-                                   : Strutil::format("Could not open \"%s\"",
-                                                     in_filename))
+                                   : Strutil::sprintf("Could not open \"%s\"",
+                                                      in_filename))
                   << "\n";
         return false;
     }
@@ -440,8 +440,8 @@ convert_file(const std::string& in_filename, const std::string& out_filename)
                 std::cerr << "iconvert ERROR: "
                           << (err.length()
                                   ? err
-                                  : Strutil::format("Could not open \"%s\"",
-                                                    out_filename))
+                                  : Strutil::sprintf("Could not open \"%s\"",
+                                                     out_filename))
                           << "\n";
                 ok = false;
                 break;

src/iinfo/iinfo.cpp

@@ -425,10 +425,10 @@ extended_format_name(TypeDesc type, int bits)
         // file than the data type we are passing.
         if (type == TypeDesc::UINT8 || type == TypeDesc::UINT16
             || type == TypeDesc::UINT32 || type == TypeDesc::UINT64)
-            return ustring::format("uint%d", bits).c_str();
+            return ustring::sprintf("uint%d", bits).c_str();
         if (type == TypeDesc::INT8 || type == TypeDesc::INT16
             || type == TypeDesc::INT32 || type == TypeDesc::INT64)
-            return ustring::format("int%d", bits).c_str();
+            return ustring::sprintf("int%d", bits).c_str();
     }
     return type.c_str();  // use the name implied by type
 }
@@ -445,11 +445,11 @@ brief_format_name(TypeDesc type, int bits = 0)
             return "f";
         if (type.basetype == TypeDesc::HALF)
             return "h";
-        return ustring::format("f%d", bits).c_str();
+        return ustring::sprintf("f%d", bits).c_str();
     } else if (type.is_signed()) {
-        return ustring::format("i%d", bits).c_str();
+        return ustring::sprintf("i%d", bits).c_str();
     } else {
-        return ustring::format("u%d", bits).c_str();
+        return ustring::sprintf("u%d", bits).c_str();
     }
     return type.c_str();  // use the name implied by type
 }
@@ -701,7 +701,7 @@ main(int argc, const char* argv[])
         if (!in) {
             std::string err = geterror();
             if (err.empty())
-                err = Strutil::format("Could not open \"%s\"", s);
+                err = Strutil::sprintf("Could not open \"%s\"", s);
             std::cerr << "iinfo ERROR: " << err << "\n";
             continue;
         }

src/include/CMakeLists.txt

@@ -7,6 +7,7 @@ if (NOT USE_EXTERNAL_PUGIXML)
           OpenImageIO/pugixml.cpp)
 endif ()
 
+
 if (VERBOSE)
     message(STATUS "Create oiioversion.h from oiioversion.h.in")
 endif ()
@@ -33,3 +34,8 @@ list(APPEND public_headers "${CMAKE_BINARY_DIR}/include/OpenImageIO/oiioversion.
 install (FILES ${public_headers}
          DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/OpenImageIO
          COMPONENT developer)
+
+file (GLOB fmt_headers OpenImageIO/fmt/*.h)
+install (FILES ${fmt_headers}
+         DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/OpenImageIO/fmt
+         COMPONENT developer)