OpenShadingLanguage icon indicating copy to clipboard operation
OpenShadingLanguage copied to clipboard

can't set shader parameter with init code to zero

Open 1-Olivier opened this issue 7 years ago • 1 comments

A shader parameter with some initialization code can't be set to zero. It looks a lot like zero is used as a magic value to know if the init code should be run or not.

The problem is easy to reproduce with this trivial shader:

shader simple(
    float a = 0.5,
    float value = 1 - a )
{
    printf("value is %f\n", value);
}

Default run is fine:

$ testshade --shader simple l
value is 0.500000

Most values work fine:

$ testshade --param value 0.8 --shader simple l
value is 0.800000

But zero runs the init code anyway:

$ testshade --param value 0.0 --shader simple l
value is 0.500000

That last case should print 0.000000, not 0.500000

The same happens with a color parameter. Tested against current master branch (7a56abdfb0b2b560415dc6e85b8861c09bbc3a77) on linux.

Oh and it seems to check for binary zero as this works:

$ testshade --param value -0.0 --shader simple l
value is -0.000000

1-Olivier avatar Jul 26 '18 20:07 1-Olivier

We found a bug that's related. When using structures with array fields and a litteral initializer, the genereted OSO contains a code snippet that initializes the array to a constant value declared before. For example this OSL snippet:

struct foo {
    float values[3];
};

surface test (foo input = {{ 0.5, 0.4, 0.7 }}) {
    Ci = color(input.values[0], input.values[1], input.values[2]) * emission();
}

Results in the following OSO (note that I simplified it for clarity):

surface test_constant_color
param	struct bloup	input
param	float[3]	input.values	0
global	closure color	Ci
const	float[3]	$const1	0.5 0.400000006 0.699999988
temp	closure color	$tmp1
const	string	$const2	"emission"
temp	color	$tmp2
const	int	$const3	0
temp	float	$tmp3
const	int	$const4	1
temp	float	$tmp4
const	int	$const5	2
temp	float	$tmp5
code input.values
	assign		input.values $const1
code ___main___
	closure		$tmp1 $const2
	aref		$tmp3 input.values $const3
	aref		$tmp4 input.values $const4
	aref		$tmp5 input.values $const5
	color		$tmp2 $tmp3 $tmp4 $tmp5
	mul		Ci $tmp1 $tmp2
	end

The important part is the line where input.values gets assigned. When this osl is opened using an OSLQuery, the default value for the struct is incorrectly set to {{0.0, 0.0, 0.0}}, even though the shader correctly outputs {0.5, 0.4, 0.7} by default. This wouldn't be such an issue if it wasn't for the fact that by default (and as far as I know, this behaviour can't be changed), calling Parameter(...) for a shader group using the default value of the parameter does nothing. When stepping into the code, we encountered this comment (instance.cpp, line 315):

// If the instance value is the same as the master's default,
// just skip the parameter, let it "keep" the default.
// Note that this can't/shouldn't happen for the indefinite-
// sized array case, which is why we have it in the 'else'
// clause of that test.

In our case, since the parser incorrectly assumed that the default value was {{0.0, 0.0, 0.0}}, when we manually set it to this same value, it falls back to the correct default value set in the OSL code.

For us, there is two parts to this problem:

  • The first one being the fact that calling Parameter(...) using the default value is interpreted as a noop. There may be a good reason for this, even though we couldn't find one.
  • The second, and real, problem is the fact that the default parameter of a "complex" type (stucture of array) is incorrectly parser by OSL.

ix-bruiz avatar Oct 04 '21 12:10 ix-bruiz