From 73f4c5f3408f29df770be2beaf475a4003bf3185 Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Fri, 3 Jul 2026 20:42:48 -0400 Subject: [PATCH] fix: convert integer channels to OpenEXR instead of reinterpreting bytes Converting an image whose channels have differing native integer depths (e.g. a Softimage PIC with a 16-bit R and an 8-bit G channel) to OpenEXR produced garbage pixels: the uint16/uint8 bytes were copied straight into the half channels and reinterpreted rather than rescaled. Three coupled issues, all rooted in treating "output supports channelformats" as "output stores these exact per-channel formats": - exroutput: spec_to_header forced spec.format to one of EXR's three stored types but left spec.channelformats naming the original (uint16/uint8) types, so the post-open spec misdescribed what was actually stored. Remap channelformats through the same mapping (and drop the list when all channels collapse to one type) so the spec is consistent with the half/float/uint the file really holds. - ImageOutput::copy_image: the native (no-conversion) fast path was taken whenever the input had channelformats. Only take it when the output will store each channel in the input's native format; otherwise fall back to the converting path. - iconvert: its by-hand copy path (used whenever the input has mixed channel formats) requested a native per-channel transfer unconditionally. Apply the same check against the opened output's stored formats and, when they differ, transfer through the output's format so values are rescaled. Regression test added to testsuite/softimage: converting the mixed 16/8-bit file to EXR now yields correctly normalized values. Assisted-by: Claude Code / Opus 4.8 Signed-off-by: Larry Gritz --- src/iconvert/iconvert.cpp | 31 +++++++++++++++++++++++++----- src/libOpenImageIO/imageoutput.cpp | 13 +++++++++++++ src/openexr.imageio/exroutput.cpp | 30 +++++++++++++++++++++-------- testsuite/softimage/ref/out.txt | 10 ++++++++++ testsuite/softimage/run.py | 8 ++++++++ 5 files changed, 79 insertions(+), 13 deletions(-) diff --git a/src/iconvert/iconvert.cpp b/src/iconvert/iconvert.cpp index f77d84f55b..9b4f25c27a 100644 --- a/src/iconvert/iconvert.cpp +++ b/src/iconvert/iconvert.cpp @@ -473,16 +473,37 @@ convert_file(const std::string& in_filename, const std::string& out_filename) "iconvert ERROR copying \"{}\" to \"{}\" :\n\t{}\n", in_filename, out_filename, out->geterror()); } else { - // Need to do it by hand for some reason. Future expansion in which - // only a subset of channels are copied, or some such. - std::vector pixels((size_t)outspec.image_bytes(true)); + // Need to do it by hand for some reason. A format of UNKNOWN + // requests a per-channel native copy to preserve mixed + // per-channel formats. That is only valid if the output + // actually stores each channel in the input's native format. + // If it remaps any channel (e.g. OpenEXR promoting + // uint16/uint8 to half), copy through the output's stored + // format so integer values are rescaled rather than + // bit-reinterpreted. + TypeDesc xferformat = outspec.format; + if (xferformat == TypeDesc::UNKNOWN) { + const ImageSpec& storedspec(out->spec()); + for (int c = 0; c < outspec.nchannels; ++c) + if (inspec.channelformat(c) + != storedspec.channelformat(c)) { + xferformat = storedspec.format; + break; + } + } + size_t bufbytes = (xferformat == TypeDesc::UNKNOWN) + ? (size_t)outspec.image_bytes(true) + : (size_t)outspec.image_pixels() + * outspec.nchannels + * xferformat.size(); + std::vector pixels(bufbytes); ok = in->read_image(subimage, miplevel, 0, outspec.nchannels, - outspec.format, &pixels[0]); + xferformat, &pixels[0]); if (!ok) { OIIO::print(stderr, "iconvert ERROR reading \"{}\": {}\n", in_filename, in->geterror()); } else { - ok = out->write_image(outspec.format, &pixels[0]); + ok = out->write_image(xferformat, &pixels[0]); if (!ok) OIIO::print(stderr, "iconvert ERROR writing \"{}\": {}\n", diff --git a/src/libOpenImageIO/imageoutput.cpp b/src/libOpenImageIO/imageoutput.cpp index 4ec5300c3a..05755fcfee 100644 --- a/src/libOpenImageIO/imageoutput.cpp +++ b/src/libOpenImageIO/imageoutput.cpp @@ -791,6 +791,19 @@ ImageOutput::copy_image(ImageInput* in) // FIXME -- a smarter implementation would read scanlines or tiles at // a time, to minimize mem footprint. bool native = supports("channelformats") && inspec.channelformats.size(); + // The native (per-channel, no-conversion) copy is only valid if this + // output actually stores each channel in the same format the input + // supplies. If the output remaps any channel (e.g. OpenEXR promoting + // uint16 to half), a raw native copy would reinterpret the bytes rather + // than convert them, so fall back to the converting path in that case. + if (native) { + const ImageSpec& outspec(spec()); + for (int c = 0; c < inspec.nchannels; ++c) + if (inspec.channelformat(c) != outspec.channelformat(c)) { + native = false; + break; + } + } TypeDesc format = native ? TypeDesc::UNKNOWN : inspec.format; std::unique_ptr pixels(new char[inspec.image_bytes(native)]); bool ok = in->read_image(in->current_subimage(), in->current_miplevel(), 0, diff --git a/src/openexr.imageio/exroutput.cpp b/src/openexr.imageio/exroutput.cpp index 300ce20fbe..3b3c29ffaa 100644 --- a/src/openexr.imageio/exroutput.cpp +++ b/src/openexr.imageio/exroutput.cpp @@ -865,14 +865,28 @@ bool OpenEXROutput::spec_to_header(ImageSpec& spec, int subimage, Imf::Header& header) { - // Force use of one of the three data types that OpenEXR supports - switch (spec.format.basetype) { - case TypeDesc::UINT: spec.format = TypeDesc::UINT; break; - case TypeDesc::FLOAT: - case TypeDesc::DOUBLE: spec.format = TypeDesc::FLOAT; break; - default: - // Everything else defaults to half - spec.format = TypeDesc::HALF; + // OpenEXR only stores three data types (half, float, uint); map anything + // else onto them. Do this for the overall format and, so the spec stays + // consistent with what is actually stored, for any per-channel formats. + auto exr_stored_format = [](TypeDesc t) -> TypeDesc { + switch (t.basetype) { + case TypeDesc::UINT: return TypeDesc::UINT; + case TypeDesc::FLOAT: + case TypeDesc::DOUBLE: return TypeDesc::FLOAT; + default: return TypeDesc::HALF; // everything else defaults to half + } + }; + spec.format = exr_stored_format(spec.format); + if (spec.channelformats.size() == size_t(spec.nchannels)) { + bool allsame = true; + for (int c = 0; c < spec.nchannels; ++c) { + spec.channelformats[c] = exr_stored_format(spec.channelformats[c]); + allsame &= (spec.channelformats[c] == spec.channelformats[0]); + } + // If every channel ended up the same, the per-channel list is + // redundant -- drop it so the spec advertises a single format. + if (allsame) + spec.channelformats.clear(); } Imath::Box2i dataWindow(Imath::V2i(spec.x, spec.y), diff --git a/testsuite/softimage/ref/out.txt b/testsuite/softimage/ref/out.txt index 471e196acf..3cbc5acdf0 100644 --- a/testsuite/softimage/ref/out.txt +++ b/testsuite/softimage/ref/out.txt @@ -59,3 +59,13 @@ src/mixed-bitdepth.pic : 2 x 1, 2 channel, uint16 softimage Stats FiniteCount: 2 2 Constant: No Monochrome: No +mixed-bitdepth.exr : 2 x 1, 2 channel, half openexr + Stats Min: 0.066956 0.333252 (float) + Stats Max: 0.200317 0.399902 (float) + Stats Avg: 0.133636 0.366577 (float) + Stats StdDev: 0.066681 0.033325 (float) + Stats NanCount: 0 0 + Stats InfCount: 0 0 + Stats FiniteCount: 2 2 + Constant: No + Monochrome: No diff --git a/testsuite/softimage/run.py b/testsuite/softimage/run.py index 8c80afe8d6..e3336c477b 100755 --- a/testsuite/softimage/run.py +++ b/testsuite/softimage/run.py @@ -19,3 +19,11 @@ # single bit depth was assumed for all channels. command += info_command ("--stats src/mixed-bitdepth.pic", info_program="iinfo", failureok=True) +# Converting that mixed-bit-depth file to a format that stores a single/other +# channel type (OpenEXR promotes both channels to half) must rescale each +# channel's values, not reinterpret its raw bytes. Convert and check the +# resulting normalized values. +command += (oiio_app("iconvert") + + "src/mixed-bitdepth.pic mixed-bitdepth.exr >> out.txt 2>&1 ;\n") +command += info_command ("mixed-bitdepth.exr", extraargs="--stats", + verbose=False, hash=False)