Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix propagation of instance transform updates from dirty instancers #69

Closed
wants to merge 1 commit into from

Conversation

nrusch
Copy link
Contributor

@nrusch nrusch commented Nov 26, 2019

Fixes propagation of changes to instance "primvars" (e.g. transforms) in response to changes to instancer dirty state.

Marking an instancer dirty also sets the DirtyInstancer bit on its source Rprim. Since the delegate's current instancing implementation is looking at the Rprim dirty bits (as opposed to the instancer dirty bis), we need to make sure instance matrices are recomputed any time the instancer itself is marked dirty in order to get proper transform updates.

@sirpalee sirpalee self-requested a review November 27, 2019 11:36
@sirpalee
Copy link
Contributor

I created a simple usda file to showcase and test the issue. Without @nrusch change the instance transforms are not updated. However, when built with this change the render delegate deadlocks and keeps using a single thread.

instances.zip

USD version: 0.20.02 dev latest
Arnold Version: 5.4.0.2
OS: Ubuntu 18

@nrusch are you able to reproduce the deadlock?

@nrusch
Copy link
Contributor Author

nrusch commented Nov 27, 2019

No, the only time I've seen things get to a deadlocked/infinite loop state is if Arnold aborts the render (since there is no way for a client to detect that that has happened other than checking the task convergence state, as outlined in #4).

However, my current build of arnold_usd isn't from the latest master, and I'm not using the USD scene delegate, so I'll test out those things one at a time later to see if I can reproduce.

@nrusch
Copy link
Contributor Author

nrusch commented Dec 5, 2019

OK, I finally got around to testing your example, but I can't reproduce the issue you're talking about. This is using a fresh build from the head of master, plus the patch from this MR.

@nrusch
Copy link
Contributor Author

nrusch commented Dec 5, 2019

Hmm, OK, I think I actually am seeing the issue. The trick is you have to modify the timecode in the USD scene delegate (so, say, render frame 1, then switch to frame 2 and try to render again).

It seems to be a deadlock situation, although the CPU isn't really doing any work. I do get this in the terminal, which may be telling:

00:01:18 383MB WARNING | unable to join thread. Invalid argument

@sirpalee sirpalee added this to To do in Sprint 16 Jan 11, 2020
@sirpalee sirpalee self-assigned this Jan 11, 2020
@sirpalee sirpalee added bug Something isn't working render delegate Related to the Arnold Render Delegate labels Jan 11, 2020
@sirpalee
Copy link
Contributor

sirpalee commented Jan 14, 2020

The deadlock does not happen anymore with Arnold-6.0.1.0, and your pull request fixes the matrices update issue. Since we still target Arnold-5.4, I'm not going to merge in your change yet, but I'll see about what's the best way to approach Arnold-5.4 support.

@nrusch
Copy link
Contributor Author

nrusch commented Jan 14, 2020

Ah cool, good to know. I haven't updated to Arnold 6 on my end, but it sounds like I should.

@sirpalee sirpalee removed this from To do in Sprint 16 Jan 24, 2020
@sirpalee sirpalee added this to In progress in Sprint 17 via automation Jan 24, 2020
@sirpalee sirpalee moved this from In progress to Review in progress in Sprint 17 Jan 24, 2020
@kikou kikou added this to To do in Sprint 18 Feb 7, 2020
@kikou kikou moved this from To do to Review in progress in Sprint 18 Feb 7, 2020
@sirpalee sirpalee closed this Feb 19, 2020
Sprint 18 automation moved this from Review in progress to Done Feb 19, 2020
@nrusch
Copy link
Contributor Author

nrusch commented Feb 19, 2020

Hey @sirpalee, did this ever get resolved in the delegate? Or is this no longer directly relevant due to refactoring, etc?

@sirpalee
Copy link
Contributor

Hey @nrusch just submitted a PR that includes your change as well: #225

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working render delegate Related to the Arnold Render Delegate
Projects
No open projects
Sprint 17
  
Review in progress
Sprint 18
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants