Set up ray types for testrender

[lgritz]

Try to set ray types correctly for rays -- a little naive now, only using "camera", "shadow", and also "diffuse" for all indirect rays (leave distinguishing between reflection, refraction, diffuse, glossy for another day, and currently testrender doesn't support subsurface or displacement, but maybe someday).

Also, for testshade, improve efficency -- don't decode raytype on every point. For each individual shade, testshade was converting the name of the raytype from a string to a ustring, then decoding to a bitfield. Pull it all out of the loops by computing the bitfield once.

[fpsunflower]

LGTM

It would be hard to split raytype much further for testrender without a fairly significant refactor to all the closures.

[lgritz]

Yeah, I figured. It would go deep into the closures, and that was much more than I was prepared to do at the moment, and also is unnecessary for what @jstone-lucasfilm needs to get unstuck, which I think is merely to distinguish camera rays from secondary rays.

[lgritz]

Amended: I forgot to assign the raytype of the ray to the SG! Sorry, @jstone-lucasfilm, fixed now. Also, I added a testsuite entry to verify. (Note to self: how many decades will it take me to fully absorb the precept that "untested code is buggy code," always dammit.)

[lgritz]

@fpsunflower Would love your opinion on this:

Now that I "fixed" this PR, the render-bumptest test fails! And it's because of this glass shader in which we try to disable the contribution of caustic rays by testing (!raytype("glossy") && !raytype("diffuse")).

Well, a minute ago when we didn't set the ray type correctly at all in testrender, it was never these ray types. Now we do, but we set all non-camera rays to think they are diffuse -- because, as noted, correctly distinguishing reflection vs refraction vs glossy vs diffuse requires the BSDF closures themselves having more smarts than they currently do in testrender -- and so all secondary rays (even reflection rays) think they are diffuse, therefore they don't shade now!

The same glass shader is in src/shaders, obviously meant to be an example, though now we see that it's a poor example (at least when used with a fixed testrender. So that's very unsatisfying.

I'm sure that we did it this way because once upon a time, SPI's glass shader probably had a similar idiom. But maybe this isn't the way we would currently recommend that anybody do it.

I think the expedient thing is to just remove the "caustic-avoiding" behavior of this shader, which we clearly didn't depend on because it never worked properly. But still curious to hear your amused take on this, Chris.

[fpsunflower]

Yeah I agree, the caustic avoidance should be done in the integrator, not the shader.

That being said - there is still a grey area where the renderer, through its own heuristics will not actually use something the shader could have worked really hard to compute. So there is still some need for a mechanism for the renderer to tell the shader "I am not going to use a,b, or c even if you give them to me". I don't think we ever came up with a satisfying design for this.

One candidate we had talked about adding (and maybe we did this via getattribute() is the maximum path roughness. This is a bit better than raytype because it less. you do a softer cutoff of some effects.

Another design we considered was giving some more explicit hints to the shader like:

• skip bump
• skip sss
• skip specular
• etc ...

The logic would even be mostly the same as raytype() -- it would just be querying a bitmask populated by the renderer with some agreed upon strings. If we design the hints to be "skip X" -- that allows the shader to ignore these hints if it wants -- they are only there for the sake of early exits.

As an example where raytypes definitely don't work - consider the connection rays in BDPT -- they only need to know transparency information, but are not technically "shadow" rays -- in fact if you allow lens connections they can be "camera" rays (!). This is where having a more explicit list of "skippable work" seems beneficial.

Another idea we discussed but never actually implemented was the idea of letting the renderer tell the compiler before runtime optimization (or during, via callbacks) which closures it is interested in. If the renderer decides a particular closure is not important (say something specular after deciding its not going to allow caustics), it would let the runtime optimization turn that into a no-op and most likely cascade into a bunch of other simplifications. We actually already do this for raytypes by giving the mask of "definitely on" and "definitely off" -- this would be another flavor of that. That being said -- the trend seems to be to move away from compiling additional specializations of shaders given the JIT compilation costs, so I think ultimately we'll want to stick to things that are cheap at to leave in for a runtime decision.

Its definitely a topic worth discussing at the next TSC, I am curious how other teams solved this. I suspect that if you have a "pattern only" integration of OSL, this problem presents itself much less, because you simply wouldn't pull on the shader outputs that generate something you don't need. So maybe this is only an issue for those folks relying on closures?

[fpsunflower]

Also that glass shader should definitely be rewritten to use the shiny new Material X closures :)

[lgritz]

I updated the PR by removing the special cases that attempted to suppress caustic rays.

Later, we should rewrite those glass shaders entirely based on the new closures, but I wanted to limit the scope creep of this PR which was really just aimed at fixing what Jonathan needed right away.

[lgritz]

On Slack, Jonathan reports that this works, so I'm going to merge.

As a note: the last batch of trouble Jonathan was having was because of this line:

<Background resolution="1024" /> 

currently, this will run the shader out of any ray context at all (therefore having no raytype) in order to populate an environment map that will be used as needed.

Remove the "resolution" part so it just says

<Background  />

and it will run the shader every time it's needed, with the correct(-ish) ray type. But... we also discovered that when you do that, no importance sampling is used at all, so it's much noisier. I think the right approach is to do both -- populate the map for importance sampling, but run the shader on the rays themselves to get higher quality visible results. Caveat emptor if you write a shader that has ray behavior that deviates significantly from the no-raytype behavior that will be used to understand the distribution of energy.