Dynamic properties clarification

[nathan-disguise]

Hi,

Currently we've been in the process of integrating OCIO and believe we've encountered an issue regarding the dynamic properties feature.

As far as we understand, they are solely supposed to be handled by the developers and are not intended to be defined or specified as part of the config as stated in the documentation. However when defining an ExposureContrastTransform, should a user fail to set a value for a property such as exposure like so:

!<ExposureContrastTransform> {style: linear, contrast: 0.5, gamma: 1.1, pivot: 0.18}

The generated processor initialises a dynamic property for the missing exposure value. Given that a config is supposed to be locked this seems to contradict the core intention here, therefore we suspect this is a bug and not intentional.

Could you please clarify whether or not this is intended behaviour. If not no worries, we're happy to assist where we can.

Many Thanks

[doug-walker]

All parameters have a default value, in the case of Exposure, that is zero. So it is expected to have that set to zero, even if you do not set it. It would not be expected to have the dynamic aspect enabled though. (There is some helper code in OCIO that will configure a viewing pipeline that will insert a dynamic exposure control, but I'll assume you are not using something like that.)

[nathan-disguise]

Thanks for clarifying the intended behaviour @doug-walker. It seems that doing this in the config does enable the dynamic properties for the exposure parameter.

It can be observed using this dummy config file and python script, which when excecuted produces the following output:

Dynamic properties exist.
Exposure present: True
Contrast present: False
Gamma present: False
Dynamic exposure value prior to change = 0.0
Changed Value to 10.0
Dynamic exposure value = 10.0

This is similar to behaviour we noticed within our software when running the dummy config.

From examining the source code and testing locally, I believe the issue is located within OCIOYaml parser which is explicitly making the values dynamic if they are missing. Given the lack of similar behaviour on some of the other transforms it seems to me this behaviour was earmarked to be removed.

(OCIOYaml.cpp line:1109)

inline void load(const YAML::Node& node, ExposureContrastTransformRcPtr& t)
{
    t = ExposureContrastTransform::Create();

    CheckDuplicates(node);

    bool dynExposure = true;
    bool dynContrast = true;
    bool dynGamma    = true;

    for (Iterator iter = node.begin(); iter != node.end(); ++iter)
    {
        const std::string & key = iter->first.as<std::string>();

        if (iter->second.IsNull() || !iter->second.IsDefined()) continue;

        if (key == "exposure")
        {
            double param;
            load(iter->second, param);
            t->setExposure(param);
            dynExposure = false;
        }
 ...
    // Missing values are dynamic.
    if (dynExposure)
    {
        t->makeExposureDynamic();
    }
    if (dynContrast)
    {
        t->makeContrastDynamic();
    }
    if (dynGamma)
    {
        t->makeGammaDynamic();
    }
}

My belief is that the only change required here would be to remove this behaviour. Certainly, if you believe this to be the case as well, I'm happy to make a contribution, no worries if not.

[doug-walker]

@nathan-disguise , thank you for the detailed description. The behavior you have noticed was intentional.

I was traveling last week when I first read your issue and unfortunately misinterpreted what you were asking about. (I thought you were asking about creating an ExposureContrastTransform from the API rather than loading it from a config.)

It is in fact useful for config authors to be able to have a way to specify where in a viewing pipeline the dynamic exposure, contrast, and gamma should happen. (Our first attempt at this allowed the config author to call out which parameters were dynamic, which we felt would be clearer, but there were objections to that.) So, yes, the ones that are not specified are interpreted as being dynamic.

Is this causing a problem for your implementation?