PREMULTIPLIED IMAGES: Blender cannot save or load premultiplied images correctly. #28085

New Issue

Troy Sobotka · 2011-07-26T06:43:35+02:00

Troy Sobotka commented

2011-07-26 06:43:35 +02:00

%%%Every properly premultiplied image fails to load properly. Further, Blender is incapable of outputting an sRGB premultiplied image.

By "properly premultiplied" it refers to a proper premultiplied image. Blender does not generate properly premultiplied images as a result of a broken transformation on color management.

Blender cannot output an alpha channel image correctly. Further, any and all premultiplied images that are properly premultiplied, fail to load correctly.

What appears to be happening is that the premultiplied values are not being respected by the color management system. It would seem that the premultiplied semi transparent values are being incorrectly passed to the sRGB to linear transformation, where they should be divided prior to such a transformation for ingestion and premultiplied just after the linear to sRGB transformation for output.

If Blender fails to do so, the resulting images are broken. This can be verified by comparing any premultiplied image generated by Blender in an external program or within its own VSE. The fringes will be broken and either too bright or too dark. Conversely, if you load a properly premultiplied image in Blender, it is also impossible to display it correctly.

Blender's VSE however, staying out of the linear domain even when converting to float, behaves correctly. Blender is in fact incapable of loading any output from itself within the VSE as a result.

Solution would be to assert that an image is premultiplied prior to ingestion and convert to straight, perform the sRGB to linear conversion on the non multiplied RGB values, and then premultiply the result. A boolean radio box in the compositor should allow an artist to force a divide on an image load, and vice versa on an output composite.

In summary, Blender creates mangled premultiplied images that can only be loaded by itself within the compositor. Taking any image out of the compositor with alpha will result in a darkened set of fringes, including loading into the VSE.

%%%

%%%Every properly premultiplied image fails to load properly. Further, Blender is incapable of outputting an sRGB premultiplied image. By "properly premultiplied" it refers to a proper premultiplied image. Blender does not generate properly premultiplied images as a result of a broken transformation on color management. Blender cannot output an alpha channel image correctly. Further, any and all premultiplied images that are properly premultiplied, fail to load correctly. What appears to be happening is that the premultiplied values are not being respected by the color management system. It would seem that the premultiplied semi transparent values are being incorrectly passed to the sRGB to linear transformation, where they should be divided prior to such a transformation for ingestion and premultiplied just after the linear to sRGB transformation for output. If Blender fails to do so, the resulting images are broken. This can be verified by comparing any premultiplied image generated by Blender in an external program or within its own VSE. The fringes will be broken and either too bright or too dark. Conversely, if you load a properly premultiplied image in Blender, it is also impossible to display it correctly. Blender's VSE however, staying out of the linear domain even when converting to float, behaves correctly. Blender is in fact incapable of loading any output from itself within the VSE as a result. Solution would be to assert that an image is premultiplied prior to ingestion and convert to straight, perform the sRGB to linear conversion on the non multiplied RGB values, and then premultiply the result. A boolean radio box in the compositor should allow an artist to force a divide on an image load, and vice versa on an output composite. In summary, Blender creates mangled premultiplied images that can only be loaded by itself within the compositor. Taking any image out of the compositor with alpha will result in a darkened set of fringes, including loading into the VSE. %%%

Troy Sobotka commented

2011-07-26 06:43:35 +02:00

Changed status to: 'Open'

Troy Sobotka commented

2011-07-26 17:22:27 +02:00

%%%Attaching two sample screenshots showing how the premultiplication values are being incorrectly set during transform.

I've looked into the code for where srgb_to_linear is happening, but I am unsure as to how to determine if the image is flagged as premul.

Perhaps we need to offer a compositor toggle on input images to indicate that the artist desires a divide operation on the alpha data. This flag could be grabbed onto via the renderer as I believe, from what I can see in the code, that the images remain in their sRGB state up to the point of rendering where they are transferred to linear? Is this correct?

The flag approach appears common in compositing packages, perhaps to avoid issues such as this.

I have managed to add the flag boolean value, but I'd love some enlightenment on where in the code I'd need to check it? Should I put it everywhere there is an sRGB to linear transformation in the rendering stack?%%%

%%%Attaching two sample screenshots showing how the premultiplication values are being incorrectly set during transform. I've looked into the code for where srgb_to_linear is happening, but I am unsure as to how to determine if the image is flagged as premul. Perhaps we need to offer a compositor toggle on input images to indicate that the artist desires a divide operation on the alpha data. This flag could be grabbed onto via the renderer as I believe, from what I can see in the code, that the images remain in their sRGB state up to the point of rendering where they are transferred to linear? Is this correct? The flag approach appears common in compositing packages, perhaps to avoid issues such as this. I have managed to add the flag boolean value, but I'd love some enlightenment on where in the code I'd need to check it? Should I put it everywhere there is an sRGB to linear transformation in the rendering stack?%%%

Troy Sobotka commented

2011-07-26 20:07:05 +02:00

%%%I am delighted to report that the nut has indeed been cracked. I have loosely hacked and patched divers.c to test my limited case, and Blender renders properly after the hack.

I had to trace with printfs every srgb_to_linear transformation and came to the conclusion that to fix this bug we should only need to worry about ingestion and output via divers.c. Perhaps Brecht or Matt may have other concerns or thoughts on this matter. The code I changed for my sample using the provided TIFF attached here is as follows:

static void imb_float_from_rect_linear(struct ImBuf *ibuf, float *fbuf)
{
float *tof = fbuf;
int i;
unsigned char *to = (unsigned char *) ibuf->rect;

  printf("***IMB_float_from_rect_linear***\n");
  
  printf("*****FIRST VALUES - R: %d, G: %d, B: %d, A: %d*****\n", (float)tof[0], (float)tof[1], (float)tof[2], (float)tof[3]);

for (i = ibuf->x * ibuf->y; i > 0; i--) 
{
    if (TRUE) { *ibuf->profile == IB_PROFILE_SRGB_PREMUL) {*if (tof- [x] != 0) {
            tof- [x] = srgb_to_linearrgb( (((float)to- [x]) * (1.0f/255.0f)) / (((float)to- [x])*(1.0f/255.0f)) );
	        tof- [x] = srgb_to_linearrgb( (((float)to- [x]) * (1.0f/255.0f)) / (((float)to- [x])*(1.0f/255.0f)) );
	        tof- [x] = srgb_to_linearrgb( (((float)to- [x]) * (1.0f/255.0f)) / (((float)to- [x])*(1.0f/255.0f)) );
	        tof- [x] = ((float)to- [x])*(1.0f/255.0f);
	    //}
	    to += 4; 
	    tof += 4;
    }
    else {
	    tof- [x] = srgb_to_linearrgb(((float)to- [x])*(1.0f/255.0f));
	    tof- [x] = srgb_to_linearrgb(((float)to- [x])*(1.0f/255.0f));
	    tof- [x] = srgb_to_linearrgb(((float)to- [x])*(1.0f/255.0f));
	    tof- [x] = ((float)to- [x])*(1.0f/255.0f);
	    to += 4; 
	    tof += 4;

}

That effectively does the divide always. It isn't a patch, but merely to prove the point on the attached TIFF.

So my best line of thought currently is to provide a toggle value that allows the artist to set the image to be SRGB_PREMUL, adding the value to the IMB definitions.

The code would then if on the value and properly divide the source RGB values prior to linearization. It would also need to be duplicated for output.

If we manage to implement this, Blender can once and for all output alpha channel images to other applications and to its own VSE. I don't believe there is a single case where Blender can output images to even it's own VSE, and I've tested many.%%%

%%%I am delighted to report that the nut has indeed been cracked. I have loosely hacked and patched divers.c to test my limited case, and Blender renders _properly_ after the hack. I had to trace with printfs every srgb_to_linear transformation and came to the conclusion that to fix this bug we should only need to worry about ingestion and output via divers.c. Perhaps Brecht or Matt may have other concerns or thoughts on this matter. The code I changed for my sample using the provided TIFF attached here is as follows: static void imb_float_from_rect_linear(struct ImBuf *ibuf, float *fbuf) { float *tof = fbuf; int i; unsigned char *to = (unsigned char *) ibuf->rect; ``` printf("***IMB_float_from_rect_linear***\n"); printf("*****FIRST VALUES - R: %d, G: %d, B: %d, A: %d*****\n", (float)tof[0], (float)tof[1], (float)tof[2], (float)tof[3]); ``` for (i = ibuf->x * ibuf->y; i > 0; i--) { if (TRUE) { *ibuf->profile == IB_PROFILE_SRGB_PREMUL) {*if (tof- [x] != 0) { tof- [x] = srgb_to_linearrgb( (((float)to- [x]) * (1.0f/255.0f)) / (((float)to- [x])*(1.0f/255.0f)) ); tof- [x] = srgb_to_linearrgb( (((float)to- [x]) * (1.0f/255.0f)) / (((float)to- [x])*(1.0f/255.0f)) ); tof- [x] = srgb_to_linearrgb( (((float)to- [x]) * (1.0f/255.0f)) / (((float)to- [x])*(1.0f/255.0f)) ); tof- [x] = ((float)to- [x])*(1.0f/255.0f); //} to += 4; tof += 4; } else { tof- [x] = srgb_to_linearrgb(((float)to- [x])*(1.0f/255.0f)); tof- [x] = srgb_to_linearrgb(((float)to- [x])*(1.0f/255.0f)); tof- [x] = srgb_to_linearrgb(((float)to- [x])*(1.0f/255.0f)); tof- [x] = ((float)to- [x])*(1.0f/255.0f); to += 4; tof += 4; ``` } ``` } } That effectively does the divide _always_. It isn't a patch, but merely to prove the point on the attached TIFF. So my best line of thought currently is to provide a toggle value that allows the artist to set the image to be SRGB_PREMUL, adding the value to the IMB definitions. The code would then if on the value and properly divide the source RGB values prior to linearization. It would also need to be duplicated for output. If we manage to implement this, Blender can once and for all output alpha channel images to other applications and to its own VSE. I don't believe there is a single case where Blender can output images to even it's own VSE, and I've tested many.%%%

Troy Sobotka commented

2011-07-26 20:08:15 +02:00

%%%Also I suspect a divide by zero in that top brace. I know it is possible, but my non zero check code was seeming to be skipped for some reason.%%%

Troy Sobotka commented

2011-07-26 20:11:44 +02:00

%%%Attaching a proper output from Blender.%%%

Troy Sobotka commented

2011-07-26 20:47:50 +02:00

%%%Adding a sample test background for compositing that is useful to composite over to greatly illustrate Blender mangling.%%%

Ton Roosendaal commented

2011-07-29 18:19:43 +02:00

%%%Thanks for all the info. It's a known issue though, the srgb code that was added doesnt work for alpha.
To tackle it we really need to some time from Matt first, to carefully check (or reconfirm) design specs for things like IB_PROFILE_SRGB. Combined with that, alpha type storage has to be improved.

I've added this report to our todo list for future reference;
http://wiki.blender.org/index.php/Dev:2.5/Source/Development/Todo/Render#Rendering%%%

%%%Thanks for all the info. It's a known issue though, the srgb code that was added doesnt work for alpha. To tackle it we really need to some time from Matt first, to carefully check (or reconfirm) design specs for things like IB_PROFILE_SRGB. Combined with that, alpha type storage has to be improved. I've added this report to our todo list for future reference; http://wiki.blender.org/index.php/Dev:2.5/Source/Development/Todo/Render#Rendering%%%

Ton Roosendaal commented

2011-07-29 18:19:43 +02:00

Changed status from 'Open' to: 'Archived'

Ton Roosendaal closed this issue

2011-07-29 18:19:43 +02:00

Troy Sobotka commented

2011-07-29 18:43:02 +02:00

%%%Not to belabor the point, but to clarify, the "the srgb code that was added doesnt work for alpha." is fundamentally incorrect.

This is a bug, and one that is relatively easily fixed. In short, the sRGB code is 100% fine with the exception being that linearization is being applied to the premultiplied RGB values. The solution is listed above, and it is very simple - divide the data first.

Sorry if I wasn't clear.%%%

%%%Not to belabor the point, but to clarify, the "the srgb code that was added doesnt work for alpha." is fundamentally incorrect. This is a bug, and one that is relatively easily fixed. In short, the sRGB code is 100% fine with the exception being that linearization is being applied to the premultiplied RGB values. The solution is listed above, and it is very simple - divide the data first. Sorry if I wasn't clear.%%%

Troy Sobotka commented

2011-07-29 18:53:45 +02:00

%%%The code above is purely a test - it divides always, which is incorrect obviously. It should ONLY divide on a premultiplied image.

The proper approach to this is use a boolean artist value "Premultiplied" that instructs the code to divide prior to linearization. This aligns with current practices outside of Blender.

My concern is that IB_premul doesn't appear to be used much of anywhere.

I think I can write the fix for this and it is not too invasive. It should also go hand-in-hand with OCIO integration.

The fundamental question is: Is IB_premul the correct flag to use here?%%%

%%%The code above is purely a test - it divides always, which is incorrect obviously. It should ONLY divide on a premultiplied image. The proper approach to this is use a boolean artist value "Premultiplied" that instructs the code to divide prior to linearization. This aligns with current practices outside of Blender. My concern is that IB_premul doesn't appear to be used much of anywhere. I think I can write the fix for this and it is not too invasive. It should also go hand-in-hand with OCIO integration. The fundamental question is: Is IB_premul the correct flag to use here?%%%

Xavier Thomas commented

2011-08-19 01:28:53 +02:00

%%%Hi, I added a patch: image_unpremul.diff

This patch modify the image propertires.

Before: the user had the option to laod the file with alpha as it is in file or to premutliply alpha at load time.
Now: the user can load the image with alpha, "as it is in file", premultiply at load time or unpremultiply at load time.
Note1: all this is done BEFORE colormanagement is applied (sRGB->linear) to stay correct.
Note2: using this with 8bits images can genertae banding and clipping.

This patch also uncomment a chunk of code in the image input node that was premultiplying a second time when it should not.

This patch does not change how the image viewer works. %%%

%%%Hi, I added a patch: image_unpremul.diff This patch modify the image propertires. Before: the user had the option to laod the file with alpha as it is in file or to premutliply alpha at load time. Now: the user can load the image with alpha, "as it is in file", premultiply at load time or unpremultiply at load time. Note1: all this is done BEFORE colormanagement is applied (sRGB->linear) to stay correct. Note2: using this with 8bits images can genertae banding and clipping. This patch also uncomment a chunk of code in the image input node that was premultiplying a second time when it should not. This patch does not change how the image viewer works. %%%

Xavier Thomas commented

2011-08-19 01:54:54 +02:00

%%%>This patch also uncomment a chunk of code in the image input node that was premultiplying a second time when it should not.
I ment: This patch COMMENT ...%%%

%%%>This patch also uncomment a chunk of code in the image input node that was premultiplying a second time when it should not. I ment: This patch COMMENT ...%%%

Xavier Thomas commented

2011-08-19 02:13:19 +02:00

%%%Added image_unpremul2.diff which fixes a typo.%%%

Xavier Thomas commented

2011-08-19 03:02:15 +02:00

%%%Added image_unpremul3.diff which fixes a spelling error and RNA description string.
The patch was test by Troy (and by me) %%%

%%%Added image_unpremul3.diff which fixes a spelling error and RNA description string. The patch was test by Troy (and by me) %%%

Troy Sobotka commented

2011-08-19 03:57:26 +02:00

%%%Tested and I can say that with Xavier's patch Blender can finally properly load the premul alpha correctly.

Also attaching my one line patch (well one line plus Xat's) to expose the alpha property in the input Image node.%%%

%%%Tested and I can say that with Xavier's patch Blender can _finally_ properly load the premul alpha correctly. Also attaching my one line patch (well one line plus Xat's) to expose the alpha property in the input Image node.%%%

Xavier Thomas commented

2011-08-24 01:50:52 +02:00

%%%When discussing in IRC with Ton and Brecht, they was worried that this patch is just a workaround, and that it would be better to tag image so that Blender internally would know when an image in key alpha or premul alpha (Actually blender assume all image to be premul alpha). After thinking and reading about it I really came to believe that this is really unnecessary. And I will try to explain why in the next points:

(Un)/Premultiplying an image are lossy operations, they should not be done just based on a tag. It is better that the user forces it, and if a user want to force a unpremul than he sure knows about it and other parts of blender should not care about it.
Blender users need (and will always need) a way to force an "unpremul" operation before colormanagement is applied (usefull sometimes for compositing). That is just what this patch does and it does it at the right place in a non intrusive way. This piece of code will always be needed , it fits well in the code-base and have no chances of coming back and haunt you later with difficult maintenance or introducing regression.%%%

%%%When discussing in IRC with Ton and Brecht, they was worried that this patch is just a workaround, and that it would be better to tag image so that Blender internally would know when an image in key alpha or premul alpha (Actually blender assume all image to be premul alpha). After thinking and reading about it I really came to believe that this is really unnecessary. And I will try to explain why in the next points: - (Un)/Premultiplying an image are lossy operations, they should not be done just based on a tag. It is better that the user forces it, and if a user want to force a unpremul than he sure knows about it and other parts of blender should not care about it. - Blender users need (and will always need) a way to force an "unpremul" operation before colormanagement is applied (usefull sometimes for compositing). That is just what this patch does and it does it at the right place in a non intrusive way. This piece of code will always be needed , it fits well in the code-base and have no chances of coming back and haunt you later with difficult maintenance or introducing regression.%%%

Brecht Van Lommel commented

2011-09-19 13:48:54 +02:00

%%%I still think this is the wrong solution. The convention is to use premulitiplied alpha in Blender, and we start saying, you should import images as key alpha to get correct color management, that confuses the situation more. It's not perfect now, but we should fix operations to assume the buffers are premultiplied alpha, not get us further into a situation where some things need key and others needs premul alpha. That is exactly where I think it will come back to haunt us later.

I think the color management code should be changed to assume the buffer is using premultiplied alpha.%%%

%%%I still think this is the wrong solution. The convention is to use premulitiplied alpha in Blender, and we start saying, you should import images as key alpha to get correct color management, that confuses the situation more. It's not perfect now, but we should fix operations to assume the buffers are premultiplied alpha, not get us further into a situation where some things need key and others needs premul alpha. That is exactly where I think it will come back to haunt us later. I think the color management code should be changed to assume the buffer is using premultiplied alpha.%%%

Troy Sobotka commented

2011-09-19 16:11:47 +02:00

%%%Conversely then there should be a large blinking warning to not ever use premultiplied sources in Blender, nor to save them if the intended destination is the VSE or an external program.

%%%

%%%Conversely then there should be a large blinking warning to not ever use premultiplied sources in Blender, nor to save them if the intended destination is the VSE or an external program. %%%

GuillermoEspertino commented

2011-09-19 19:15:07 +02:00

%%%@brecht: File formats do that. If you take, for instance, a proper PNG (at least, following the spec which is what most of the applications do) it always stores unassociated alpha. Blender has to deal with that (and actually it does, hence the "premul" switch to premultiply straight alpha textures, for instance).
The problem here is that Blender's CM is always linearizing the inputs before taking care of how alpha is stored.
If you want to fix blender's pipeline to always use premultiplied alpha, then you'll have to remove support for straight alpha formats (and it won't be a solution either, since some formats like tiff can have associated or unassociated alpha).
If there's a better way to manage alpha premultiplication than adding an option to switch between associated/unassociated alpha when importing the file, then no other compositing package figured it out yet. They all offer the ability to select the alpha "mode" upon importing the assets.
As long as you support different file formats you have to deal with that. You can't avoid it.%%%

%%%@brecht: File formats do that. If you take, for instance, a proper PNG (at least, following the spec which is what most of the applications do) it always stores unassociated alpha. Blender has to deal with that (and actually it does, hence the "premul" switch to premultiply straight alpha textures, for instance). The problem here is that Blender's CM is always linearizing the inputs before taking care of how alpha is stored. If you want to fix blender's pipeline to always use premultiplied alpha, then you'll have to remove support for straight alpha formats (and it won't be a solution either, since some formats like tiff can have associated or unassociated alpha). If there's a better way to manage alpha premultiplication than adding an option to switch between associated/unassociated alpha when importing the file, then no other compositing package figured it out yet. They all offer the ability to select the alpha "mode" upon importing the assets. As long as you support different file formats you have to deal with that. You can't avoid it.%%%

Troy Sobotka commented

2011-09-29 07:12:22 +02:00

%%%Based on the sagely advice of Brecht, I've created the following patch. It deals with ingestion of images via an artist selectable toggle stored in the Image and transferred to the ImBuf on copies, duplications, etc. It also uses the existing color management flag in RenderData for controlled output via the Shading box where Color Managment is selectable.

In summary, there are two artist driven toggles. The first deals with ingestion via the Image itself. The second via the Scene properties for output.

This combination allows for accurate conversion from existing premultiplied alpha images, from existing mangled Blender premultiplied images, and permits correct saving of premultiplied images.

The outstanding TODOs on this patch would be to likely deal with textures and other areas that IMB_float_from_rect is not used. I am quite certain that the myriad of labyrinthine code twists that have spawned up in Blender also have some edge cases that are not covered.

That said, this patch at least allows Blender to input and output premultiplied alpha images correctly without being too intrusive.%%%

%%%Based on the sagely advice of Brecht, I've created the following patch. It deals with ingestion of images via an artist selectable toggle stored in the Image and transferred to the ImBuf on copies, duplications, etc. It also uses the existing color management flag in RenderData for controlled output via the Shading box where Color Managment is selectable. In summary, there are two artist driven toggles. The first deals with ingestion via the Image itself. The second via the Scene properties for output. This combination allows for accurate conversion from existing premultiplied alpha images, from existing mangled Blender premultiplied images, and permits correct saving of premultiplied images. The outstanding TODOs on this patch would be to likely deal with textures and other areas that IMB_float_from_rect is not used. I am quite certain that the myriad of labyrinthine code twists that have spawned up in Blender also have some edge cases that are not covered. That said, this patch at least allows Blender to input and output premultiplied alpha images correctly without being too intrusive.%%%

Troy Sobotka commented

2011-09-30 20:53:01 +02:00

%%%Updated the patch to include Brecht's design considerations.

Added div by zero case check that should also serve to preserve RGB values in a premultiplied image when alpha is set to 0.
Migrated flags to both DNA_image_types.h and IMB_imbuf_types.h respectively.
Retained individual int flag variables cm_flags to allow for dereferencing sync as seen in image functions. (EG tempimage = *sourceimage)%%%

%%%Updated the patch to include Brecht's design considerations. * Added div by zero case check that should also serve to preserve RGB values in a premultiplied image when alpha is set to 0. * Migrated flags to both DNA_image_types.h and IMB_imbuf_types.h respectively. * Retained individual int flag variables cm_flags to allow for dereferencing sync as seen in image functions. (EG tempimage = *sourceimage)%%%

Troy Sobotka commented

2011-12-22 08:31:41 +01:00

%%%Updated patch to r42820.%%%

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

PREMULTIPLIED IMAGES: Blender cannot save or load premultiplied images correctly. #28085