forked from PixarAnimationStudios/OpenUSD
-
Notifications
You must be signed in to change notification settings - Fork 0
ies:AngleScale - findings and questions #4
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
Open
pmolodo
wants to merge
30
commits into
pr/usd-lux-update
Choose a base branch
from
ies-angleScale-findings-questions
base: pr/usd-lux-update
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pmolodo
commented
Sep 3, 2024
Owner
Author
There was a problem hiding this comment.
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).
1422fb3 to
0d32ae1
Compare
1524662 to
fd2d558
Compare
6671ceb to
b237085
Compare
0d32ae1 to
99a1b70
Compare
b237085 to
547c22a
Compare
99a1b70 to
f0992c4
Compare
547c22a to
5547016
Compare
f0992c4 to
2c9e40e
Compare
5547016 to
45b8af6
Compare
2c9e40e to
f36b3fa
Compare
45b8af6 to
4d63935
Compare
f36b3fa to
70af94a
Compare
5286654 to
3436293
Compare
70af94a to
694a278
Compare
3436293 to
17ec01b
Compare
694a278 to
c7389fb
Compare
17ec01b to
6594288
Compare
c7389fb to
54db536
Compare
6594288 to
1468c1a
Compare
54db536 to
b1d80aa
Compare
4976f42 to
b3bebc3
Compare
b1d80aa to
36a8076
Compare
b3bebc3 to
a3eb31b
Compare
- it requires a blank line before $$ equations on own line $$ - it doesn't render math equations inside of lists
for bimodal formula
- add iesAngleScale images / movies locally
Fixed display of latex formulas on github
36a8076 to
bf4aa51
Compare
a3eb31b to
911b43a
Compare
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: