HdLight::Sync performance regression #2416
Comments
|
The call to _GetLightParamValueFromMaterial was intended to cover a speculative case which in retrospect may not ever matter. I'll see if I can speed it up for your case. If not, we can probably drop it (as you found). Thanks for catching this. (The case was "What if a downstream general-purpose edit was made to lights within the material resource for render delegates which use the legacy GetLightParamValue interface? If both are present, which should be used?". Longer term, I think the material resource [or rather the "material" data source] will be used for lights for all renderers. Shorter term, it seems unlikely that that speculative case will ever matter.) |
|
Filed as internal issue #USD-8287 |
|
The fix for this is available in dev now: Removing the call to _GetLightParamValueFromMaterial restores performance. This is paired with changes to UsdImagingStageSceneIndex to also provide values for non-material-resource-based lights through the "light" data source. This provides the forwards compatibility intended by the original change but without the performance hit. |
|
Confirmed this gets things back up to 125fps in the provided test ... much appreciated! |
…Delegate::GetLightParamValue as it has significant overhead - return light parameters within "light" data source from UsdImagingStageSceneIndex in order to satisfy render delegates which don't make use of GetMaterialResource for lights. - add both time-varying and property change invalidation for the "light" data source within UsdImagingStageSceneIndex Some render delegates (such as hdSt) query light parameter values through the legacy GetLightParamValue() interface. Newer delegates use the GetMaterialResource() to querylight parameters. Front-end emulation does produce a "material" data source from GetLightParamValue calls to legacy delegates. Back-end emulation was prioritizating that "material" resource over the "light" data source in order to unify these approaches. However, because each call to GetLightParamValue() results in the full material network being rebuilt, this introduced significant light sync performance overhead. One goal of unifying access to be from the "material" data source was to have a single point for downstream overrides to address (regardless of which the render delegate uses). We have not yet encountered cases where that is relevant. UsdImagingStageSceneIndex was only providing the "material" data source form. So removing GetLightParamValue mapping alone would break hdSt cases. Rather than enforcing the single mapping in back-end emulation, it's straightforward for UsdImagingStageSceneIndex to also provide "light" data source access (which it does for other fields already). We can remove that once all render delegates are making using of GetMaterialResource (or directly using data sources). Fixes PixarAnimationStudios#2416 (Internal change: 2274849) # Conflicts: # pxr/usdImaging/usdImaging/primAdapter.cpp
Description of Issue
This change: 1f87b7b made light syncing about 40x slower. In Houdini, our Sync function went from 0.000055s/light to 0.0023s/light. In usdview running storm, playback of the attached file with about 100 animated lights goes from 125FPS to 15FPS.
Steps to Reproduce
System Information (OS, Hardware)
Tested on Linux and Windows
Package Versions
USD 23.05
The text was updated successfully, but these errors were encountered: