Improved IFF support reading and writing z buffers by mikaelsundell · Pull Request #4673 · AcademySoftwareFoundation/OpenImageIO

mikaelsundell · 2025-03-19T19:07:31Z

Description

Added support for reading and writing ZBUF chunks, also known as Z-buffers or depth maps
Removed noproxy versions, no longer needed

Checklist:

I have read the contribution guidelines.
I have updated the documentation, if applicable. (Check if there is no
need to update the documentation, for example if this is a bug fix that
doesn't change the API.)
I have ensured that the change is tested somewhere in the testsuite
(adding new test cases if necessary).
If I added or modified a C++ API call, I have also amended the
corresponding Python bindings (and if altering ImageBufAlgo functions, also
exposed the new functionality as oiiotool options).
My code follows the prevailing code style of this project. If I haven't
already run clang-format before submitting, I definitely will look at the CI
test that runs clang-format and fix anything that it highlights as being
nonconforming.

lgritz · 2025-03-19T19:32:05Z

+                    } else {
+                        // skip to the next block.
+                        if (!ioseek(chunksize, SEEK_CUR)) {
+                            errorfmt("IFF error io seek failed");


The ImageInput helper methods ioseek() and ioread() handle calling errorfmt for you. If they fail, you just need to return false, you don't need to clutter with another error message call.

lgritz · 2025-03-19T19:33:04Z

-        readimg();
+    if (m_buf.empty()) {
+        if (!readimg()) {
+            errorfmt("IFF error could not read image for tile");


If readimg() itself calls errorfmt for each way that it can fail, you don't need to do it again here.

lgritz · 2025-03-19T19:37:05Z

+    // IFF image files only supports UINT8 and UINT16.  If something
+    // else was requested, revert to the one most likely to be readable
+    // by any IFF reader: UINT8
+
+    TypeDesc base_format = spec.format;
+    if (base_format != TypeDesc::UINT8 && base_format != TypeDesc::UINT16) {
+        errorfmt("Unsupported format {}. Converting to UINT8.", base_format);
+        base_format = TypeDesc::UINT8;
+    }


I think that most of the readers pick the supported data type that loses the least precision versus the one requested. So if they ask for something at least as big as uint16, we should probably give them uint16 rather than uint8, which is even less fidelity.

mikaelsundell · 2025-03-28T05:44:47Z

There will be an update on this one today!

lgritz · 2025-03-28T20:40:46Z

            // skip to the next block
-            if (!ioseek(chunksize))
+            if (!ioseek(chunksize, SEEK_CUR)) {
+                errorfmt("IFF error io seek failed");


Not necessary -- the ioseek method itself calls errorfmt if it fails. All you have to do here is return.

lgritz · 2025-03-28T21:00:36Z

Unfortunately, this dumps core on some example iff files I have. It's not something I can send. Let me see if I can at least point you to the part of the code where it crashes.

lgritz · 2025-03-28T21:22:03Z

+                    for (uint16_t px = xmin; px <= xmax; px++) {
+                        uint8_t* out_p = out_dy + px * m_header.pixel_bytes()
+                                         + m_header.rgba_channels_bytes() + c;
+                        *out_p++ = *in_p++;
+                    }


For an RGBAZ iff file (uint8 RGBA channels, float Z channel), I'm getting crashes at line 852 when releasing a std::vector (presumably the "in" vector declared at line 834).

I suspect that this loop may be the culprit. Are you sure about those offets? I believe that here, we are taking the compressed single z channel (stored at this point in scratch and copying it to the right postion in the output buffer. But the

... + m_header.rgba_channels_bytes() + c

looks wrong to me, and also the next line

*outp++ = *in_p++;

These are uint8_t pointers, so you're incrementing them by 1 each here, but isn't the data in "in" really float (the Z channel) and isn't the spacing of the output buffer going to be m_header.pixel_bytes() ?

Or am I misunderstanding what's happening here?

The reason this happens is that each call to uncompress_rle_channel decompresses floats and uint16 values byte by byte, while also handling planar storage and byte order at the same time. That’s why uint8_t is used across uint8, uint16, and float decompression and compression—it's one RLE run per byte, not per channel in this case.

pixel_bytes() is full RGB(A)+Z while rgba_channel_bytes() is RGB(A) only cause RGBA and Z chunks are stored in the actual IFF layout while we store interleaved/ packed.

My guess is that there’s a mismatch between the expected chunk size and what’s actually in the chunk (RLE vs uncompressed etc), which might be causing the issue. Are you able to open your sample files in Nuke without any problems? Or maybe try FCheck on a Windows machine? I could be wrong of course.

I could write out the binary layout if we can't reproduce it with other non-confidential files. May I ask—what software wrote the IFF files, maybe that's an easier way to the issue. After all, you are probably right there is a run over somewhere and a byte is all it needs. There must be a glitch in the code unless the original software you used writes IFF in a different way or with incorrect or non-aligned chunk sizes. If it works in Nuke etc it should work in my code.

This old legacy format is fun—if you had asked me to write it today, I would’ve done things slightly differently. 😄

The good news is that I have at least 2 different iff files that crash here -- they are RGBAZ (uint8 for RGBA, float for Z), RLE compression. The bad news is that they are production frames and I can't send them out.

They appear to be Maya playblast output. They're from different shows at different times, so I think maybe it won't be difficult for you to reproduce from scratch on your end?

Will try loading with Nuke and report back...

BTW, those failing frames do not crash in the mainline OIIO (not using this PR's patch)... but then again, I think it may simply be ignoring the Z channel entirely in that case, so I wouldn't say they are "correct" exactly.

Nuke does load it. I'm not sure how to verify that it interprets the depth values correctly.

Must be fixed! if the test files are coming from Maya it should be an easy thing to reproduce. You are able to open the earlier test file from Nicholas, it's also Maya and RGBAZ.

Nuke will scream in red ERROR if the Z buffer reading is off, my code must be off/ wrong somewhere. Let's try to find it, try to open the Nicholas file and see if you get the same problem. I will also switch between a few different configuration and see if I can trigger the issue.

Visualise depth in rgba mode using something like an Expression node putting in (-1/z)/max value in R channel and zero in G and B. Should give you an OK visualisation.

Are you doing something like a RGBAZ channels from iff => exr with oiiotool when testing?

Which earlier test file from Nicholas?

I fail just doing a straightforward iconvert foo.iff foo.exr or iinfo -v --stats or oiiotool, yes.

mikaelsundell · 2025-03-30T11:04:02Z

New version in place with both support for uncompressed data (not verified with real IFF/z-buffer files) and the use of spans to avoid buffer runovers. I also removed some redundant error messages. Hopefully this will either fire in debug or switch over to non compressed.

lgritz · 2025-04-07T22:46:46Z

Just to make sure we're all on the same page, this PR as it currently stands no longer crashes on my problem iff files, but it does (gracefully) error on the file with a "Could not read : uncompressed span overflow at offset ..."

lgritz · 2025-04-07T22:47:51Z

(Mikael and I have been discussing this off-channel, but wanted to be sure people knew progress was being made here.)

mikaelsundell · 2025-04-08T10:28:58Z

Thanks Larry, will be back this weekend to finish up, out traveling this week.

lgritz · 2025-04-08T19:11:18Z

Excuse my cluttering things up, but I'm using this PR as a guinea pig to request a GH Copilot review just to see what it says. Feel free to ignore any of it, this is just an experiment.

Copilot

Copilot reviewed 7 out of 8 changed files in this pull request and generated no comments.

Files not reviewed (1)

src/iff.imageio/CMakeLists.txt: Language not supported

Comments suppressed due to low confidence (2)

src/iff.imageio/iffoutput.cpp:165

The write_scanline function always returns false after a successful memcpy. Returning true here would indicate the scanline was written successfully.

return false;

src/iff.imageio/iffoutput.cpp:156

Verify that the updated check_open parameters (with a maximum channel value of 5) correctly support only RGB, RGBA, and RGBAZ formats as desired.

if (!check_open(mode, spec, { 0, 8192, 0, 8192, 0, 1, 0, 5 }))

mikaelsundell · 2025-04-12T09:34:32Z

Great! I'm back now, will finish this up in the next couple of days, will make sure I copilot my changes before commit. Let the bot battle begin! :-)

…ffers or depth maps Removed noproxy versions, no longer needed Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

Improved format precision handling: now falls back to UINT16 instead of UINT8 for high-precision formats Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

Added initial implementation of z-buffer decoding using span Added support for uncompressed z-buffer tiles Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

Fixed channel ordering issues in uncompressed data. Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

mikaelsundell · 2025-04-21T19:53:43Z

New commits, spans + reading and writing of uncompressed IFF data verified. I've tested both uncompressed + rle with nuke and photoshop and both works for RGBAZ in 8/16-bit.

Hopefully these are the updates we need to close this PR.

Updated uncompressed z-buffer writing to use spans Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

lgritz

Overall, LGTM and works on my iff files that once had trouble.

I did spot some problems with errorfmt calls getting confused about printf vs the newer format specifiers (throughout OIIO, all formatted string functions ending in "fmt" use the new syntax, not the old printf one). Those will need to get cleaned up, so some searching to be sure to find all of them.

With those changes, I think we are ready to merge this.

lgritz · 2025-04-30T02:30:23Z


    // helper to uncompress a rle channel
-    size_t uncompress_rle_channel(const uint8_t* in, uint8_t* out, int size);
+    size_t uncompress_rle_channel(span<uint8_t> in, span<uint8_t> out,


Let's make that cspan<uint8_t> in since it's const data?

lgritz · 2025-04-30T02:33:49Z

+                                                             in_span, tw * th);
+                        if (used > scratch_span.size()) {
+                            errorfmt(
+                                "RLE uncompress exceeds span size (channel byte %d)",


All the functions ending in fmt take the new std::format idioms, so this should be:

Suggested change

"RLE uncompress exceeds span size (channel byte %d)",

"RLE uncompress exceeds span size (channel byte {})",

lgritz · 2025-04-30T02:34:05Z

+
+                            for (uint16_t px = xmin; px <= xmax; ++px) {
+                                if (offset >= in_span.size()) {
+                                    errorfmt("in_span underflow at (%u, %u)",


Suggested change

errorfmt("in_span underflow at (%u, %u)",

errorfmt("in_span underflow at ({}, {})",

lgritz · 2025-04-30T02:34:25Z

+
+                                if (sl_span.empty()) {
+                                    errorfmt(
+                                        "scanline span overflow at (%u, %u)",


Suggested change

"scanline span overflow at (%u, %u)",

"scanline span overflow at ({}, {})",

lgritz · 2025-04-30T02:34:51Z

+
+                        if (pixel_offset + m_header.zbuffer_bytes()
+                            > scratch_span.size()) {
+                            errorfmt("in span overflow at pixel (%u, %u)", px,


Suggested change

errorfmt("in span overflow at pixel (%u, %u)", px,

errorfmt("in span overflow at pixel ({}, {})", px,

I'm not going to mark all of them, you can do it as well as I can

Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

lgritz

LGTM!

…SoftwareFoundation#4673) Added support for reading and writing ZBUF chunks, also known as Z-buffers or depth maps. Removed noproxy versions, no longer needed. --------- Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

mikaelsundell · 2025-05-12T10:49:52Z

Awesome 🙂

…SoftwareFoundation#4673) Added support for reading and writing ZBUF chunks, also known as Z-buffers or depth maps. Removed noproxy versions, no longer needed. --------- Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com> Signed-off-by: Scott Wilson <scott@propersquid.com>

…SoftwareFoundation#4673) Added support for reading and writing ZBUF chunks, also known as Z-buffers or depth maps. Removed noproxy versions, no longer needed. --------- Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

mikaelsundell changed the title ~~Added support for reading and writing z buffers~~ Improved IFF support reading and writing floating point z buffers Mar 19, 2025

lgritz reviewed Mar 19, 2025

View reviewed changes

mikaelsundell changed the title ~~Improved IFF support reading and writing floating point z buffers~~ Improved IFF support reading and writing z buffers Mar 19, 2025

lgritz reviewed Mar 28, 2025

View reviewed changes

lgritz requested a review from Copilot April 8, 2025 19:11

Copilot AI reviewed Apr 8, 2025

View reviewed changes

mikaelsundell added 9 commits April 21, 2025 21:25

Added support for reading and writing ZBUF chunks, also known as Z-bu…

fefcb05

…ffers or depth maps Removed noproxy versions, no longer needed Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

Removed pack_zbuffer_channel, no longer needed

54a3bb6

Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

Removed unnecessary error message for reading native tiles

6395ef9

Improved format precision handling: now falls back to UINT16 instead of UINT8 for high-precision formats Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

Fixed accidental removal of TypeDesc base_format

54f036e

Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

Removed unnecessary errorfmt - I/O errors are already handled by OIIO

4040e12

Added initial implementation of z-buffer decoding using span Added support for uncompressed z-buffer tiles Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

Fixed issue with pixel_bytes used for rowstride instead of zbuffer_bytes

7d53f51

Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

Refactored to use spans for input and output buffers.

38bea1d

Fixed channel ordering issues in uncompressed data. Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

Removed test code for non used variables

c4894aa

Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

clang format fix

60d03bd

Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

lgritz added enhancement Improvement of existing/working features. file formats Image file formats, ImageInput, ImageOutput labels Apr 21, 2025

Removed unused parameter in tests

5b72c41

Updated uncompressed z-buffer writing to use spans Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

lgritz requested changes Apr 30, 2025

View reviewed changes

Fixed incorrect usage of %d/%u instead of new syntax {} for errorfmt

8778996

Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>

mikaelsundell requested a review from lgritz May 11, 2025 12:34

lgritz approved these changes May 11, 2025

View reviewed changes

lgritz merged commit 946a9d3 into AcademySoftwareFoundation:main May 11, 2025
30 checks passed

BrewTestBot mentioned this pull request Oct 2, 2025

openimageio 3.1.6.1 Homebrew/homebrew-core#246692

Merged

	"RLE uncompress exceeds span size (channel byte %d)",
	"RLE uncompress exceeds span size (channel byte {})",

	errorfmt("in_span underflow at (%u, %u)",
	errorfmt("in_span underflow at ({}, {})",

	"scanline span overflow at (%u, %u)",
	"scanline span overflow at ({}, {})",

	errorfmt("in span overflow at pixel (%u, %u)", px,
	errorfmt("in span overflow at pixel ({}, {})", px,

Conversation

mikaelsundell commented Mar 19, 2025

Description

Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikaelsundell commented Mar 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgritz commented Mar 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikaelsundell Mar 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikaelsundell commented Mar 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgritz commented Apr 7, 2025

Uh oh!

lgritz commented Apr 7, 2025

Uh oh!

mikaelsundell commented Apr 8, 2025

Uh oh!

lgritz commented Apr 8, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

mikaelsundell commented Apr 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mikaelsundell commented Apr 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgritz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgritz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mikaelsundell commented May 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

mikaelsundell Mar 28, 2025 •

edited

Loading

mikaelsundell commented Mar 30, 2025 •

edited

Loading

mikaelsundell commented Apr 12, 2025 •

edited

Loading

mikaelsundell commented Apr 21, 2025 •

edited

Loading