PixarAnimationStudios
UsdLux update
Description of Change(s)
This is a draft PR that is NOT meant to be merged, intended to provide a focal point for discussion. There are several points in the described behaviour that need agreement from the community.
This PR adds clarifying documentation of the quantities and behaviour of the parameters in UsdLux, all of which are expressed as changes to the doc comments and overview.dox in pxr/usd/usdLux.
It also adds a reference implementation to the hdEmbree example delegate, by modifying that delegate to perform direct lighting according to the specification in the (revised) UsdLux documentation. If no lights are in the stage, hdEmbree falls back to ambient occlusion, as before.
There is a separate repo, here https://github.com/anderslanglands/luxtest there does a bunch of "golden image" unit test of the behaviour and compares renderers. It's all a bit manual at the moment but it generates this result: https://anderslanglands.github.io/luxtest/luxtest.html
Questions for the USD maintainers:
- What's the best way of incorporating the changes to UsdLux here. Since it implies a behavioral change on everyone implementing it, it feels like versioning up the schema would be the right approach? If so, what code changes are required to do that exactly?
- Are we OK with modifying the Embree delegate in this way, or would it be preferred to split it out into a separate delegate? Or do we want to just lock the new behaviour behind a feature flag/env var?
- What sort of testing functionality do we want to include? It might be nice to include some part of the luxtest repo to allow people to run tests of a delegate against the reference easily. Not sure how to go about setting that up.
Hi Anders, looking forward to reading this more closely.
Could you perhaps submit 51616a6 as a separate PR? That functionality's generally useful and has been requested by others.
@meshula I already did here: https://github.com/PixarAnimationStudios/OpenUSD/pull/2674 although weirdly I can't; find that from the PR search. Looking at it now I probably shouldn't have made that against release but against dev. I'll delete that, rebase and resubmit.
OK I've made a new PR rebased on dev here: https://github.com/PixarAnimationStudios/OpenUSD/pull/2762
The IES parser in general looks very nice. I added some notes that should enable parsing all the variants I had collected for a test corpus.
Here's a radially symmetric file for testing
IESNA:LM-63-2002
[TEST] 15905
[TESTLAB] LUMINAIRE TESTING LABORATORY, INC.
[ISSUEDATE] 06-11-2009
[MANUFAC] COLOR KINETICS
[LUMCAT] C-SPLASH 2-FROSTED-ALL ON
[LUMINAIRE] CAST BRASS HOUSING, FROSTED GLASS ENCLOSURE.
[LAMP] 6 RED, 6 GREEN, 6 BLUE LEDS WITH CLEAR PLASTIC OPTICS BELOW EACH.
[LAMPCAT]
[BALLAST] PDS-60 24V POWER SUPPLY WITH COLOR DIAL
[OTHER] ELECTRICAL VALUES: 120.0VAC, 0.3514A, 41.71W
[OTHER] NOTE: THIS TEST WAS PERFORMED USING THE CALIBRATED
[OTHER] PHOTODETECTOR METHOD OF ABSOLUTE PHOTOMETRY.*
[OTHER] MOUNTING: BRACKET/SURFACE
TILT=NONE
1 515 1.0 361 1 1 1 -0.542 -0.542 0.000
1.0 1 41.7
0.0 0.5 1.0 1.5 2.0 2.5 3.0 3.5 4.0 4.5 5.0 5.5 6.0 6.5 7.0 7.5 8.0 8.5 9.0 9.5
10.0 10.5 11.0 11.5 12.0 12.5 13.0 13.5 14.0 14.5 15.0 15.5 16.0 16.5 17.0
17.5 18.0 18.5 19.0 19.5 20.0 20.5 21.0 21.5 22.0 22.5 23.0 23.5 24.0 24.5
25.0 25.5 26.0 26.5 27.0 27.5 28.0 28.5 29.0 29.5 30.0 30.5 31.0 31.5 32.0
32.5 33.0 33.5 34.0 34.5 35.0 35.5 36.0 36.5 37.0 37.5 38.0 38.5 39.0 39.5
40.0 40.5 41.0 41.5 42.0 42.5 43.0 43.5 44.0 44.5 45.0 45.5 46.0 46.5 47.0
47.5 48.0 48.5 49.0 49.5 50.0 50.5 51.0 51.5 52.0 52.5 53.0 53.5 54.0 54.5
55.0 55.5 56.0 56.5 57.0 57.5 58.0 58.5 59.0 59.5 60.0 60.5 61.0 61.5 62.0
62.5 63.0 63.5 64.0 64.5 65.0 65.5 66.0 66.5 67.0 67.5 68.0 68.5 69.0 69.5
70.0 70.5 71.0 71.5 72.0 72.5 73.0 73.5 74.0 74.5 75.0 75.5 76.0 76.5 77.0
77.5 78.0 78.5 79.0 79.5 80.0 80.5 81.0 81.5 82.0 82.5 83.0 83.5 84.0 84.5
85.0 85.5 86.0 86.5 87.0 87.5 88.0 88.5 89.0 89.5 90.0 90.5 91.0 91.5 92.0
92.5 93.0 93.5 94.0 94.5 95.0 95.5 96.0 96.5 97.0 97.5 98.0 98.5 99.0 99.5
100.0 100.5 101.0 101.5 102.0 102.5 103.0 103.5 104.0 104.5 105.0 105.5 106.0
106.5 107.0 107.5 108.0 108.5 109.0 109.5 110.0 110.5 111.0 111.5 112.0 112.5
113.0 113.5 114.0 114.5 115.0 115.5 116.0 116.5 117.0 117.5 118.0 118.5 119.0
119.5 120.0 120.5 121.0 121.5 122.0 122.5 123.0 123.5 124.0 124.5 125.0 125.5
126.0 126.5 127.0 127.5 128.0 128.5 129.0 129.5 130.0 130.5 131.0 131.5 132.0
132.5 133.0 133.5 134.0 134.5 135.0 135.5 136.0 136.5 137.0 137.5 138.0 138.5
139.0 139.5 140.0 140.5 141.0 141.5 142.0 142.5 143.0 143.5 144.0 144.5 145.0
145.5 146.0 146.5 147.0 147.5 148.0 148.5 149.0 149.5 150.0 150.5 151.0 151.5
152.0 152.5 153.0 153.5 154.0 154.5 155.0 155.5 156.0 156.5 157.0 157.5 158.0
158.5 159.0 159.5 160.0 160.5 161.0 161.5 162.0 162.5 163.0 163.5 164.0 164.5
165.0 165.5 166.0 166.5 167.0 167.5 168.0 168.5 169.0 169.5 170.0 170.5 171.0
171.5 172.0 172.5 173.0 173.5 174.0 174.5 175.0 175.5 176.0 176.5 177.0 177.5
178.0 178.5 179.0 179.5 180.0
0.0
1996 1994 1984 1967 1945 1916 1880 1840 1794 1743 1689 1632 1572 1510 1447 1384
1320 1255 1192 1131 1069 1010 954 900 847 797 749 704 661 620 583 547 513 482
453 425 400 376 354 333 314 296 280 264 250 236 224 212 202 191 182 173 165
157 150 143 137 131 126 120 115 111 106 102 98 95 91 88 85 82 79 76 74 71 69
67 65 63 61 59 57 55 54 52 51 49 48 46 45 44 43 41 40 39 38 37 36 35 34 33 32
31 30 30 29 28 27 26 26 25 24 24 23 22 22 21 20 20 19 19 18 18 17 17 16 15 15
15 14 13 13 13 12 12 11 11 10 10 9 9 9 8 8 7 7 7 6 6 5 5 5 4 4 3 3 3 2 2 2 1 1
1 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
0 0 0 0 0
Thanks I’ll give that a go. The IES parser is lifted straight from Cycles.
On Sat, 28 Oct 2023 at 08:11, Nick Porcino @.***> wrote:
Here's a radially symmetric file for testing
IESNA:LM-63-2002 [TEST] 15905 [TESTLAB] LUMINAIRE TESTING LABORATORY, INC. [ISSUEDATE] 06-11-2009 [MANUFAC] COLOR KINETICS [LUMCAT] C-SPLASH 2-FROSTED-ALL ON [LUMINAIRE] CAST BRASS HOUSING, FROSTED GLASS ENCLOSURE. [LAMP] 6 RED, 6 GREEN, 6 BLUE LEDS WITH CLEAR PLASTIC OPTICS BELOW EACH. [LAMPCAT] [BALLAST] PDS-60 24V POWER SUPPLY WITH COLOR DIAL [OTHER] ELECTRICAL VALUES: 120.0VAC, 0.3514A, 41.71W [OTHER] NOTE: THIS TEST WAS PERFORMED USING THE CALIBRATED [OTHER] PHOTODETECTOR METHOD OF ABSOLUTE PHOTOMETRY.* [OTHER] MOUNTING: BRACKET/SURFACE TILT=NONE 1 515 1.0 361 1 1 1 -0.542 -0.542 0.000 1.0 1 41.7 0.0 0.5 1.0 1.5 2.0 2.5 3.0 3.5 4.0 4.5 5.0 5.5 6.0 6.5 7.0 7.5 8.0 8.5 9.0 9.5 10.0 10.5 11.0 11.5 12.0 12.5 13.0 13.5 14.0 14.5 15.0 15.5 16.0 16.5 17.0 17.5 18.0 18.5 19.0 19.5 20.0 20.5 21.0 21.5 22.0 22.5 23.0 23.5 24.0 24.5 25.0 25.5 26.0 26.5 27.0 27.5 28.0 28.5 29.0 29.5 30.0 30.5 31.0 31.5 32.0 32.5 33.0 33.5 34.0 34.5 35.0 35.5 36.0 36.5 37.0 37.5 38.0 38.5 39.0 39.5 40.0 40.5 41.0 41.5 42.0 42.5 43.0 43.5 44.0 44.5 45.0 45.5 46.0 46.5 47.0 47.5 48.0 48.5 49.0 49.5 50.0 50.5 51.0 51.5 52.0 52.5 53.0 53.5 54.0 54.5 55.0 55.5 56.0 56.5 57.0 57.5 58.0 58.5 59.0 59.5 60.0 60.5 61.0 61.5 62.0 62.5 63.0 63.5 64.0 64.5 65.0 65.5 66.0 66.5 67.0 67.5 68.0 68.5 69.0 69.5 70.0 70.5 71.0 71.5 72.0 72.5 73.0 73.5 74.0 74.5 75.0 75.5 76.0 76.5 77.0 77.5 78.0 78.5 79.0 79.5 80.0 80.5 81.0 81.5 82.0 82.5 83.0 83.5 84.0 84.5 85.0 85.5 86.0 86.5 87.0 87.5 88.0 88.5 89.0 89.5 90.0 90.5 91.0 91.5 92.0 92.5 93.0 93.5 94.0 94.5 95.0 95.5 96.0 96.5 97.0 97.5 98.0 98.5 99.0 99.5 100.0 100.5 101.0 101.5 102.0 102.5 103.0 103.5 104.0 104.5 105.0 105.5 106.0 106.5 107.0 107.5 108.0 108.5 109.0 109.5 110.0 110.5 111.0 111.5 112.0 112.5 113.0 113.5 114.0 114.5 115.0 115.5 116.0 116.5 117.0 117.5 118.0 118.5 119.0 119.5 120.0 120.5 121.0 121.5 122.0 122.5 123.0 123.5 124.0 124.5 125.0 125.5 126.0 126.5 127.0 127.5 128.0 128.5 129.0 129.5 130.0 130.5 131.0 131.5 132.0 132.5 133.0 133.5 134.0 134.5 135.0 135.5 136.0 136.5 137.0 137.5 138.0 138.5 139.0 139.5 140.0 140.5 141.0 141.5 142.0 142.5 143.0 143.5 144.0 144.5 145.0 145.5 146.0 146.5 147.0 147.5 148.0 148.5 149.0 149.5 150.0 150.5 151.0 151.5 152.0 152.5 153.0 153.5 154.0 154.5 155.0 155.5 156.0 156.5 157.0 157.5 158.0 158.5 159.0 159.5 160.0 160.5 161.0 161.5 162.0 162.5 163.0 163.5 164.0 164.5 165.0 165.5 166.0 166.5 167.0 167.5 168.0 168.5 169.0 169.5 170.0 170.5 171.0 171.5 172.0 172.5 173.0 173.5 174.0 174.5 175.0 175.5 176.0 176.5 177.0 177.5 178.0 178.5 179.0 179.5 180.0 0.0 1996 1994 1984 1967 1945 1916 1880 1840 1794 1743 1689 1632 1572 1510 1447 1384 1320 1255 1192 1131 1069 1010 954 900 847 797 749 704 661 620 583 547 513 482 453 425 400 376 354 333 314 296 280 264 250 236 224 212 202 191 182 173 165 157 150 143 137 131 126 120 115 111 106 102 98 95 91 88 85 82 79 76 74 71 69 67 65 63 61 59 57 55 54 52 51 49 48 46 45 44 43 41 40 39 38 37 36 35 34 33 32 31 30 30 29 28 27 26 26 25 24 24 23 22 22 21 20 20 19 19 18 18 17 17 16 15 15 15 14 13 13 13 12 12 11 11 10 10 9 9 9 8 8 7 7 7 6 6 5 5 5 4 4 3 3 3 2 2 2 1 1 1 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
— Reply to this email directly, view it on GitHub https://github.com/PixarAnimationStudios/OpenUSD/pull/2758#issuecomment-1783386068, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOYQXPBUPHSKOEBBWZLFK3YBQBNVAVCNFSM6AAAAAA6P2SV46VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBTGM4DMMBWHA . You are receiving this because you authored the thread.Message ID: @.***>
Thanks Dan I did not realise that!
On Thu, 9 Nov 2023 at 10:42, Dan Y @.***> wrote:
@.**** commented on this pull request.
In pxr/usd/usdLux/lightAPI.h https://github.com/PixarAnimationStudios/OpenUSD/pull/2758#discussion_r1387231263 :
@@ -62,6 +62,38 @@ class SdfAssetPath; /// regardless of whether LightAPI is included as a built-in API of the prim /// type (e.g. RectLight or DistantLight) or is applied directly to a Gprim /// that should be treated as a light. +/// +/// Quantities and Units
I could be wrong about this, but for the usdLux header comment changes, I believe you might need to update these first in usdLux's schema.usda and then use usdGenSchema.py to generate the headers with the updated doxygen comments. See for example https://github.com/PixarAnimationStudios/OpenUSD/blob/dev/pxr/usd/usdLux/schema.usda#L49-L54
— Reply to this email directly, view it on GitHub https://github.com/PixarAnimationStudios/OpenUSD/pull/2758#pullrequestreview-1721325856, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOYQXN6ZEAUDJTUNISJTY3YDP4FHAVCNFSM6AAAAAA6P2SV46VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMRRGMZDKOBVGY . You are receiving this because you authored the thread.Message ID: @.***>
Hi, author of the Cycles IES parser here!
You might want to have a look at https://projects.blender.org/blender/blender/pulls/114689 - the topic of handling Type A files also came up on our end a few weeks ago, so I've updated the parser to be able to handle them. As part of that, I've also removed the angle checks in the processing code - while the IES spec is explicit about which specific angle ranges are allowed, I've found many files deviate from that, so Cycles now doesn't care anymore and just tries to make the best out of any input.
@anderslanglands - apologies, you can revert the change to usdrecord . We made a similar change (thanks to a patch from @creijon) on Feb 8th that is just hung up on getting pushed to gitHub due to the pending 24.03 release.
@anderslanglands - apologies, you can revert the change to
usdrecord. We made a similar change (thanks to a patch from @creijon) on Feb 8th that is just hung up on getting pushed to gitHub due to the pending 24.03 release.
You mean the change to add a disableDefaultLight option?
Yep. --spiff
On Thu, Feb 15, 2024 at 8:23 AM Anders Langlands @.***> wrote:
@anderslanglands https://github.com/anderslanglands - apologies, you can revert the change to usdrecord . We made a similar change (thanks to a patch from @creijon https://github.com/creijon) on Feb 8th that is just hung up on getting pushed to gitHub due to the pending 24.03 release.
You mean the change to add a disableDefaultLight option?
— Reply to this email directly, view it on GitHub https://github.com/PixarAnimationStudios/OpenUSD/pull/2758#issuecomment-1946479525, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPOU2G6DPHFZHZ636S5OADYTYZARAVCNFSM6AAAAAA6P2SV46VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNBWGQ3TSNJSGU . You are receiving this because you commented.Message ID: @.***>
@anderslanglands et al, following up on one of the discrepancies exposed here between prman and other renderers, which is not well-specified in UsdLux, is default light visibility-to-camera. After discussing internally and with a few external folks, we'd like to make the following brief proposal, which we're motivated to deploy in the nearish term.
We're not ready to address "generalized" or "per-ray type" visibility generally, yet, but lights and light behavior are already special enough that we are comfortable adding a specific affordance for lights that perhaps models what we'd promote later, more generally.
Behavior
The common behavior seems to be that all lights -- except for "geometry lights" like MeshLights and VolumeLights -- should be invisible by default to camera rays, though sometimes you will want to override this behavior (like rendering the texture of a DomeLight).
Affordance
We propose adding to LightAPI the following attribute:
token visibility:camera = "invisible"
and then overriding it in MeshLightAPI and VolumeLightAPI:
token visibility:camera = "inherited" (
customData = {
bool apiSchemaOverride = 1
}
)
Semantics
For camera rays, we'll consider the new attribute a "most local" opinion over the pre-existing inherited visibility attribute. Since we can only set visibility:camera to "invisible" or "inherited", if the light itself is already "normal" invisible, we cannot use this attribute to re-vis it to camera rays. To start, this attribute will not inherit, being part of the LightAPI schema specifically.
Questions
- We're saying the new attribute can be animated... is that right?
- We don't think this will require a version-bump of LightAPI, and only, potentially prman and Storm renders will be impacted. Internally we already model this behavior in RenderMan by manually adding the prman-specific camera-visibility attributes on all of our light models. Sound OK?
@spiffmon great! My only concern is that I do think visibility:camera should be able to "re-vis" an invisible light. The common use case for this is having two different representations of a light: one for the camera and one for secondary rays. Most common example of this is having a high-res HDRI visible to camera only but a convolved HDRI visible to only secondary rays. This is still a common request for realtime-oriented workflows where sampling the high-res HDRI directly for illumination can be too noisy for low sample counts.
In response to your questions, yes it should be animatable, and I think it's fine not to bump API version
@anderslanglands , making per-ray visibility be tri-state is a good topic to continue discussing, but I'm not sure it's the solution to the problem you describe. We've gathered quite alot of feedback from our artists that needing to manage two lights (most often exactly as you say, one "sampled" light and one "visible", where the sampled one is a cheaper analytic light) and having to keep them in sync is a problem, and that was the dominating reason for turning Light into LightAPI, so that we could have a MeshLightAPI. Ironically, Mesh Lights remain a problem because they are most often the lights that we want to have an analytically sampled-representation for... this is a project we hope to tackle at some point.
So my response to your example would be: let's adjust the DomeLight schema to allow specification of a lightSampling texture in addition to the current "backdrop" texture and whatever other controls we need to manage its use? That way you still only need to manage one light in the scenegraph.
Any other arguments for visibility:camera to be tri-state?
Yes that seems reasonable. I do worry about potential future scope creep in "... and whatever other controls we need to manage its use" but as you say the key point is that both "lights" should be identical aside from the map.
I'm ok with this in that case. I might suggest inverting the sense of the naming you proposed to make the new texture attribute the "backdrop" or "camera" texture, since a DomeLight is primarily a light after all, but that's just naming.