UsdImagingDelegate::InvokeExtComputation uses an IndexPath as a CachePath

williamkrick commented 4 years ago

Description of Issue

In UsdImagingDelegate::InvokeExtComputation the computationId parameter is passed into _GetHdPrimInfo(), which is expecting a CachePath, but the computationId is an IndexPath. You can see similar code in the related functions in UsdImagingDelegate, such as GetExtComputationKernel, that convert the computationId to a CachePath before using it in various ways.

I'm working in MayaUSD trying to evaluate some HdExtCompCpuComputation objects. When I call resolve on my HdBufferSource it calls through into InvokeExtComputation and fails because the lookup in _GetHdPrimInfo fails because it is using an IndexPath as a CachePath.

I thought that this would also break CPU implementations of USDSkel in USDView, but I couldn't necessarily reproduce the issue there. I tried setting USDSKELIMAGING_FORCE_CPU_COMPUTE=1 and HD_ENABLE_GPU_COMPUTE=0 and the meshes turn black (I guess normals are missing), but I still see the deformation occurring. I thought that no deformation would occur because InvokeExtComputation would fail to call UsdSkelImagingSkeletonAdapter::InvokeComputation.

I did see some talk on USD-interest about this kind of thing: https://groups.google.com/g/usd-interest/c/16w7EMACyrs/m/HZtniAlkBgAJ https://groups.google.com/g/usd-interest/c/Liv5iLYMWZU/m/ccN4J7sMAgAJ

But those discussions hadn't pinpointed the issue in InvokeExtComputation that I am seeing as a problem. Something is going on that I still don't understand because I didn't expect the meshes to continue to deform, but the fix to InvokeExtComputation seems reasonable so I figured I would suggest it.

It would also be nice if there was a way to configure skeletonAdapter so that it can correctly create the CPU computations without having to set USDSKELIMAGING_FORCE_CPU_COMPUTE. The main issue is _IsEnabledAggregatorComputation is only configurable by environment variable, so I have to duplicate all the code in skeletonAdapter just to prevent that aggregation stuff from happening (which is fine, just not nice).

Steps to Reproduce

set USDSKELIMAGING_FORCE_CPU_COMPUTE=1 and HD_ENABLE_GPU_COMPUTE=0 to try and force USDSkel to use the CPU implementation.
Run USDView and open HumanFemale.walk.usd.
The character shows up black, and the deformation works.

System Information (OS, Hardware)

Windows 10

Package Versions

USD Version: 0.20.8-beta1-7c888ac

Build Flags

  Building                      Shared libraries
    Variant                     RelWithDebInfo
    Imaging                     On
      Ptex support:             Off
      OpenVDB support:          Off
      OpenImageIO support:      Off
      OpenColorIO support:      Off
      PRMan support:            Off
    UsdImaging                  On
      usdview:                  On
    Python support              On
      Python Debug:             Off
      Python 3:                 Off
    Documentation               Off
    Tests                       Off
    Examples                    On
    Tutorials                   On
    Tools                       On
    Alembic Plugin              Off
      HDF5 support:             Off
    Draco Plugin                Off
    MaterialX Plugin            Off

  Dependencies                  zlib, boost, GLEW, OpenSubdiv
  Build arguments               boost: --with-date_time --with-thread --with-system --with-filesystem
                                USD: -DPYSIDEUICBINARY=C:/dev/maya/builds/main/RelWithDebInfo/runTime/bin2/pyside2-uic -DPYSIDE_IS_SCRIPT=TRUE

jtran56 commented 4 years ago

Filed as internal issue #USD-6355

rajabala commented 3 years ago

Thanks for reporting the issue! You're correct - we do need to translate the render index computationId argument in UsdImagingDelegate::InvokeExtComputation to the usdImaging cachePath. It doesn't fail with usdview because the UsdImagingDelegate uses a delegateId of "/", so there's no prefix added and it matches the scene hierarchy. We hit a verify when USDIMAGINGGL_ENGINE_DEBUG_SCENE_DELEGATE_ID is set to "/Foo", which goes away once we do the additional path translation.

Re: the black image with CPU computations in Storm -- we'll take a look!

PixarAnimationStudios / OpenUSD