GetColorSpaceFromFilepath should handle v1 configs better

[doug-walker]

OCIO v2 introduced the new FileRules section of the config to add some new features around file path mapping. A new set of API calls related to that was also added. The intention is that any v1 clients that were using parseColorSpaceFromString would update to use getColorSpaceFromFilepath. The name was changed because there is a difference in behavior with the new system that developers need to think about when upgrading. ParseColorSpaceFromString still exists and still does what it used to do, but it is listed as deprecated.

The first difference is that getColorSpaceFromFilepath uses the new FileRules, so it can do the original v1 algorithm of searching for a color space name in the path (if the config author enables it), but it can also do a lot more, such as map specific file extensions to a specified color space.

The second difference is that getColorSpaceFromFilepath always returns a valid color space. This was a request from app developers that always need a default (which is a common scenario).

The behavior of parseColorSpaceFromString (in v1 and v2) actually depends on the value of the strictparsing attribute of the config. If strictparsing is true, parseColorSpaceFromString returns an empty string if the path did not contain a color space name. If strictparsing is false, it will try to return the default role in that scenario, if it exists and points to a legitimate color space (otherwise it still returns an empty string).

If an application using v2 wants to do something special in the case where the path does not contain a color space name, the suggested code is as follows:

if (config->filepathOnlyMatchesDefaultRule(path)  &&  config->isStrictParsingEnabled())  {
     do what you want to do here 
}

But most apps (in my estimation) do not need to do this and should simply be able to call getColorSpaceFromFilepath to set the default color space for a file. This should be the case regardless of the version of the config file. Ideally, the application should not need to check the version of the config and sometimes call parseColorSpaceFromString and other times call getColorSpaceFromFilepath.

The reason I created this issue is that there is currently a problem with getColorSpaceFromFilepath when used with v1 configs (which of course do not have the FileRules section). It currently is not searching the path for a color space name. We will fix that but we also need to decide what getColorSpaceFromFilepath will return for a v1 config if the default role is not present or not valid. (Remember, the call promises to return a valid color space and developers should be able to rely on this.) Here is the proposal:

if (config->getMajorVersion() == 1) {
    if PathUtils.cpp ParseColorSpaceFromString finds a match, return it
    else if the default role is a valid color space, return it
    else if there is a color space such that lower(cs) == "raw", return the first one
    else if there is a color space with isdata == true, return the first one
    else return the first color space in the config
}

The motivation for looking for a raw or data space is to essentially not apply any color management in that case. (This is essentially what OCIO does when it needs to fall back to its internal default config.) However, since ociocheck will warn if the default role is missing or does not point to a legitimate color space, these "fall-back fall-backs" would never be reached on a correct config.

Note that for v2 configs, no fall-back is needed since config validation ensures that either the default role or default FileRule provides a valid color space.

Side note: I also noticed a bug in both the v1 and v2 documentation. The section on strictparsing says this:
"However, if the colorspace cannot be determined and strictparsing: true, it will produce an error."
But as stated above, this is incorrect, it does not (v2) and did not (v1) throw, it returns an empty string.

[lgritz]

That all makes sense to me, and I'm convinced that the new getColorSpaceFromFilepath is altogether better. My complications are partly due to the fact that OIIO can't control which OCIO it's being built against, so I need to make sure that when people build against OCIO v1 it still works, and for that matter, when they build against v2 but use a v1 config, behavior shouldn't change if they're using it in production (or for my unit tests, for that matter). For the combination of OIIO built against OCIOv2 and also using a v2 config, I'm very happy to rely on getColorSpaceFromFilepath's out-of-the-box behavior, even if that's different from v1 (going to a new config should be expected to include changing improvements, I think).

[lgritz]

The new file rules are very nice, and a big improvement. Its addition had slipped under the radar for me entirely, until I started to get deprecation warnings about parseColorSpaceFromString, forcing me to notice the docs about the new function. :-)

[doug-walker]

Closing this as fixed with PR #1417. @hodoulp, this should be included in the 2.0.2 release.