Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API won't write the environment section if config version is set to 1.0 #1903

Closed
kenmcgaugh opened this issue Nov 8, 2023 · 3 comments · Fixed by #1904
Closed

API won't write the environment section if config version is set to 1.0 #1903

kenmcgaugh opened this issue Nov 8, 2023 · 3 comments · Fixed by #1904
Assignees

Comments

@kenmcgaugh
Copy link

When serializing a config via the API, the environment section is omitted if the config's version is set to 1.0. However the environment section is supported in OpenColorIO-1.0.9 (the version currently used in our pipeline). I believe the offending line is here:

if (configMajorVersion >= 2)

@remia
Copy link
Collaborator

remia commented Nov 9, 2023

We also faced the same issue, our workaround for now has been to simply add the environment back manually after the .ocio file is generated (which is trivial to do). I believe there was a discussion on Slack a while back about this issue but it is now long gone unfortunately. I remember mention of backward compatibility issues, @doug-walker do you remember anything?

@kenmcgaugh
Copy link
Author

That is also my current workaround. I serialise to a string and then insert the environment settings before writing it to disk.

@doug-walker
Copy link
Collaborator

Here is a relevant section from OpenColorIO.h:

     * \brief The EnvironmentMode controls the behavior of loadEnvironment.
     *  * ENV_ENVIRONMENT_LOAD_PREDEFINED - Only update vars already added to the Context.
     *  * ENV_ENVIRONMENT_LOAD_ALL        - Load all env. vars into the Context.
     *
     * \note Loading ALL the env. vars may reduce performance and reduce cache efficiency.
     *
     * Client programs generally will not use these methods because the EnvironmentMode is
     * set automatically when a Config is loaded.  If the Config has an "environment"
     * section, the mode is set to LOAD_PREDEFINED, and otherwise set to LOAD_ALL.

Note that if the environment section exists, it must list all of the context vars used in the config.

For v2 configs, it is essentially always in LOAD_PREDEFINED mode because serialization always includes the environment section. We felt this was important for both performance and readability of configs.

We did not want to impose this on v1 configs and so the intent was to not write an empty environment section in that case. But unfortunately the code was sloppy and did not check to see if the environment had values in it, in which case it should be written. We will fix that. It was quite rare to use the environment section with v1 configs, which is probably why no one has flagged it so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants