Fix wrong display returned by getDefaultDisplay() (#638)

* Fix window-specific issue in GetEnv()

While the content of result was correct, its length was set
to a wrong size.

This is a patch by Patrick Hodoul.

* Fix wrong display returned by getDefaultDisplay()

Solved by ignoring environment variable if it's an empty string.
Ideally, need to test it to be NULL instead to support obscure
case when empty string is passed. Unfortunately, this is not
possible with the current API, and is unlikely scenario anyway.

The issue was caused by wrong logic around parsing display/views
override environment variable: if it is missing, it was converted
to an empty string. That empty string was considered an override
of one element display, with an empty name.

Additionally, make it so getDisplay() returns displays in order
of either active_displays configuration variable or environment
variable override. the issue there was caused by wrong order of
arguments to vector intersection.
This commit is contained in:
Sergey Sharybin authored and Michael Dolan committed 2019-01-29 14:33:15 -08:00
1 parent c302a2163d
commit 718cc7f9f7
3 files changed
+406 -12

No files matched your search

+374 -2
View File
@@ -274,11 +274,17 @@ OCIO_NAMESPACE_ENTER
{
std::string activeDisplays;
Platform::Getenv(OCIO_ACTIVE_DISPLAYS_ENVVAR, activeDisplays);
SplitStringEnvStyle(activeDisplaysEnvOverride_, activeDisplays.c_str());
activeDisplays = pystring::strip(activeDisplays);
if (!activeDisplays.empty()) {
SplitStringEnvStyle(activeDisplaysEnvOverride_, activeDisplays.c_str());
}
std::string activeViews;
Platform::Getenv(OCIO_ACTIVE_VIEWS_ENVVAR, activeViews);
SplitStringEnvStyle(activeViewsEnvOverride_, activeViews.c_str());
activeViews = pystring::strip(activeViews);
if (!activeViews.empty()) {
SplitStringEnvStyle(activeViewsEnvOverride_, activeViews.c_str());
}
defaultLumaCoefs_.resize(3);
defaultLumaCoefs_[0] = DEFAULT_LUMA_COEFF_R;
@@ -2585,4 +2591,370 @@ OIIO_ADD_TEST(Config, range_serialization)
}
OIIO_ADD_TEST(Config, display)
{
static const std::string SIMPLE_PROFILE_HEADER =
"ocio_profile_version: 1\n"
"\n"
"search_path: luts\n"
"strictparsing: true\n"
"luma: [0.2126, 0.7152, 0.0722]\n"
"\n"
"roles:\n"
" default: raw\n"
" scene_linear: lnh\n"
"\n"
"displays:\n"
" sRGB_1:\n"
" - !<View> {name: Raw, colorspace: raw}\n"
" sRGB_2:\n"
" - !<View> {name: Raw, colorspace: raw}\n"
" sRGB_3:\n"
" - !<View> {name: Raw, colorspace: raw}\n"
"\n";
static const std::string SIMPLE_PROFILE_FOOTER =
"\n"
"colorspaces:\n"
" - !<ColorSpace>\n"
" name: raw\n"
" family: \"\"\n"
" equalitygroup: \"\"\n"
" bitdepth: unknown\n"
" isdata: false\n"
" allocation: uniform\n"
"\n"
" - !<ColorSpace>\n"
" name: lnh\n"
" family: \"\"\n"
" equalitygroup: \"\"\n"
" bitdepth: unknown\n"
" isdata: false\n"
" allocation: uniform\n";
{
std::string myProfile =
SIMPLE_PROFILE_HEADER
+
"active_displays: []\n"
"active_views: []\n"
+ SIMPLE_PROFILE_FOOTER;
std::istringstream is(myProfile);
OCIO::ConstConfigRcPtr config;
OIIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is));
OIIO_CHECK_EQUAL(config->getNumDisplays(), 3);
OIIO_CHECK_EQUAL(std::string(config->getDisplay(0)), std::string("sRGB_1"));
OIIO_CHECK_EQUAL(std::string(config->getDisplay(1)), std::string("sRGB_2"));
OIIO_CHECK_EQUAL(std::string(config->getDisplay(2)), std::string("sRGB_3"));
OIIO_CHECK_EQUAL(std::string(config->getDefaultDisplay()), "sRGB_1");
}
{
std::string myProfile =
SIMPLE_PROFILE_HEADER
+
"active_displays: [sRGB_1]\n"
"active_views: []\n"
+ SIMPLE_PROFILE_FOOTER;
std::istringstream is(myProfile);
OCIO::ConstConfigRcPtr config;
OIIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is));
OIIO_CHECK_EQUAL(config->getNumDisplays(), 1);
OIIO_CHECK_EQUAL(std::string(config->getDisplay(0)), std::string("sRGB_1"));
OIIO_CHECK_EQUAL(std::string(config->getDefaultDisplay()), "sRGB_1");
}
{
std::string myProfile =
SIMPLE_PROFILE_HEADER
+
"active_displays: [sRGB_2, sRGB_1]\n"
"active_views: []\n"
+ SIMPLE_PROFILE_FOOTER;
std::istringstream is(myProfile);
OCIO::ConstConfigRcPtr config;
OIIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is));
OIIO_CHECK_EQUAL(config->getNumDisplays(), 2);
OIIO_CHECK_EQUAL(std::string(config->getDisplay(0)), std::string("sRGB_2"));
OIIO_CHECK_EQUAL(std::string(config->getDisplay(1)), std::string("sRGB_1"));
OIIO_CHECK_EQUAL(std::string(config->getDefaultDisplay()), "sRGB_2");
}
{
std::string myProfile =
SIMPLE_PROFILE_HEADER
+
"active_displays: []\n"
"active_views: []\n"
+ SIMPLE_PROFILE_FOOTER;
const std::string active_displays(
std::string(OCIO::OCIO_ACTIVE_DISPLAYS_ENVVAR) + "= sRGB_3, sRGB_2");
putenv(const_cast<char *>(active_displays.c_str()));
std::istringstream is(myProfile);
OCIO::ConstConfigRcPtr config;
OIIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is));
OIIO_CHECK_EQUAL(config->getNumDisplays(), 2);
OIIO_CHECK_EQUAL(std::string(config->getDisplay(0)), std::string("sRGB_3"));
OIIO_CHECK_EQUAL(std::string(config->getDisplay(1)), std::string("sRGB_2"));
OIIO_CHECK_EQUAL(std::string(config->getDefaultDisplay()), "sRGB_3");
}
{
std::string myProfile =
SIMPLE_PROFILE_HEADER
+
"active_displays: [sRGB_2, sRGB_1]\n"
"active_views: []\n"
+ SIMPLE_PROFILE_FOOTER;
const std::string active_displays(
std::string(OCIO::OCIO_ACTIVE_DISPLAYS_ENVVAR) + "= sRGB_3, sRGB_2");
putenv(const_cast<char *>(active_displays.c_str()));
std::istringstream is(myProfile);
OCIO::ConstConfigRcPtr config;
OIIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is));
OIIO_CHECK_EQUAL(config->getNumDisplays(), 2);
OIIO_CHECK_EQUAL(std::string(config->getDisplay(0)), std::string("sRGB_3"));
OIIO_CHECK_EQUAL(std::string(config->getDisplay(1)), std::string("sRGB_2"));
OIIO_CHECK_EQUAL(std::string(config->getDefaultDisplay()), "sRGB_3");
}
{
const std::string active_displays(
std::string(OCIO::OCIO_ACTIVE_DISPLAYS_ENVVAR) + "="); // No value
putenv(const_cast<char *>(active_displays.c_str()));
std::string myProfile =
SIMPLE_PROFILE_HEADER
+
"active_displays: [sRGB_2, sRGB_1]\n"
"active_views: []\n"
+ SIMPLE_PROFILE_FOOTER;
std::istringstream is(myProfile);
OCIO::ConstConfigRcPtr config;
OIIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is));
OIIO_CHECK_EQUAL(config->getNumDisplays(), 2);
OIIO_CHECK_EQUAL(std::string(config->getDisplay(0)), std::string("sRGB_2"));
OIIO_CHECK_EQUAL(std::string(config->getDisplay(1)), std::string("sRGB_1"));
OIIO_CHECK_EQUAL(std::string(config->getDefaultDisplay()), "sRGB_2");
}
{
const std::string active_displays(
std::string(OCIO::OCIO_ACTIVE_DISPLAYS_ENVVAR) + "= "); // No value, but misleading space
putenv(const_cast<char *>(active_displays.c_str()));
std::string myProfile =
SIMPLE_PROFILE_HEADER
+
"active_displays: [sRGB_2, sRGB_1]\n"
"active_views: []\n"
+ SIMPLE_PROFILE_FOOTER;
std::istringstream is(myProfile);
OCIO::ConstConfigRcPtr config;
OIIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is));
OIIO_CHECK_EQUAL(config->getNumDisplays(), 2);
OIIO_CHECK_EQUAL(std::string(config->getDisplay(0)), std::string("sRGB_2"));
OIIO_CHECK_EQUAL(std::string(config->getDisplay(1)), std::string("sRGB_1"));
OIIO_CHECK_EQUAL(std::string(config->getDefaultDisplay()), "sRGB_2");
}
}
OIIO_ADD_TEST(Config, view)
{
static const std::string SIMPLE_PROFILE_HEADER =
"ocio_profile_version: 1\n"
"\n"
"search_path: luts\n"
"strictparsing: true\n"
"luma: [0.2126, 0.7152, 0.0722]\n"
"\n"
"roles:\n"
" default: raw\n"
" scene_linear: lnh\n"
"\n"
"displays:\n"
" sRGB_1:\n"
" - !<View> {name: View_1, colorspace: raw}\n"
" - !<View> {name: View_2, colorspace: raw}\n"
" sRGB_2:\n"
" - !<View> {name: View_2, colorspace: raw}\n"
" - !<View> {name: View_3, colorspace: raw}\n"
" sRGB_3:\n"
" - !<View> {name: View_3, colorspace: raw}\n"
" - !<View> {name: View_1, colorspace: raw}\n"
"\n";
static const std::string SIMPLE_PROFILE_FOOTER =
"\n"
"colorspaces:\n"
" - !<ColorSpace>\n"
" name: raw\n"
" family: \"\"\n"
" equalitygroup: \"\"\n"
" bitdepth: unknown\n"
" isdata: false\n"
" allocation: uniform\n"
"\n"
" - !<ColorSpace>\n"
" name: lnh\n"
" family: \"\"\n"
" equalitygroup: \"\"\n"
" bitdepth: unknown\n"
" isdata: false\n"
" allocation: uniform\n";
{
std::string myProfile =
SIMPLE_PROFILE_HEADER
+
"active_displays: []\n"
"active_views: []\n"
+ SIMPLE_PROFILE_FOOTER;
std::istringstream is(myProfile);
OCIO::ConstConfigRcPtr config;
OIIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is));
OIIO_CHECK_EQUAL(std::string(config->getDefaultView("sRGB_1")), "View_1");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_1", 0)), "View_1");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_1", 1)), "View_2");
OIIO_CHECK_EQUAL(std::string(config->getDefaultView("sRGB_2")), "View_2");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_2", 0)), "View_2");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_2", 1)), "View_3");
OIIO_CHECK_EQUAL(std::string(config->getDefaultView("sRGB_3")), "View_3");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_3", 0)), "View_3");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_3", 1)), "View_1");
}
{
std::string myProfile =
SIMPLE_PROFILE_HEADER
+
"active_displays: []\n"
"active_views: [View_3]\n"
+ SIMPLE_PROFILE_FOOTER;
std::istringstream is(myProfile);
OCIO::ConstConfigRcPtr config;
OIIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is));
OIIO_CHECK_EQUAL(std::string(config->getDefaultView("sRGB_1")), "View_1");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_1", 0)), "View_1");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_1", 1)), "View_2");
OIIO_CHECK_EQUAL(std::string(config->getDefaultView("sRGB_2")), "View_3");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_2", 0)), "View_2");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_2", 1)), "View_3");
OIIO_CHECK_EQUAL(std::string(config->getDefaultView("sRGB_3")), "View_3");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_3", 0)), "View_3");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_3", 1)), "View_1");
}
{
std::string myProfile =
SIMPLE_PROFILE_HEADER
+
"active_displays: []\n"
"active_views: [View_3, View_2, View_1]\n"
+ SIMPLE_PROFILE_FOOTER;
std::istringstream is(myProfile);
OCIO::ConstConfigRcPtr config;
OIIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is));
OIIO_CHECK_EQUAL(std::string(config->getDefaultView("sRGB_1")), "View_2");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_1", 0)), "View_1");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_1", 1)), "View_2");
OIIO_CHECK_EQUAL(std::string(config->getDefaultView("sRGB_2")), "View_3");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_2", 0)), "View_2");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_2", 1)), "View_3");
OIIO_CHECK_EQUAL(std::string(config->getDefaultView("sRGB_3")), "View_3");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_3", 0)), "View_3");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_3", 1)), "View_1");
}
{
std::string myProfile =
SIMPLE_PROFILE_HEADER
+
"active_displays: []\n"
"active_views: []\n"
+ SIMPLE_PROFILE_FOOTER;
const std::string active_displays(
std::string(OCIO::OCIO_ACTIVE_VIEWS_ENVVAR) + "= View_3, View_2");
putenv(const_cast<char *>(active_displays.c_str()));
std::istringstream is(myProfile);
OCIO::ConstConfigRcPtr config;
OIIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is));
OIIO_CHECK_EQUAL(std::string(config->getDefaultView("sRGB_1")), "View_2");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_1", 0)), "View_1");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_1", 1)), "View_2");
OIIO_CHECK_EQUAL(std::string(config->getDefaultView("sRGB_2")), "View_3");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_2", 0)), "View_2");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_2", 1)), "View_3");
OIIO_CHECK_EQUAL(std::string(config->getDefaultView("sRGB_3")), "View_3");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_3", 0)), "View_3");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_3", 1)), "View_1");
}
{
std::string myProfile =
SIMPLE_PROFILE_HEADER
+
"active_displays: []\n"
"active_views: []\n"
+ SIMPLE_PROFILE_FOOTER;
const std::string active_displays(
std::string(OCIO::OCIO_ACTIVE_VIEWS_ENVVAR) + "="); // No value.
putenv(const_cast<char *>(active_displays.c_str()));
std::istringstream is(myProfile);
OCIO::ConstConfigRcPtr config;
OIIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is));
OIIO_CHECK_EQUAL(std::string(config->getDefaultView("sRGB_1")), "View_1");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_1", 0)), "View_1");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_1", 1)), "View_2");
OIIO_CHECK_EQUAL(std::string(config->getDefaultView("sRGB_2")), "View_2");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_2", 0)), "View_2");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_2", 1)), "View_3");
OIIO_CHECK_EQUAL(std::string(config->getDefaultView("sRGB_3")), "View_3");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_3", 0)), "View_3");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_3", 1)), "View_1");
}
{
std::string myProfile =
SIMPLE_PROFILE_HEADER
+
"active_displays: []\n"
"active_views: []\n"
+ SIMPLE_PROFILE_FOOTER;
const std::string active_displays(
std::string(OCIO::OCIO_ACTIVE_VIEWS_ENVVAR) + "= "); // No value, but misleading space
putenv(const_cast<char *>(active_displays.c_str()));
std::istringstream is(myProfile);
OCIO::ConstConfigRcPtr config;
OIIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is));
OIIO_CHECK_EQUAL(std::string(config->getDefaultView("sRGB_1")), "View_1");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_1", 0)), "View_1");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_1", 1)), "View_2");
OIIO_CHECK_EQUAL(std::string(config->getDefaultView("sRGB_2")), "View_2");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_2", 0)), "View_2");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_2", 1)), "View_3");
OIIO_CHECK_EQUAL(std::string(config->getDefaultView("sRGB_3")), "View_3");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_3", 0)), "View_3");
OIIO_CHECK_EQUAL(std::string(config->getView("sRGB_3", 1)), "View_1");
}
}
#endif // OCIO_UNIT_TEST
+2 -2
View File
@@ -114,13 +114,13 @@ OCIO_NAMESPACE_ENTER
// Apply the env override if it's not empty.
if(!activeDisplaysEnvOverride.empty())
{
displayCache = IntersectStringVecsCaseIgnore(displayMasterList, activeDisplaysEnvOverride);
displayCache = IntersectStringVecsCaseIgnore(activeDisplaysEnvOverride, displayMasterList);
if(!displayCache.empty()) return;
}
// Otherwise, aApply the active displays if it's not empty.
else if(!activeDisplays.empty())
{
displayCache = IntersectStringVecsCaseIgnore(displayMasterList, activeDisplays);
displayCache = IntersectStringVecsCaseIgnore(activeDisplays, displayMasterList);
if(!displayCache.empty()) return;
}
+30 -8
View File
@@ -59,8 +59,9 @@ void Getenv (const char* name, std::string& value)
}
else
{
value.resize(len+1);
::snprintf(&value[0], len, "%s", val);
// NB: len is the sizeof() of a string ( i.e. not its strlen() )
value = val;
value.resize(len-1);
if(val) free(val);
}
#else
@@ -138,12 +139,33 @@ OIIO_ADD_TEST(Platform, getenv)
OIIO_ADD_TEST(Platform, putenv)
{
const std::string value("MY_DUMMY_ENV=SomeValue");
::putenv(const_cast<char*>(value.c_str()));
std::string env;
OCIO::Platform::Getenv("MY_DUMMY_ENV", env);
OIIO_CHECK_ASSERT(!env.empty());
OIIO_CHECK_ASSERT(0==strcmp("SomeValue", env.c_str()));
{
const std::string value("MY_DUMMY_ENV=SomeValue");
::putenv(const_cast<char*>(value.c_str()));
std::string env;
OCIO::Platform::Getenv("MY_DUMMY_ENV", env);
OIIO_CHECK_ASSERT(!env.empty());
OIIO_CHECK_ASSERT(0==strcmp("SomeValue", env.c_str()));
OIIO_CHECK_EQUAL(strlen("SomeValue"), env.size());
}
{
const std::string value("MY_DUMMY_ENV= ");
::putenv(const_cast<char*>(value.c_str()));
std::string env;
OCIO::Platform::Getenv("MY_DUMMY_ENV", env);
OIIO_CHECK_ASSERT(!env.empty());
OIIO_CHECK_ASSERT(0==strcmp(" ", env.c_str()));
OIIO_CHECK_EQUAL(strlen(" "), env.size());
}
{
const std::string value("MY_DUMMY_ENV=");
::putenv(const_cast<char*>(value.c_str()));
std::string env;
OCIO::Platform::Getenv("MY_DUMMY_ENV", env);
OIIO_CHECK_ASSERT(env.empty());
}
}
OIIO_ADD_TEST(Platform, string_compare)