Skip to content

Improve lights#238

Merged
bsavery merged 1 commit intoGPUOpen-LibrariesAndSDKs:masterfrom
hshakula:light-improvements
Feb 26, 2020
Merged

Improve lights#238
bsavery merged 1 commit intoGPUOpen-LibrariesAndSDKs:masterfrom
hshakula:light-improvements

Conversation

@hshakula
Copy link
Copy Markdown
Contributor

@hshakula hshakula commented Feb 21, 2020

Description of Changes

Support light shaping as much as possible - Point, Spot, and IES lights.

Existing issues:

  • In Full quality spotlight's penumbra falloff does not match with Karma (or RenderMan) - Tahoe has different attenuation function. From the other side Hybrid's spotlight matches with Karma almost ideally (but still differs in light intensity)
  • Shadows from spotlights are always hard despite the distance to the light (same as Karma but differs with RenderMan)

Rework area lights for Hybrid engine

Hybrid supports arbitrarily shaped area lights only in High quality, in low and medium quality they do not affect the scene. In low and medium qualities it's possible to use a rectangular mesh (LTC polygonal light) as the shape of our area light. So to fully support area lights in Hybrid I approximated them all as a set of rect area lights:

  • Sphere and cylinder approximated by the cube (6 separate rect meshes)
  • Disk approximated by one rect mesh

Resulted meshes match proportions of original meshes. Also, to keep total emission from the light same as would emit original mesh, I scale the resulted meshes so that their area matches the area of original meshes

To simplify implementation was decided to apply this approximation of lights for all of the Hybrid qualities. Otherwise, it would require us to resync Hydra light primitives from HdRprApi what would introduce encapsulation hell - we would need to keep track of every Hydra light primitive object in HdRprApi class, right now only HdRenderIndex knows about all existing light primitives which obviously not under our control. This can be revisited later.

Make area lights invisible in Full quality

The motivation here is to match Karma's behavior. I left ability for the user to fall back to the old behavior via environment variable setting - HDRPR_VISIBLE_LIGHTS

Technical steps

  • Combine classes of area lights - HdRprDiskLight, HdRprRectLight, HdRprSphereLight and HdRprCylinderLight - into one HdRprLight class. The reason here is that each area light type can be shaped into one of the shapes from UsdLuxShapingAPI, i.e. each area light type can represent one of UsdLuxShapingAPI's shapes or corresponding light type. This is controlled dynamically via light parameters.
  • HdRprLight handles:
    • UsdLuxShapingAPI and dynamically decides light type on the scene.
    • area light representation - arbitrary shape or rect approximated.

@bsavery
Copy link
Copy Markdown
Contributor

bsavery commented Feb 22, 2020

Very nice change.

My only question is with the visibility:
The motivation here is to match Karma's behavior. I left ability for the user to fall back to the old behavior via environment variable setting - HDRPR_VISIBLE_LIGHTS

What does Storm / RenderMan do as opposed to Karma?

@hshakula
Copy link
Copy Markdown
Contributor Author

I can’t say about storm right now - it does not work for me in Houdini. RenderMan’s default behavior is visible lights but you have an ability to control its visibility in refraction, transparency and for primary rays. From another side karma’s lights are always invisible and there’s no way to change it AFAIK. I can actually expose in some way RPR’s visibility flags but I would prefer to do it on request or when it will be supported in hybrid, if you think otherwise let me know (anyway I will add it to my todo list)

@bsavery
Copy link
Copy Markdown
Contributor

bsavery commented Feb 24, 2020

I guess my question here was "Do we need to have the ability to fall back to the old behavior". If the new one is preferred.

@hshakula
Copy link
Copy Markdown
Contributor Author

This behavior preferred for us right now only because hdRpr mainly used in Houdini but for some other type of software, it might be preferred to show area lights.

The best solution here is to add visibility flags to the primvar API of our lights so that the user could control it more explicitly. What do you think? Should I implement it within this PR?

@hshakula
Copy link
Copy Markdown
Contributor Author

Just checked - Storm does not show lights either

@bsavery
Copy link
Copy Markdown
Contributor

bsavery commented Feb 26, 2020

Ok. I just think the ability to revert behavior is not needed. Especially if we implement the visibility flags.

@hshakula
Copy link
Copy Markdown
Contributor Author

I am all for removing this later when visibility flags are implemented.

@bsavery
Copy link
Copy Markdown
Contributor

bsavery commented Feb 26, 2020

Ok. Please add that task.

@bsavery bsavery merged commit 8f24be7 into GPUOpen-LibrariesAndSDKs:master Feb 26, 2020
@hshakula hshakula deleted the light-improvements branch February 26, 2020 21:04
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