AcademySoftwareFoundation / OpenShadingLanguage

Advanced shading language for production GI renderers
BSD 3-Clause "New" or "Revised" License
2.09k stars 357 forks source link

Zero derivs for interactive params when needed #1700

Closed aconty closed 1 year ago

aconty commented 1 year ago

Sometimes an interactive param is used in a way that gets flagged as needing derivs. But we were neither allocating space for that nor zeroing them. This resulted on memory corruption.

Description

This fixes incorrect renders in a production context where params are often tagged as needing derivs.

Checklist:

lgritz commented 1 year ago

Hmmm, I think this will work, and I think it's fine to accept. But I wonder if a better fix long-term is that we should be turning derivatives off for a param that is marked as interactive. It won't have non-zero derivs, so any computations done using its derivative are pointless anyway.

aconty commented 1 year ago

I thought about that, but I wasn't familiar with all the moving parts and didn't try.

lgritz commented 1 year ago

I can take a look at that angle tomorrow when I'm back in the office.

If you need a workaround today for builds, it's fine to merge this, it doesn't hurt, I just think we can do better... if we can figure out the right spot to disable the derivs.

aconty commented 1 year ago

We can wait for your take on it tomorrow

lgritz commented 1 year ago

I'm going to merge this now so we can do an internal release, then I will come back to the problem afterwards and see if I can make it disable derivs entirely for these params that are marked interactive. (I don't want to hold up the fix waiting for me to get that right.)