AcademySoftwareFoundation / OpenShadingLanguage

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

Fix userdata derivatives for interpolated params on GPU #1685

Closed lecocqp closed 1 year ago

lecocqp commented 1 year ago

Description

Following #1657, it turns out we forgot to zero initialize the userdata derivatives for the optix path. This leads to some rendering glitches caused by the memory garbage left in those derivatives.

Tests

Checklist:

linux-foundation-easycla[bot] commented 1 year ago

CLA Missing ID CLA Not Signed

lgritz commented 1 year ago

Pascal, you can fix the DCO by adding a sign-off:

git commit --amend -s
git push origin <branchname> --force

For the EasyCLA, it seems you weren't on the SPI approved list. (How did that happen? Is this really your first OSL PR?) Anyway, I added you, so you should see something to click that will let you accept it and they it will be cleared for this and any future PRs.

lgritz commented 1 year ago

Don't worry about the two test failures for icc and icx -- that's just Intel's servers being flaky and the CI runs unable to download those compilers. It comes and goes.

lecocqp commented 1 year ago

Yes, very first OSL PR, I only contributed to OIIO so far. For the EasyCLA should I proceed as a Corporate or Individual contributor?

lgritz commented 1 year ago

Corporate, tell it you're with Sony Imageworks, I've already put you on our list.

lecocqp commented 1 year ago

I completed the CLA authorization request. Is there something else I need to do for getting the CLA failure resolved? Another push force commit?

lgritz commented 1 year ago

I'm not sure, I'll try to figure it out.

lgritz commented 1 year ago

Hmmm, can you just diddle it and re-push:

git commit --amend       # force a sha change, don't edit anything
git push origin blah --force

maybe that will get it unstuck.

lecocqp commented 1 year ago

I re-pushed the branch but same thing. The EasyCLA process says this: "The Corporate CLA process requires you to be approved by your organization's CLA Manager. A CLA Manager at your organization will receive your request via email immediately after you submit it. To expedite the process, you can follow up with them directly.".

Maybe you (or someone else?) have receive an email for approving the authorization?

lgritz commented 1 year ago

I am the CLA manager for the company. I see you already on the approved list, and there are no additional requests waiting for my approval. If the rest of the CI passes, I may be able to just force a merge and override the check. I just don't know why EasyCLA is still giving me trouble. @jmertic do you have any insights?

lgritz commented 1 year ago

Pascal, is it possible that the email associated with the commit doesn't match the email associated with your github account?

lecocqp commented 1 year ago

Oh you're right! I didn't realize that the commit was signed off with my Imageworks email instead of my gmail address. I'll look into it tomorrow.

lgritz commented 1 year ago

I think you should be able to fix with

git commit --amend --author "Pascal Lecocq <your@email>" -s

and then re-push --force

But also, I just added your work email to the permissions list, so it may just work with another force push? (Though changing the email to be consistent will make it so that future PRs all look like the same person instead of you appearing under multiple aliases.)

lecocqp commented 1 year ago

I setup my personal email for that specific repo in the .git/config using a [user] tag. I just pushed another commit, let's see how it goes.