facebookresearch · jturner65 · May 3, 2021 · May 3, 2021 · May 3, 2021 · May 3, 2021
diff --git a/docs/pages/attributesJSON.rst b/docs/pages/attributesJSON.rst
@@ -230,3 +230,6 @@ The Light Setup attributes JSON should contain a single cell named "lights" that
 "type"
 	- string
 	- 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.
diff --git a/examples/tutorials/lighting_tutorial.py b/examples/tutorials/lighting_tutorial.py
@@ -99,7 +99,7 @@ def main(show_imgs=True, save_imgs=False):
 
     # create and register new light setup:
     my_scene_lighting_setup = [
-        LightInfo(vector=[0.0, 2.0, 0.6, 0.0], model=LightPositionModel.GLOBAL)
+        LightInfo(vector=[0.0, 2.0, 0.6, 0.0], model=LightPositionModel.Global)
     ]
     sim.set_light_setup(my_scene_lighting_setup, "my_scene_lighting")
 
@@ -142,7 +142,7 @@ def main(show_imgs=True, save_imgs=False):
 
     # create a custom light setup
     my_default_lighting = [
-        LightInfo(vector=[2.0, 2.0, 1.0, 0.0], model=LightPositionModel.CAMERA)
+        LightInfo(vector=[2.0, 2.0, 1.0, 0.0], model=LightPositionModel.Camera)
     ]
     # overwrite the default DEFAULT_LIGHTING_KEY light setup
     sim.set_light_setup(my_default_lighting)
@@ -165,7 +165,7 @@ def main(show_imgs=True, save_imgs=False):
         LightInfo(
             vector=[2.0, 1.5, 5.0, 1.0],
             color=[0.0, 100.0, 100.0],
-            model=LightPositionModel.GLOBAL,
+            model=LightPositionModel.Global,
         )
     ]
     sim.set_light_setup(light_setup_2, "my_custom_lighting")
@@ -196,7 +196,7 @@ def main(show_imgs=True, save_imgs=False):
         LightInfo(
             vector=[0.0, 0.0, 1.0, 0.0],
             color=[1.6, 1.6, 1.4],
-            model=LightPositionModel.CAMERA,
+            model=LightPositionModel.Camera,
         )
     ]
 

diff --git a/src/esp/bindings/GfxBindings.cpp b/src/esp/bindings/GfxBindings.cpp
@@ -161,9 +161,9 @@ void initGfxBindings(py::module& m) {
   py::enum_<LightPositionModel>(
       m, "LightPositionModel",
       R"(Defines the coordinate frame of a light source.)")
-      .value("CAMERA", LightPositionModel::CAMERA)
-      .value("GLOBAL", LightPositionModel::GLOBAL)
-      .value("OBJECT", LightPositionModel::OBJECT);
+      .value("Camera", LightPositionModel::Camera)
+      .value("Global", LightPositionModel::Global)
+      .value("Object", LightPositionModel::Object);
 
   py::enum_<LightType>(
       m, "LightType", R"(Defines the type of light described by the LightInfo)")
@@ -178,7 +178,7 @@ void initGfxBindings(py::module& m) {
       .def(py::init())
       .def(py::init<Magnum::Vector4, Magnum::Color3, LightPositionModel>(),
            "vector"_a, "color"_a = Magnum::Color3{1},
-           "model"_a = LightPositionModel::GLOBAL)
+           "model"_a = LightPositionModel::Global)
       .def_readwrite("vector", &LightInfo::vector)
       .def_readwrite("color", &LightInfo::color)
       .def_readwrite("model", &LightInfo::model)

diff --git a/src/esp/gfx/LightSetup.cpp b/src/esp/gfx/LightSetup.cpp
@@ -28,11 +28,11 @@ Magnum::Vector4 getLightPositionRelativeToCamera(
                         "w == 1 for a point light");
 
   switch (light.model) {
-    case LightPositionModel::OBJECT:
+    case LightPositionModel::Object:
       return transformationMatrix * light.vector;
-    case LightPositionModel::GLOBAL:
+    case LightPositionModel::Global:
       return cameraMatrix * light.vector;
-    case LightPositionModel::CAMERA:
+    case LightPositionModel::Camera:
       return light.vector;
   }
 

diff --git a/src/esp/gfx/LightSetup.h b/src/esp/gfx/LightSetup.h
@@ -17,11 +17,11 @@ namespace gfx {
 
 enum class LightPositionModel {
   /** @brief Light position is relative to the camera */
-  CAMERA = 0,
+  Camera = 0,
   /** @brief Light position is relative to scene */
-  GLOBAL = 1,
+  Global = 1,
   /** @brief Light position is relative to the object being rendered */
-  OBJECT = 2,
+  Object = 2,
 };
 
 enum class LightType {
@@ -40,7 +40,7 @@ struct LightInfo {
   // directional light with no distance attenuation.
   Magnum::Vector4 vector;
   Magnum::Color3 color{1};
-  LightPositionModel model = LightPositionModel::GLOBAL;
+  LightPositionModel model = LightPositionModel::Global;
 };
 
 bool operator==(const LightInfo& a, const LightInfo& b);

diff --git a/src/esp/metadata/attributes/LightLayoutAttributes.cpp b/src/esp/metadata/attributes/LightLayoutAttributes.cpp
@@ -13,7 +13,13 @@ const std::map<std::string, esp::gfx::LightType>
     LightInstanceAttributes::LightTypeNamesMap = {
         {"point", esp::gfx::LightType::Point},
         {"directional", esp::gfx::LightType::Directional},
-};
+        {"spot", esp::gfx::LightType::Spot}};
+
+const std::map<std::string, esp::gfx::LightPositionModel>
+    LightInstanceAttributes::LightPositionNamesMap = {
+        {"global", esp::gfx::LightPositionModel::Global},
+        {"camera", esp::gfx::LightPositionModel::Camera},
+        {"object", esp::gfx::LightPositionModel::Object}};
 
 LightInstanceAttributes::LightInstanceAttributes(const std::string& handle)
     : AbstractAttributes("LightInstanceAttributes", handle) {
@@ -22,6 +28,7 @@ LightInstanceAttributes::LightInstanceAttributes(const std::string& handle)
   setColor({1.0, 1.0, 1.0});
   setIntensity(1.0);
   setType(static_cast<int>(esp::gfx::LightType::Point));
+  setPositionModel(static_cast<int>(esp::gfx::LightPositionModel::Global));
   // ignored for all but spot lights
   setInnerConeAngle(0.0_radf);
   setOuterConeAngle(90.0_degf);

diff --git a/src/esp/metadata/attributes/LightLayoutAttributes.h b/src/esp/metadata/attributes/LightLayoutAttributes.h
@@ -22,9 +22,19 @@ class LightInstanceAttributes : public AbstractAttributes {
   /**
    * @brief Constant static map to provide mappings from string tags to @ref
    * esp::gfx::LightType values.  This will be used to map values set in json
-   * for light type to @ref esp::gfx::LightType.  Keys must be lowercase.
+   * for light type to @ref esp::gfx::LightType.  Keys must be lowercase - will
+   * support any case values in JSON.
    */
   static const std::map<std::string, esp::gfx::LightType> LightTypeNamesMap;
+
+  /**
+   * @brief Constant static map to provide mappings from string tags to @ref
+   * esp::gfx::LightPositionModel values.  This will be used to map values set
+   * in json to specify what translations are measured from for a lighting
+   * instance.
+   */
+  static const std::map<std::string, esp::gfx::LightPositionModel>
+      LightPositionNamesMap;
   explicit LightInstanceAttributes(const std::string& handle = "");
 
   /**
@@ -61,6 +71,16 @@ class LightInstanceAttributes : public AbstractAttributes {
   void setType(int type) { setInt("type", type); }
   int getType() const { return getInt("type"); }
 
+  /**
+   * @brief Get/Set the position model to use when placing the light - whether
+   * the lights translation should be relative to the camera, the global scene
+   * origin, or some object.
+   */
+  void setPositionModel(int position_model) {
+    setInt("position_model", position_model);
+  }
+  int getPositionModel() const { return getInt("position_model"); }
+
   /**
    * @brief Get/Set inner cone angle for spotlights.  Should be ignored for
    * other lights

diff --git a/src/esp/metadata/managers/LightLayoutAttributesManager.cpp b/src/esp/metadata/managers/LightLayoutAttributesManager.cpp
@@ -120,19 +120,49 @@ void LightLayoutAttributesManager::setLightInstanceValsFromJSONDoc(
                                lightAttribs->setIntensity(intensity);
                              });
 
+  // set frame of reference for light transformation
+  int posMdleVal = -1;
+  std::string tmpPosMdleVal = "";
+  if (io::readMember<std::string>(jsonConfig, "position_model",
+                                  tmpPosMdleVal)) {
+    std::string strToLookFor = Cr::Utility::String::lowercase(tmpPosMdleVal);
+    if (LightInstanceAttributes::LightPositionNamesMap.count(strToLookFor)) {
+      posMdleVal = static_cast<int>(
+          LightInstanceAttributes::LightPositionNamesMap.at(strToLookFor));
+    } else {
+      LOG(WARNING)
+          << "LightLayoutAttributesManager::setLightInstanceValsFromJSONDoc : "
+             "Position Model Value in JSON : `"
+          << posMdleVal
+          << "` does not map to a valid "
+             "LightInstanceAttributes::LightPositionNamesMap value, so "
+             "defaulting LightInfo position model to "
+             "esp::gfx::LightPositionModel::Global.";
+      posMdleVal = static_cast<int>(esp::gfx::LightPositionModel::Global);
+    }
+    lightAttribs->setPositionModel(posMdleVal);
+  }
+
   // type of light - should map to enum values in esp::gfx::LightType
   int typeVal = -1;
-  std::string tmpVal = "";
-  if (io::readMember<std::string>(jsonConfig, "type", tmpVal)) {
-    std::string strToLookFor = Cr::Utility::String::lowercase(tmpVal);
-    if (LightInstanceAttributes::LightTypeNamesMap.count(strToLookFor)) {
+  std::string tmpTypeVal = "";
+  if (io::readMember<std::string>(jsonConfig, "type", tmpTypeVal)) {
+    std::string strToLookFor = Cr::Utility::String::lowercase(tmpTypeVal);
+    if (strToLookFor.compare("spot") == 0) {
+      // TODO remove this if block to support spot lights
+      LOG(WARNING)
+          << "LightLayoutAttributesManager::setLightInstanceValsFromJSONDoc : "
+             "Type spotlight specified in JSON not currently supported, so "
+             "defaulting LightInfo type to esp::gfx::LightType::Point.";
+      typeVal = static_cast<int>(esp::gfx::LightType::Point);
+    } else if (LightInstanceAttributes::LightTypeNamesMap.count(strToLookFor)) {
       typeVal = static_cast<int>(
           LightInstanceAttributes::LightTypeNamesMap.at(strToLookFor));
     } else {
       LOG(WARNING)
           << "LightLayoutAttributesManager::setLightInstanceValsFromJSONDoc : "
-             "Type Value in json : `"
-          << tmpVal
+             "Type Value in JSON : `"
+          << tmpTypeVal
           << "` does not map to a valid "
              "LightInstanceAttributes::LightTypeNamesMap value, so "
              "defaulting LightInfo type to esp::gfx::LightType::Point.";
@@ -150,10 +180,13 @@ void LightLayoutAttributesManager::setLightInstanceValsFromJSONDoc(
   // read spotlight params
   if (jsonConfig.HasMember("spot")) {
     if (!jsonConfig["spot"].IsObject()) {
+      // TODO prune NOTE: component when spotlights are supported
       LOG(WARNING)
           << "LightLayoutAttributesManager::setValsFromJSONDoc : \"spot\" "
              "cell in JSON config unable to be parsed to set "
-             "spotlight parameters so skipping.";
+             "spotlight parameters so skipping.  NOTE : Spotlights not "
+             "currently supported, so cone anble values are ignored and light "
+             "will be created as a point light.";
     } else {
       const auto& spotArea = jsonConfig["spot"];
       // set inner cone angle
@@ -170,7 +203,7 @@ void LightLayoutAttributesManager::setLightInstanceValsFromJSONDoc(
             lightAttribs->setOuterConeAngle(outerConeAngle);
           });
     }
-  }  // if member spot present
+  }  // if JSON object 'spot' present
 }  // LightLayoutAttributesManager::setValsFromJSONDoc
 
 LightLayoutAttributes::ptr LightLayoutAttributesManager::initNewObjectInternal(
@@ -208,8 +241,12 @@ gfx::LightSetup LightLayoutAttributesManager::createLightSetupFromAttributes(
     if (numLightInstances == 0) {
       // setup default LightInfo instances - lifted from LightSetup.cpp.
       // TODO create default attributes describing these lights?
-      return gfx::LightSetup{{{1.0, 1.0, 0.0, 0.0}, {0.75, 0.75, 0.75}},
-                             {{-0.5, 0.0, 1.0, 0.0}, {0.4, 0.4, 0.4}}};
+      return gfx::LightSetup{{.vector = {1.0, 1.0, 0.0, 0.0},
+                              .color = {0.75, 0.75, 0.75},
+                              .model = gfx::LightPositionModel::Global},
+                             {.vector = {-0.5, 0.0, 1.0, 0.0},
+                              .color = {0.4, 0.4, 0.4},
+                              .model = gfx::LightPositionModel::Global}};
     } else {
       const std::map<std::string, LightInstanceAttributes::ptr>&
           lightInstances = lightLayoutAttributes->getLightInstances();
@@ -218,6 +255,8 @@ gfx::LightSetup LightLayoutAttributesManager::createLightSetupFromAttributes(
         const LightInstanceAttributes::ptr& lightAttr = elem.second;
         const int type = lightAttr->getType();
         const gfx::LightType typeEnum = static_cast<gfx::LightType>(type);
+        const gfx::LightPositionModel posModelEnum =
+            static_cast<gfx::LightPositionModel>(lightAttr->getPositionModel());
         const Magnum::Color3 color =
             lightAttr->getColor() * lightAttr->getIntensity();
         Magnum::Vector4 lightVector;
@@ -240,7 +279,8 @@ gfx::LightSetup LightLayoutAttributesManager::createLightSetupFromAttributes(
             lightVector = {lightAttr->getPosition(), 1.0f};
           }
         }  // switch on type
-        res.push_back({lightVector, color});
+        res.push_back(
+            {.vector = lightVector, .color = color, .model = posModelEnum});
       }  // for each light instance described
     }    // if >0 light instances described
   }      // lightLayoutAttributes of requested name exists

diff --git a/src/tests/AttributesManagersTest.cpp b/src/tests/AttributesManagersTest.cpp
@@ -459,6 +459,7 @@ TEST_F(AttributesManagersTest, AttributesManagers_LightJSONLoadTest) {
         "intensity": -0.1,
         "color": [2,1,-1],
         "type": "directional",
+        "position_model" : "camera",
         "spot": {
           "innerConeAngle": -0.75,
           "outerConeAngle": -1.57
@@ -486,6 +487,8 @@ TEST_F(AttributesManagersTest, AttributesManagers_LightJSONLoadTest) {
   ASSERT_EQ(lightAttr->getIntensity(), -0.1);
   ASSERT_EQ(lightAttr->getType(),
             static_cast<int>(esp::gfx::LightType::Directional));
+  ASSERT_EQ(lightAttr->getPositionModel(),
+            static_cast<int>(esp::gfx::LightPositionModel::Camera));
   ASSERT_EQ(lightAttr->getInnerConeAngle(), -0.75_radf);
   ASSERT_EQ(lightAttr->getOuterConeAngle(), -1.57_radf);
 }  // AttributesManagers_LightJSONLoadTest

diff --git a/src/tests/SimTest.cpp b/src/tests/SimTest.cpp
@@ -133,10 +133,10 @@ struct SimTest : Cr::TestSuite::Tester {
 
   LightSetup lightSetup1{{Magnum::Vector4{1.0f, 1.5f, 0.5f, 0.0f},
                           {5.0, 5.0, 0.0},
-                          LightPositionModel::CAMERA}};
+                          LightPositionModel::Camera}};
   LightSetup lightSetup2{{Magnum::Vector4{0.0f, 0.5f, 1.0f, 0.0f},
                           {0.0, 5.0, 5.0},
-                          LightPositionModel::CAMERA}};
+                          LightPositionModel::Camera}};
 };
 struct {
   // display name for sim being tested

diff --git a/tests/test_light_setup.py b/tests/test_light_setup.py
@@ -22,7 +22,7 @@ def test_set_default_light_setup(make_cfg_settings):
         assert sim.get_light_setup() == light_setup
 
         # ensure modifications to local light setup variable are not reflected in sim
-        light_setup[0].model = LightPositionModel.CAMERA
+        light_setup[0].model = LightPositionModel.Camera
         assert sim.get_light_setup() != light_setup
 
         sim.set_light_setup(light_setup, DEFAULT_LIGHTING_KEY)