Port from master to RB-2.0 - Adsk Contrib - Improve File Rules suppor…

…t for v1 (#1445) * Adsk Contrib - Improve File Rules support for v1 configs (#1417) * Adsk Contrib - Improve file rules for v1 configs Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Adsk Contrib - Emergency GPU build fix (#1391) Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Adsk Contrib - Fix some bugs found by Maya and SonarCloud (#1403) * Adsk Contrib - Fix some bugs found by Maya and SonarCloud Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Fix a Windows warning Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * After Effects and Photoshop plug-in updates (#1373) * Set Mac OS deployment target to 10.10 Signed-off-by: Brendan Bolles <brendan@fnordware.com> * More 10.10 targets Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Ditch ADD_EXTRA_BUILTINS Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Getting strange callback when AE effect is pasted Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Use NDEBUG in AE project Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Disable radio buttons with no config Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Update version Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Move to AE 2021 SDK, set PF_OutFlag2_MUTABLE_RENDER_SEQUENCE_DATA_SLOWER Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Define OCIO_DEPRECATED in After Effects builds Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Verify change from color space menu Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Handle situation where color space has a '/' in it Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Windows family separator fix Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Invert everything Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Invert everything Windows Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Disable more controls Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Incorporate invert into LUT export Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Get rid of fullPaths stuff Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Fix changed config settings retention Signed-off-by: Brendan Bolles <brendan@fnordware.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Add OpenColorIOConfig.cmake generation (#1397) * Initial OpenColorIO Config CMake implementation Signed-off-by: Rémi Achard <remiachard@gmail.com> * Remove macro, improve documentation Signed-off-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Use CMake visibility flags (#1411) * Update install doc minimal version numbers Signed-off-by: Rémi Achard <remiachard@gmail.com> * Use CMake visibility presets variables Signed-off-by: Rémi Achard <remiachard@gmail.com> * Fix python package case Signed-off-by: Rémi Achard <remiachard@gmail.com> * Allow advanced user to override symbol visibility Signed-off-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Fix Windows build Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Update include/OpenColorIO/OpenColorIO.h Co-authored-by: Michael Dolan <michdolan@gmail.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Update src/OpenColorIO/FileRules.cpp Co-authored-by: Michael Dolan <michdolan@gmail.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Hide dependency symbol visibility (#1409) (#1416) When creating libOpenColorIO.so, we lacked the linker commands that hide symbol visibility from the dependent libraries, which is necessary to prevent OCIO from exporting the symbols from Expat and the other dependencies that OCIO needs to use internally but is not trying intentionally to expose via its API. Failing to do this can result in symbol clashes and all sorts of subtle errors if OCIO is used in an app that also uses and is linked against a potentially different version of Expat (or any of the other deps). Signed-off-by: Larry Gritz <lg@larrygritz.com> Co-authored-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Adsk Contrib - Improve DX11 & Cg fragment shader language support (#1406) * Adsk Contrib - Improve DX11 support Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Improve Cg support Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Improve GPU comments Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Fix unit test failures Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> Co-authored-by: Brendan Bolles <brendan@fnordware.com> Co-authored-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Michael Dolan <michdolan@gmail.com> Co-authored-by: Larry Gritz <lg@larrygritz.com> Co-authored-by: doug-walker <43830961+doug-walker@users.noreply.github.com> * Add missing file change Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> Co-authored-by: Brendan Bolles <brendan@fnordware.com> Co-authored-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Michael Dolan <michdolan@gmail.com> Co-authored-by: Larry Gritz <lg@larrygritz.com> Co-authored-by: doug-walker <43830961+doug-walker@users.noreply.github.com>
AcademySoftwareFoundation · Aug 12, 2021 · d7b41f6 · d7b41f6
1 parent 9eb736b
commit d7b41f6
Show file tree

Hide file tree

Showing 9 changed files with 722 additions and 236 deletions.
diff --git a/docs/guides/authoring/overview.rst b/docs/guides/authoring/overview.rst
@@ -300,40 +300,40 @@ to use to control other types of tasks not listed below.
 
 * ``color_picking`` - colors in a color-selection UI can be displayed
   in this space, while selecting colors in a different working space
-  (e.g. ``scene_linear`` or ``texture_paint``)
+  (e.g. ``scene_linear`` or ``texture_paint``).
 
 * ``color_timing`` - color space used for applying color corrections,
   e.g. user-specified grade within an image viewer (if the application
   uses the ``DisplayTransform::setDisplayCC`` API method)
 
 * ``compositing_log`` - a log color space used for certain processing
   operations (plate resizing, pulling keys, degrain, etc). Used by the
-  OCIOLogConvert Nuke node
+  OCIOLogConvert Nuke node.
 
 * ``data`` - used when writing data outputs such as normals, depth
   data, and other "non color" data. The color space in this role should
   typically have ``data: true`` specified, so no color transforms are
-  applied
+  applied.
 
 * ``default`` - when ``strictparsing: false``, this color space is used
-  as a fallback. If not defined, the ``scene_linear`` role is used
+  as a fallback.
 
 * ``matte_paint`` - color space which matte-paintings are created in
   (for more information, :ref:`see the guide on baking ICC profiles
   for Photoshop <userguide-bakelut-photoshop>`, and
-  :ref:`config-spivfx`)
+  :ref:`config-spivfx`).
 
 * ``reference`` - the color space against which the other color spaces
-  are defined
+  are defined.
 
 .. note::
    The reference role has sometimes been misinterpreted as being the
    space in which "reference art" is stored in.
 
 * ``scene_linear`` - the scene-referred linear-to-light color space,
-  often the same as the reference space (see:ref:`faq-terminology`)
+  often the same as the reference space (see:ref:`faq-terminology`).
 
 * ``texture_paint`` - similar to ``matte_paint`` but for painting
   textures for 3D objects (see the description of texture painting in
-  :ref:`SPI's pipeline <config-spipipeline-texture>`)
+  :ref:`SPI's pipeline <config-spipipeline-texture>`).
 
diff --git a/docs/guides/authoring/rules.rst b/docs/guides/authoring/rules.rst
@@ -115,7 +115,7 @@ determine the colorspace ``lnf`` (it being the right-most substring
 containing a colorspace name)
 
 However, if the colorspace cannot be determined and ``strictparsing:
-true``, it will produce an error.
+true``, it will return an empty string.
 
 If the colorspace cannot be determined and ``strictparsing: false``,
 the default role will be used. This allows unhandled images to operate

diff --git a/include/OpenColorIO/OpenColorIO.h b/include/OpenColorIO/OpenColorIO.h
@@ -994,7 +994,8 @@ class OCIOEXPORT Config
      */
     void setFileRules(ConstFileRulesRcPtr fileRules);
 
-    ///  Get the color space of the first rule that matched filePath.
+    /// Get the color space of the first rule that matched filePath. (For v1 configs, this is
+    /// equivalent to calling parseColorSpaceFromString with strictparsing set to false.)
     const char * getColorSpaceFromFilepath(const char * filePath) const;
 
     /**
@@ -1149,64 +1150,56 @@ class OCIOEXPORT Config
 extern OCIOEXPORT std::ostream& operator<< (std::ostream&, const Config&);
 
 
-
-// TODO: Move to .rst
-// FileRules
-// *********
-// The File Rules are a set of filepath to color space mappings that are evaluated
-// from first to last. The first rule to match is what determines which color space is
-// returned. There are four types of rules available. Each rule type has a name key that may
-// be used by applications to refer to that rule. Name values must be unique i.e. using a
-// case insensitive comparison. The other keys depend on the rule type:
-//
-// - Basic Rule: This is the basic rule type that uses Unix glob style pattern matching and
-//   is thus very easy to use. It contains the keys:
-//
-//   * name: Name of the rule
-//
-//   * colorspace: Color space name to be returned.
-//
-//   * pattern: Glob pattern to be used for the main part of the name/path.
-//
-//   * extension: Glob pattern to be used for the file extension. Note that if glob tokens
-//     are not used, the extension will be used in a non-case-sensitive way by default.
-//
-// - Regex Rule: This is similar to the basic rule but allows additional capabilities for
-//   power-users. It contains the keys:
-//
-//   * name: Name of the rule
-//
-//   * colorspace: Color space name to be returned.
-//
-//   * regex: Regular expression to be evaluated.
-//
-// - OCIO v1 style Rule: This rule allows the use of the OCIO v1 style, where the string
-//   is searched for color space names from the config. This rule may occur 0 or 1 times
-//   in the list. The position in the list prioritizes it with respect to the other rules.
-//   StrictParsing is not used. If no color space is found in the path, the rule will not
-//   match and the next rule will be considered.
-//   \see FileRules::insertPathSearchRule.
-//   It has the key:
-//
-//   * name: Must be "ColorSpaceNamePathSearch".
-//
-// - Default Rule: The file_rules must always end with this rule. If no prior rules match,
-//   this rule specifies the color space applications will use.
-//   \see FileRules::setDefaultRuleColorSpace.
-//   It has the keys:
-//
-//   * name: must be "Default".
-//
-//   * colorspace : Color space name to be returned.
-//
-// Custom string keys and associated string values may be used to convey app or
-// workflow-specific information, e.g. whether the color space should be left as is
-// or converted into a working space.
-//
-// Getters and setters are using the rule position, they will throw if the position is not
-// valid. If the rule at the specified position does not implement the requested property
-// getter will return NULL and setter will throw.
-//
+/**
+ * \brief
+ * The File Rules are a set of filepath to color space mappings that are evaluated
+ * from first to last. The first rule to match is what determines which color space is
+ * returned. There are four types of rules available. Each rule type has a name key that may
+ * be used by applications to refer to that rule. Name values must be unique i.e. using a
+ * case insensitive comparison. The other keys depend on the rule type:
+ *
+ * * *Basic Rule*: This is the basic rule type that uses Unix glob style pattern matching and
+ *   is thus very easy to use. It contains the keys:
+ *     * name: Name of the rule
+ *     * colorspace: Color space name to be returned.
+ *     * pattern: Glob pattern to be used for the main part of the name/path.
+ *     * extension: Glob pattern to be used for the file extension. Note that if glob tokens
+ *       are not used, the extension will be used in a non-case-sensitive way by default.
+ * 
+ * * *Regex Rule*: This is similar to the basic rule but allows additional capabilities for
+ *   power-users. It contains the keys:
+ *     * name: Name of the rule
+ *     * colorspace: Color space name to be returned.
+ *     * regex: Regular expression to be evaluated.
+ * 
+ * * *OCIO v1 style Rule*: This rule allows the use of the OCIO v1 style, where the string
+ *   is searched for color space names from the config. This rule may occur 0 or 1 times
+ *   in the list. The position in the list prioritizes it with respect to the other rules.
+ *   StrictParsing is not used. If no color space is found in the path, the rule will not
+ *   match and the next rule will be considered.
+ *   see \ref insertPathSearchRule.
+ *   It has the key:
+ *     * name: Must be "ColorSpaceNamePathSearch".
+ * 
+ * * *Default Rule*: The file_rules must always end with this rule. If no prior rules match,
+ *   this rule specifies the color space applications will use.
+ *   see \ref setDefaultRuleColorSpace.
+ *   It has the keys:
+ *     * name: must be "Default".
+ *     * colorspace : Color space name to be returned.
+ *
+ * Custom string keys and associated string values may be used to convey app or
+ * workflow-specific information, e.g. whether the color space should be left as is
+ * or converted into a working space.
+ *
+ * Getters and setters are using the rule position, they will throw if the position is not
+ * valid. If the rule at the specified position does not implement the requested property
+ * getter will return NULL and setter will throw.
+ *
+ * When loading a v1 config, a set of FileRules are created with ColorSpaceNamePathSearch followed
+ * by the Default rule pointing to the default role. This allows getColorSpaceFromFilepath to emulate
+ * OCIO v1 code that used parseColorSpaceFromString with strictparsing set to false.
+ */
 
 class OCIOEXPORT FileRules
 {

diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp
@@ -192,24 +192,6 @@ void GetColorSpaceReferences(std::set<std::string> & colorSpaceNames,
     }
 }
 
-static constexpr char AddedDefault[]{ "added_default_rule_colorspace" };
-
-void FindAvailableName(const ColorSpaceSetRcPtr & colorspaces, std::string & csname)
-{
-    int i = 0;
-    csname = AddedDefault;
-    while (true)
-    {
-        if (!colorspaces->hasColorSpace(csname.c_str()))
-        {
-            break;
-        }
-
-        csname = AddedDefault + std::to_string(i);
-        ++i;
-    }
-}
-
 // Views are stored in two vectors of objects, using pointers to temporarily group them.
 typedef std::vector<const View *> ViewPtrVec;
 
@@ -591,56 +573,6 @@ class Config::Impl
 
     static ConstConfigRcPtr Read(std::istream & istream, const char * filename);
 
-    // Upgrade from v1 to v2.
-    void upgradeFromVersion1ToVersion2() noexcept
-    {
-        // V2 adds file_rules and these require a default rule. We try to initialize the default
-        // rule using the default role. If the default role doesn't exist, we look for a Raw
-        // ColorSpace with isdata true. If that is not found either, we add new ColorSpace
-        // named "added_default_rule_colorspace".
-
-        m_majorVersion = 2;
-        m_minorVersion = 0;
-
-        const char * rname = LookupRole(m_roles, ROLE_DEFAULT);
-        if (!hasColorSpace(rname))
-        {
-            std::string defaultCS;
-            bool addNewDefault = true;
-            // The default role doesn't exist so look for a color space named "raw"
-            // (not case-sensitive) with isdata true.
-            const int csindex = getColorSpaceIndex("raw");
-            if (-1 != csindex)
-            {
-                auto cs = m_allColorSpaces->getColorSpaceByIndex(csindex);
-                if (cs->isData())
-                {
-                    // "Raw" color space can be used for default.
-                    addNewDefault = false;
-                    defaultCS = cs->getName();
-                }
-            }
-
-            if (addNewDefault)
-            {
-                FindAvailableName(m_allColorSpaces, defaultCS);
-                auto newCS = ColorSpace::Create();
-                newCS->setName(defaultCS.c_str());
-                newCS->setIsData(true);
-                m_allColorSpaces->addColorSpace(newCS);
-                // Put the added CS in the inactive list to avoid it showing up in user menus.
-                if (!m_inactiveColorSpaceNamesConf.empty())
-                {
-                    m_inactiveColorSpaceNamesConf += ",";
-                }
-                m_inactiveColorSpaceNamesConf += defaultCS;
-                setInactiveColorSpaces(m_inactiveColorSpaceNamesConf.c_str());
-            }
-
-            m_fileRules->setColorSpace(m_fileRules->getNumEntries() - 1, defaultCS.c_str());
-        }
-    }
-
     // Validate view object that can be a config defined shared view or a display-defined view.
     void validateView(const std::string & display, const View & view, bool checkUseDisplayName) const
     {
@@ -1274,15 +1206,19 @@ void Config::upgradeToLatestVersion() noexcept
     {
         if (wasVersion == 1)
         {
-            m_impl->upgradeFromVersion1ToVersion2();
+            UpdateFileRulesFromV1ToV2(*this, m_impl->m_fileRules);
+
+            // The instance version is now 2.0
+            m_impl->m_majorVersion = 2;
+            m_impl->m_minorVersion = 0;
         }
+
         static_assert(LastSupportedMajorVersion == 2, "Config: Handle newer versions");
         setMajorVersion(LastSupportedMajorVersion);
         setMinorVersion(LastSupportedMinorVersion[LastSupportedMajorVersion - 1]);
     }
 }
 
-
 ConfigRcPtr Config::createEditableCopy() const
 {
     ConfigRcPtr config = Config::Create();
@@ -1762,26 +1698,17 @@ void Config::validate() const
 
     ///// FileRules
 
-    // All Config objects have a fileRules object, regardless of version. This object is
-    // initialized to have a defaultRule with the color space set to "default" (i.e., the default
-    // role). The fileRules->validate call will validate that all color spaces used in rules
-    // exist, or if they are roles that they point to a color space that exists. Because this would
-    // cause validate to improperly fail on v1 configs (since they are not required to actually
-    // contain file rules), we don't do this check on v1 configs when there is only one rule.
-    if (getMajorVersion() >= 2 || getImpl()->m_fileRules->getNumEntries() != 1)
+    try
     {
-        try
-        {
-            getImpl()->m_fileRules->getImpl()->validate(*this);
-        }
-        catch (const Exception & e)
-        {
-            std::ostringstream os;
-            os << "Config failed validation. File rules failed with: ";
-            os << e.what();
-            getImpl()->m_validationtext = os.str();
-            throw Exception(getImpl()->m_validationtext.c_str());
-        }
+        getImpl()->m_fileRules->getImpl()->validate(*this);
+    }
+    catch (const Exception & e)
+    {
+        std::ostringstream os;
+        os << "Config failed validation. File rules failed with: ";
+        os << e.what();
+        getImpl()->m_validationtext = os.str();
+        throw Exception(getImpl()->m_validationtext.c_str());
     }
 
     ///// Resolve all file Transforms using context variables.
@@ -4723,7 +4650,7 @@ void Config::Impl::checkVersionConsistency() const
 
     // Check for the file rules.
 
-    if (m_majorVersion < 2 && m_fileRules->getNumEntries() > 1)
+    if (m_majorVersion < 2 && m_fileRules->getNumEntries() > 2)
     {
         throw Exception("Only version 2 (or higher) can have file rules.");
     }