[Bf-committers] REVIEW: Predivide for premultiplied images patch. ATTN: Brecht

[Brecht Van Lommel]

Hi,

A quick review:

* The BLI_CM_ should not be in blenlib, there should be separate flags
in DNA_image_types.h and IMB_imbuf_types.h. This adds a bit more code,
but these things should be decoupled.
* I'd also be inclined to not add a separate cm_flags but use the
existing flag members in ImBuf and Image.
* Predivide should check if the alpha is zero and avoid dividing in such cases.
* It would help performance to compute invalpha = 1.0f/alpha; and then
multiply by that instead of doing 3 divisions.
* I think the predivide flag from the scene should be set in a few
more places, like for compositing out nodes, viewer nodes, .. . Now
it's only being done on rendering an animation to disk.
* It's also not clear to me yet that the predivide happens in all
places it's needed, will need to check this more closely.

I think of this as a temporary solution that we have to leave off by
default for compatibility now, but should be turned on by default once
we have a better premul/key system, where we actually know which
buffers are in which alpha format.

Brecht.

On Thu, Sep 29, 2011 at 7:30 AM, Troy Sobotka <troy.sobotka at gmail.com> wrote:
> In the ever ongoing deluxe saga of trying to get Blender to correctly
> ingest premultiplied alpha images such as TIFFs, the following patch
> is offered for review. Special thanks go to Brecht for his extreme
> patience and sagely advice.
>
> ATTN: This patch also adds in the three lines of code to properly
> update the window region when the update signal is set. This code was
> supplied by Brecht.
>
> Based on feedback from Brecht, the patch now is purely artist driven.
> The artist toggles are available via the Image Input node and the UV
> Image propertiers pane for ingestion. For output, if Color Management
> is selected, a Predivide box is offered in the Properties box in the
> Shading region.
>
> Q: What does it do?
> A: The patch predivides the RGB values when the artist driven toggles
> are selected. This permits accurate sRGB to linear and vice versa
> conversions.
>
> Q: Does it allow me to load premultiplied alpha TIFF images correctly
> or use my premultiplied Blender renders in the VSE?
> A: Yes. In the case of the latter, you will need to unmangle the
> existing renders via the following technique.
>
> Q: Does it allow me to unmangle or reuse legacy renders I have made
> with Blender using premultiplied alpha?
> A: Yes. In the case of previously mangled alpha images, the correct
> process would be to leave Predivide off for ingestion and turn
> Predivide on for output.
>
> Q: What is the correct workflow involved with premultiplied data
> ingestion into a linear environment?
> A: The correct workflow is to select Predivide for all sRGB
> premultiplied alpha images and only select Predivide if you are saving
> with a premultiplied alpha.
>
> Q: Is there a set of images to test this with?
> A: Yes. In the attached bug report there is a well crafted
> premultiplied alpha foreground plate and a complimentary fully opaque
> background. This pattern designed by Guillermo Espertino will quickly
> reveal inconsistencies in sRGB premultiplied conversions. It would be
> excellent if Blender experts of their respective domains would use the
> two images to test where we need to extend this technique.
>
> http://projects.blender.org/tracker/?func=detail&atid=498&aid=28085&group_id=9
>
> Technical details:
>
> 1) Adds one flag to ImBuf and the corresponding iff for plugins.
> 2) Adds one variable to Image to hold the cm_flags.
> 3) Adds one bitwise flag define for use with the color_mgt_flag in
> RenderData for Scene access.
> 4) Modifies the IMB_linear_from_rect and IMB_rect_from_linear to honor
> the input and output flags.
> 5) Modifies pipeline.c to honor the Scene setting flag.
> 6) A few adjustments within image.c to copy the new cm_flags in the
> Image struct to the ImBuf struct.
> 7) Updates the update window manager code according to instructions by Brecht.
>
> With respect,
> TJS
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers
>