From c65b022e6207865702357d07d19b5d576dbfb136 Mon Sep 17 00:00:00 2001 From: Brecht Van Lommel Date: Mon, 11 Apr 2022 19:38:45 +0200 Subject: [PATCH] Fix OpenColorIO error with configs using * Don't assume the display colorspace name fully defines the transform to display space, this is not true in OpenColorIO 2 where view transforms may be defined in more complexs ways than just specifying a colorspace. * In places where we need to store the display colorspace name, resolve token manually. Ref T96590 --- intern/opencolorio/fallback_impl.cc | 9 ++--- intern/opencolorio/ocio_capi.cc | 6 ++-- intern/opencolorio/ocio_capi.h | 3 +- intern/opencolorio/ocio_impl.cc | 14 ++++++-- intern/opencolorio/ocio_impl.h | 9 +++-- intern/opencolorio/ocio_impl_glsl.cc | 2 +- source/blender/imbuf/IMB_colormanagement.h | 4 --- source/blender/imbuf/intern/colormanagement.c | 33 +++++++++---------- 8 files changed, 46 insertions(+), 34 deletions(-) diff --git a/intern/opencolorio/fallback_impl.cc b/intern/opencolorio/fallback_impl.cc index aaab8b4e6b9..d78b34d3c92 100644 --- a/intern/opencolorio/fallback_impl.cc +++ b/intern/opencolorio/fallback_impl.cc @@ -461,12 +461,13 @@ OCIO_ConstProcessorRcPtr *FallbackImpl::createDisplayProcessor(OCIO_ConstConfigR const char * /*display*/, const char * /*look*/, const float scale, - const float exponent) + const float exponent, + const bool inverse) { FallbackTransform transform; - transform.type = TRANSFORM_LINEAR_TO_SRGB; - transform.scale = scale; - transform.exponent = exponent; + transform.type = (inverse) ? TRANSFORM_SRGB_TO_LINEAR : TRANSFORM_LINEAR_TO_SRGB; + transform.scale = (inverse && scale != 0.0f) ? 1.0f / scale : scale; + transform.exponent = (inverse && exponent != 0.0f) ? 1.0f / exponent : exponent; return (OCIO_ConstProcessorRcPtr *)new FallbackProcessor(transform); } diff --git a/intern/opencolorio/ocio_capi.cc b/intern/opencolorio/ocio_capi.cc index 91784a288c8..5e4c2a87a0b 100644 --- a/intern/opencolorio/ocio_capi.cc +++ b/intern/opencolorio/ocio_capi.cc @@ -250,9 +250,11 @@ OCIO_ConstProcessorRcPtr *OCIO_createDisplayProcessor(OCIO_ConstConfigRcPtr *con const char *display, const char *look, const float scale, - const float exponent) + const float exponent, + const bool inverse) { - return impl->createDisplayProcessor(config, input, view, display, look, scale, exponent); + return impl->createDisplayProcessor( + config, input, view, display, look, scale, exponent, inverse); } OCIO_PackedImageDesc *OCIO_createOCIO_PackedImageDesc(float *data, diff --git a/intern/opencolorio/ocio_capi.h b/intern/opencolorio/ocio_capi.h index 5c036ec263a..9bd4ec374e2 100644 --- a/intern/opencolorio/ocio_capi.h +++ b/intern/opencolorio/ocio_capi.h @@ -166,7 +166,8 @@ OCIO_ConstProcessorRcPtr *OCIO_createDisplayProcessor(OCIO_ConstConfigRcPtr *con const char *display, const char *look, const float scale, - const float exponent); + const float exponent, + const bool inverse); OCIO_PackedImageDesc *OCIO_createOCIO_PackedImageDesc(float *data, long width, diff --git a/intern/opencolorio/ocio_impl.cc b/intern/opencolorio/ocio_impl.cc index ca1b7cc42e1..8d9c5dd2d49 100644 --- a/intern/opencolorio/ocio_impl.cc +++ b/intern/opencolorio/ocio_impl.cc @@ -254,7 +254,12 @@ const char *OCIOImpl::configGetDisplayColorSpaceName(OCIO_ConstConfigRcPtr *conf const char *view) { try { - return (*(ConstConfigRcPtr *)config)->getDisplayViewColorSpaceName(display, view); + const char *name = (*(ConstConfigRcPtr *)config)->getDisplayViewColorSpaceName(display, view); + /* OpenColorIO does not resolve this token for us, so do it ourselves. */ + if (strcasecmp(name, "") == 0) { + return display; + } + return name; } catch (Exception &exception) { OCIO_reportException(exception); @@ -655,7 +660,8 @@ OCIO_ConstProcessorRcPtr *OCIOImpl::createDisplayProcessor(OCIO_ConstConfigRcPtr const char *display, const char *look, const float scale, - const float exponent) + const float exponent, + const bool inverse) { ConstConfigRcPtr config = *(ConstConfigRcPtr *)config_; @@ -718,6 +724,10 @@ OCIO_ConstProcessorRcPtr *OCIOImpl::createDisplayProcessor(OCIO_ConstConfigRcPtr group->appendTransform(et); } + if (inverse) { + group->setDirection(TRANSFORM_DIR_INVERSE); + } + /* Create processor from transform. This is the moment were OCIO validates * the entire transform, no need to check for the validity of inputs above. */ ConstProcessorRcPtr *p = MEM_new(__func__); diff --git a/intern/opencolorio/ocio_impl.h b/intern/opencolorio/ocio_impl.h index d42fa58121f..f8397c62e52 100644 --- a/intern/opencolorio/ocio_impl.h +++ b/intern/opencolorio/ocio_impl.h @@ -85,7 +85,8 @@ class IOCIOImpl { const char *display, const char *look, const float scale, - const float exponent) = 0; + const float exponent, + const bool inverse) = 0; virtual OCIO_PackedImageDesc *createOCIO_PackedImageDesc(float *data, long width, @@ -201,7 +202,8 @@ class FallbackImpl : public IOCIOImpl { const char *display, const char *look, const float scale, - const float exponent); + const float exponent, + const bool inverse); OCIO_PackedImageDesc *createOCIO_PackedImageDesc(float *data, long width, @@ -290,7 +292,8 @@ class OCIOImpl : public IOCIOImpl { const char *display, const char *look, const float scale, - const float exponent); + const float exponent, + const bool inverse); OCIO_PackedImageDesc *createOCIO_PackedImageDesc(float *data, long width, diff --git a/intern/opencolorio/ocio_impl_glsl.cc b/intern/opencolorio/ocio_impl_glsl.cc index 87f9d8837c7..150ed1a58bb 100644 --- a/intern/opencolorio/ocio_impl_glsl.cc +++ b/intern/opencolorio/ocio_impl_glsl.cc @@ -603,7 +603,7 @@ static OCIO_GPUDisplayShader &getGPUDisplayShader( OCIO_ConstProcessorRcPtr *processor_to_scene_linear = OCIO_configGetProcessorWithNames( config, input, ROLE_SCENE_LINEAR); OCIO_ConstProcessorRcPtr *processor_to_display = OCIO_createDisplayProcessor( - config, ROLE_SCENE_LINEAR, view, display, look, 1.0f, 1.0f); + config, ROLE_SCENE_LINEAR, view, display, look, 1.0f, 1.0f, false); /* Create shader descriptions. */ if (processor_to_scene_linear && processor_to_display) { diff --git a/source/blender/imbuf/IMB_colormanagement.h b/source/blender/imbuf/IMB_colormanagement.h index 7cf2c02e657..97030d44047 100644 --- a/source/blender/imbuf/IMB_colormanagement.h +++ b/source/blender/imbuf/IMB_colormanagement.h @@ -256,10 +256,6 @@ void IMB_colormanagement_display_settings_from_ctx( struct ColorManagedViewSettings **r_view_settings, struct ColorManagedDisplaySettings **r_display_settings); -const char *IMB_colormanagement_get_display_colorspace_name( - const struct ColorManagedViewSettings *view_settings, - const struct ColorManagedDisplaySettings *display_settings); - /** * Acquire display buffer for given image buffer using specified view and display settings. */ diff --git a/source/blender/imbuf/intern/colormanagement.c b/source/blender/imbuf/intern/colormanagement.c index 193fda01816..53aa74edc61 100644 --- a/source/blender/imbuf/intern/colormanagement.c +++ b/source/blender/imbuf/intern/colormanagement.c @@ -794,9 +794,8 @@ void IMB_colormanagement_display_settings_from_ctx( } } -const char *IMB_colormanagement_get_display_colorspace_name( - const ColorManagedViewSettings *view_settings, - const ColorManagedDisplaySettings *display_settings) +static const char *get_display_colorspace_name(const ColorManagedViewSettings *view_settings, + const ColorManagedDisplaySettings *display_settings) { OCIO_ConstConfigRcPtr *config = OCIO_getCurrentConfig(); @@ -815,8 +814,7 @@ static ColorSpace *display_transform_get_colorspace( const ColorManagedViewSettings *view_settings, const ColorManagedDisplaySettings *display_settings) { - const char *colorspace_name = IMB_colormanagement_get_display_colorspace_name(view_settings, - display_settings); + const char *colorspace_name = get_display_colorspace_name(view_settings, display_settings); if (colorspace_name) { return colormanage_colorspace_get_named(colorspace_name); @@ -837,8 +835,14 @@ static OCIO_ConstCPUProcessorRcPtr *create_display_buffer_processor(const char * const float scale = (exposure == 0.0f) ? 1.0f : powf(2.0f, exposure); const float exponent = (gamma == 1.0f) ? 1.0f : 1.0f / max_ff(FLT_EPSILON, gamma); - OCIO_ConstProcessorRcPtr *processor = OCIO_createDisplayProcessor( - config, from_colorspace, view_transform, display, (use_look) ? look : "", scale, exponent); + OCIO_ConstProcessorRcPtr *processor = OCIO_createDisplayProcessor(config, + from_colorspace, + view_transform, + display, + (use_look) ? look : "", + scale, + exponent, + false); OCIO_configRelease(config); @@ -923,10 +927,8 @@ static OCIO_ConstCPUProcessorRcPtr *display_from_scene_linear_processor( OCIO_ConstProcessorRcPtr *processor = NULL; if (view_name && config) { - const char *view_colorspace = OCIO_configGetDisplayColorSpaceName( - config, display->name, view_name); - processor = OCIO_configGetProcessorWithNames( - config, global_role_scene_linear, view_colorspace); + processor = OCIO_createDisplayProcessor( + config, global_role_scene_linear, view_name, display->name, NULL, 1.0f, 1.0f, false); OCIO_configRelease(config); } @@ -955,10 +957,8 @@ static OCIO_ConstCPUProcessorRcPtr *display_to_scene_linear_processor(ColorManag OCIO_ConstProcessorRcPtr *processor = NULL; if (view_name && config) { - const char *view_colorspace = OCIO_configGetDisplayColorSpaceName( - config, display->name, view_name); - processor = OCIO_configGetProcessorWithNames( - config, view_colorspace, global_role_scene_linear); + processor = OCIO_createDisplayProcessor( + config, global_role_scene_linear, view_name, display->name, NULL, 1.0f, 1.0f, true); OCIO_configRelease(config); } @@ -1730,8 +1730,7 @@ static bool is_ibuf_rect_in_display_space(ImBuf *ibuf, if ((view_settings->flag & COLORMANAGE_VIEW_USE_CURVES) == 0 && view_settings->exposure == 0.0f && view_settings->gamma == 1.0f) { const char *from_colorspace = ibuf->rect_colorspace->name; - const char *to_colorspace = IMB_colormanagement_get_display_colorspace_name(view_settings, - display_settings); + const char *to_colorspace = get_display_colorspace_name(view_settings, display_settings); ColorManagedLook *look_descr = colormanage_look_get_named(view_settings->look); if (look_descr != NULL && !STREQ(look_descr->process_space, "")) { return false;