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

Adsk Contrib - Allow read and write of file rules without color space validation #1976

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 11 additions & 15 deletions src/OpenColorIO/OCIOYaml.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4034,7 +4034,7 @@ inline void load(const YAML::Node & node, ViewingRulesRcPtr & vr)
catch (Exception & ex)
{
std::ostringstream os;
os << "File rules: " << ex.what();
os << "Viewing rules: " << ex.what();
throwError(node, os.str().c_str());
}
}
Expand Down Expand Up @@ -4658,13 +4658,15 @@ inline void load(const YAML::Node& node, ConfigRcPtr & config, const char* filen
config->setWorkingDir(configrootdir.c_str());
}

auto defaultCS = config->getColorSpace(ROLE_DEFAULT);
if (!fileRulesFound)
{
if (config->getMajorVersion() >= 2)
{
if (!defaultCS)
if (!config->hasRole(ROLE_DEFAULT))
{
// Note that no validation of the default color space is done (e.g. to check that
// it exists in the config) in order to enable loading configs that are only
// partially complete. The caller may use config->validate() after, if desired.
throwError(node, "The config must contain either a Default file rule or "
"the 'default' role.");
}
Expand All @@ -4683,6 +4685,7 @@ inline void load(const YAML::Node& node, ConfigRcPtr & config, const char* filen
else
{
// If default role is also defined.
auto defaultCS = config->getColorSpace(ROLE_DEFAULT);
if (defaultCS)
{
const auto defaultRule = fileRules->getNumEntries() - 1;
Expand Down Expand Up @@ -4815,18 +4818,11 @@ inline void save(YAML::Emitter & out, const Config & config)
const char* role = config.getRoleName(i);
if(role && *role)
{
ConstColorSpaceRcPtr colorspace = config.getColorSpace(role);
if(colorspace)
{
out << YAML::Key << role;
out << YAML::Value << config.getColorSpace(role)->getName();
}
else
{
std::ostringstream os;
os << "Colorspace associated to the role '" << role << "', does not exist.";
throw Exception(os.str().c_str());
}
// Note that no validation of the name strings is done here (e.g. to check that
// they exist in the config) in order to enable serializing configs that are only
// partially complete. The caller may use config->validate() first, if desired.
out << YAML::Key << role;
out << YAML::Value << config.getRoleColorSpace(i);
}
}
out << YAML::EndMap;
Expand Down
10 changes: 0 additions & 10 deletions tests/cpu/Config_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1911,16 +1911,6 @@ OCIO_ADD_TEST(Config, context_variable_with_search_path_v2)
}
}

OCIO_ADD_TEST(Config, role_without_colorspace)
{
OCIO::ConfigRcPtr config = OCIO::Config::Create()->createEditableCopy();
config->setRole("reference", "UnknownColorSpace");

std::ostringstream os;
OCIO_CHECK_THROW_WHAT(config->serialize(os), OCIO::Exception,
"Colorspace associated to the role 'reference', does not exist");
}

OCIO_ADD_TEST(Config, env_colorspace_name)
{
// Unset the env. variable to make sure the test start in the right environment.
Expand Down
72 changes: 72 additions & 0 deletions tests/cpu/FileRules_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1831,6 +1831,78 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory)
}
}

OCIO_ADD_TEST(FileRules, read_write_incomplete_configs)
{
// It should be possible to read and write configs where that are syntactically valid
// but which are incomplete and hence would not pass validation.

// The default role references a color space that has not been defined yet.
{
constexpr char CONFIG[] = { R"(ocio_profile_version: 2
roles:
default: cs2

colorspaces:
- !<ColorSpace>
name: raw
)" };

// Test read works.
std::istringstream iss;
iss.str(CONFIG);
OCIO::ConstConfigRcPtr cfg;
OCIO_CHECK_NO_THROW(cfg = OCIO::Config::CreateFromStream(iss));
OCIO_REQUIRE_ASSERT(cfg);

// Test write works.
std::ostringstream os;
OCIO_CHECK_NO_THROW(cfg->serialize(os));

// Test that validate catches the problem.
OCIO_CHECK_THROW_WHAT(
cfg->validate(),
OCIO::Exception,
"Config failed role validation. The role 'default' refers to a color space, 'cs2', "\
"which is not defined."
);
}

// FileRules Default rule references a color space that has not been defined yet.
{
constexpr char CONFIG[] = { R"(ocio_profile_version: 2

file_rules:
- !<Rule> {name: Default, colorspace: cs2}

displays:
sRGB:
- !<View> {name: Raw, colorspace: raw}
colorspaces:
- !<ColorSpace>
name: raw
)" };

// Test read works.
std::istringstream iss;
iss.str(CONFIG);
OCIO::ConstConfigRcPtr cfg;
OCIO_CHECK_NO_THROW(cfg = OCIO::Config::CreateFromStream(iss));
OCIO_REQUIRE_ASSERT(cfg);

// Test write works.
std::ostringstream os;
OCIO_CHECK_NO_THROW(cfg->serialize(os));

// Test that validate catches the problem.
OCIO_CHECK_THROW_WHAT(
cfg->validate(),
OCIO::Exception,
"File rules: rule named 'Default' is referencing 'cs2' that is neither "\
"a color space nor a named transform."
);
}
}

OCIO_ADD_TEST(FileRules, config_v2_wrong_rule)
{
// 2 default rules.
Expand Down
Loading