DDS:

• I have proposed changes/fixes to OpenImageIO that fix various cases where it does not handle certain DDS correctly, compared to D16087. https://github.com/OpenImageIO/oiio/pull/3573
• Will do performance measurements next; very initial look does not look terribly good though. Will need to dig more.

Personally wouldn't worry too much about the thumbnailing, except for EXR. That format gave us actual bug reports and needed special treatment. But then I only added same for JPEG and WebP for completeness. Although the thumbnailing code for those latter formats are a (slight) improvement, I doubt anyone would notice if that was removed.

DDS: the various tweaks/fixes for OIIO DDS support as mentioned above have landed to upstream, and were just released as OIIO v2.4.4.2.

DDS performance: I've done a quick attempt at replacing Blender's DDS ImBuf code with OIIO. Initial results were that OIIO code was about 4x slower at loading DXT/BCn compressed DDS files compared to D16087. I did some low hanging fruit in OIIO itself to speed that up (just landed in master as #3583), but that still leaves OIIO at around 2x slower compared to Blender code. Getting the remaining speedup would be much more involved due to how OIIO is structured (there are some memory copies that are hard to avoid).

Something like this might be an issue for other formats that we'd want to switch to OIIO, particularly the ones that are "fast to decode" and then an extra one or two memory copies done by OIIO are a substantial cost. Would probably have to be evaluated for each and every format.

Png and jpg would perhaps be the two formats where performance actually is important. Mainly as these are used as playback caching.

@Jeroen Bakker (jbakker) - Png and jpg would perhaps be the two formats where performance actually is important. Mainly as these are used as playback caching.

Don't quote me, because it has been a while since I looked at this. But using EXR for that type of use could be worth investigating one day. Depends on the formats and compression, but it might be possible that it can give us both smaller file sizes AND faster reads.

Someone had mentioned (Troy probably) that EXR was fast reading and I looked into the idea of using that for our preview thumbnails. Compared to our current PNGs they were smaller and much faster to read. I was quite surprised.

In T101413#1428643, @Harley Acheson (harley) wrote:

EXR was fast reading and I looked into the idea of using that for our preview thumbnails. Compared to our current PNGs they were smaller and much faster to read. I was quite surprised.

It's not surprising. PNG is mostly bottlenecked by a completely serial zlib decoding process (which means PNG reading/writing can't be multi-threaded, at all). Whereas EXR splits up the image into chunks of 16-pixel rows and compresses them (when using "zip" aka zlib compression) separately. Which is exactly what allows it to use multi-threading on both reading & writing. This did not use to be a big thing back when disk I/O was the bottleneck, but all that has changed with SSDs.

I think this is great for all the reasons listed. Additionally, potential benefits are consistency with Hydra, shipping OIIO python bindings with the same supported file formats, and maybe even replacing our ImBuf with oiio::ImageBuf.

For thumbnails reading we can keep our own code for a bit, but it doesn't have to delay replacing the rest of the JPEG/WebP/OpenEXR code. IRIS support we could consider removing entirely, it's really only there for historical reasons at this point.

@Brecht Van Lommel (brecht) I'm probably at the point where I can start prepping the individual formats for review this week. I'd start with the support code and the PSD format since that already uses oiio. After that, things can go as fast or as slow as we'd like depending on the time of reviewers with higher priority items on their plates. There's a few items remaining to work through though: Testing and dependencies.

Testing
For testing image loading, I'm using the regular python unittest framework to open each file and then compare Image attributes with what I expect (is_float, colorspace, channel count, and alpha_mode). There's no comparison of what the image looks like however. We'd need a picture of what things look like in the Image Editor or similar. I'm not sure what the best option is here.

For save testing, I have a separate script which can create and save the images as necessary. They're created from scratch with a script and each image contains alpha and pixel values either clamped at 1 for 8bit or >1 for float buffers. From there I can use idiff for comparison against the saved references. But this doesn't fit with the render_test system very well actually. Should I just use the unittest framework here too and perform all the idiff calls myself? The main loss would be the nice html report unless I also implement that piece too.

Dependencies
Lastly, when it comes time to do PNG, oiio becomes a de-facto required dependency for Blender (e.g. the icons are loaded through PNG). At minimum we'd need to prevent the ability to disable that cmake option, and require it for building, but maybe this is a larger discussion point?

I'm not entirely sure what it means to review this per file format, I was imaging it to be one change where a single load/save code replaces a bunch of file formats. But maybe it's more convenient to do it in smaller parts, it's not clear to me.

For testing image loading, I would compare against a saved OpenEXR reference image. I think what we need to test is that the scene linear values match, display is a different concern I think.

If using the render test mechanism doesn't work well, it's fine to use a separate Python script. But it should still use the same BLENDER_TEST_UPDATE=1 environment variable as we have for renders and modifiers to update tests. In general it's convenient to be able to add an extra image for testing to some folder without having to update a Python file along with it.

Our own PNG loading code is something we might want to keep for lite builds, so OIIO is not strictly a required dependency.

Think we may need to re-evaluate the purpose of the lite build, afaik it was always meant as something that builds fast with minimal features. I wouldn't oppose making oiio a required dep (but that be an admin call to make) leading to a slightly more featured lite, as long as it doesn't impact build time, which i expect would be rather minimal especially compared to some non optional components of a lite build like geometry nodes (seriously, why is this a non-optional component? it's *very* expensive to build) nor am i expecting any regressions in link time , since we're going dynamic with oiio in 3.5. footprint on disk will be slightly larger, but afaik minimal footprint was never really a goal of a lite build?

I think lite is also about keeping Blender easy to build against system libraries. It's not something I personally care much about, but would like to have opinion of maybe @Campbell Barton (campbellbarton), @Sergey Sharybin (sergey) or @Bastien Montagne (mont29) about this.

The formats maintain their own is_a, load, and save functions still - so no single loader/saver. This was necessary as the saves are very different, some of the loads require specific ImBuf manipulation, formats like DDS have additional code all to their own, and to aid in the review so folks just need to worry about 1 format at a time. I published the WIP all-up patch here: D16640: WIP: Use OpenImageIO for loading and saving nearly all image formats

For the image load test, I'm not sure how to do the comparison though. During development there were many times the image would load incorrectly (too dark, washed out, or ) due to missing a setting on the ImBuf or oiio etc. Would I load both the format image and the exr reference and do a pixel-by-pixel check in python?

Ok, I see that to keep things strictly compatible some code file format. I think it may be easiest to review this in 3 steps like: add tests, add OIIO utilities, replace file formats. I don't mind having it split up per file format if you prefer, it's just not needed from my side as a reviewer.

For the loading test, I was thinking you load the image, save it as EXR, then compare that to a reference EXR. If the EXR saving code is correct, then I think that should catch cases where the image was too dark, washed out, etc.

Looking at the test in bl_imbuf_load.py, the way I would make that work is to write out both an EXR and a text file (with "is_float" and other metadata), per image. And then compare both to reference files, generated by running tests with BLENDER_TEST_UPDATE=1. That way we can easily add images for testing or add a lot of additional metadata to the text file.

Sorry, was a bit busy to reply faster.

I do not any issues of making OIIO a required dependency. It is easily available on majority of distros now, and I do not see it as a complication of installing libopenimageio-dev alongside with other dependencies.

In fact, I'd rather see core to be streamlined and the amount of permutations reduced. In my experience, those fine-grained optional features and ifdef are crippling the code, introduce compilation errors and bugs, and overall introduce more issues than they solve.

Am also totally fine making OIIO a mandatory dependency of Blender, as already said this is an easily available library these days.

Alright, I've implemented the tests with the suggestions above. Thanks for the guidance! D16657: Tests: Add tests for image format saving and loading

The 3 checkin approach of tests, then support code, then all formats is ok with me. It's actually a bit simpler so it's entirely up to what reviewers would like.

Lastly, if push comes to shove, we don't have to rush this in for 3.5. I'll be around and able to make changes but if folks have other higher priority things to do, especially considering the holidays, than by all means this can wait until the next train.