Use OpenImageIO for loading and saving nearly all image formats #101413
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
18 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#101413
Loading…
Reference in New Issue
No description provided.
Delete Branch "%!s(<nil>)"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Proposal
Use OpenImageIO (OIIO) to manage our image loading/saving code for nearly all supported formats.
Re-organized this task to better indicate the status now that the groundwork and most formats were completed. See text below the table for the original Plan, Design, and Implementation sequence.
Status
The following table outlines the fate of each format given the Goals and Issues below.
Original
Pros:
Cons:
Unknown:
Why now
OIIO 2.4.x introduces complete support for "IO proxies" which allows it to read/write images directly in memory. This is required for fitting in with Blender's current systems and was a key point of contention in the past.
Recent work in the DDS space has caused this topic to surface again: #101405 (DDS image format improvements)
Implementation Goals
Goals
Non-Goals
Approach
Each format will retain custom load/save entry points inside the format registration array:
ImFileType IMB_FILE_TYPES- [ ]insidefiletype.cAll the machinery surrounding usage of that system will remain unchanged.The entry points for each format will use simple, declarative, APIs from OIIO to setup the proper configuration for the operation in question.
They will then call into newly created OIIO read/write routines to handle all the processing in a centralized and standardized way (i.e. common handling of
IB_test,IB_mem,IB_metadataflags etc.)Open Issues
Issue 1 Efficient thumbnail extraction doesn't exist in OIIO
Recent additions to Blender provide efficient thumbnail extraction and generation for JPEG, EXR, and WebP for the file browser. We will extract embedded thumbnails directly from the file if they exist, or will generate them otherwise, and do so as efficiently as possible by making use of size-hints that the low-level APIs can use to prevent the need for additional memory or resizing of images.
OIIO does have a
get_thumbnailAPI, however it is insufficient for our needs due to two reasons. The first is that it doesn't exist for the formats we would need for parity. The second is that it currently doesn't have any size-hinting and adding it would be considered a breaking change which means it wouldn't show up until OIIO 2.5.Issue 2 JPEG-2000 feature parity
Several of JPEG-2000's options aren't supported by OIIO: Compression quality, YCC colorspace, and
OPJ_CINEMA2K_48are not present. Additionally the "J2K" codec must be saved as .j2k (not .j2c) or otherwise it'll be processed incorrectly.Closed Issues
Issue 3 DDS textures
It turns out that Eevee makes use of the raw DDS compressed data in certain scenarios. Blender will load in the uncompressed data as well as the compressed data. This was very surprising to discover! We can keep doing this but we'll have to step a little bit outside of pure OIIO during loading. Either way we can still maintain parity here with a very small portion of low-level code.
DDS normal map compressed formats are now supported as of recent OIIO 2.4.4.2 and should no longer be an issue
Plan
The code required for each image format is well-insulated and can be checked in individually.
The common reading/writing code will be checked in alongside the first format to be converted.
Possible checkin sequence:
Cleanup: Remove
IMB_gettileand related support functions as they are dead code and would complicate the TIFF conversionCleanup: Fix slightly error-prone colorspace handling in the load loop (see note below)
Per-Format conversion
In Parallel
Notes
IB_testflag at all or not optimally. These will be fixed during their conversion.IB_mem. We will get this for "free" for all formats during their conversion.effective_colorspacevariable is not cleared between iterations. It is possible for a file to load far enough to set a colorspace but then fail shortly afterwards. In practice this probably wouldn't happen though.Risks
Testing
Image handling is extremely nuanced. Proper review and testing will take time. All of the following must be checked and validated with folks familiar in the matter:
channels/planes(images can "look" ok but these details can differ)The above items need to be inspected in 2 situations: 1) With an older blender version loading newly saved images and 2) With the new blender version loading previously saved images. (i.e. general cross testing)
Automation to prevent regressions would be nice. I'm not sure what's possible/allowable with our frameworks for verifying that list above. If we want to test all formats and all options we're looking at ~100 separate image files that would need to be verified.
Performance
This is mostly for Blender-specific scenarios as Cycles has always depended on OIIO for bulk load of texture data.
A quick test of the Playback caching scenario using
Timeline->Playback->Play Every Frameto force-load all the images did not show anything outright broken at least for PNGs so far:100 frame 1080p Image sequence of PNGs
master Took 9.05 seconds
prototype Took 8.94 seconds
100 frame 4k Image sequence of PNGs
master Took 20.1 seconds
prototype Took 20.1 seconds
Q/A
Q. Is it worth doing if we can't convert all the formats?
A. PSD files are already using OIIO and Cycles already uses OIIO for all but a few situations. Even if we don't get all of the formats, we're still looking at thousands of lines of code that can be removed. Considering that the new code would be simpler and more declarative, it makes the proposal worth exploring at least.
Q. What happens if the thumbnail issue isn't solved?
A. There's 2 middle grounds that we could explore:
- Skip those formats entirely but convert all the others
- Use OIIO for Load/Save for those formats but keep the thumbnail code using the low level libraries
The second option has the benefit that we'll still reduce some unneeded code. Both options have the downside that Blender still requires the library to be linked in.
Q. What about perf?
A. Beyond the scenarios mentioned above there's the following:
.is_acheck might be slower. OIIO does fast signature checks but if that passes it then proceeds to do more invasive work before returning. I don't know how much this will affect things in practice. The.is_achecks are done in 2 cases -- thumbnails (sigh…) and some packed file scenarios.Working examples
Current all-up diff here now: D16640: WIP: Use OpenImageIO for loading and saving nearly all image formats
Added subscriber: @deadpin
Changed status from 'Needs Triage' to: 'Confirmed'
Added subscribers: @aras_p, @Jeroen-Bakker
Added subscriber: @EAW
Added subscriber: @Dangry
Added subscriber: @LazyDodo
Added subscriber: @TheRedWaxPolice
Added subscriber: @Harley
Added subscriber: @Yuro
Added subscriber: @GeorgiaPacific
Added subscriber: @ThomasDinges
DDS:
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.
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.
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.
Added subscriber: @HooglyBoogly
Added subscriber: @Baardaap
Added subscriber: @brecht
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
ImBufwithoiio::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.
Added subscriber: @SteffenD
@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
idifffor comparison against the saved references. But this doesn't fit with therender_testsystem very well actually. Should I just use the unittest framework here too and perform all theidiffcalls 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=1environment 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?
Added subscribers: @mont29, @Sergey, @ideasman42
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 @ideasman42, @Sergey or @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 withBLENDER_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.
Glad to see this being worked on. Another issue this will solve is with BMPs. Blender currently only supports this format partially. Only
BI_RGBcompression seem to be supported.BI_BITFIELDSwhich is another commonly used method along with RLE isn't supported, also losing support for an alpha channel. OpenImageIO seems to have much wider support for Bitmaps.