Don't try to ReParameter symbols not in the group #1693
Conversation
| int paramindex = layer->findparam(ustring(paramname), | ||
| false /* don't go to master */); |
There was a problem hiding this comment.
I have a suggestion:
First, look for it with search_master=false. If that succeeds, we're done.
If that search fails, try again with search_master=true. If that succeeds, then you know it's the case that the parameter did exist, but was optimized away.
I think that in such a case, ReParameter should return true. It's not a failure in the same sense as using a parameter that doesn't exist in the shader, or using a parameter not marked as interactive. It's simply that changing the value of the parameter has no effect on the appearance, so you don't need to communicate the new value to the GPU or whatever. But I still consider that a success in terms of changing the value and being able to get the right picture when you rerender.
There was a problem hiding this comment.
Remember that I'm simultaneously working on another fix in which if you try to ReParameter something that isn't allowed to vary (say, because not folding it would make something illegal for the GPU), the false it returns can be interpreted by the renderer as "if I want to vary that parameter, I need to re-send the shader group and re-JIT". So I don't want to throw back false failures for harmless things where we can draw the right picture without re-compiling.
Sometimes, after optimization, symbols tagged as interactive get removed because they are unused. But ReParameter is failing with errors because it goes looking for the removed symbol to master, which is, of course, not interactive. This patch avoids that so the symbol is not found and we just return false. Signed-off-by: Alejandro Conty <[email protected]>
Sometimes, after optimization, symbols tagged as interactive get removed because they are unused. But ReParameter is failing with errors because it goes looking for the removed symbol to master, which is, of course, not interactive. This patch avoids that so the symbol is not found, and then we just return false.
Description
We found this issue in production because code was being removed, then symbols too, then reparameter failed.
There is no other way to prevent the error than from inside OSL. I'm suggesting this change but any other way to
handle it would work for us.
Checklist: