OLCheckPrimaryGamut output inGamut

[JorisVincent]

The function OLCheckPrimaryGamut returns a boolean inGamut. It is unclear whether this refers to the original input primaries being in gamut, or to the (possibly truncated) output primaries being in gamut. It is currently doing the former: even if primaries are truncated to be within gamut, it returns false if they were not already. However, the documentation says:

%    inGamut                  - Boolean scalar. True if returned primaries
%                               are in gamut, false if not.  You can only
%                               get false if checkPrimaryOutOfRange is
%                               false.

This is NOT how it currently is acting. Is this a bug in code, or a bug in documentation?

[JorisVincent]

This is not just a pedantic question; we rely on this output in several functions, e.g. OLPrimaryInvSolveChrom:

    %% Are initial primaries in gamut 
    [initialPrimaries,inGamut] = OLCheckPrimaryGamut(initialPrimaries, ...
        'primaryHeadroom',maxLumPrimaryHeadroom, ...
        'primaryTolerance',p.Results.primaryTolerance, ...
        'checkPrimaryOutOfRange',false);
    if (inGamut)
        % If we're good, break out of loop
        break;
    end

[DavidBrainard]

I looked at this. I believe the routine to be written as intended and its documentation to be correct.

The purpose of this routine is to check the primaries, but to accept a very small violation of gamut as in gamut. In cases where the primaries are just a hair out of gamut, it puts them in gamut. It does not, however, touch input primaries that are out of gamut by more than the primaryTolerance (default 1e-6).

This routine is needed to deal with the fact that routines like fmincon allow a small violation of their constraints, so that primaries that are basically fine can be technically out of gamut.

The return status really is about what is returned, which can differ (by primaryTolerance) from what is passed.

Note that you can obtain a strict check by setting primaryTolerance to 0. And that primaryTolerance is different from primaryHeadroom.

I expanded the header comments to provide a fuller description along the lines of this comment.

Closing.

[JorisVincent]

Thanks for clarifying the header comment. However, if inGamut is supposed to indicated whether the returned primaries are in gamut, then there is a bug in the code, when the flag checkPrimaryOutOfRange is set to false:

 %% Initially out of gamut, but checkPrimaryOutOfRange false, so forced to truncate to gamut
[outputPrimary,inGamut,gamutMargin] = OLCheckPrimaryGamut(-0.01,'checkPrimaryOutOfRange',false)

returns:

outputPrimary =

     0

inGamut =

  logical

   0

gamutMargin =

    0.0100

The outputPrimary has been truncated to gamut, as intended, but the inGamut indicates false.

[JorisVincent]

I've issued a PR to remove the line that leads to the aforementioned bug. Don't want to just merge it in, because of our reliance on this return argument in several functions, e.g. OLPrimaryInvSolveChrom

[DavidBrainard]

Rejected PR in favor of keeping old behavior and improving comments, now done. There is some chance that some piece of code counts on the current behavior.