Skip to content

Support specifying position model/frame of reference for lighting instances#1210

Merged
jturner65 merged 4 commits into
facebookresearch:masterfrom
jturner65:MM_LightConfig_PosModel
May 3, 2021
Merged

Support specifying position model/frame of reference for lighting instances#1210
jturner65 merged 4 commits into
facebookresearch:masterfrom
jturner65:MM_LightConfig_PosModel

Conversation

@jturner65

Copy link
Copy Markdown
Contributor

Motivation and Context

This PR introduces JSON support, and appropriate testing and documentation, for setting the desired position model/frame of reference for a lighting instance. With this setting, the user can specify whether they wish for a particular light info/instance to be placed in the scene relative to the global origin or the camera. Object-relative positioning is also fully supported in the configuration, but only as a stub currently in the underlying lighting code.

The enum describing the position model value has also been refactored to follow our naming conventions, from ALL_CAPS to pascal case, in the c++ and python code.

How Has This Been Tested

All existing and added c++ and python test cases pass

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have completed my CLA (see CONTRIBUTING)
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label May 3, 2021
Comment thread src/esp/gfx/LightSetup.cpp
@jturner65 jturner65 merged commit 70769ba into facebookresearch:master May 3, 2021
@jturner65 jturner65 deleted the MM_LightConfig_PosModel branch May 3, 2021 17:54
- The type of the light. "point" and "directional" are currently supported.
"position_model"
- string
- They frame to use to place the light. "global", meaning stage's origin, and "camera", meaning place relative to a (potentially moving) camera, are currently supported.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh, the "object" mode cannot be supported at the moment.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, this would require more configuration specifications at the scene instance level, to specify which object instance the light should "orbit"

@bigbike bigbike May 3, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not "orbit". Just stay fixed relative to the object.
In that case, the "light" should be in the scene graph, and be attached to a child node of that object.

@jturner65 jturner65 May 3, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed orbit :). (but yeah, I was kind of abusing terminology there, excuse me).

@bigbike bigbike left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is great! Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Do not delete this pull request or issue due to inactivity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants