On 1/29/21 8:49 PM, Cousin Ricky wrote:
> On 2021-01-29 1:57 PM (-4), William F Pokorny wrote:
>> I'm going to blame the couple of you reviving a dormant thread about 
>> color to grey conversions for this! Always knock on wood after 
>> mentioning a feature... ;-)
> 
> *forum Kung-Fu champion takes a bow*
> 

As you should!

...
> 
> I did further tests, and it appears that .gray/.grey simply returns the 
> value of the .green channel.  This is a seriously stinky green bug!
> 

Yep, you figured it out more quickly than I did.

The problem was introduced at commit:
   7d04c132 [parser] Modify token ID handling. Jan 22, 2019.

where the code was changed incorrectly to reach for the green value. 
Users running any v3.8 version after that commit are exposed to wrong 
answers.

The unsolved mystery is how we are sometimes getting the right result 
despite the mistake! My guess is library vector optimizations sometimes 
sticking the right value in [1] too, but I don't really know.

If you have a version in hand returning the right results, you are 
probably OK continuing to use it. However, users should check their 
version is actually working if running any version >= 7d04c132.

> Looks like we'll have to use one of these macros for the time being:
> 
> #macro DotGray (Color)
>    #local C = color Color;
>    (0.2126 * C.red + 0.7152 * C.green + 0.0722 * C.blue) // ITU-R BT.709
> #end
> 
> ---or---
> 
> #macro DotGrey (Colour)
>    #local C = colour Colour;
>    (0.2126 * C.red + 0.7152 * C.green + 0.0722 * C.blue) // ITU-R BT.709
> #end
> 

Yes, or for those compiling v3.8 at home, change the CASE(GRAY_TOKEN) 
code in source/parser/parser_expressions.cpp to read:

   CASE(GRAY_TOKEN)
       *Terms=1;  // TODO. Redundant? Is other than >=1 possible?
                  // Being explicit is safe for check below.
       Express[0]=PreciseRGBFTColour(Express).Greyscale();
       i=0;       // i set to 0 at top. Again, being explicit the value
                  // we want is in Express[0] is safe.
   END_CASE

and re-compile.

> 
> I'd mostly ditched .gray for DotGray() anyway, since Christoph revealed 
> that the .gray formula was inaccurate in the real world, so I wouldn't 
> have run across this bug, at least not without re-rendering one of my 
> old scenes in 3.8.  The only situations I can think of in the past 4 
> years where I *might* have used .gray would be on gray scale pigments, 
> for which, based on my findings, this bug would not manifest.
> 

In the povr branch the plan is to enable options (currently 5) by 
configuration, with Rec.709 being the default.

What color to grey means is a muddy business when you dig in. I plan to 
enable five because those align with using 3 coefficients, but there are 
more complicated methods. Rec.2020 has two main stream methods and one 
would require gamma twiddling pre and post calculation.

> I'm using openSUSE, but prima facie this doesn't sound like an operating 
> system-dependent issue.

No and I appreciate the bug confirmation.

Bill P.

Post a reply to this message