On 28/12/2010, at 8:57 AM, Jeremy Selan wrote:

Thanks, I've addressed all comments and rolled it into master.

'Default' seems a little redundant in this names. config.getDefaultContext() vs config.getCurrentContext()
'Config' also seems redundant in these names. config.getConfigRootDir() vs config.getWorkingDir()
'context' varies it's position in few of the argument list.

Fixed.

What would cause the g_fileExistsCache to be flushed? I could see this could be problematic if a client app was running for a while and files started to change on disk.

Currently, nothing causes it to be flushed. Adding a library-wide
"flush cache" call is at the top of my todo list. (It was already
written up as github issue #46)

In the future is the idea to add a section into the yaml which LoadEnvironmentVariables() can use to limit which variables it looks up?

When I started adding the code, there didnt seem to be a clear
advantage to adding complexity by requiring all env-vars to be
pre-declared. Any advantages you can think of? Only one that comes
to my mind would be a place to add a default value, if the value is
not otherwise defined in the environment.

I guess I was mainly thinking about speed, I have seen people create a hell of a lot of environment variables. But your right we can add that complexity later when / if that becomes a problem.

.malcolm

-- Jeremy

On Fri, Dec 24, 2010 at 3:31 AM, Malcolm Humphreys
<malcolmh...@...> wrote:

I really like most of this change, I have a few comments which are mostly cosmetic.

'Default' seems a little redundant in this names.
config.getDefaultContext() vs config.getCurrentContext()
config.getDefaultSearchPath() vs config.getSearchPath()
config.setDefaultSearchPath() vs config.setSearchPath()

'Config' also seems redundant in these names.
config.getConfigRootDir() vs config.getWorkingDir()
config.setConfigRootDir() vs config.setWorkingDir()

'context' varies it's position in few of the argument list. It would be nice if this was more consistent. ie. BuildColorSpaceOps(ops, config, context, src, dst) vs getProcessor(src, dst, context)

It feels better to me to change getProcessor to (context, src, dst) but either way would work.

Can we rename GetPreppedSearchPath to something like ResolveSearchPaths?

What would cause the g_fileExistsCache to be flushed? I could see this could be problematic if a client app was running for a while and files started to change on disk.

In the future is the idea to add a section into the yaml which LoadEnvironmentVariables() can use to limit which variables it looks up?
eg.
context:
JOB: None
SEQ: common
SHOT: None

.malcolm

On 24/12/2010, at 9:06 AM, Jeremy Selan wrote:

Commits:
1a54ec8d76

Resolves Issue #16:
http://github.com/imageworks/OpenColorIO/issues#issue/16

This commit enables color transforms to leverage environment variables
+ search paths to determine the lut location. This will enable
workflows where the looks changes across different
shots/sequences/etc. For apps which work in the context of a single
shot, no changes are required. And, for apps which span multiple
shots (such as a playback utility), a mechanism is provided to update
the variables at runtime.

We introduce a new class, OCIO::Context, that is responsible for all
of the work involved in turning a short filename into full path. (it
handles both envvar interpolation and search paths). Profiles, on
construction, automatically get a default context which contains the
full environment. New versions of getProcessor have been added that
take a context as an argument, allowing the default context to be
overridden at runtime. If no context is provided, the default is
used.

-- Jeremy