From 7e0f4490c3af3c2571a0110d87f7d4e128b17ea0 Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Thu, 23 Apr 2026 10:55:24 -0700 Subject: [PATCH] fix: Relax OpenColorIO exception handling In PR 5114, just a few weeks ago, I was trying to catch every possible OCIO exception. That was a good idea, but I was over-eager and treated every OCIO exception as if something really bad happened. I should have recognized that OCIO enjoys throwing exceptions for things where I would have just used an empty return value to indicate a failure to do what was asked. So this patch is heavily revising the approach as follows: * Exceptions thrown because it couldn't read a config properly (it was missing or broken), or couldn't produce a ColorProcessor -- those are real problems that we should treat as serious errors. * Exceptions thrown because it can't satisfy a simple request (like "what is the name of the color space for this role" in a case where the role you passed is not known by the config) -- we should catch those exceptions, but it's not an "error" for OIIO, we just return an empty string because there is no color space for that role. Also beefed up some of the documentation of our ColorConfig class methods to describe what happens with certain kinds of failures. Signed-off-by: Larry Gritz --- src/include/OpenImageIO/color.h | 25 ++++++--- src/libOpenImageIO/color_ocio.cpp | 86 +++++++++++-------------------- 2 files changed, 48 insertions(+), 63 deletions(-) diff --git a/src/include/OpenImageIO/color.h b/src/include/OpenImageIO/color.h index 99eba56396..10dd04ac0e 100644 --- a/src/include/OpenImageIO/color.h +++ b/src/include/OpenImageIO/color.h @@ -121,11 +121,12 @@ class OIIO_API ColorConfig { int getColorSpaceIndex(string_view name) const; /// Get the name of the color space representing the named role, - /// or NULL if none could be identified. + /// or nullptr if none could be identified. const char* getColorSpaceNameByRole(string_view role) const; /// Get the data type that OCIO thinks this color space is. The name - /// may be either a color space name or a role. + /// may be either a color space name or a role. For an unknown space or + /// any error, return TypeUnknown. OIIO::TypeDesc getColorSpaceDataType(string_view name, int* bits) const; /// Retrieve the full list of known color space names, as a vector @@ -133,13 +134,14 @@ class OIIO_API ColorConfig { std::vector getColorSpaceNames() const; /// Get the name of the color space family of the named color space, - /// or NULL if none could be identified. + /// or nullptr if none could be identified. const char* getColorSpaceFamilyByName(string_view name) const; // Get the number of Roles defined in this configuration int getNumRoles() const; - /// Query the name of the specified Role. + /// Query the name of the specified Role, or return nullptr if there is no + /// role with that index. const char* getRoleByIndex(int index) const; /// Retrieve the full list of known Roles, as a vector of strings. @@ -148,7 +150,8 @@ class OIIO_API ColorConfig { /// Get the number of Looks defined in this configuration int getNumLooks() const; - /// Query the name of the specified Look. + /// Query the name of the specified Look, or return nullptr if there is no + /// look with that index. const char* getLookNameByIndex(int index) const; /// Retrieve the full list of known look names, as a vector of strings. @@ -157,7 +160,8 @@ class OIIO_API ColorConfig { /// Get the number of NamedTransforms defined in this configuration int getNumNamedTransforms() const; - /// Query the name of the specified NamedTransform. + /// Query the name of the specified NamedTransform, or nullptr if there is + /// no NamedTransform with that index. const char* getNamedTransformNameByIndex(int index) const; /// Retrieve the full list of known NamedTransforms, as a vector of strings @@ -221,7 +225,8 @@ class OIIO_API ColorConfig { /// Get the number of displays defined in this configuration int getNumDisplays() const; - /// Query the name of the specified display. + /// Query the name of the specified display, or nullptr if there is no + /// display with that index. const char* getDisplayNameByIndex(int index) const; /// Retrieve the full list of known display names, as a vector of @@ -236,7 +241,9 @@ class OIIO_API ColorConfig { /// display will be used. int getNumViews(string_view display = "") const; - /// Query the name of the specified view for the specified display + /// Query the name of the specified view for the specified display, or + /// nullptr if there is no view with that index or if the display is not + /// found. const char* getViewNameByIndex(string_view display, int index) const; /// Retrieve the full list of known view names for the display, as a @@ -247,12 +254,14 @@ class OIIO_API ColorConfig { /// Query the name of the default view for the specified display. If the /// display is empty or not specified, the default display will be used. /// This version does not consider the input color space. + /// Returns nullptr for failure. const char* getDefaultViewName(string_view display = "") const; /// Query the name of the default view for the specified display, given /// the input color space. If `display` is "default" or an empty string, /// the default display will be used. The input color space is used to /// determine the most appropriate default view for the given display. + /// Returns nullptr for failure. const char* getDefaultViewName(string_view display, string_view inputColorSpace) const; diff --git a/src/libOpenImageIO/color_ocio.cpp b/src/libOpenImageIO/color_ocio.cpp index 1c0a63b221..badfb7c966 100644 --- a/src/libOpenImageIO/color_ocio.cpp +++ b/src/libOpenImageIO/color_ocio.cpp @@ -446,8 +446,7 @@ ColorConfig::Impl::inventory() scene_linear_alias = lin->getName(); return; // If any non-"raw" spaces were defined, we're done } - } catch (OCIO::Exception& e) { - error("Exception from OCIO: {}", e.what()); + } catch (...) { } } @@ -684,8 +683,7 @@ ColorConfig::Impl::classify_by_conversions(CSInfo& cs) cs.setflag(CSInfo::is_ACEScg | CSInfo::is_linear_response, ACEScg_alias); } - } catch (OCIO::Exception& e) { - error("Exception from OCIO: {}", e.what()); + } catch (...) { } } @@ -782,8 +780,7 @@ ColorConfig::Impl::IdentifyBuiltinColorSpace(const char* name) const try { return OCIO::Config::IdentifyBuiltinColorSpace(config_, builtinconfig_, name); - } catch (OCIO::Exception& e) { - error("Exception from OCIO: {}", e.what()); + } catch (...) { } return nullptr; } @@ -952,8 +949,7 @@ ColorConfig::getColorSpaceFamilyByName(string_view name) const std::string(name).c_str()); if (c) return c->getFamily(); - } catch (OCIO::Exception& e) { - getImpl()->error("Exception from OCIO: {}", e.what()); + } catch (...) { } } return nullptr; @@ -986,8 +982,7 @@ ColorConfig::getRoleByIndex(int index) const try { if (getImpl()->config_ && !disable_ocio) return getImpl()->config_->getRoleName(index); - } catch (OCIO::Exception& e) { - getImpl()->error("Exception from OCIO: {}", e.what()); + } catch (...) { } return nullptr; } @@ -1010,8 +1005,7 @@ ColorConfig::getNumLooks() const try { if (getImpl()->config_ && !disable_ocio) return getImpl()->config_->getNumLooks(); - } catch (OCIO::Exception& e) { - getImpl()->error("Exception from OCIO: {}", e.what()); + } catch (...) { } return 0; } @@ -1024,8 +1018,7 @@ ColorConfig::getLookNameByIndex(int index) const try { if (getImpl()->config_ && !disable_ocio) return getImpl()->config_->getLookNameByIndex(index); - } catch (OCIO::Exception& e) { - getImpl()->error("Exception from OCIO: {}", e.what()); + } catch (...) { } return nullptr; } @@ -1060,8 +1053,7 @@ ColorConfig::Impl::isColorSpaceLinear(string_view name) const OCIO::REFERENCE_SPACE_SCENE) || config_->isColorSpaceLinear(c_str(name), OCIO::REFERENCE_SPACE_DISPLAY); - } catch (const std::exception& e) { - error("ColorConfig error: {}", e.what()); + } catch (...) { return false; } } @@ -1121,9 +1113,7 @@ ColorConfig::getColorSpaceNameByRole(string_view role) const // role); return c->getName(); } - } catch (OCIO::Exception& e) { - getImpl()->error("Exception from OCIO: {}", e.what()); - return nullptr; + } catch (...) { } } @@ -1132,7 +1122,7 @@ ColorConfig::getColorSpaceNameByRole(string_view role) const || Strutil::iequals(role, "scene_linear")) return "linear"; - return NULL; // Dunno what role + return nullptr; // Dunno what role } @@ -1168,8 +1158,7 @@ ColorConfig::getColorSpaceDataType(string_view name, int* bits) const case OCIO::BIT_DEPTH_F32: *bits = 32; return TypeDesc::FLOAT; } } - } catch (OCIO::Exception& e) { - getImpl()->error("Exception from OCIO: {}", e.what()); + } catch (...) { } } return TypeUnknown; @@ -1183,8 +1172,7 @@ ColorConfig::getNumDisplays() const try { if (getImpl()->config_ && !disable_ocio) return getImpl()->config_->getNumDisplays(); - } catch (OCIO::Exception& e) { - getImpl()->error("Exception from OCIO: {}", e.what()); + } catch (...) { } return 0; } @@ -1197,8 +1185,7 @@ ColorConfig::getDisplayNameByIndex(int index) const try { if (getImpl()->config_ && !disable_ocio) return getImpl()->config_->getDisplay(index); - } catch (OCIO::Exception& e) { - getImpl()->error("Exception from OCIO: {}", e.what()); + } catch (...) { } return nullptr; } @@ -1225,8 +1212,7 @@ ColorConfig::getNumViews(string_view display) const if (getImpl()->config_ && !disable_ocio) return getImpl()->config_->getNumViews( std::string(display).c_str()); - } catch (OCIO::Exception& e) { - getImpl()->error("Exception from OCIO: {}", e.what()); + } catch (...) { } return 0; } @@ -1242,8 +1228,7 @@ ColorConfig::getViewNameByIndex(string_view display, int index) const if (getImpl()->config_ && !disable_ocio) return getImpl()->config_->getView(std::string(display).c_str(), index); - } catch (OCIO::Exception& e) { - getImpl()->error("Exception from OCIO: {}", e.what()); + } catch (...) { } return nullptr; } @@ -1269,8 +1254,7 @@ ColorConfig::getDefaultDisplayName() const try { if (getImpl()->config_ && !disable_ocio) return getImpl()->config_->getDefaultDisplay(); - } catch (OCIO::Exception& e) { - getImpl()->error("Exception from OCIO: {}", e.what()); + } catch (...) { } return nullptr; } @@ -1285,8 +1269,7 @@ ColorConfig::getDefaultViewName(string_view display) const try { if (getImpl()->config_ && !disable_ocio) return getImpl()->config_->getDefaultView(c_str(display)); - } catch (OCIO::Exception& e) { - getImpl()->error("Exception from OCIO: {}", e.what()); + } catch (...) { } return nullptr; } @@ -1305,8 +1288,7 @@ ColorConfig::getDefaultViewName(string_view display, if (getImpl()->config_ && !disable_ocio) return getImpl()->config_->getDefaultView(c_str(display), c_str(inputColorSpace)); - } catch (OCIO::Exception& e) { - getImpl()->error("Exception from OCIO: {}", e.what()); + } catch (...) { } return nullptr; } @@ -1324,8 +1306,7 @@ ColorConfig::getDisplayViewColorSpaceName(const std::string& display, if (strcmp(c_str(name), "") == 0) name = display; return c_str(name); - } catch (OCIO::Exception& e) { - getImpl()->error("Exception from OCIO: {}", e.what()); + } catch (...) { } } return nullptr; @@ -1341,8 +1322,7 @@ ColorConfig::getDisplayViewLooks(const std::string& display, if (getImpl()->config_ && !disable_ocio) return getImpl()->config_->getDisplayViewLooks(display.c_str(), view.c_str()); - } catch (OCIO::Exception& e) { - getImpl()->error("Exception from OCIO: {}", e.what()); + } catch (...) { } return nullptr; } @@ -1355,8 +1335,7 @@ ColorConfig::getNumNamedTransforms() const try { if (getImpl()->config_ && !disable_ocio) return getImpl()->config_->getNumNamedTransforms(); - } catch (OCIO::Exception& e) { - getImpl()->error("Exception from OCIO: {}", e.what()); + } catch (...) { } return 0; } @@ -1369,8 +1348,7 @@ ColorConfig::getNamedTransformNameByIndex(int index) const try { if (getImpl()->config_ && !disable_ocio) return getImpl()->config_->getNamedTransformNameByIndex(index); - } catch (OCIO::Exception& e) { - getImpl()->error("Exception from OCIO: {}", e.what()); + } catch (...) { } return nullptr; } @@ -1432,8 +1410,7 @@ ColorConfig::Impl::resolve(string_view name) const OCIO::ConstColorSpaceRcPtr cs = config->getColorSpace(c_str(name)); if (cs) return cs->getName(); - } catch (OCIO::Exception& e) { - error("Exception from OCIO: {}", e.what()); + } catch (...) { } } // OCIO did not know this name as a color space, role, or alias. @@ -1559,7 +1536,9 @@ class ColorProcessor_OCIO final : public ColorProcessor { chanstride, xstride, ystride); m_cpuproc->apply(pid); } catch (OCIO::Exception& e) { - OIIO::print("OCIO error in apply: {}\n", e.what()); + OIIO::errorfmt("OCIO error in apply: {}\n", e.what()); + // FIXME -- some day, we should make ColorProcessor::apply return + // a status, and we should indicate here that it failed. } } @@ -2032,8 +2011,7 @@ ColorConfig::getColorSpaceFromFilepath(string_view str) const string_view r = getImpl()->config_->getColorSpaceFromFilepath( s.c_str()); return r; - } catch (OCIO::Exception& e) { - getImpl()->error("Exception from OCIO: {}", e.what()); + } catch (...) { } } // Fall back on parseColorSpaceFromString @@ -2051,8 +2029,7 @@ ColorConfig::getColorSpaceFromFilepath(string_view str, string_view default_cs, s.c_str()); if (!getImpl()->config_->filepathOnlyMatchesDefaultRule(s.c_str())) return r; - } catch (OCIO::Exception& e) { - getImpl()->error("Exception from OCIO: {}", e.what()); + } catch (...) { } } if (cs_name_match) { @@ -2068,8 +2045,7 @@ ColorConfig::filepathOnlyMatchesDefaultRule(string_view str) const { try { return getImpl()->config_->filepathOnlyMatchesDefaultRule(c_str(str)); - } catch (OCIO::Exception& e) { - getImpl()->error("Exception from OCIO: {}", e.what()); + } catch (...) { } return false; } @@ -2248,8 +2224,8 @@ ColorConfig::get_color_interop_id(string_view colorspace) const std::string(resolve(colorspace)).c_str()); if (c) interop_id = c->getInteropID(); - } catch (OCIO::Exception& e) { - getImpl()->error("Exception from OCIO: {}", e.what()); + } catch (...) { + interop_id = nullptr; } if (interop_id) { return interop_id;