Skip to content

Conversation

@pmolodo
Copy link
Owner

@pmolodo pmolodo commented Sep 3, 2024

Description of Change(s)

NOTE: Not a real PR - just a place to post a document and gather feedback

Read + leave notes on this document:

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given these options:

  • RenderMan
  • Karma
  • A: Bimodal
  • B: Add vAngleScaleOrigin attr
  • C: Add 3 attrs (vAngleScaleOrigin, hAngleScaleOrigin, angleScaleDirection)

my vote is:

B: Add vAngleScaleOrigin attr

...because it seems the best compromise between present needs (backwards compatible mapping for existing scenes assuming either RenderMan or Karma behavior) and future (providing intuitive profile scaling for lights in any direction).

@pmolodo pmolodo force-pushed the ies-angleScale-findings-questions branch from 1524662 to fd2d558 Compare September 23, 2024 18:11
@pmolodo pmolodo force-pushed the ies-angleScale-findings-questions branch from 6671ceb to b237085 Compare October 9, 2024 00:25
@pmolodo pmolodo force-pushed the ies-angleScale-findings-questions branch from b237085 to 547c22a Compare October 21, 2024 17:06
@pmolodo pmolodo force-pushed the ies-angleScale-findings-questions branch from 547c22a to 5547016 Compare October 22, 2024 18:11
@pmolodo pmolodo force-pushed the ies-angleScale-findings-questions branch from 5547016 to 45b8af6 Compare October 23, 2024 17:30
@pmolodo pmolodo force-pushed the ies-angleScale-findings-questions branch from 45b8af6 to 4d63935 Compare October 23, 2024 18:20
@pmolodo pmolodo force-pushed the ies-angleScale-findings-questions branch from 5286654 to 3436293 Compare January 28, 2025 22:27
@pmolodo pmolodo force-pushed the ies-angleScale-findings-questions branch from 3436293 to 17ec01b Compare January 28, 2025 22:32
@pmolodo pmolodo force-pushed the ies-angleScale-findings-questions branch from 17ec01b to 6594288 Compare February 5, 2025 17:28
@pmolodo pmolodo force-pushed the ies-angleScale-findings-questions branch from 6594288 to 1468c1a Compare February 5, 2025 17:49
@pmolodo pmolodo force-pushed the pr/usd-lux-update branch from 54db536 to b1d80aa Compare June 11, 2025 22:22
@pmolodo pmolodo force-pushed the ies-angleScale-findings-questions branch from 4976f42 to b3bebc3 Compare June 11, 2025 22:22
@pmolodo pmolodo force-pushed the pr/usd-lux-update branch from b1d80aa to 36a8076 Compare July 24, 2025 16:46
@pmolodo pmolodo force-pushed the ies-angleScale-findings-questions branch from b3bebc3 to a3eb31b Compare July 24, 2025 16:46
pmolodo and others added 24 commits July 24, 2025 14:23
- add iesAngleScale images / movies locally
Fixed display of latex formulas on github
@pmolodo pmolodo force-pushed the pr/usd-lux-update branch from 36a8076 to bf4aa51 Compare July 24, 2025 21:26
@pmolodo pmolodo force-pushed the ies-angleScale-findings-questions branch from a3eb31b to 911b43a Compare July 24, 2025 21:26
pmolodo pushed a commit that referenced this pull request Jan 26, 2026
dirty expression prior to removing it from the collection tables.

Reported by the testLightLinkingSceneIndex test under asan, as:

=1343067==ERROR: AddressSanitizer: heap-use-after-free on address 0x60c0009bd598 at pc 0x7f2b478c6b23 bp 0x7ffff1bf6350 sp 0x7ffff1bf6340
READ of size 8 at 0x60c0009bd598 thread T0
    #0 0x7f2b478c6b22 in std::vector<SdfPathExpression::Op, std::allocator<SdfPathExpression::Op> >::size() const /usr/lib/gcc/x86_64-redhat-linux/11/../../../../include/c++/11/bits/stl_vector.h:919
    #1 0x7f2b478c6b22 in std::vector<SdfPathExpression::Op, std::allocator<SdfPathExpression::Op> >::vector(std::vector<SdfPathExpression::Op, std::allocator<SdfPathExpression::Op> > const&) /usr/lib/gcc/x86_64-redhat-linux/11/../../../../include/c++/11/bits/stl_vector.h:555
    #2 0x7f2b478c6b22 in SdfPathExpression::SdfPathExpression(SdfPathExpression const&) /depts/tools/build_archive/dev/builds/2379295/inst/fedora-gcc64-opt-asan/pxr/include/pxr/usd/sdf/pathExpression.h:54
    #3 0x7f2b478cf89d in std::pair<SdfPathExpression, std::optional<std::pair<SdfPath, TfToken> > >::pair<std::pair<SdfPath, TfToken> const&, true>(SdfPathExpression const&, std::pair<SdfPath, TfToken> const&) /usr/lib/gcc/x86_64-redhat-linux/11/../../../../include/c++/11/bits/stl_pair.h:337
    #4 0x7f2b478cf89d in HdsiLightLinkingSceneIndex_Impl::_Cache::ProcessCollection(SdfPath const&, TfToken const&, SdfPathExpression const&) imaging/hdsi/lightLinkingSceneIndex.cpp:156

(Internal change: 2379722)
pmolodo pushed a commit that referenced this pull request Jan 26, 2026
pxr/usdImaging.

This change introduces a prototype implementation for supporting UsdSkel
native instancing in Storm, utilizing vertex shader-based skinning.

In Hydra 1.0, when UsdSkel was natively instanced, USD scene composition
would deduplicate the scene graph. However, the SkelRootAdapter would
de-instance itself and all of its descendants during imaging due to
architectural limitations.

Hydra 2.0's native instancing model is based on binding signatures,
meaning that UsdSkel instances can only share a prototype if they have
identical skel binding signatures. In practice, this becomes problematic
because a common UsdSkel use case involves overriding skel:animation per
instance to apply unique animations - such as in a crowd of 1,000
HumanFemale_Group instances, each playing a different animation clip.
This variation causes the NiPrototypePropagatingSceneIndex to treat them
as distinct and thus de-instance them.

Our proposal addresses this by relocating skel:animation to a primvar
before NiInstanceAggregation. This ensures all instances share the same
skel binding signature, allowing proper aggregation. The per-instance
animation data is then aggregated automatically as an instance primvar
on the instancer, which can be accessed downstream with minimal code
changes.

During the SkeletonResolvingSceneIndex stage, we restore the
per-instance skel:animation data and aggregate it into tensored
skinningTransforms and blendShapeWeights, without requiring significant
schema changes to ResolvedSkeleton. A new "range" attribute is added to
handle cases where blend shapes vary in size between animations,
ensuring correct reconstruction downstream.

In PointsResolvingSceneIndex, we disable the skinning extComputation but
reuse its input generation logic. These inputs are now emitted as
primvars to feed vertex shaders directly, instead of compute shaders.
Note that currently all the tensored per-vertex and per-instance
primvars are all concatenated and passed to the vertex shader as
constant primvars (see TODO #3 and #4).  This is also potentially the
reason why the vertex shader is quite a bit slower than the compute
shader even though it's a direct port. We have to do two indirect index
lookups from primvar inputs and Nsight profiling shows that we're
spending 30% of the time waiting for memory fetch.

On the Storm side, the existing skinning compute shaders have been
ported to shared skinning methods in skinning.glsl, with added instance
support (current shared between the two skinnable point-based prims:
mesh and basisCurves).

This entire feature is gated behind the HD_ENABLE_DEFERRED_SKINNING
environment variable or the HdSkinningSettings::IsSkinningDeferred()
API call (his also requires HGI_ENABLE_VULKAN=1 for more complex test
scenes like HumanFemale_Group).

Added imaging test coverage for this implementation.

TODO:
1. fix DQS (skinningMethod is getting aggregated currently)
2. testUsdImagingGLUsdSkelInstancing is very slow for deferred skinning
   in test.usda, the majority of the time is spent in shader
   compilation, we'll file a separate ticket for that.
3. use existing skel:jointIndices and skel:jointWeights vertex primvars
   on the mesh instead of influences constant primvar from
   extComputation. This will require adding tensor value support to
   vertex primvars.
4. convert skinningTransforms and blendShapeWeights from constant
   primvars to instance primvars so we don't need the instance indexing
   logic in the vertex shader. This will require add tensor value
   support to instance primvars.

(Internal change: 2390250)
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

Successfully merging this pull request may close these issues.

2 participants