Skip to content

Commit

Permalink
Merge pull request #24 from autodesk-forks/flame-category-fix
Browse files Browse the repository at this point in the history
Allow ColorSpaceMenuHelper to include color spaces that are missing categories
  • Loading branch information
doug-walker authored Oct 26, 2024
2 parents 583cac6 + 7b24cca commit 45d3aba
Show file tree
Hide file tree
Showing 12 changed files with 155 additions and 91 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@

Include all named transforms (or not) to :ref:`ColorSpaceMenuHelper`. Default is not to include named transforms.

.. py:method:: ColorSpaceMenuParameters.setTreatNoCategoriesAsAny(self: PyOpenColorIO.ColorSpaceMenuParameters, includeNamedTransforms: bool = True) -> None
:module: PyOpenColorIO

When searching for color spaces using app or user categories, treat color spaces that have no categories as if they had any categories. Default is not to treat them this way.

.. py:method:: ColorSpaceMenuParameters.setIncludeRoles(self: PyOpenColorIO.ColorSpaceMenuParameters, includeRoles: bool = True) -> None
:module: PyOpenColorIO
Expand Down
6 changes: 6 additions & 0 deletions include/OpenColorIO/OpenColorAppHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,12 @@ class OCIOEXPORT ColorSpaceMenuParameters
virtual void setIncludeNamedTransforms(bool include) noexcept = 0;
virtual bool getIncludeNamedTransforms() const noexcept = 0;

/**
* When searching for color spaces using app or user categories, treat color spaces that have
* no categories as if they had any categories. Default is not to treat them this way.
*/
virtual void setTreatNoCategoryAsAny(bool value) noexcept = 0;
virtual bool getTreatNoCategoryAsAny() const noexcept = 0;

/**
* App categories is a comma separated list of categories. If appCategories is not NULL and
Expand Down
24 changes: 17 additions & 7 deletions src/OpenColorIO/apphelpers/CategoryHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ bool HasEncoding(const T & elt, const std::string & encoding)

ColorSpaceVec GetColorSpaces(ConstConfigRcPtr config,
bool includeColorSpaces,
bool treatNoCategoryAsAny,
SearchReferenceSpaceType colorSpaceType,
const Categories & categories,
const Encodings & encodings)
Expand All @@ -62,11 +63,13 @@ ColorSpaceVec GetColorSpaces(ConstConfigRcPtr config,
auto cs = config->getColorSpace(config->getColorSpaceNameByIndex(colorSpaceType,
COLORSPACE_ACTIVE,
idx));

const bool skipCategory = treatNoCategoryAsAny && cs->getNumCategories() == 0;
for (const auto & cat : categories)
{
for (const auto & enc : encodings)
{
if (HasCategory(cs, cat) && HasEncoding(cs, enc))
if ((skipCategory || HasCategory(cs, cat)) && HasEncoding(cs, enc))
{
AddElement(css, cs.get());
}
Expand All @@ -79,6 +82,7 @@ ColorSpaceVec GetColorSpaces(ConstConfigRcPtr config,

ColorSpaceVec GetColorSpaces(ConstConfigRcPtr config,
bool includeColorSpaces,
bool treatNoCategoryAsAny,
SearchReferenceSpaceType colorSpaceType,
const Categories & categories)
{
Expand All @@ -91,9 +95,11 @@ ColorSpaceVec GetColorSpaces(ConstConfigRcPtr config,
auto cs = config->getColorSpace(config->getColorSpaceNameByIndex(colorSpaceType,
COLORSPACE_ACTIVE,
idx));

const bool skipCategory = treatNoCategoryAsAny && cs->getNumCategories() == 0;
for (const auto & cat : categories)
{
if (HasCategory(cs, cat))
if (skipCategory || HasCategory(cs, cat))
{
AddElement(css, cs.get());
}
Expand Down Expand Up @@ -267,7 +273,7 @@ StringUtils::StringVec ExtractItems(const char * strings)

ColorSpaceNames FindColorSpaceNames(ConstConfigRcPtr config, const Categories & categories)
{
ColorSpaceVec allCS = GetColorSpaces(config, true, SEARCH_REFERENCE_SPACE_ALL, categories);
ColorSpaceVec allCS = GetColorSpaces(config, true, false, SEARCH_REFERENCE_SPACE_ALL, categories);
return GetNames(allCS);
}

Expand Down Expand Up @@ -338,6 +344,7 @@ Infos FindColorSpaceInfos(ConstConfigRcPtr config,
const Categories & userCategories,
bool includeColorSpaces,
bool includeNamedTransforms,
bool treatNoCategoryAsAny,
const Encodings & encodings,
SearchReferenceSpaceType colorSpaceType)
{
Expand Down Expand Up @@ -369,7 +376,7 @@ Infos FindColorSpaceInfos(ConstConfigRcPtr config,

if (!encsIgnored)
{
appCS = GetColorSpaces(config, includeColorSpaces, colorSpaceType,
appCS = GetColorSpaces(config, includeColorSpaces, treatNoCategoryAsAny, colorSpaceType,
appCategories, encodings);
appNT = GetNamedTransforms(config, includeNamedTransforms, appCategories,
encodings);
Expand All @@ -381,7 +388,9 @@ Infos FindColorSpaceInfos(ConstConfigRcPtr config,
{
encsIgnored = true;
log.m_ignoreEncodings = !encodings.empty();
appCS = GetColorSpaces(config, includeColorSpaces, colorSpaceType, appCategories);
appCS = GetColorSpaces(config, includeColorSpaces, treatNoCategoryAsAny, colorSpaceType,
appCategories);

appNT = GetNamedTransforms(config, includeNamedTransforms, appCategories);
appSize = appCS.size() + appNT.size();

Expand Down Expand Up @@ -424,7 +433,8 @@ Infos FindColorSpaceInfos(ConstConfigRcPtr config,
{
// 3b) Items using user categories.

userCS = GetColorSpaces(config, includeColorSpaces, colorSpaceType, userCategories);
userCS = GetColorSpaces(config, includeColorSpaces, treatNoCategoryAsAny, colorSpaceType,
userCategories);
userNT = GetNamedTransforms(config, includeNamedTransforms, userCategories);
userSize = userCS.size() + userNT.size();
if (userSize == 0)
Expand Down Expand Up @@ -464,7 +474,7 @@ Infos FindColorSpaceInfos(ConstConfigRcPtr config,
{
// If not already computed, compute list with app categories and no
// encodings.
appCSNoEncodings = GetColorSpaces(config, includeColorSpaces,
appCSNoEncodings = GetColorSpaces(config, includeColorSpaces, treatNoCategoryAsAny,
colorSpaceType, appCategories);
appNTNoEncodings = GetNamedTransforms(config, includeNamedTransforms,
appCategories);
Expand Down
1 change: 1 addition & 0 deletions src/OpenColorIO/apphelpers/CategoryHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ Infos FindColorSpaceInfos(ConstConfigRcPtr config,
const Categories & userCategories,
bool includeColorSpaces,
bool includeNamedTransforms,
bool treatNoCategoryAsAny,
const Encodings & encodings,
SearchReferenceSpaceType colorSpaceType);

Expand Down
13 changes: 13 additions & 0 deletions src/OpenColorIO/apphelpers/ColorSpaceHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ void ColorSpaceMenuParametersImpl::setParameters(ConstColorSpaceMenuParametersRc
m_includeColorSpaces = impl.m_includeColorSpaces;
m_includeRoles = impl.m_includeRoles;
m_includeNamedTransforms = impl.m_includeNamedTransforms;
m_treatNoCategoryAsAny = impl.m_treatNoCategoryAsAny;
m_colorSpaceType = impl.m_colorSpaceType;
m_additionalColorSpaces = impl.m_additionalColorSpaces;
}
Expand Down Expand Up @@ -268,6 +269,16 @@ bool ColorSpaceMenuParametersImpl::getIncludeNamedTransforms() const noexcept
return m_includeNamedTransforms;
}

void ColorSpaceMenuParametersImpl::setTreatNoCategoryAsAny(bool value) noexcept
{
m_treatNoCategoryAsAny = value;
}

bool ColorSpaceMenuParametersImpl::getTreatNoCategoryAsAny() const noexcept
{
return m_treatNoCategoryAsAny;
}

void ColorSpaceMenuParametersImpl::addColorSpace(const char * name) noexcept
{
if (name && *name && !StringUtils::Contain(m_additionalColorSpaces, name))
Expand Down Expand Up @@ -334,6 +345,7 @@ std::ostream & operator<<(std::ostream & os, const ColorSpaceMenuParameters & p)
os << ", includeColorSpaces: " << (p.getIncludeColorSpaces() ? "true" : "false");
os << ", includeRoles: " << (p.getIncludeRoles() ? "true" : "false");
os << ", includeNamedTransforms: " << (p.getIncludeNamedTransforms() ? "true" : "false");
os << ", treatNoCategoryAsAny: " << (p.getTreatNoCategoryAsAny() ? "true" : "false");
if (impl.m_colorSpaceType == SEARCH_REFERENCE_SPACE_SCENE)
{
os << ", colorSpaceType: scene";
Expand Down Expand Up @@ -481,6 +493,7 @@ void ColorSpaceMenuHelperImpl::refresh()
allAppCategories, allUserCategories,
m_parameters.m_includeColorSpaces,
m_parameters.m_includeNamedTransforms,
m_parameters.m_treatNoCategoryAsAny,
allEncodings, m_parameters.m_colorSpaceType);
}

Expand Down
4 changes: 4 additions & 0 deletions src/OpenColorIO/apphelpers/ColorSpaceHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ class ColorSpaceMenuParametersImpl : public ColorSpaceMenuParameters
bool getIncludeRoles() const noexcept override;
void setIncludeNamedTransforms(bool include) noexcept override;
bool getIncludeNamedTransforms() const noexcept override;
void setTreatNoCategoryAsAny(bool value) noexcept override;
bool getTreatNoCategoryAsAny() const noexcept override;

SearchReferenceSpaceType getSearchReferenceSpaceType() const noexcept override;
void setSearchReferenceSpaceType(SearchReferenceSpaceType colorspaceType) noexcept override;

Expand All @@ -127,6 +130,7 @@ class ColorSpaceMenuParametersImpl : public ColorSpaceMenuParameters
bool m_includeColorSpaces = true;
bool m_includeRoles = false;
bool m_includeNamedTransforms = false;
bool m_treatNoCategoryAsAny = false;
SearchReferenceSpaceType m_colorSpaceType = SEARCH_REFERENCE_SPACE_ALL;

StringUtils::StringVec m_additionalColorSpaces;
Expand Down
4 changes: 2 additions & 2 deletions src/OpenColorIO/ops/fixedfunction/FixedFunctionOpCPU.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1547,7 +1547,7 @@ void applyHSYToRGB(const void * inImg, void * outImg, long numPixels, float min0
const float * in = (const float *)inImg;
float * out = (float *)outImg;

for(unsigned idx=0; idx<numPixels; ++idx)
for(long idx=0; idx<numPixels; ++idx)
{
float hue = in[0] - 1.f/6.f;
float sat = in[1];
Expand Down Expand Up @@ -1658,7 +1658,7 @@ void applyRGBToHSY(const void * inImg, void * outImg, long numPixels, float min0
const float loGain = 5.f;
const float satLo = distRgb * loGain;
const float maxLum = 0.01f;
const float minLum = maxLum * 0.1;
const float minLum = maxLum * 0.1f;
const float alpha = CLAMP( (luma - minLum) / (maxLum - minLum), 0.f, 1.f );
sat = satLo + alpha * (satHi - satLo);
sat *= 1.4f;
Expand Down
8 changes: 8 additions & 0 deletions src/bindings/python/apphelpers/PyColorSpaceHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ void bindPyColorSpaceMenuHelpers(py::module & m)
bool includeColorSpaces,
SearchReferenceSpaceType searchReferenceSpaceType,
bool includeNamedTransforms,
bool treatNoCategoryAsAny,
const std::string & appCategories,
const std::string & encodings,
const std::string & userCategories,
Expand All @@ -90,13 +91,15 @@ void bindPyColorSpaceMenuHelpers(py::module & m)
p->setIncludeColorSpaces(includeColorSpaces);
p->setIncludeRoles(includeRoles);
p->setIncludeNamedTransforms(includeNamedTransforms);
p->setTreatNoCategoryAsAny(treatNoCategoryAsAny);
return p;
}),
"config"_a.none(false),
"role"_a.none(false) = "",
"includeColorSpaces"_a = true,
"searchReferenceSpaceType"_a = SEARCH_REFERENCE_SPACE_ALL,
"includeNamedTransforms"_a = false,
"treatNoCategoryAsAny"_a = false,
"appCategories"_a.none(false) = "",
"encodings"_a.none(false) = "",
"userCategories"_a.none(false) = "",
Expand Down Expand Up @@ -126,6 +129,11 @@ void bindPyColorSpaceMenuHelpers(py::module & m)
.def("setIncludeNamedTransforms", &ColorSpaceMenuParameters::setIncludeNamedTransforms,
"includeNamedTransforms"_a = true,
DOC(ColorSpaceMenuParameters, setIncludeNamedTransforms))
.def("getTreatNoCategoryAsAny", &ColorSpaceMenuParameters::getTreatNoCategoryAsAny,
DOC(ColorSpaceMenuParameters, getTreatNoCategoryAsAny))
.def("setTreatNoCategoryAsAny", &ColorSpaceMenuParameters::setTreatNoCategoryAsAny,
"treatNoCategoryAsAny"_a = false,
DOC(ColorSpaceMenuParameters, setTreatNoCategoryAsAny))
.def("getEncodings", &ColorSpaceMenuParameters::getEncodings,
DOC(ColorSpaceMenuParameters, getEncodings))
.def("setEncodings", &ColorSpaceMenuParameters::setEncodings, "encodings"_a.none(false),
Expand Down
16 changes: 8 additions & 8 deletions tests/cpu/apphelpers/CategoryHelpers_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ OCIO_ADD_TEST(CategoryHelpers, basic)
{
OCIO::Categories categories{ "file-io", "working-space" };
OCIO::Encodings encodings{ "sdr-video", "log" };
OCIO::ColorSpaceVec css = OCIO::GetColorSpaces(config, true,
OCIO::ColorSpaceVec css = OCIO::GetColorSpaces(config, true, false,
OCIO::SEARCH_REFERENCE_SPACE_SCENE,
categories, encodings);
OCIO_REQUIRE_EQUAL(css.size(), 3);
Expand All @@ -71,15 +71,15 @@ OCIO_ADD_TEST(CategoryHelpers, basic)
{
OCIO::Categories categories{ "file-io", "working-space" };
OCIO::Encodings encodings{ "sdr-video", "log" };
OCIO::ColorSpaceVec css = OCIO::GetColorSpaces(config, false,
OCIO::ColorSpaceVec css = OCIO::GetColorSpaces(config, false, false,
OCIO::SEARCH_REFERENCE_SPACE_SCENE,
categories, encodings);
OCIO_REQUIRE_EQUAL(css.size(), 0);
}
{
OCIO::Categories categories{};
OCIO::Encodings encodings{ "sdr-video", "log" };
OCIO::ColorSpaceVec css = OCIO::GetColorSpaces(config, true,
OCIO::ColorSpaceVec css = OCIO::GetColorSpaces(config, true, false,
OCIO::SEARCH_REFERENCE_SPACE_SCENE,
categories, encodings);
OCIO_CHECK_EQUAL(css.size(), 0);
Expand All @@ -91,17 +91,17 @@ OCIO_ADD_TEST(CategoryHelpers, basic)
{
OCIO::Categories categories{ "file-io", "working-space" };
OCIO::Encodings encodings{};
OCIO::ColorSpaceVec css = OCIO::GetColorSpaces(config, true,
OCIO::ColorSpaceVec css = OCIO::GetColorSpaces(config, true, false,
OCIO::SEARCH_REFERENCE_SPACE_SCENE,
categories, encodings);
OCIO_CHECK_EQUAL(css.size(), 0);
css = OCIO::GetColorSpaces(config, true, OCIO::SEARCH_REFERENCE_SPACE_SCENE, categories);
css = OCIO::GetColorSpaces(config, true, false, OCIO::SEARCH_REFERENCE_SPACE_SCENE, categories);
OCIO_CHECK_EQUAL(css.size(), 7);
}
{
OCIO::Categories categories{ "file-io", "working-space" };
OCIO::Encodings encodings{ "sdr-video", "log" };
OCIO::ColorSpaceVec css = OCIO::GetColorSpaces(config, true,
OCIO::ColorSpaceVec css = OCIO::GetColorSpaces(config, true, false,
OCIO::SEARCH_REFERENCE_SPACE_DISPLAY,
categories, encodings);
OCIO_REQUIRE_EQUAL(css.size(), 2);
Expand All @@ -111,7 +111,7 @@ OCIO_ADD_TEST(CategoryHelpers, basic)
{
OCIO::Categories categories{ "file-io", "working-space" };
OCIO::Encodings encodings{ "sdr-video", "log" };
OCIO::ColorSpaceVec css = OCIO::GetColorSpaces(config, true,
OCIO::ColorSpaceVec css = OCIO::GetColorSpaces(config, true, false,
OCIO::SEARCH_REFERENCE_SPACE_ALL,
categories, encodings);
OCIO_REQUIRE_EQUAL(css.size(), 5);
Expand All @@ -123,7 +123,7 @@ OCIO_ADD_TEST(CategoryHelpers, basic)
}
{
OCIO::Categories categories{ "file-io", "working-space" };
OCIO::ColorSpaceVec css = OCIO::GetColorSpaces(config, true,
OCIO::ColorSpaceVec css = OCIO::GetColorSpaces(config, true, false,
OCIO::SEARCH_REFERENCE_SPACE_ALL,
categories);
OCIO_REQUIRE_EQUAL(css.size(), 10);
Expand Down
5 changes: 5 additions & 0 deletions tests/cpu/apphelpers/ColorSpaceHelpers_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,9 +224,14 @@ active_views: []
OCIO_CHECK_NO_THROW(menuHelper = OCIO::ColorSpaceMenuHelper::Create(params));
OCIO_CHECK_EQUAL(menuHelper->getNumColorSpaces(), 0);

params->setIncludeColorSpaces(true);
params->setAppCategories("basic-2d");
params->setSearchReferenceSpaceType(OCIO::SEARCH_REFERENCE_SPACE_SCENE);
params->setTreatNoCategoryAsAny(true);
OCIO_CHECK_NO_THROW(menuHelper = OCIO::ColorSpaceMenuHelper::Create(params));
OCIO_CHECK_EQUAL(menuHelper->getNumColorSpaces(), 2);

params->setTreatNoCategoryAsAny(false);
OCIO_CHECK_NO_THROW(menuHelper = OCIO::ColorSpaceMenuHelper::Create(params));
OCIO_CHECK_EQUAL(menuHelper->getNumColorSpaces(), 1);

Expand Down
Loading

0 comments on commit 45d3aba

Please sign in to comment.