Skip to content

Commit

Permalink
feat(ImageBuf): auto print uncaught IB errors
Browse files Browse the repository at this point in the history
Advanced users know to check error returns and retrieve errors.

New users often just try OIIO calls with no error checks when they
don't get behavior they want, ask questions to the dev team. We
hypothesize that if they only saw the text of the error messages that
they never retrieved, they could probably self-diagnose many of their
problems.

This patch makes an ImageBuf, when it is destryed, print any pending
error messages that were never retrieved. It happens by default, but
can be disabled by setting the new global attribute
"imagebuf:print_uncaught_errors" to 0 (it defaults to 1).

This doesn't cover everything, but for ImageBuf and ImageBufAlgo,
maybe it will help some users? It could annoy advanced users who
intend to not retrieve error messages (why?) but still don't want
this printed. But that's who the new attribute is for.

Signed-off-by: Larry Gritz <[email protected]>
  • Loading branch information
lgritz committed Aug 23, 2023
1 parent 56c734a commit b031222
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 1 deletion.
16 changes: 15 additions & 1 deletion src/include/OpenImageIO/imageio.h
Original file line number Diff line number Diff line change
Expand Up @@ -2905,7 +2905,7 @@ OIIO_API std::string geterror(bool clear = true);
/// you should raise this limit. Setting the limit to 0 means having no
/// limit.
///
/// - `int log_times`
/// - `int log_times` (0)
///
/// When the `"log_times"` attribute is nonzero, `ImageBufAlgo` functions
/// are instrumented to record the number of times they were called and
Expand All @@ -2923,6 +2923,20 @@ OIIO_API std::string geterror(bool clear = true);
/// the log information. When the `log_times` attribute is disabled,
/// there is no additional performance cost.
///
/// - `imagebuf:print_uncaught_errors` (1)
///
/// If nonzero, an `ImageBuf` upon destruction will print any error messages
/// that were never retrieved by its `geterror()` method. While this may
/// seem chaotic, we are presuming that any well-written library or
/// application will proactively check error codes and retrieve errors, so
/// will never print anything upon destruction. But for less sophisticated
/// applications (or users), this is very useful for forcing display of
/// error messages so that users can see relevant errors even if they never
/// check them explicitly, thus self-diagnose their troubles before asking
/// the project dev deam for help. Advanced users who for some reason desire
/// to neither retrieve errors themselves nor have them printed in this
/// manner can disable the behavior by setting this attribute to 0.
///
OIIO_API bool attribute(string_view name, TypeDesc type, const void* val);

/// Shortcut attribute() for setting a single integer.
Expand Down
10 changes: 10 additions & 0 deletions src/libOpenImageIO/imagebuf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,16 @@ ImageBufImpl::~ImageBufImpl()
// else init_spec requested the system-wide shared cache, which
// does not need to be destroyed.
clear();

// Upon destruction, print uncaught errors to help users who don't know
// how to properly check for errors.
if (!m_err.empty() /* Note: safe becausethis is the dtr */
&& pvt::imagebuf_print_uncaught_errors) {
OIIO::print(
"An ImageBuf was destroyed with a pending error message that was never\n"
"retrieved via ImageBuf::geterror(). This was the error message:\n{}\n",
m_err);
}
}


Expand Down
12 changes: 12 additions & 0 deletions src/libOpenImageIO/imagebuf_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,16 @@ test_write_over()



static void
test_uncaught_error()
{
ImageBuf buf;
buf.error("Boo!");
// buf exists scope and is destroyed without anybody retrieving the error.
}



int
main(int /*argc*/, char* /*argv*/[])
{
Expand Down Expand Up @@ -534,6 +544,8 @@ main(int /*argc*/, char* /*argv*/[])

test_write_over();

test_uncaught_error();

Filesystem::remove("A_imagebuf_test.tif");
return unit_test_failures;
}
9 changes: 9 additions & 0 deletions src/libOpenImageIO/imageio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ int dds_bc5normal(0);
int limit_channels(1024);
int limit_imagesize_MB(std::min(32 * 1024,
int(Sysutil::physical_memory() >> 20)));
int imagebuf_print_uncaught_errors(1);
ustring font_searchpath(Sysutil::getenv("OPENIMAGEIO_FONTS"));
ustring plugin_searchpath(OIIO_DEFAULT_PLUGIN_SEARCHPATH);
std::string format_list; // comma-separated list of all formats
Expand Down Expand Up @@ -344,6 +345,10 @@ attribute(string_view name, TypeDesc type, const void* val)
limit_imagesize_MB = *(const int*)val;
return true;
}
if (name == "imagebuf:print_uncaught_errors" && type == TypeInt) {
imagebuf_print_uncaught_errors = *(const int*)val;
return true;
}
if (name == "use_tbb" && type == TypeInt) {
oiio_use_tbb = *(const int*)val;
return true;
Expand Down Expand Up @@ -472,6 +477,10 @@ getattribute(string_view name, TypeDesc type, void* val)
*(int*)val = dds_bc5normal;
return true;
}
if (name == "imagebuf:print_uncaught_errors" && type == TypeInt) {
*(int*)val = imagebuf_print_uncaught_errors;
return true;
}
if (name == "use_tbb" && type == TypeInt) {
*(int*)val = oiio_use_tbb;
return true;
Expand Down
1 change: 1 addition & 0 deletions src/libOpenImageIO/imageio_pvt.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ extern int openexr_core;
extern int limit_channels;
extern int limit_imagesize_MB;
extern int opencv_version;
extern int imagebuf_print_uncaught_errors;
extern OIIO_UTIL_API int oiio_use_tbb; // This lives in libOpenImageIO_Util
OIIO_API const std::vector<std::string>& font_dirs();
OIIO_API const std::vector<std::string>& font_file_list();
Expand Down

0 comments on commit b031222

Please sign in to comment.