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

Regression with ComputeFlattened and Value Clips in 20.08 #1344

Closed
chrisrydalch opened this issue Sep 29, 2020 · 6 comments
Closed

Regression with ComputeFlattened and Value Clips in 20.08 #1344

chrisrydalch opened this issue Sep 29, 2020 · 6 comments

Comments

@chrisrydalch
Copy link
Contributor

chrisrydalch commented Sep 29, 2020

Description of Issue

As of USD 20.08/Dev, Primvars::ComputeFlattened fails to get values for stitched, value-clip driven primvars. These value clip-driven primvars also report to be indexed, when they are not.

Steps to Reproduce

In the attached example, the asset_A.usda has the primvars defined and result_A.usda has the clip applied.

  1. Open the attached computeflattened_primvars.zip file
  2. With USD 20.08 or Dev (Commit 533030f), run get_flattened_primvars.py)
  3. The output is displayed similar to this:
		Asset Flattened		Asset Indexed?		Result Flattened	Result Indexed?
displayColor	Vec3fArray		False			Error		    	True
myDouble	DoubleArray		False			Error		    	True

This is in contrast to USD 20.05:

		Asset Flattened		Asset Indexed?		Result Flattened	Result Indexed?
displayColor	Vec3fArray		False			Vec3fArray		False
myDouble	DoubleArray		False			DoubleArray		False

System Information (OS, Hardware)

CentOS 7

Internal issue is BSSUSD-52

Thanks!

computeflattened_primvars.zip

@sunyab
Copy link
Contributor

sunyab commented Sep 30, 2020

Hi @chrisrydalch, this is an issue with the manifest layer generated in usdstitchclips and a consequence of the changes to value clips behavior in the 20.08 release -- I'll go into more details below. We are currently gearing up for the 20.11 release and a full fix for this likely will not make it in to that release. There are a few workarounds:

  • The simplest would be to remove the 'manifestAssetPath' specified in the resulting stitched file. USD will generate an in-memory manifest at runtime that should be correct and give the expected result.

  • If the runtime generation of the manifest is too expensive, you can use helper API on UsdClipsAPI to generate a manifest and then update the 'manifestAssetPath' to use that instead. For example:

clipLayers = [Sdf.Layer.FindOrOpen(p) for p in glob.glob("clip/clip.*.usda")]
manifest = Usd.ClipsAPI.GenerateClipManifestFromLayers(clipLayers, "/grid")
manifest.Export('clip.manifest.usda')

Details:
As of 20.08, the clip manifest is required for value clips. All varying attributes authored in the manifest are assumed to have values authored in the clips and because of that, they are considered to always have authored values. UsdGeomPrimvar's IsIndexed check relies on this, which is why its value has changed between 20.08 and 20.05.

usdstitchclips currently uses the topology layer it generates as the manifest as well. Because this topology layer contains declarations for the indices attribute, it triggers the behavior I described above and causes the issue you're seeing. I think the right fix for this is to update usdstitchclips to generate a manifest layer that is separate from the topology layer. In the meantime, removing the 'manifestAssetPath' or pointing it to the one generated by UsdClipsAPI avoids this issue.

Let me know what you think!

@chrisrydalch
Copy link
Contributor Author

Hey @sunyab! This is great info, thank you for the break down as well as some options. I feel like every time value clips need to be dived into deeper, there's some nuance that I missed before 😁 but this all makes sense.

For us I think we'll rely on the ClipsAPI to generate a manifest; we're directly rendering from USD, so we're more sensitive to that big runtime hit. For example, one of the first shots to get a decent number of crowds didn't use manifests, so the shot was 5-7x slower to open in USD 20.08 vs 20.05 - ouch! 😬 Even in a simple test, the 5-7x hit seemed pretty consistent; does that sound expected to you?

As for usdstitchclips, are you open to a PR, or would you rather wait and take care of it internally after 20.11 is out the door?

Thank you again for the help, we really appreciate it!

@sunyab
Copy link
Contributor

sunyab commented Oct 2, 2020

@chrisrydalch The 5-7x slowdown is a little surprising to me, I don't believe we had observed anything similar here. The automatic manifest generation does need to open and traverse each clip though -- can you say how many clip layers are being used in general?

For usdstitchclips, I think it's something we'll want to take care of internally after 20.11 is released. I have a nearly completed fix but I need to look at it a bit more and do more testing.

@jilliene
Copy link

jilliene commented Oct 2, 2020

Filed as internal issue #USD-6384

@chrisrydalch
Copy link
Contributor Author

Thanks @sunyab! I'm not sure how many are in the production examples it does seem to vary, but we observed a similar relative slow-down in a simple flag, which had 60 layers (layer-per-frame). Having the manifest though solves both that slow-down and this primvar-indices issue, but if you feel it's worth looking into more, I can clean up that example and log it.

Thanks!

@sunyab
Copy link
Contributor

sunyab commented Oct 7, 2020

@chrisrydalch Having an example would definitely be helpful! In the meantime I'll try to spend some time looking closer at the real fix once we get the ball rolling on the 20.11 release.

kohakukun pushed a commit to autodesk-forks/USD that referenced this issue Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants