From 4439241434e945127876f107823bf242fd33469f Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Mon, 27 Jul 2020 21:49:47 -0700 Subject: [PATCH 1/2] More gradual fmt conversion Another step toward switching to std::format notation. The basic strategy is to make sure that all the classes that have errorf (etc) methods using printf notation will also have errorfmt (etc) methods using std::format notation. These will coexist for a while to let people switch, before the printf ones will be marked deprecated and then eventually removed. In some cases where the methods aren't really part of the public API, but only are used internally, we are already skipping to the deprecation marking. Signed-off-by: Larry Gritz --- src/include/OpenImageIO/errorhandler.h | 48 +++++++- src/include/OpenImageIO/imagebuf.h | 11 +- src/include/OpenImageIO/imageio.h | 26 ++++- src/include/OpenImageIO/strutil.h | 1 + src/include/OpenImageIO/typedesc.h | 17 ++- src/libOpenImageIO/imagebuf.cpp | 18 +-- src/libtexture/environment.cpp | 8 +- src/libtexture/imagecache.cpp | 54 ++++----- src/libtexture/imagecache_pvt.h | 6 + src/libtexture/texture3d.cpp | 14 +-- src/libtexture/texture_pvt.h | 6 + src/libtexture/texturesys.cpp | 44 ++++---- src/libutil/typedesc.cpp | 147 +++++++++++++++++++++---- src/tiff.imageio/tiffinput.cpp | 2 +- 14 files changed, 309 insertions(+), 93 deletions(-) diff --git a/src/include/OpenImageIO/errorhandler.h b/src/include/OpenImageIO/errorhandler.h index 80c4e55704..6c60b485fa 100644 --- a/src/include/OpenImageIO/errorhandler.h +++ b/src/include/OpenImageIO/errorhandler.h @@ -144,7 +144,7 @@ class OIIO_API ErrorHandler { // // Formatted output with printf notation. Use these if you specifically // want printf-notation, even after format() changes to python notation - // for OIIO 2.1. + // in some future OIIO release. // template void infof(const char* format, const Args&... args) @@ -188,6 +188,52 @@ class OIIO_API ErrorHandler { #endif } + // + // Formatted output with std::format notation. Use these if you + // specifically want std::format-notation, even before format() changes + // to the new notation in some future OIIO release. + // + template + void infofmt(const char* format, const Args&... args) + { + if (verbosity() >= VERBOSE) + info(Strutil::fmt::format(format, args...)); + } + + template + void warningfmt(const char* format, const Args&... args) + { + if (verbosity() >= NORMAL) + warning(Strutil::fmt::format(format, args...)); + } + + template + void errorfmt(const char* format, const Args&... args) + { + error(Strutil::fmt::format(format, args...)); + } + + template + void severefmt(const char* format, const Args&... args) + { + severe(Strutil::fmt::format(format, args...)); + } + + template + void messagefmt(const char* format, const Args&... args) + { + if (verbosity() > QUIET) + message(Strutil::fmt::format(format, args...)); + } + + template + void debugfmt(const char* format, const Args&... args) + { +#ifndef NDEBUG + debug(Strutil::fmt::format(format, args...)); +#endif + } + /// One built-in handler that can always be counted on to be present /// and just echoes the error messages to the console (stdout or /// stderr, depending on the error category). diff --git a/src/include/OpenImageIO/imagebuf.h b/src/include/OpenImageIO/imagebuf.h index 3427b0a88d..1d5defe2aa 100644 --- a/src/include/OpenImageIO/imagebuf.h +++ b/src/include/OpenImageIO/imagebuf.h @@ -940,7 +940,7 @@ class OIIO_API ImageBuf { /// Error reporting for ImageBuf: call this with Python / {fmt} / /// std::format style formatting specification. template - void fmterror(const char* fmt, const Args&... args) const + void errorfmt(const char* fmt, const Args&... args) const { error(Strutil::fmt::format(fmt, args...)); } @@ -962,6 +962,15 @@ class OIIO_API ImageBuf { error(Strutil::format(fmt, args...)); } + // Error reporting for ImageBuf: call this with Python / {fmt} / + // std::format style formatting specification. + template + OIIO_DEPRECATED("use `errorfmt` instead") + void fmterror(const char* fmt, const Args&... args) const + { + error(Strutil::fmt::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; diff --git a/src/include/OpenImageIO/imageio.h b/src/include/OpenImageIO/imageio.h index ad3408eb78..673ea216b2 100644 --- a/src/include/OpenImageIO/imageio.h +++ b/src/include/OpenImageIO/imageio.h @@ -1595,6 +1595,14 @@ class OIIO_API ImageInput { /// Error reporting for the plugin implementation: call this with /// fmt::format-like arguments. template + void errorfmt(const char* fmt, const Args&... args) const { + append_error(Strutil::fmt::format (fmt, args...)); + } + + // Error reporting for the plugin implementation: call this with + // fmt::format-like arguments. + template + OIIO_DEPRECATED("use `errorfmt` instead") void fmterror(const char* fmt, const Args&... args) const { append_error(Strutil::fmt::format (fmt, args...)); } @@ -2208,6 +2216,14 @@ class OIIO_API ImageOutput { /// Error reporting for the plugin implementation: call this with /// fmt::format-like arguments. template + void errorfmt(const char* fmt, const Args&... args) const { + append_error(Strutil::fmt::format (fmt, args...)); + } + + // Error reporting for the plugin implementation: call this with + // fmt::format-like arguments. + template + OIIO_DEPRECATED("use `errorfmt` instead") void fmterror(const char* fmt, const Args&... args) const { append_error(Strutil::fmt::format (fmt, args...)); } @@ -2722,8 +2738,16 @@ typedef bool (*wrap_impl) (int &coord, int origin, int width); /// output to stderr for debugging statements. OIIO_API void debug (string_view str); -/// debug output with `fmt`/`std::format` conventions. +/// debug output with `std::format` conventions. +template +void debugfmt (const char* fmt, const T1& v1, const Args&... args) +{ + debug (Strutil::fmt::format(fmt, v1, args...)); +} + +// (Unfortunate old synonym) template +OIIO_DEPRECATED("use `debugfmt` instead") void fmtdebug (const char* fmt, const T1& v1, const Args&... args) { debug (Strutil::fmt::format(fmt, v1, args...)); diff --git a/src/include/OpenImageIO/strutil.h b/src/include/OpenImageIO/strutil.h index 339f077d27..e4d5bfb00d 100644 --- a/src/include/OpenImageIO/strutil.h +++ b/src/include/OpenImageIO/strutil.h @@ -236,6 +236,7 @@ std::string OIIO_API vsprintf (const char *fmt, va_list ap) /// Return a std::string formatted like Strutil::format, but passed /// already as a va_list. This is not guaranteed type-safe and is not /// extensible like format(). Use with caution! +OIIO_DEPRECATED("use `vsprintf` instead") std::string OIIO_API vformat (const char *fmt, va_list ap) OPENIMAGEIO_PRINTF_ARGS(1,0); diff --git a/src/include/OpenImageIO/typedesc.h b/src/include/OpenImageIO/typedesc.h index ce29386584..8a2db249a1 100644 --- a/src/include/OpenImageIO/typedesc.h +++ b/src/include/OpenImageIO/typedesc.h @@ -489,6 +489,9 @@ struct tostring_formatting { const char *uint_fmt = "%u"; const char *reserved2 = ""; const char *reserved3 = ""; + bool use_sprintf = true; + + enum Notation { STDFORMAT }; tostring_formatting() = default; tostring_formatting(const char *int_fmt, const char *float_fmt = "%g", @@ -498,12 +501,24 @@ struct tostring_formatting { const char *array_end = "}", const char *array_sep = ",", int flags = escape_strings, const char *uint_fmt = "%u"); + + // Alternative ctr for std::format notation. You must pass STDFORMAT + // as the first argument. + tostring_formatting(Notation notation, + const char *int_fmt = "{}", const char *uint_fmt = "{}", + const char *float_fmt = "{}", + const char *string_fmt = "\"{}\"", const char *ptr_fmt = "{}", + const char *aggregate_begin = "(", const char *aggregate_end = ")", + const char *aggregate_sep = ",", const char *array_begin = "{", + const char *array_end = "}", const char *array_sep = ",", + int flags = escape_strings); }; /// Return a string containing the data values formatted according -/// to the type and the optional formatting control arguments. +/// to the type and the optional formatting control arguments. Will be +/// deprecated someday as printf formatting falls out of favor. OIIO_API std::string tostring(TypeDesc type, const void* data, const tostring_formatting& fmt = {}); diff --git a/src/libOpenImageIO/imagebuf.cpp b/src/libOpenImageIO/imagebuf.cpp index 5a5636548f..c4e6787f78 100644 --- a/src/libOpenImageIO/imagebuf.cpp +++ b/src/libOpenImageIO/imagebuf.cpp @@ -108,7 +108,7 @@ class ImageBufImpl { template void error(const char* fmt, const Args&... args) const { - error(Strutil::sprintf(fmt, args...)); + error(Strutil::fmt::format(fmt, args...)); } template @@ -932,12 +932,12 @@ ImageBufImpl::read(int subimage, int miplevel, int chbegin, int chend, auto input = ImageInput::open(m_name.string(), m_configspec.get(), m_rioproxy); if (!input) { - errorf("%s", OIIO::geterror()); + error("{}", OIIO::geterror()); return false; } input->threads(threads()); // Pass on our thread policy if (!input->read_native_deep_image(subimage, miplevel, m_deepdata)) { - errorf("%s", input->geterror()); + error("{}", input->geterror()); return false; } m_spec = m_nativespec; // Deep images always use native data @@ -1044,11 +1044,11 @@ ImageBufImpl::read(int subimage, int miplevel, int chbegin, int chend, m_pixels_valid = true; } else { m_pixels_valid = false; - errorf("%s", in->geterror()); + error("{}", in->geterror()); } } else { m_pixels_valid = false; - errorf("%s", OIIO::geterror()); + error("{}", OIIO::geterror()); } return m_pixels_valid; } @@ -1063,7 +1063,7 @@ ImageBufImpl::read(int subimage, int miplevel, int chbegin, int chend, m_pixels_valid = true; } else { m_pixels_valid = false; - errorf("%s", m_imagecache->geterror()); + error("{}", m_imagecache->geterror()); } return m_pixels_valid; @@ -1218,7 +1218,7 @@ ImageBuf::write(ImageOutput* out, ProgressCallback progress_callback, } } if (!ok) - errorf("%s", out->geterror()); + error("{}", out->geterror()); return ok; } @@ -1263,7 +1263,7 @@ ImageBuf::write(string_view _filename, TypeDesc dtype, string_view _fileformat, auto out = ImageOutput::create(fileformat); if (!out) { - errorf("%s", geterror()); + error("{}", geterror()); return false; } out->threads(threads()); // Pass on our thread policy @@ -1315,7 +1315,7 @@ ImageBuf::write(string_view _filename, TypeDesc dtype, string_view _fileformat, } if (!out->open(filename.c_str(), newspec)) { - errorf("%s", out->geterror()); + error("{}", out->geterror()); return false; } if (!write(out.get(), progress_callback, progress_callback_data)) diff --git a/src/libtexture/environment.cpp b/src/libtexture/environment.cpp index a11b0cf45e..40583015d4 100644 --- a/src/libtexture/environment.cpp +++ b/src/libtexture/environment.cpp @@ -351,8 +351,8 @@ TextureSystemImpl::environment(TextureHandle* texture_handle_, int s = m_imagecache->subimage_from_name(texturefile, options.subimagename); if (s < 0) { - errorf("Unknown subimage \"%s\" in texture \"%s\"", - options.subimagename, texturefile->filename()); + error("Unknown subimage \"{}\" in texture \"{}\"", + options.subimagename, texturefile->filename()); return missing_texture(options, nchannels, result, dresultds, dresultdt); } @@ -360,8 +360,8 @@ TextureSystemImpl::environment(TextureHandle* texture_handle_, options.subimagename.clear(); } if (options.subimage < 0 || options.subimage >= texturefile->subimages()) { - errorf("Unknown subimage \"%s\" in texture \"%s\"", - options.subimagename, texturefile->filename()); + error("Unknown subimage \"{}\" in texture \"{}\"", options.subimagename, + texturefile->filename()); return missing_texture(options, nchannels, result, dresultds, dresultdt); } diff --git a/src/libtexture/imagecache.cpp b/src/libtexture/imagecache.cpp index 4990983381..838c0de219 100644 --- a/src/libtexture/imagecache.cpp +++ b/src/libtexture/imagecache.cpp @@ -828,7 +828,7 @@ ImageCacheFile::read_tile(ImageCachePerThreadInfo* thread_info, int subimage, if (!ok) { std::string err = inp->geterror(); if (!err.empty() && errors_should_issue()) - imagecache().errorf("%s", err); + imagecache().error("{}", err); } if (ok) { @@ -982,7 +982,7 @@ ImageCacheFile::read_untiled(ImageCachePerThreadInfo* thread_info, if (!ok) { std::string err = inp->geterror(); if (!err.empty() && errors_should_issue()) - imagecache().errorf("%s", err); + imagecache().error("{}", err); } size_t b = (y1 - y0 + 1) * spec.scanline_bytes(); thread_info->m_stats.bytes_read += b; @@ -1023,7 +1023,7 @@ ImageCacheFile::read_untiled(ImageCachePerThreadInfo* thread_info, if (!ok) { std::string err = inp->geterror(); if (!err.empty() && errors_should_issue()) - imagecache().errorf("%s", err); + imagecache().error("{}", err); } size_t b = spec.image_bytes(); thread_info->m_stats.bytes_read += b; @@ -1151,7 +1151,7 @@ ImageCacheFile::mark_broken(string_view error) if (!error.size()) error = string_view("unknown error"); m_broken_message = error; - imagecache().errorf("%s", error); + imagecache().error("{}", error); invalidate_spec(); } @@ -1515,8 +1515,8 @@ ImageCacheTile::read(ImageCachePerThreadInfo* thread_info) // (! m_valid) m_used = false; // Don't let it hold mem if invalid if (file.mod_time() != Filesystem::last_write_time(file.filename())) - file.imagecache().errorf( - "File \"%s\" was modified after being opened by OIIO", + file.imagecache().error( + "File \"{}\" was modified after being opened by OIIO", file.filename()); #if 0 std::cerr << "(1) error reading tile " << m_id.x() << ' ' << m_id.y() @@ -2502,7 +2502,7 @@ ImageCacheImpl::get_image_info(ustring filename, int subimage, int miplevel, ImageCachePerThreadInfo* thread_info = get_perthread_info(); ImageCacheFile* file = find_file(filename, thread_info, nullptr); if (!file && dataname != s_exists) { - errorf("Invalid image file \"%s\"", filename); + error("Invalid image file \"{}\"", filename); return false; } return get_image_info(file, thread_info, subimage, miplevel, dataname, @@ -2566,8 +2566,8 @@ ImageCacheImpl::get_image_info(ImageCacheFile* file, std::string* errptr = m_errormessage.get(); if (errptr) errptr->clear(); - errorf("Invalid image file \"%s\": %s", file->filename(), - file->broken_error_message()); + error("Invalid image file \"{}\": {}", file->filename(), + file->broken_error_message()); } return false; } @@ -2617,14 +2617,14 @@ ImageCacheImpl::get_image_info(ImageCacheFile* file, // reference to the spec. if (subimage < 0 || subimage >= file->subimages()) { if (file->errors_should_issue()) - errorf("Unknown subimage %d (out of %d)", subimage, - file->subimages()); + error("Unknown subimage {} (out of {})", subimage, + file->subimages()); return false; } if (miplevel < 0 || miplevel >= file->miplevels(subimage)) { if (file->errors_should_issue()) - errorf("Unknown mip level %d (out of %d)", miplevel, - file->miplevels(subimage)); + error("Unknown mip level {} (out of {})", miplevel, + file->miplevels(subimage)); return false; } @@ -2808,7 +2808,7 @@ ImageCacheImpl::imagespec(ustring filename, int subimage, int miplevel, ImageCachePerThreadInfo* thread_info = get_perthread_info(); ImageCacheFile* file = find_file(filename, thread_info, nullptr); if (!file) { - errorf("Image file \"%s\" not found", filename); + error("Image file \"{}\" not found", filename); return NULL; } return imagespec(file, thread_info, subimage, miplevel, native); @@ -2830,8 +2830,8 @@ ImageCacheImpl::imagespec(ImageCacheFile* file, file = verify_file(file, thread_info, true); if (file->broken()) { if (file->errors_should_issue()) - errorf("Invalid image file \"%s\": %s", file->filename(), - file->broken_error_message()); + error("Invalid image file \"{}\": {}", file->filename(), + file->broken_error_message()); return NULL; } if (file->is_udim()) { @@ -2840,14 +2840,14 @@ ImageCacheImpl::imagespec(ImageCacheFile* file, } if (subimage < 0 || subimage >= file->subimages()) { if (file->errors_should_issue()) - errorf("Unknown subimage %d (out of %d)", subimage, - file->subimages()); + error("Unknown subimage {} (out of {})", subimage, + file->subimages()); return NULL; } if (miplevel < 0 || miplevel >= file->miplevels(subimage)) { if (file->errors_should_issue()) - errorf("Unknown mip level %d (out of %d)", miplevel, - file->miplevels(subimage)); + error("Unknown mip level {} (out of {})", miplevel, + file->miplevels(subimage)); return NULL; } const ImageSpec* spec = native ? &file->nativespec(subimage, miplevel) @@ -2904,7 +2904,7 @@ ImageCacheImpl::get_pixels(ustring filename, int subimage, int miplevel, ImageCachePerThreadInfo* thread_info = get_perthread_info(); ImageCacheFile* file = find_file(filename, thread_info); if (!file) { - errorf("Image file \"%s\" not found", filename); + error("Image file \"{}\" not found", filename); return false; } return get_pixels(file, thread_info, subimage, miplevel, xbegin, xend, @@ -2929,8 +2929,8 @@ ImageCacheImpl::get_pixels(ImageCacheFile* file, file = verify_file(file, thread_info); if (file->broken()) { if (file->errors_should_issue()) - errorf("Invalid image file \"%s\": %s", file->filename(), - file->broken_error_message()); + error("Invalid image file \"{}\": {}", file->filename(), + file->broken_error_message()); return false; } if (file->is_udim()) { @@ -2939,14 +2939,14 @@ ImageCacheImpl::get_pixels(ImageCacheFile* file, } if (subimage < 0 || subimage >= file->subimages()) { if (file->errors_should_issue()) - errorf("get_pixels asked for nonexistent subimage %d of \"%s\"", - subimage, file->filename()); + error("get_pixels asked for nonexistent subimage {} of \"{}\"", + subimage, file->filename()); return false; } if (miplevel < 0 || miplevel >= file->miplevels(subimage)) { if (file->errors_should_issue()) - errorf("get_pixels asked for nonexistent MIP level %d of \"%s\"", - miplevel, file->filename()); + error("get_pixels asked for nonexistent MIP level {} of \"{}\"", + miplevel, file->filename()); return false; } diff --git a/src/libtexture/imagecache_pvt.h b/src/libtexture/imagecache_pvt.h index 0e7fd2ea9f..5dd97d7bf6 100644 --- a/src/libtexture/imagecache_pvt.h +++ b/src/libtexture/imagecache_pvt.h @@ -1021,6 +1021,12 @@ class ImageCacheImpl : public ImageCache { { append_error(Strutil::sprintf(fmt, args...)); } + /// Internal error reporting routine, with std::format-like arguments. + template + void error(const char* fmt, const Args&... args) const + { + append_error(Strutil::fmt::format(fmt, args...)); + } void error(const char* msg) const { append_error(msg); } /// Append a string to the current error message diff --git a/src/libtexture/texture3d.cpp b/src/libtexture/texture3d.cpp index 54a8f54379..5011823b68 100644 --- a/src/libtexture/texture3d.cpp +++ b/src/libtexture/texture3d.cpp @@ -139,8 +139,8 @@ TextureSystemImpl::texture3d(TextureHandle* texture_handle_, int s = m_imagecache->subimage_from_name(texturefile, options.subimagename); if (s < 0) { - errorf("Unknown subimage \"%s\" in texture \"%s\"", - options.subimagename, texturefile->filename()); + error("Unknown subimage \"{}\" in texture \"{}\"", + options.subimagename, texturefile->filename()); return missing_texture(options, nchannels, result, dresultds, dresultdt, dresultdr); } @@ -148,8 +148,8 @@ TextureSystemImpl::texture3d(TextureHandle* texture_handle_, options.subimagename.clear(); } if (options.subimage < 0 || options.subimage >= texturefile->subimages()) { - errorf("Unknown subimage \"%s\" in texture \"%s\"", - options.subimagename, texturefile->filename()); + error("Unknown subimage \"{}\" in texture \"{}\"", options.subimagename, + texturefile->filename()); return missing_texture(options, nchannels, result, dresultds, dresultdt, dresultdr); } @@ -370,7 +370,7 @@ TextureSystemImpl::accum3d_sample_closest( ttex - tile_t, rtex - tile_r, tile_chbegin, tile_chend); bool ok = find_tile(id, thread_info, true); if (!ok) - errorf("%s", m_imagecache->geterror()); + error("{}", m_imagecache->geterror()); TileRef& tile(thread_info->tile); if (!tile || !ok) return false; @@ -518,7 +518,7 @@ TextureSystemImpl::accum3d_sample_bilinear( id.xyz(stex[0] - tile_s, ttex[0] - tile_t, rtex[0] - tile_r); bool ok = find_tile(id, thread_info, true); if (!ok) - errorf("%s", m_imagecache->geterror()); + error("{}", m_imagecache->geterror()); TileRef& tile(thread_info->tile); if (!tile->valid()) return false; @@ -555,7 +555,7 @@ TextureSystemImpl::accum3d_sample_bilinear( rtex[k] - tile_r); bool ok = find_tile(id, thread_info, firstsample); if (!ok) - errorf("%s", m_imagecache->geterror()); + error("{}", m_imagecache->geterror()); firstsample = false; TileRef& tile(thread_info->tile); if (!tile->valid()) diff --git a/src/libtexture/texture_pvt.h b/src/libtexture/texture_pvt.h index a0a5cf0cbe..8d15851751 100644 --- a/src/libtexture/texture_pvt.h +++ b/src/libtexture/texture_pvt.h @@ -542,6 +542,12 @@ class TextureSystemImpl : public TextureSystem { { append_error(Strutil::sprintf(fmt, args...)); } + /// Internal error reporting routine, with std::format-like arguments. + template + void error(const char* fmt, const Args&... args) const + { + append_error(Strutil::fmt::format(fmt, args...)); + } /// Append a string to the current error message void append_error(const std::string& message) const; diff --git a/src/libtexture/texturesys.cpp b/src/libtexture/texturesys.cpp index 5d70736417..18013d49d4 100644 --- a/src/libtexture/texturesys.cpp +++ b/src/libtexture/texturesys.cpp @@ -545,7 +545,7 @@ TextureSystemImpl::get_texture_info(ustring filename, int subimage, if (!ok) { std::string err = m_imagecache->geterror(); if (!err.empty()) - errorf("%s", err); + error("{}", err); } return ok; } @@ -565,7 +565,7 @@ TextureSystemImpl::get_texture_info(TextureHandle* texture_handle, if (!ok) { std::string err = m_imagecache->geterror(); if (!err.empty()) - errorf("%s", err); + error("{}", err); } return ok; } @@ -580,7 +580,7 @@ TextureSystemImpl::get_imagespec(ustring filename, int subimage, if (!ok) { std::string err = m_imagecache->geterror(); if (!err.empty()) - errorf("%s", err); + error("{}", err); } return ok; } @@ -599,7 +599,7 @@ TextureSystemImpl::get_imagespec(TextureHandle* texture_handle, if (!ok) { std::string err = m_imagecache->geterror(); if (!err.empty()) - errorf("%s", err); + error("{}", err); } return ok; } @@ -611,7 +611,7 @@ TextureSystemImpl::imagespec(ustring filename, int subimage) { const ImageSpec* spec = m_imagecache->imagespec(filename, subimage); if (!spec) - errorf("%s", m_imagecache->geterror()); + error("{}", m_imagecache->geterror()); return spec; } @@ -628,7 +628,7 @@ TextureSystemImpl::imagespec(TextureHandle* texture_handle, if (!spec) { std::string err = m_imagecache->geterror(); if (!err.empty()) - errorf("%s", err); + error("{}", err); } return spec; } @@ -644,7 +644,7 @@ TextureSystemImpl::get_texels(ustring filename, TextureOpt& options, PerThreadInfo* thread_info = m_imagecache->get_perthread_info(); TextureFile* texfile = find_texturefile(filename, thread_info); if (!texfile) { - errorf("Texture file \"%s\" not found", filename); + error("Texture file \"{}\" not found", filename); return false; } return get_texels((TextureHandle*)texfile, (Perthread*)thread_info, options, @@ -666,25 +666,25 @@ TextureSystemImpl::get_texels(TextureHandle* texture_handle_, TextureFile* texfile = verify_texturefile((TextureFile*)texture_handle_, thread_info); if (!texfile) { - errorf("Invalid texture handle NULL"); + error("Invalid texture handle NULL"); return false; } if (texfile->broken()) { if (texfile->errors_should_issue()) - errorf("Invalid texture file \"%s\"", texfile->filename()); + error("Invalid texture file \"{}\"", texfile->filename()); return false; } int subimage = options.subimage; if (subimage < 0 || subimage >= texfile->subimages()) { - errorf("get_texel asked for nonexistent subimage %d of \"%s\"", - subimage, texfile->filename()); + error("get_texel asked for nonexistent subimage {} of \"{}\"", subimage, + texfile->filename()); return false; } if (miplevel < 0 || miplevel >= texfile->miplevels(subimage)) { if (texfile->errors_should_issue()) - errorf("get_texel asked for nonexistent MIP level %d of \"%s\"", - miplevel, texfile->filename()); + error("get_texel asked for nonexistent MIP level {} of \"{}\"", + miplevel, texfile->filename()); return false; } const ImageSpec& spec(texfile->spec(subimage, miplevel)); @@ -754,7 +754,7 @@ TextureSystemImpl::get_texels(TextureHandle* texture_handle_, if (!ok) { std::string err = m_imagecache->geterror(); if (!err.empty()) - errorf("%s", err); + error("{}", err); } return ok; } @@ -1024,8 +1024,8 @@ TextureSystemImpl::texture(TextureHandle* texture_handle_, int s = m_imagecache->subimage_from_name(texturefile, options.subimagename); if (s < 0) { - errorf("Unknown subimage \"%s\" in texture \"%s\"", - options.subimagename, texturefile->filename()); + error("Unknown subimage \"{}\" in texture \"{}\"", + options.subimagename, texturefile->filename()); return missing_texture(options, nchannels, result, dresultds, dresultdt); } @@ -1965,7 +1965,7 @@ TextureSystemImpl::sample_closest( id.xy(stex - tile_s, ttex - tile_t); bool ok = find_tile(id, thread_info, sample == 0); if (!ok) - errorf("%s", m_imagecache->geterror()); + error("{}", m_imagecache->geterror()); TileRef& tile(thread_info->tile); if (!tile || !ok) { allok = false; @@ -2150,7 +2150,7 @@ TextureSystemImpl::sample_bilinear( id.xy(sttex[S0] - tile_st[S0], sttex[T0] - tile_st[T0]); bool ok = find_tile(id, thread_info, sample == 0); if (!ok) - errorf("%s", m_imagecache->geterror()); + error("{}", m_imagecache->geterror()); TileRef& tile(thread_info->tile); if (!tile->valid()) return false; @@ -2211,7 +2211,7 @@ TextureSystemImpl::sample_bilinear( id.xy(tile_edge[S0 + i], tile_edge[T0 + j]); bool ok = find_tile(id, thread_info, sample == 0); if (!ok) - errorf("%s", m_imagecache->geterror()); + error("{}", m_imagecache->geterror()); if (!thread_info->tile->valid()) { return false; } @@ -2515,7 +2515,7 @@ TextureSystemImpl::sample_bicubic( id.xy(stex[0] - tile_s, ttex[0] - tile_t); bool ok = find_tile(id, thread_info, sample == 0); if (!ok) - errorf("%s", m_imagecache->geterror()); + error("{}", m_imagecache->geterror()); TileRef& tile(thread_info->tile); if (!tile) { return false; @@ -2585,8 +2585,8 @@ TextureSystemImpl::sample_bicubic( id.xy(tile_s_edge[i], tile_t_edge[j]); bool ok = find_tile(id, thread_info, sample == 0); if (!ok) - errorf("%s", m_imagecache->geterror()); - OIIO_DASSERT(thread_info->tile->id() == id); + error("{}", m_imagecache->geterror()); + DASSERT(thread_info->tile->id() == id); if (!thread_info->tile->valid()) return false; } diff --git a/src/libutil/typedesc.cpp b/src/libutil/typedesc.cpp index 377486247e..a0249441cf 100644 --- a/src/libutil/typedesc.cpp +++ b/src/libutil/typedesc.cpp @@ -327,6 +327,21 @@ tostring_formatting::tostring_formatting( +tostring_formatting::tostring_formatting( + Notation notation, const char* int_fmt, const char* uint_fmt, + const char* float_fmt, const char* string_fmt, const char* ptr_fmt, + const char* aggregate_begin, const char* aggregate_end, + const char* aggregate_sep, const char* array_begin, const char* array_end, + const char* array_sep, int flags) + : tostring_formatting(int_fmt, float_fmt, string_fmt, ptr_fmt, + aggregate_begin, aggregate_end, aggregate_sep, + array_begin, array_end, array_sep, flags, uint_fmt) +{ + use_sprintf = false; +} + + + template inline std::string sprintt(TypeDesc type, const char* format, const tostring_formatting& fmt, @@ -388,6 +403,68 @@ sprintt(TypeDesc type, const char* format, const tostring_formatting& fmt, +template +inline std::string +formatt(TypeDesc type, const char* format, const tostring_formatting& fmt, + const T* v) +{ + std::string val; + if (type.arraylen) + val += fmt.array_begin; + const size_t n = type.arraylen ? type.arraylen : 1; + for (size_t i = 0; i < n; ++i) { + if (type.aggregate > 1) + val += fmt.aggregate_begin; + for (int j = 0; j < (int)type.aggregate; ++j, ++v) { + val += Strutil::fmt::format(format, *v); + if (type.aggregate > 1 && j < type.aggregate - 1) + val += fmt.aggregate_sep; + } + if (type.aggregate > 1) + val += fmt.aggregate_end; + if (i < n - 1) + val += fmt.array_sep; + } + if (type.arraylen) + val += fmt.array_end; + return val; +} + + + +inline std::string +formatt(TypeDesc type, const char* format, const tostring_formatting& fmt, + const char** v) +{ + std::string val; + if (type.arraylen) + val += fmt.array_begin; + const size_t n = type.arraylen ? type.arraylen : 1; + for (size_t i = 0; i < n; ++i) { + if (type.aggregate > 1) + val += fmt.aggregate_begin; + for (int j = 0; j < (int)type.aggregate; ++j, ++v) { + if (fmt.flags & tostring_formatting::escape_strings) + val += Strutil::fmt::format(format, + *v ? Strutil::escape_chars(*v) + : std::string()); + else + val += Strutil::fmt::format(format, *v ? *v : ""); + if (type.aggregate > 1 && j < type.aggregate - 1) + val += fmt.aggregate_sep; + } + if (type.aggregate > 1) + val += fmt.aggregate_end; + if (i < n - 1) + val += fmt.array_sep; + } + if (type.arraylen) + val += fmt.array_end; + return val; +} + + + // From OpenEXR inline unsigned int bitField(unsigned int value, int minBit, int maxBit) @@ -413,16 +490,29 @@ tostring(TypeDesc type, const void* data, const tostring_formatting& fmt) // Perhaps there is a way to use CType<> with a dynamic argument? switch (type.basetype) { case TypeDesc::UNKNOWN: - return sprintt(type, fmt.ptr_fmt, fmt, (void**)data); - case TypeDesc::NONE: return sprintt(type, "None", fmt, (void**)data); + return fmt.use_sprintf ? sprintt(type, fmt.ptr_fmt, fmt, (void**)data) + : formatt(type, fmt.ptr_fmt, fmt, (void**)data); + case TypeDesc::NONE: + return fmt.use_sprintf ? sprintt(type, "None", fmt, (void**)data) + : formatt(type, "None", fmt, (void**)data); case TypeDesc::UCHAR: - return sprintt(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, - (unsigned char*)data); - case TypeDesc::CHAR: return sprintt(type, fmt.int_fmt, fmt, (char*)data); + return fmt.use_sprintf + ? sprintt(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, + (unsigned char*)data) + : formatt(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, + (unsigned char*)data); + case TypeDesc::CHAR: + return fmt.use_sprintf ? sprintt(type, fmt.int_fmt, fmt, (char*)data) + : formatt(type, fmt.int_fmt, fmt, (char*)data); case TypeDesc::USHORT: - return sprintt(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, - (uint16_t*)data); - case TypeDesc::SHORT: return sprintt(type, fmt.int_fmt, fmt, (short*)data); + return fmt.use_sprintf + ? sprintt(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, + (uint16_t*)data) + : formatt(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, + (uint16_t*)data); + case TypeDesc::SHORT: + return fmt.use_sprintf ? sprintt(type, fmt.int_fmt, fmt, (short*)data) + : formatt(type, fmt.int_fmt, fmt, (short*)data); case TypeDesc::UINT: if (type.vecsemantics == TypeDesc::RATIONAL && type.aggregate == TypeDesc::VEC2) { @@ -445,8 +535,11 @@ tostring(TypeDesc type, const void* data, const tostring_formatting& fmt) return Strutil::sprintf("%02d:%02d:%02d:%02d", hours, minutes, seconds, frame); } - return sprintt(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, - (unsigned int*)data); + return fmt.use_sprintf + ? sprintt(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, + (unsigned int*)data) + : formatt(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, + (unsigned int*)data); case TypeDesc::INT: if (type.elementtype() == TypeRational) { std::string out; @@ -458,24 +551,40 @@ tostring(TypeDesc type, const void* data, const tostring_formatting& fmt) } return out; } - return sprintt(type, fmt.int_fmt, fmt, (int*)data); + return fmt.use_sprintf ? sprintt(type, fmt.int_fmt, fmt, (int*)data) + : formatt(type, fmt.int_fmt, fmt, (int*)data); case TypeDesc::UINT64: - return sprintt(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, - (const uint64_t*)data); + return fmt.use_sprintf + ? sprintt(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, + (const uint64_t*)data) + : formatt(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, + (const uint64_t*)data); case TypeDesc::INT64: - return sprintt(type, fmt.int_fmt, fmt, (const int64_t*)data); + return fmt.use_sprintf + ? sprintt(type, fmt.int_fmt, fmt, (const int64_t*)data) + : formatt(type, fmt.int_fmt, fmt, (const int64_t*)data); case TypeDesc::HALF: - return sprintt(type, fmt.float_fmt, fmt, (const half*)data); + return fmt.use_sprintf + ? sprintt(type, fmt.float_fmt, fmt, (const half*)data) + : formatt(type, fmt.float_fmt, fmt, (const half*)data); case TypeDesc::FLOAT: - return sprintt(type, fmt.float_fmt, fmt, (const float*)data); + return fmt.use_sprintf + ? sprintt(type, fmt.float_fmt, fmt, (const float*)data) + : formatt(type, fmt.float_fmt, fmt, (const float*)data); case TypeDesc::DOUBLE: - return sprintt(type, fmt.float_fmt, fmt, (const double*)data); + return fmt.use_sprintf + ? sprintt(type, fmt.float_fmt, fmt, (const double*)data) + : formatt(type, fmt.float_fmt, fmt, (const double*)data); case TypeDesc::STRING: if (!type.is_array() && !(fmt.flags & tostring_formatting::quote_single_string)) return *(const char**)data; - return sprintt(type, fmt.string_fmt, fmt, (const char**)data); - case TypeDesc::PTR: return sprintt(type, fmt.ptr_fmt, fmt, (void**)data); + return fmt.use_sprintf + ? sprintt(type, fmt.string_fmt, fmt, (const char**)data) + : formatt(type, fmt.string_fmt, fmt, (const char**)data); + case TypeDesc::PTR: + return fmt.use_sprintf ? sprintt(type, fmt.ptr_fmt, fmt, (void**)data) + : formatt(type, fmt.ptr_fmt, fmt, (void**)data); default: #ifndef NDEBUG return Strutil::sprintf(" (base %d, agg %d vec %d)", diff --git a/src/tiff.imageio/tiffinput.cpp b/src/tiff.imageio/tiffinput.cpp index 8b3b6f1dd7..14c8060b53 100644 --- a/src/tiff.imageio/tiffinput.cpp +++ b/src/tiff.imageio/tiffinput.cpp @@ -487,7 +487,7 @@ oiio_tiff_last_error() static void my_error_handler(const char* /*str*/, const char* format, va_list ap) { - oiio_tiff_last_error() = Strutil::vformat(format, ap); + oiio_tiff_last_error() = Strutil::vsprintf(format, ap); } From fc709dafebf3e4ebaecca32bd0cf7ceb098e281a Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Thu, 30 Jul 2020 11:44:01 -0700 Subject: [PATCH 2/2] Function rename for clarity Signed-off-by: Larry Gritz --- src/libutil/typedesc.cpp | 105 ++++++++++++++++++++------------------- 1 file changed, 55 insertions(+), 50 deletions(-) diff --git a/src/libutil/typedesc.cpp b/src/libutil/typedesc.cpp index a0249441cf..6b171f173a 100644 --- a/src/libutil/typedesc.cpp +++ b/src/libutil/typedesc.cpp @@ -343,9 +343,9 @@ tostring_formatting::tostring_formatting( template -inline std::string -sprintt(TypeDesc type, const char* format, const tostring_formatting& fmt, - const T* v) +static std::string +sprint_type(TypeDesc type, const char* format, const tostring_formatting& fmt, + const T* v) { std::string val; if (type.arraylen) @@ -371,9 +371,9 @@ sprintt(TypeDesc type, const char* format, const tostring_formatting& fmt, -inline std::string -sprintt(TypeDesc type, const char* format, const tostring_formatting& fmt, - const char** v) +static std::string +sprint_type(TypeDesc type, const char* format, const tostring_formatting& fmt, + const char** v) { std::string val; if (type.arraylen) @@ -404,9 +404,9 @@ sprintt(TypeDesc type, const char* format, const tostring_formatting& fmt, template -inline std::string -formatt(TypeDesc type, const char* format, const tostring_formatting& fmt, - const T* v) +static std::string +format_type(TypeDesc type, const char* format, const tostring_formatting& fmt, + const T* v) { std::string val; if (type.arraylen) @@ -432,9 +432,9 @@ formatt(TypeDesc type, const char* format, const tostring_formatting& fmt, -inline std::string -formatt(TypeDesc type, const char* format, const tostring_formatting& fmt, - const char** v) +static std::string +format_type(TypeDesc type, const char* format, const tostring_formatting& fmt, + const char** v) { std::string val; if (type.arraylen) @@ -490,29 +490,32 @@ tostring(TypeDesc type, const void* data, const tostring_formatting& fmt) // Perhaps there is a way to use CType<> with a dynamic argument? switch (type.basetype) { case TypeDesc::UNKNOWN: - return fmt.use_sprintf ? sprintt(type, fmt.ptr_fmt, fmt, (void**)data) - : formatt(type, fmt.ptr_fmt, fmt, (void**)data); + return fmt.use_sprintf + ? sprint_type(type, fmt.ptr_fmt, fmt, (void**)data) + : format_type(type, fmt.ptr_fmt, fmt, (void**)data); case TypeDesc::NONE: - return fmt.use_sprintf ? sprintt(type, "None", fmt, (void**)data) - : formatt(type, "None", fmt, (void**)data); + return fmt.use_sprintf ? sprint_type(type, "None", fmt, (void**)data) + : format_type(type, "None", fmt, (void**)data); case TypeDesc::UCHAR: return fmt.use_sprintf - ? sprintt(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, - (unsigned char*)data) - : formatt(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, - (unsigned char*)data); + ? sprint_type(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, + (unsigned char*)data) + : format_type(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, + (unsigned char*)data); case TypeDesc::CHAR: - return fmt.use_sprintf ? sprintt(type, fmt.int_fmt, fmt, (char*)data) - : formatt(type, fmt.int_fmt, fmt, (char*)data); + return fmt.use_sprintf + ? sprint_type(type, fmt.int_fmt, fmt, (char*)data) + : format_type(type, fmt.int_fmt, fmt, (char*)data); case TypeDesc::USHORT: return fmt.use_sprintf - ? sprintt(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, - (uint16_t*)data) - : formatt(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, - (uint16_t*)data); + ? sprint_type(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, + (uint16_t*)data) + : format_type(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, + (uint16_t*)data); case TypeDesc::SHORT: - return fmt.use_sprintf ? sprintt(type, fmt.int_fmt, fmt, (short*)data) - : formatt(type, fmt.int_fmt, fmt, (short*)data); + return fmt.use_sprintf + ? sprint_type(type, fmt.int_fmt, fmt, (short*)data) + : format_type(type, fmt.int_fmt, fmt, (short*)data); case TypeDesc::UINT: if (type.vecsemantics == TypeDesc::RATIONAL && type.aggregate == TypeDesc::VEC2) { @@ -536,10 +539,10 @@ tostring(TypeDesc type, const void* data, const tostring_formatting& fmt) seconds, frame); } return fmt.use_sprintf - ? sprintt(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, - (unsigned int*)data) - : formatt(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, - (unsigned int*)data); + ? sprint_type(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, + (unsigned int*)data) + : format_type(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, + (unsigned int*)data); case TypeDesc::INT: if (type.elementtype() == TypeRational) { std::string out; @@ -551,40 +554,42 @@ tostring(TypeDesc type, const void* data, const tostring_formatting& fmt) } return out; } - return fmt.use_sprintf ? sprintt(type, fmt.int_fmt, fmt, (int*)data) - : formatt(type, fmt.int_fmt, fmt, (int*)data); + return fmt.use_sprintf + ? sprint_type(type, fmt.int_fmt, fmt, (int*)data) + : format_type(type, fmt.int_fmt, fmt, (int*)data); case TypeDesc::UINT64: return fmt.use_sprintf - ? sprintt(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, - (const uint64_t*)data) - : formatt(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, - (const uint64_t*)data); + ? sprint_type(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, + (const uint64_t*)data) + : format_type(type, fmt.uint_fmt ? fmt.uint_fmt : "%u", fmt, + (const uint64_t*)data); case TypeDesc::INT64: return fmt.use_sprintf - ? sprintt(type, fmt.int_fmt, fmt, (const int64_t*)data) - : formatt(type, fmt.int_fmt, fmt, (const int64_t*)data); + ? sprint_type(type, fmt.int_fmt, fmt, (const int64_t*)data) + : format_type(type, fmt.int_fmt, fmt, (const int64_t*)data); case TypeDesc::HALF: return fmt.use_sprintf - ? sprintt(type, fmt.float_fmt, fmt, (const half*)data) - : formatt(type, fmt.float_fmt, fmt, (const half*)data); + ? sprint_type(type, fmt.float_fmt, fmt, (const half*)data) + : format_type(type, fmt.float_fmt, fmt, (const half*)data); case TypeDesc::FLOAT: return fmt.use_sprintf - ? sprintt(type, fmt.float_fmt, fmt, (const float*)data) - : formatt(type, fmt.float_fmt, fmt, (const float*)data); + ? sprint_type(type, fmt.float_fmt, fmt, (const float*)data) + : format_type(type, fmt.float_fmt, fmt, (const float*)data); case TypeDesc::DOUBLE: return fmt.use_sprintf - ? sprintt(type, fmt.float_fmt, fmt, (const double*)data) - : formatt(type, fmt.float_fmt, fmt, (const double*)data); + ? sprint_type(type, fmt.float_fmt, fmt, (const double*)data) + : format_type(type, fmt.float_fmt, fmt, (const double*)data); case TypeDesc::STRING: if (!type.is_array() && !(fmt.flags & tostring_formatting::quote_single_string)) return *(const char**)data; return fmt.use_sprintf - ? sprintt(type, fmt.string_fmt, fmt, (const char**)data) - : formatt(type, fmt.string_fmt, fmt, (const char**)data); + ? sprint_type(type, fmt.string_fmt, fmt, (const char**)data) + : format_type(type, fmt.string_fmt, fmt, (const char**)data); case TypeDesc::PTR: - return fmt.use_sprintf ? sprintt(type, fmt.ptr_fmt, fmt, (void**)data) - : formatt(type, fmt.ptr_fmt, fmt, (void**)data); + return fmt.use_sprintf + ? sprint_type(type, fmt.ptr_fmt, fmt, (void**)data) + : format_type(type, fmt.ptr_fmt, fmt, (void**)data); default: #ifndef NDEBUG return Strutil::sprintf(" (base %d, agg %d vec %d)",