Skip to content

Commit

Permalink
cleanup: change raw cerr stream output to print (#4258)
Browse files Browse the repository at this point in the history 
A small piece of the ongoing switch to std::format/print style text
output. This part is for error and debugging output when reading/writing
exr images.

I also switched conditional compilation to the DBG macro idiom that I've
used elsewhere, where it takes a combination of DEBUG compile and
setting of environment variable OIIO_DEBUG_OPENEXR to trigger the printf
style debugging (the first use case of this was in our color management
code, which I also modify slightly here to conform).

Also opportunistically simplify gcd declaration now that we're C++17
minimum.

Signed-off-by: Larry Gritz <[email protected]>
  • Loading branch information
@lgritz
lgritz authored May 18, 2024
1 parent d868624 commit 41c8949
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 68 deletions.
3 changes: 2 additions & 1 deletion src/libOpenImageIO/color_ocio.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ static const Imath::C3f test_colors[n_test_colors]


#if 0 || !defined(NDEBUG) /* allow color configuration debugging */
static bool colordebug = Strutil::stoi(Sysutil::getenv("OIIO_COLOR_DEBUG"));
static bool colordebug = Strutil::stoi(Sysutil::getenv("OIIO_DEBUG_COLOR"))
|| Strutil::stoi(Sysutil::getenv("OIIO_DEBUG_ALL"));
# define DBG(...) \
if (colordebug) \
Strutil::print(__VA_ARGS__)
Expand Down
28 changes: 13 additions & 15 deletions src/openexr.imageio/exr_pvt.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
#include <OpenImageIO/imageio.h>
#include <OpenImageIO/platform.h>
#include <OpenImageIO/string_view.h>
#include <OpenImageIO/strutil.h>
#include <OpenImageIO/sysutil.h>
#include <OpenImageIO/typedesc.h>

#include <ImathBox.h>
Expand All @@ -24,25 +26,21 @@

#define OPENEXR_HAS_FLOATVECTOR 1

#define ENABLE_READ_DEBUG_PRINTS 0

#define ENABLE_EXR_DEBUG_PRINTS 0

OIIO_PLUGIN_NAMESPACE_BEGIN

#if OIIO_CPLUSPLUS_VERSION >= 17 || defined(__cpp_lib_gcd_lcm)
using std::gcd;
// Lots of debugging printf turned on for DEBUG builds or if you define
// ENABLE_EXR_DEBUG_PRINTS above, *AND* the "OIIO_DEBUG_OPENEXR" environment
// variable is set to something numerically non-zero.
#if ENABLE_EXR_DEBUG_PRINTS || !defined(NDEBUG) /* allow debugging */
static bool exrdebug = Strutil::stoi(Sysutil::getenv("OIIO_DEBUG_OPENEXR"))
|| Strutil::stoi(Sysutil::getenv("OIIO_DEBUG_ALL"));
# define DBGEXR(...) \
if (exrdebug) \
Strutil::print(__VA_ARGS__)
#else
template<class M, class N, class T = std::common_type_t<M, N>>
inline T
gcd(M a, N b)
{
while (b) {
T t = b;
b = a % b;
a = t;
}
return a;
}
# define DBGEXR(...)
#endif


Expand Down
16 changes: 7 additions & 9 deletions src/openexr.imageio/exrinput.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ OpenEXRInput::PartInfo::parse_header(OpenEXRInput* in,
r[1] = static_cast<int>(d);
spec.attribute(oname, TypeRational, r);
} else {
int f = static_cast<int>(gcd(int64_t(n), int64_t(d)));
int f = static_cast<int>(std::gcd(int64_t(n), int64_t(d)));
if (f > 1) {
int r[2];
r[0] = n / f;
Expand All @@ -598,7 +598,8 @@ OpenEXRInput::PartInfo::parse_header(OpenEXRInput* in,
}
} else {
#if 0
std::cerr << " unknown attribute " << type << ' ' << name << "\n";
print(std::cerr, " unknown attribute '{}' name '{}'\n",
type, name);
#endif
}
}
Expand Down Expand Up @@ -1128,8 +1129,8 @@ OpenEXRInput::read_native_scanlines(int subimage, int miplevel, int ybegin,
if (!seek_subimage(subimage, miplevel))
return false;
chend = clamp(chend, chbegin + 1, m_spec.nchannels);
// std::cerr << "openexr rns " << ybegin << ' ' << yend << ", channels "
// << chbegin << "-" << (chend-1) << "\n";
DBGEXR("openexr rns {} {}-{}, channels {}-{}", ybegin, yend, chbegin,
chend - 1);

// Compute where OpenEXR needs to think the full buffers starts.
// OpenImageIO requires that 'data' points to where the client wants
Expand Down Expand Up @@ -1258,11 +1259,8 @@ OpenEXRInput::read_native_tiles(int subimage, int miplevel, int xbegin,
"OpenEXRInput::read_native_tiles is not supported for luminance-chroma images");
return false;
}
#if 0
std::cerr << "openexr rnt " << xbegin << ' ' << xend << ' ' << ybegin
<< ' ' << yend << ", chans " << chbegin
<< "-" << (chend-1) << "\n";
#endif
DBGEXR("openexr rnt {} {}-{}, chans {}-{}", xbegin, xend, ybegin, yend,
chend - 1);
if (!m_tiled_input_part
|| !m_spec.valid_tile_range(xbegin, xend, ybegin, yend, zbegin, zend)) {
errorfmt("called OpenEXRInput::read_native_tiles without an open file");
Expand Down
67 changes: 24 additions & 43 deletions src/openexr.imageio/exrinput_c.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,8 @@ oiio_exr_error_handler(exr_const_context_t ctxt, exr_result_t code,
}

// this should only happen from valid_file check, do we care?
//std::cerr << "EXR error with no valid context ("
// << exr_get_error_code_as_string(code) << "): " << msg
// << std::endl;
// print(std::cerr, "EXR error with no valid context ({}): {}\n",
// exr_get_error_code_as_string(code), msg);
}

static int64_t
Expand Down Expand Up @@ -418,8 +417,9 @@ OpenEXRCoreInput::open(const std::string& name, ImageSpec& newspec,
m_userdata.m_io = nullptr;
return false;
}
#if ENABLE_READ_DEBUG_PRINTS
exr_print_context_info(m_exr_context, 1);
#if ENABLE_EXR_DEBUG_PRINTS || !defined(NDEBUG) /* allow debugging */
if (exrdebug)
exr_print_context_info(m_exr_context, 1);
#endif
rv = exr_get_count(m_exr_context, &m_nsubimages);
if (rv != EXR_ERR_SUCCESS) {
Expand Down Expand Up @@ -660,7 +660,7 @@ OpenEXRCoreInput::PartInfo::parse_header(OpenEXRCoreInput* in,
r[1] = static_cast<int>(d);
spec.attribute(oname, TypeRational, r);
} else {
int f = static_cast<int>(gcd(int64_t(n), int64_t(d)));
int f = static_cast<int>(std::gcd(int64_t(n), int64_t(d)));
if (f > 1) {
int r[2];
r[0] = n / f;
Expand Down Expand Up @@ -733,7 +733,8 @@ OpenEXRCoreInput::PartInfo::parse_header(OpenEXRCoreInput* in,
case EXR_ATTR_TILEDESC:
default:
#if 0
std::cerr << " unknown attribute type '" << attr->type_name << "' in name: '" << attr->name << "'" << std::endl;
print(std::cerr, " unknown attribute type '{}' in name '{}'\n",
attr->type_name, attr->name);
#endif
break;
}
Expand Down Expand Up @@ -1181,16 +1182,10 @@ OpenEXRCoreInput::read_native_scanlines(int subimage, int miplevel, int ybegin,
if (rv != EXR_ERR_SUCCESS)
return false;

#if ENABLE_READ_DEBUG_PRINTS
{
lock_guard lock(*this);
std::cerr << "exr rns " << m_userdata.m_io->filename() << ":"
<< subimage << ":" << miplevel << " scans (" << ybegin << '-'
<< yend << "|" << (yend - ybegin) << ")[" << chbegin << "-"
<< (chend - 1) << "] -> pb " << pixelbytes << " sb "
<< scanlinebytes << " spc " << scansperchunk << std::endl;
}
#endif
DBGEXR("exr rns {}:{}:{} scans ({}-{}|{}}[{}-{}] -> pb {} sb {} spc {}\n",
m_userdata.m_io->filename(), subimage, miplevel, ybegin, yend,
yend - ybegin, chbegin, chend - 1, pixelbytes, scanlinebytes,
scansperchunk);
int endy = spec.y + spec.height;
yend = std::min(endy, yend);
int ychunkstart = spec.y
Expand Down Expand Up @@ -1358,13 +1353,10 @@ OpenEXRCoreInput::read_native_tile(int subimage, int miplevel, int x, int y,
scanlinebytes);
}

#if ENABLE_READ_DEBUG_PRINTS
std::cerr << "openexr rnt single " << m_userdata.m_io->filename() << " si "
<< subimage << " mip " << miplevel << " pos " << x << ' ' << y
<< "\n -> tile " << tx << ", " << ty << ", pixbytes "
<< pixelbytes << " scan " << scanlinebytes << " tilesz " << tilew
<< "x" << tileh << std::endl;
#endif
DBGEXR(
"openexr rnt single {} si {} mip {} pos {} {} -> tile {} {} pixbytes {} scan {} tilesz {}x{}\n",
m_userdata.m_io->filename(), subimage, miplevel, x, y, tx, ty,
pixelbytes, scanlinebytes, tilew, tileh);

uint8_t* cdata = static_cast<uint8_t*>(data);
size_t chanoffset = 0;
Expand All @@ -1379,12 +1371,9 @@ OpenEXRCoreInput::read_native_tile(int subimage, int miplevel, int x, int y,
curchan.user_line_stride
= scanlinebytes; //curchan.width * pixelbytes;
chanoffset += chanbytes;
#if ENABLE_READ_DEBUG_PRINTS
std::cerr << " chan " << c << " tile " << tx << ", " << ty
<< ": linestride " << curchan.user_line_stride
<< " tilesize " << curchan.width << " x "
<< curchan.height << std::endl;
#endif
DBGEXR(" chan {} tile {}, {}: linestride {} tilesize {} x {}\n",
c, tx, ty, curchan.user_line_stride, curchan.width,
curchan.height);
break;
}
}
Expand Down Expand Up @@ -1465,19 +1454,11 @@ OpenEXRCoreInput::read_native_tiles(int subimage, int miplevel, int xbegin,

size_t scanlinebytes = size_t(nxtiles) * size_t(tilew) * pixelbytes;

#if ENABLE_READ_DEBUG_PRINTS
{
lock_guard lock(*this);
std::cerr << "exr rnt " << m_userdata.m_io->filename() << ":"
<< subimage << ":" << miplevel << " (" << xbegin << ' '
<< xend << ' ' << ybegin << ' ' << yend << "|"
<< (xend - xbegin) << "x" << (yend - ybegin) << ")["
<< chbegin << "-" << (chend - 1) << "] -> t " << firstxtile
<< ", " << firstytile << " n " << nxtiles << ", " << nytiles
<< " pb " << pixelbytes << " sb " << scanlinebytes << " tsz "
<< tilew << "x" << tileh << std::endl;
}
#endif
DBGEXR(
"exr rnt {}:{}:{} ({}-{}|{}x{})[{}-{}] -> t {}, {} n {}, {} pb {} sb {} tsz {}x{}\n",
m_userdata.m_io->filename(), subimage, miplevel, xbegin, xend,
xend - xbegin, ybegin, yend, chbegin, chend - 1, firstxtile, firstytile,
nxtiles, nytiles, pixelbytes, scanlinebytes, tilew, tileh);

std::atomic<bool> ok(true);
parallel_for_2D(
Expand Down

0 comments on commit 41c8949

Please sign in to comment.