Painting on multi-layer OpenEXR files
Needs RevisionPublic
Actions

Authored by Christian Schoenebeck (Crudebyte) on Mar 15 2015, 6:18 PM.

Details

Reviewers

Sergey Sharybin (sergey)

Summary

Short Description: This patch extends Blender's internal paint tool for being used with multi-layer OpenEXR files.

NOTE: I posted this patch on IRC in December (2014), since I had no time at that point to go through the hassle of creating a Blender dev account here, etc. Not sure if anybody applied this patch in the meantime or not.

Revision Context: This is a patch against the Blender 2.72b tarball release version.

Background: The paint tool of Blender <= 2.72b is limited to ImageBufs with exactly 4 channels. Since most render passes (except the "combined" and "vector" passes) however are stored to and loaded from .exr files with 3 channels and less, the paint tool did not allow to paint to most render pass layers of .exr files at all and showed an error message "Image requires 4 color channels to paint" on user's brush actions instead. This patch implements generic painting on ImageBufs with arbitrary amount of channels to resolve this usability constraint regarding OpenEXR files. The old blend functions, optimized for 4 channels are still used if the provided ImageBuf is using 4 channels.

From usability point of view, I find this an important fix regarding OpenEXR compatibility with Blender. Because currently there is no open source solution at all to paint to multi-layer OpenEXR files. There is apparently an OpenEXR plugin available for Gimp, however that Gimp plugin only provides import functionality, which will truncate the color channels to 8 bit resolution, which actually renders it useless, because it destroys the purpose of the OpenEXR format of keeping the data in HDR during the entire post processing process.

See also my initial post about this issue on Stackexchange:
http://blender.stackexchange.com/questions/19600/painting-on-openexr-multilayer-images

I am using this patch for more than 3 months in production now. Encountered no negative issues so far.

Diff Detail

Event Timeline

Christian Schoenebeck (Crudebyte) updated this revision to Diff 3751.Mar 15 2015, 6:18 PM

Christian Schoenebeck (Crudebyte) retitled this revision from to Painting on multi-layer OpenEXR files.

Christian Schoenebeck (Crudebyte) updated this object.

Christian Schoenebeck (Crudebyte) added a reviewer: Campbell Barton (campbellbarton).Mar 15 2015, 6:26 PM

The gen functions seems to be weird to me.

First of all. why are they limited to 4 channels of source2 only? For example, considering blend_color_add_float_gen, you can totally get a sum of two RGA images, no issues what's so ever.

Second, they are incorrect when you've got RGBA source and BW output. Current implementation will use R as the BW value, but in fact it should use luminance of the color instead.

Third, i'm not sure we need to have dedicated functions. All those functions are used by painting only by the looks of it, so why not to always pass number of channels and let functions to do optimized part when all source and destination are 4 channels?

This revision now requires changes to proceed.Mar 18 2015, 2:35 PM

Campbell Barton (campbellbarton) added inline comments.Mar 18 2015, 2:41 PM

source/blender/blenlib/intern/math_color_blend_inline.c
1179	Theres a reasonable amount of code here yet... When the source isn't 4 channels, NOP's run. RGBA -> single channel isnt doing color conversion. Behavior for RGB -> RGB is doing NOP's when it can do something useful in some cases. Theres a lot of effort here for something which is working rather questionably with mixing different channel counts. It could be less hassle just to copy channels to `float[4]`, operate on them. then copy back.
source/blender/imbuf/intern/rectop.c
695–708	Would just have add `mul_vn_vnfl` math function and not bother with switch. (also for `switch (sbuf->channels)` below)

In D1186#21519, @Sergey Sharybin (sergey) wrote:

The gen functions seems to be weird to me.

First of all. why are they limited to 4 channels of source2 only? For example, considering blend_color_add_float_gen, you can totally get a sum of two RGA images, no issues what's so ever.

Well, "noop" is a bit incorrect term in this case. Actually I just took that "noop" code block and its comment over from the original 4 channel implementations. The actual "noop" behavior differs from function to function. The added generic functions are also not limited to 4 channels, they also work on buffers with more than 4 channels. However they assume that an alpha channel is always the 4th channel (that is zero based index 3). I took that xxxA assumption because:

I did not see any indicator in the current data structures about which channel shall be the alpha channel, and I thought fixing this "the correct way" would require a lot more changes in the entire code base of Blender, because wherever ImageBufs are created/initialized by Blender, additional code would be needed to set such a new member variable regarding the index of the alpha channel in the ImageBuf.
From what I saw all buffers with alpha channels in Blender were RGBA, so I thought that i.e. RGA would be rather theoretical.
"Paint tool on OpenEXR multi-layer files" seems to be the only use case for generic blend functions in Blender so far.

Or in other words the xxxA assumption was made as a decision due to the ratio of usefullness / effort.

Second, they are incorrect when you've got RGBA source and BW output. Current implementation will use R as the BW value, but in fact it should use luminance of the color instead.

Sure, there might be things to fix here and there. The amount of code involved is not small, so it is definitely not perfect, but I needed some kind of fix for this huge use case constraint regarding OpenEXR files. It was a real show stopper during production not being able to modify pixels of OpenEXR layers directly with any kind of existing open source tool.

Third, i'm not sure we need to have dedicated functions. All those functions are used by painting only by the looks of it, so why not to always pass number of channels and let functions to do optimized part when all source and destination are 4 channels?

Because I though that people might complain if would remove the existing 4 channel optimized functions, i.e. in case they are used / going to be used at places in the code base where always only 4 channels are actually used. Because at such places it would decrease performance if the generic functions must be used instead. And I saw no issue in just leaving the old 4 channels functions.

Why did I not pack the 4 channel optimized code into the generic functions? Because like stated above, those generic blend functions are probably not used in other parts of Blender that soon, except the internal paint tool. And for the latter, the user does not feel any performance difference whether a channel optimized implementation is used or not.

Unfortunately I can't provide another revision at this point due to my current schedules. I just posted this patch here recently in case somebody might find it useful, encountering the same usability constraint we had with OpenEXR and Blender paint tool. For me the provided patch did the job and the concerns you came up with are partly right, but are rather of theoretical nature IMO.

source/blender/blenlib/intern/math_color_blend_inline.c
1179	I saw no easy way to just copy channels to float 4 and back. That was actually my first thought as well. It would require a lot of code changes all over the place in Blender and you would need to dig a lot of holes to pass informations about what the "original" format was, etc. So after reviewing the current code base I decided that it was easier to introduce generic functions, which might be incorrect in certain aspects here and there, definitely, but I thought these issues could be fixed with certain iterations of development when time comes.
source/blender/imbuf/intern/rectop.c
695–708	Sure.