Color Management: various improvements and fixes for image saving

* Respect the image file color space setitng for saving in various cases where
  it was previously ignored. Previously it would often use the sRGB or Linear
  color space even when not selected.
* For the Save As operator, add a Color Space option in the file browser to
  choose the color space of the file. Previously this was chosen automatically,
  now it's possible to e.g. resave a Linear image as Linear ACES.
* When changing the file format, the colorspace is automatically changed to an
  appropriate color space for the file format. This already happened before, but
  there was no visibility or control in the operator settings for this.
* Don't change color space when using the Save operator to save over the same
  file.
* Fix missing color space conversion for 16 bit PNGs, where it assumed wrongly
  assumed ibuf->rect would be used for saving. Add BKE_image_format_is_byte to
  more accurately test this.

Fixes T74610

Ref T68926

Differential Revision: https://developer.blender.org/D14899
This commit is contained in:
Brecht Van Lommel 2022-05-13 16:03:26 +02:00 committed by Brecht Van Lommel
parent 1e4cd98f0a
commit 5baa3ecda6
Notes: blender-bot 2024-02-06 20:53:08 +01:00
Referenced by issue #101329, Regression: EXR 'JPG Preview' doesn't use color space anymore
Referenced by issue #101331, Image "Save As" and changing colorspace: invalid colorspace change
Referenced by issue #99750, Crash with 'File Output' node
Referenced by issue #98444, Regression: image.save_render() command changes color profile and file type when saving.
Referenced by issue #74610, Saving image from image editor resets Image Texture color space
Referenced by issue #68926, Color Management Improvements
Referenced by issue #108754, Test bf_imbuf_save fails with an assert in debug builds (DPX 8-bit save)
Referenced by pull request #117914, Fix #108754: remove overactive color management assert
Referenced by commit 434bb0c93f, Fix #108754: remove overactive color management assert
7 changed files with 226 additions and 102 deletions

View File

@ -76,6 +76,8 @@ char BKE_imtype_from_arg(const char *arg);
void BKE_image_format_from_imbuf(struct ImageFormatData *im_format, const struct ImBuf *imbuf);
void BKE_image_format_to_imbuf(struct ImBuf *ibuf, const struct ImageFormatData *imf);
bool BKE_image_format_is_byte(const struct ImageFormatData *imf);
/* Color Management */
void BKE_image_format_color_management_copy(struct ImageFormatData *imf,

View File

@ -35,6 +35,10 @@ typedef struct ImageSaveOptions {
bool save_copy;
bool save_as_render;
bool do_newpath;
/* Keep track of previous values for auto updates in UI. */
bool prev_save_as_render;
int prev_imtype;
} ImageSaveOptions;
bool BKE_image_save_options_init(ImageSaveOptions *opts,
@ -44,6 +48,7 @@ bool BKE_image_save_options_init(ImageSaveOptions *opts,
struct ImageUser *iuser,
const bool guess_path,
const bool save_as_render);
void BKE_image_save_options_update(struct ImageSaveOptions *opts, struct Image *ima);
void BKE_image_save_options_free(struct ImageSaveOptions *opts);
bool BKE_image_save(struct ReportList *reports,

View File

@ -911,6 +911,11 @@ void BKE_image_format_from_imbuf(ImageFormatData *im_format, const ImBuf *imbuf)
im_format->planes = imbuf->planes;
}
bool BKE_image_format_is_byte(const ImageFormatData *imf)
{
return (imf->depth == R_IMF_CHAN_DEPTH_8) && (BKE_imtype_valid_depths(imf->imtype) & imf->depth);
}
/* Color Management */
void BKE_image_format_color_management_copy(ImageFormatData *imf, const ImageFormatData *imf_src)

View File

@ -121,9 +121,16 @@ bool BKE_image_save_options_init(ImageSaveOptions *opts,
opts->im_format.stereo3d_format = *ima->stereo3d_format;
opts->im_format.views_format = ima->views_format;
/* Render output: colorspace from render settings. */
BKE_image_format_color_management_copy_from_scene(&opts->im_format, scene);
}
/* Default to saving in the same colorspace as the image setting. */
if (!save_as_render) {
BKE_color_managed_colorspace_settings_copy(&opts->im_format.linear_colorspace_settings,
&ima->colorspace_settings);
}
opts->im_format.color_management = R_IMF_COLOR_MANAGEMENT_FOLLOW_SCENE;
if (ima->source == IMA_SRC_TILED) {
@ -177,11 +184,57 @@ bool BKE_image_save_options_init(ImageSaveOptions *opts,
}
}
/* Copy for detecting UI changes. */
opts->prev_save_as_render = opts->save_as_render;
opts->prev_imtype = opts->im_format.imtype;
BKE_image_release_ibuf(ima, ibuf, lock);
return (ibuf != NULL);
}
void BKE_image_save_options_update(ImageSaveOptions *opts, Image *image)
{
/* Auto update color space when changing save as render and file type. */
if (opts->save_as_render) {
if (!opts->prev_save_as_render) {
if (ELEM(image->type, IMA_TYPE_R_RESULT, IMA_TYPE_COMPOSITE)) {
BKE_image_format_init_for_write(&opts->im_format, opts->scene, NULL);
}
else {
BKE_image_format_color_management_copy_from_scene(&opts->im_format, opts->scene);
}
}
}
else {
if (opts->prev_save_as_render) {
/* Copy colorspace from image settings. */
BKE_color_managed_colorspace_settings_copy(&opts->im_format.linear_colorspace_settings,
&image->colorspace_settings);
}
else if (opts->im_format.imtype != opts->prev_imtype) {
const bool linear_float_output = BKE_imtype_requires_linear_float(opts->im_format.imtype);
/* TODO: detect if the colorspace is linear, not just equal to scene linear. */
const bool is_linear = IMB_colormanagement_space_name_is_scene_linear(
opts->im_format.linear_colorspace_settings.name);
/* If changing to a linear float or byte format, ensure we have a compatible color space. */
if (linear_float_output && !is_linear) {
STRNCPY(opts->im_format.linear_colorspace_settings.name,
IMB_colormanagement_role_colorspace_name_get(COLOR_ROLE_DEFAULT_FLOAT));
}
else if (!linear_float_output && is_linear) {
STRNCPY(opts->im_format.linear_colorspace_settings.name,
IMB_colormanagement_role_colorspace_name_get(COLOR_ROLE_DEFAULT_BYTE));
}
}
}
opts->prev_save_as_render = opts->save_as_render;
opts->prev_imtype = opts->im_format.imtype;
}
void BKE_image_save_options_free(ImageSaveOptions *opts)
{
BKE_image_format_free(&opts->im_format);
@ -243,12 +296,17 @@ static void image_save_post(ReportList *reports,
BLI_path_rel(ima->filepath, relbase); /* only after saving */
}
ColorManagedColorspaceSettings old_colorspace_settings;
BKE_color_managed_colorspace_settings_copy(&old_colorspace_settings, &ima->colorspace_settings);
IMB_colormanagement_colorspace_from_ibuf_ftype(&ima->colorspace_settings, ibuf);
if (!BKE_color_managed_colorspace_settings_equals(&old_colorspace_settings,
&ima->colorspace_settings)) {
*r_colorspace_changed = true;
/* Update image file color space when saving to another color space. */
const bool linear_float_output = BKE_imtype_requires_linear_float(opts->im_format.imtype);
if (!opts->save_as_render || linear_float_output) {
if (opts->im_format.linear_colorspace_settings.name[0] &&
!BKE_color_managed_colorspace_settings_equals(
&ima->colorspace_settings, &opts->im_format.linear_colorspace_settings)) {
BKE_color_managed_colorspace_settings_copy(&ima->colorspace_settings,
&opts->im_format.linear_colorspace_settings);
*r_colorspace_changed = true;
}
}
}
@ -314,12 +372,12 @@ static bool image_save_single(ReportList *reports,
/* we need renderresult for exr and rendered multiview */
rr = BKE_image_acquire_renderresult(opts->scene, ima);
bool is_mono = rr ? BLI_listbase_count_at_most(&rr->views, 2) < 2 :
BLI_listbase_count_at_most(&ima->views, 2) < 2;
bool is_exr_rr = rr && ELEM(imf->imtype, R_IMF_IMTYPE_OPENEXR, R_IMF_IMTYPE_MULTILAYER) &&
RE_HasFloatPixels(rr);
bool is_multilayer = is_exr_rr && (imf->imtype == R_IMF_IMTYPE_MULTILAYER);
int layer = (iuser && !is_multilayer) ? iuser->layer : -1;
const bool is_mono = rr ? BLI_listbase_count_at_most(&rr->views, 2) < 2 :
BLI_listbase_count_at_most(&ima->views, 2) < 2;
const bool is_exr_rr = rr && ELEM(imf->imtype, R_IMF_IMTYPE_OPENEXR, R_IMF_IMTYPE_MULTILAYER) &&
RE_HasFloatPixels(rr);
const bool is_multilayer = is_exr_rr && (imf->imtype == R_IMF_IMTYPE_MULTILAYER);
const int layer = (iuser && !is_multilayer) ? iuser->layer : -1;
/* error handling */
if (rr == nullptr) {

View File

@ -219,6 +219,12 @@ OutputSingleLayerOperation::OutputSingleLayerOperation(const Scene *scene,
image_input_ = nullptr;
BKE_image_format_init_for_write(&format_, scene, format);
if (!save_as_render) {
/* If not saving as render, stop IMB_colormanagement_imbuf_for_write using this
* colorspace for conversion. */
format_.linear_colorspace_settings.name[0] = '\0';
}
BLI_strncpy(path_, path, sizeof(path_));
view_name_ = view_name;

View File

@ -1775,6 +1775,7 @@ static int image_save_as_exec(bContext *C, wmOperator *op)
if (op->customdata) {
isd = op->customdata;
image_save_options_from_op(bmain, &isd->opts, op);
BKE_image_save_options_update(&isd->opts, isd->image);
}
else {
isd = image_save_as_init(C, op);
@ -1794,9 +1795,14 @@ static int image_save_as_exec(bContext *C, wmOperator *op)
return OPERATOR_FINISHED;
}
static bool image_save_as_check(bContext *UNUSED(C), wmOperator *op)
static bool image_save_as_check(bContext *C, wmOperator *op)
{
Main *bmain = CTX_data_main(C);
ImageSaveData *isd = op->customdata;
image_save_options_from_op(bmain, &isd->opts, op);
BKE_image_save_options_update(&isd->opts, isd->image);
return WM_operator_filesel_ensure_ext_imtype(op, &isd->opts.im_format);
}
@ -1839,7 +1845,7 @@ static void image_save_as_draw(bContext *UNUSED(C), wmOperator *op)
ImageSaveData *isd = op->customdata;
PointerRNA imf_ptr;
const bool is_multiview = RNA_boolean_get(op->ptr, "show_multiview");
const bool use_color_management = RNA_boolean_get(op->ptr, "save_as_render");
const bool save_as_render = RNA_boolean_get(op->ptr, "save_as_render");
uiLayoutSetPropSep(layout, true);
uiLayoutSetPropDecorate(layout, false);
@ -1852,7 +1858,14 @@ static void image_save_as_draw(bContext *UNUSED(C), wmOperator *op)
/* image template */
RNA_pointer_create(NULL, &RNA_ImageFormatSettings, &isd->opts.im_format, &imf_ptr);
uiTemplateImageSettings(layout, &imf_ptr, use_color_management);
uiTemplateImageSettings(layout, &imf_ptr, save_as_render);
if (!save_as_render) {
PointerRNA linear_settings_ptr = RNA_pointer_get(&imf_ptr, "linear_colorspace_settings");
uiLayout *col = uiLayoutColumn(layout, true);
uiItemS(col);
uiItemR(col, &linear_settings_ptr, "name", 0, IFACE_("Color Space"), ICON_NONE);
}
/* multiview template */
if (is_multiview) {

View File

@ -2445,68 +2445,77 @@ void IMB_colormanagement_imbuf_make_display_space(
colormanagement_imbuf_make_display_space(ibuf, view_settings, display_settings, false);
}
static ImBuf *imbuf_ensure_editable(ImBuf *ibuf, ImBuf *colormanaged_ibuf, bool allocate_result)
{
if (colormanaged_ibuf != ibuf) {
/* Is already an editable copy. */
return colormanaged_ibuf;
}
if (allocate_result) {
/* Copy full image buffer. */
colormanaged_ibuf = IMB_dupImBuf(ibuf);
IMB_metadata_copy(colormanaged_ibuf, ibuf);
return colormanaged_ibuf;
}
else {
/* Render pipeline is constructing image buffer itself,
* but it's re-using byte and float buffers from render result make copy of this buffers
* here sine this buffers would be transformed to other color space here. */
if (ibuf->rect && (ibuf->mall & IB_rect) == 0) {
ibuf->rect = MEM_dupallocN(ibuf->rect);
ibuf->mall |= IB_rect;
}
if (ibuf->rect_float && (ibuf->mall & IB_rectfloat) == 0) {
ibuf->rect_float = MEM_dupallocN(ibuf->rect_float);
ibuf->mall |= IB_rectfloat;
}
return ibuf;
}
}
ImBuf *IMB_colormanagement_imbuf_for_write(ImBuf *ibuf,
bool save_as_render,
bool allocate_result,
const ImageFormatData *image_format)
{
ImBuf *colormanaged_ibuf = ibuf;
const bool is_movie = BKE_imtype_is_movie(image_format->imtype);
const bool requires_linear_float = BKE_imtype_requires_linear_float(image_format->imtype);
const bool do_alpha_under = image_format->planes != R_IMF_PLANES_RGBA;
/* Update byte buffer if exists but invalid. */
if (ibuf->rect_float && ibuf->rect &&
(ibuf->userflags & (IB_DISPLAY_BUFFER_INVALID | IB_RECT_INVALID)) != 0) {
IMB_rect_from_float(ibuf);
ibuf->userflags &= ~(IB_RECT_INVALID | IB_DISPLAY_BUFFER_INVALID);
}
const bool do_colormanagement_display = save_as_render && (is_movie || !requires_linear_float);
const bool do_colormanagement_linear = save_as_render && requires_linear_float &&
image_format->linear_colorspace_settings.name[0] &&
!IMB_colormanagement_space_name_is_scene_linear(
image_format->linear_colorspace_settings.name);
/* Detect if we are writing to a file format that needs a linear float buffer. */
const bool linear_float_output = BKE_imtype_requires_linear_float(image_format->imtype);
if (do_colormanagement_display || do_colormanagement_linear || do_alpha_under) {
if (allocate_result) {
colormanaged_ibuf = IMB_dupImBuf(ibuf);
}
else {
/* Render pipeline is constructing image buffer itself,
* but it's re-using byte and float buffers from render result make copy of this buffers
* here sine this buffers would be transformed to other color space here.
*/
/* Detect if we are writing output a byte buffer, which we would need to create
* with color management conversions applied. This may be for either applying the
* display transform for renders, or a user specified color space for the file. */
const bool byte_output = BKE_image_format_is_byte(image_format);
if (ibuf->rect && (ibuf->mall & IB_rect) == 0) {
ibuf->rect = MEM_dupallocN(ibuf->rect);
ibuf->mall |= IB_rect;
}
BLI_assert(!(byte_output && linear_float_output));
if (ibuf->rect_float && (ibuf->mall & IB_rectfloat) == 0) {
ibuf->rect_float = MEM_dupallocN(ibuf->rect_float);
ibuf->mall |= IB_rectfloat;
}
}
}
/* If we're saving from RGBA to RGB buffer then it's not
* so much useful to just ignore alpha -- it leads to bad
* artifacts especially when saving byte images.
/* If we're saving from RGBA to RGB buffer then it's not so much useful to just ignore alpha --
* it leads to bad artifacts especially when saving byte images.
*
* What we do here is we're overlaying our image on top of
* background color (which is currently black).
* What we do here is we're overlaying our image on top of background color (which is currently
* black). This is quite much the same as what Gimp does and it seems to be what artists expects
* from saving.
*
* This is quite much the same as what Gimp does and it
* seems to be what artists expects from saving.
*
* Do a conversion here, so image format writers could
* happily assume all the alpha tricks were made already.
* helps keep things locally here, not spreading it to
* all possible image writers we've got.
* Do a conversion here, so image format writers could happily assume all the alpha tricks were
* made already. helps keep things locally here, not spreading it to all possible image writers
* we've got.
*/
if (do_alpha_under) {
if (image_format->planes != R_IMF_PLANES_RGBA) {
float color[3] = {0, 0, 0};
colormanaged_ibuf = imbuf_ensure_editable(ibuf, colormanaged_ibuf, allocate_result);
if (colormanaged_ibuf->rect_float && colormanaged_ibuf->channels == 4) {
IMB_alpha_under_color_float(
colormanaged_ibuf->rect_float, colormanaged_ibuf->x, colormanaged_ibuf->y, color);
@ -2520,69 +2529,95 @@ ImBuf *IMB_colormanagement_imbuf_for_write(ImBuf *ibuf,
}
}
if (do_colormanagement_display) {
/* Color management with display and view transform. */
bool make_byte = false;
if (save_as_render && !linear_float_output) {
/* Render output: perform conversion to display space using view transform. */
colormanaged_ibuf = imbuf_ensure_editable(ibuf, colormanaged_ibuf, allocate_result);
/* for proper check whether byte buffer is required by a format or not
* should be pretty safe since this image buffer is supposed to be used for
* saving only and ftype would be overwritten a bit later by BKE_imbuf_write
*/
colormanaged_ibuf->ftype = BKE_imtype_to_ftype(image_format->imtype,
&colormanaged_ibuf->foptions);
/* if file format isn't able to handle float buffer itself,
* we need to allocate byte buffer and store color managed
* image there
*/
const ImFileType *type = IMB_file_type_from_ibuf(colormanaged_ibuf);
if (type != NULL) {
if ((type->save != NULL) && (type->flag & IM_FTYPE_FLOAT) == 0) {
make_byte = true;
}
}
/* perform color space conversions */
colormanagement_imbuf_make_display_space(colormanaged_ibuf,
&image_format->view_settings,
&image_format->display_settings,
make_byte);
byte_output);
if (colormanaged_ibuf->rect_float) {
/* float buffer isn't linear anymore,
/* Float buffer isn't linear anymore,
* image format write callback should check for this flag and assume
* no space conversion should happen if ibuf->float_colorspace != NULL
*/
* no space conversion should happen if ibuf->float_colorspace != NULL. */
colormanaged_ibuf->float_colorspace = display_transform_get_colorspace(
&image_format->view_settings, &image_format->display_settings);
if (byte_output) {
colormanaged_ibuf->rect_colorspace = colormanaged_ibuf->float_colorspace;
}
}
}
else if (do_colormanagement_linear) {
/* Color management transform to another linear color space. */
if (!colormanaged_ibuf->rect_float) {
IMB_float_from_rect(colormanaged_ibuf);
imb_freerectImBuf(colormanaged_ibuf);
else {
/* Linear render or regular file output: conversion between two color spaces. */
/* Detect which color space we need to convert between. */
const char *from_colorspace = (ibuf->rect_float && !(byte_output && ibuf->rect)) ?
/* From float buffer. */
(ibuf->float_colorspace) ? ibuf->float_colorspace->name :
global_role_scene_linear :
/* From byte buffer. */
(ibuf->rect_colorspace) ? ibuf->rect_colorspace->name :
global_role_default_byte;
const char *to_colorspace = image_format->linear_colorspace_settings.name;
/* TODO: can we check with OCIO if color spaces are the same but have different names? */
if (to_colorspace[0] == '\0' || STREQ(from_colorspace, to_colorspace)) {
/* No conversion needed, but may still need to allocate byte buffer for output. */
if (byte_output && !ibuf->rect) {
ibuf->rect_colorspace = ibuf->float_colorspace;
IMB_rect_from_float(ibuf);
}
}
else {
/* Color space conversion needed. */
colormanaged_ibuf = imbuf_ensure_editable(ibuf, colormanaged_ibuf, allocate_result);
if (colormanaged_ibuf->rect_float) {
const char *from_colorspace = (ibuf->float_colorspace) ? ibuf->float_colorspace->name :
global_role_scene_linear;
const char *to_colorspace = image_format->linear_colorspace_settings.name;
if (byte_output) {
colormanaged_ibuf->rect_colorspace = colormanage_colorspace_get_named(to_colorspace);
IMB_colormanagement_transform(colormanaged_ibuf->rect_float,
colormanaged_ibuf->x,
colormanaged_ibuf->y,
colormanaged_ibuf->channels,
from_colorspace,
to_colorspace,
false);
if (colormanaged_ibuf->rect) {
/* Byte to byte. */
IMB_colormanagement_transform_byte_threaded((unsigned char *)colormanaged_ibuf->rect,
colormanaged_ibuf->x,
colormanaged_ibuf->y,
colormanaged_ibuf->channels,
from_colorspace,
to_colorspace);
}
else {
/* Float to byte. */
IMB_rect_from_float(colormanaged_ibuf);
}
}
else {
if (!colormanaged_ibuf->rect_float) {
/* Byte to float. */
IMB_float_from_rect(colormanaged_ibuf);
imb_freerectImBuf(colormanaged_ibuf);
/* This conversion always goes to scene linear. */
from_colorspace = global_role_scene_linear;
}
if (colormanaged_ibuf->rect_float) {
/* Float to float. */
IMB_colormanagement_transform(colormanaged_ibuf->rect_float,
colormanaged_ibuf->x,
colormanaged_ibuf->y,
colormanaged_ibuf->channels,
from_colorspace,
to_colorspace,
false);
colormanaged_ibuf->float_colorspace = colormanage_colorspace_get_named(to_colorspace);
}
}
}
}
if (colormanaged_ibuf != ibuf) {
IMB_metadata_copy(colormanaged_ibuf, ibuf);
}
return colormanaged_ibuf;
}