From ce49aa1202e3ee8f03bfbce33480c64b95cf2e8f Mon Sep 17 00:00:00 2001
From: Cody Glassman <glassmancody.info@gmail.com>
Date: Sat, 4 Jun 2022 13:35:27 +0000
Subject: [PATCH] Attach lights at origin when missing AttachLight node

---
 CHANGELOG.md                       |  1 +
 apps/opencs/view/render/object.cpp |  2 +-
 apps/openmw/mwrender/animation.cpp |  2 +-
 components/sceneutil/lightutil.cpp | 28 ++--------------------------
 components/sceneutil/lightutil.hpp |  5 ++---
 5 files changed, 7 insertions(+), 31 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a427f1e7c1..2cbc553151 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -119,6 +119,7 @@
     Bug #6718: Throwable weapons cause arrow enchantment effect to be applied to the whole body
     Bug #6730: LoopGroup stalls animation after playing :Stop frame until another animation is played
     Bug #6753: Info records without a DATA subrecords are loaded incorrectly
+    Bug #6794: Light sources are attached to mesh bounds centers instead of mesh origins when AttachLight NiNode is missing
     Feature #890: OpenMW-CS: Column filtering
     Feature #1465: "Reset" argument for AI functions
     Feature #2491: Ability to make OpenMW "portable"
diff --git a/apps/opencs/view/render/object.cpp b/apps/opencs/view/render/object.cpp
index 2def43df70..93c348ecc5 100644
--- a/apps/opencs/view/render/object.cpp
+++ b/apps/opencs/view/render/object.cpp
@@ -140,7 +140,7 @@ void CSVRender::Object::update()
     if (light)
     {
         bool isExterior = false; // FIXME
-        SceneUtil::addLight(mBaseNode, light, Mask_ParticleSystem, Mask_Lighting, isExterior);
+        SceneUtil::addLight(mBaseNode, light, Mask_Lighting, isExterior);
     }
 }
 
diff --git a/apps/openmw/mwrender/animation.cpp b/apps/openmw/mwrender/animation.cpp
index 0ec8d8885d..f50ef156b9 100644
--- a/apps/openmw/mwrender/animation.cpp
+++ b/apps/openmw/mwrender/animation.cpp
@@ -1507,7 +1507,7 @@ namespace MWRender
     {
         bool exterior = mPtr.isInCell() && mPtr.getCell()->getCell()->isExterior();
 
-        mExtraLightSource = SceneUtil::addLight(parent, esmLight, Mask_ParticleSystem, Mask_Lighting, exterior);
+        mExtraLightSource = SceneUtil::addLight(parent, esmLight, Mask_Lighting, exterior);
         mExtraLightSource->setActorFade(mAlpha);
     }
 
diff --git a/components/sceneutil/lightutil.cpp b/components/sceneutil/lightutil.cpp
index 07b41ac814..031899848d 100644
--- a/components/sceneutil/lightutil.cpp
+++ b/components/sceneutil/lightutil.cpp
@@ -2,7 +2,6 @@
 
 #include <osg/Light>
 #include <osg/Group>
-#include <osg/ComputeBoundsVisitor>
 
 #include <components/esm3/loadligh.hpp>
 #include <components/fallback/fallback.hpp>
@@ -11,7 +10,6 @@
 #include "lightcontroller.hpp"
 #include "util.hpp"
 #include "visitor.hpp"
-#include "positionattitudetransform.hpp"
 
 namespace SceneUtil
 {
@@ -58,34 +56,12 @@ namespace SceneUtil
         light->setQuadraticAttenuation(quadraticAttenuation);
     }
 
-    osg::ref_ptr<LightSource> addLight(osg::Group* node, const ESM::Light* esmLight, unsigned int partsysMask, unsigned int lightMask, bool isExterior)
+    osg::ref_ptr<LightSource> addLight(osg::Group* node, const ESM::Light* esmLight, unsigned int lightMask, bool isExterior)
     {
         SceneUtil::FindByNameVisitor visitor("AttachLight");
         node->accept(visitor);
 
-        osg::Group* attachTo = nullptr;
-        if (visitor.mFoundNode)
-        {
-            attachTo = visitor.mFoundNode;
-        }
-        else
-        {
-            osg::ComputeBoundsVisitor computeBound;
-            computeBound.setTraversalMask(~partsysMask);
-            // We want the bounds of all children of the node, ignoring the node's local transformation
-            // So do a traverse(), not accept()
-            computeBound.traverse(*node);
-
-            // PositionAttitudeTransform seems to be slightly faster than MatrixTransform
-            osg::ref_ptr<SceneUtil::PositionAttitudeTransform> trans(new SceneUtil::PositionAttitudeTransform);
-            trans->setPosition(computeBound.getBoundingBox().center());
-            trans->setNodeMask(lightMask);
-
-            node->addChild(trans);
-
-            attachTo = trans;
-        }
-
+        osg::Group* attachTo = visitor.mFoundNode ? visitor.mFoundNode : node;
         osg::ref_ptr<LightSource> lightSource = createLightSource(esmLight, lightMask, isExterior, osg::Vec4f(0,0,0,1));
         attachTo->addChild(lightSource);
         return lightSource;
diff --git a/components/sceneutil/lightutil.hpp b/components/sceneutil/lightutil.hpp
index 1ddfa3d45f..aabeec58ab 100644
--- a/components/sceneutil/lightutil.hpp
+++ b/components/sceneutil/lightutil.hpp
@@ -26,13 +26,12 @@ namespace SceneUtil
 
     /// @brief Convert an ESM::Light to a SceneUtil::LightSource, and add it to a sub graph.
     /// @note If the sub graph contains a node named "AttachLight" (case insensitive), then the light is added to that.
-    /// Otherwise, the light is added in the center of the node's bounds.
+    /// Otherwise, the light is attached directly to the root node of the subgraph.
     /// @param node The sub graph to add a light to
     /// @param esmLight The light definition coming from the game files containing radius, color, flicker, etc.
-    /// @param partsysMask Node mask to ignore when computing the sub graph's bounding box.
     /// @param lightMask Mask to assign to the newly created LightSource.
     /// @param isExterior Is the light outside? May be used for deciding which attenuation settings to use.
-    osg::ref_ptr<LightSource> addLight (osg::Group* node, const ESM::Light* esmLight, unsigned int partsysMask, unsigned int lightMask, bool isExterior);
+    osg::ref_ptr<LightSource> addLight (osg::Group* node, const ESM::Light* esmLight, unsigned int lightMask, bool isExterior);
 
     /// @brief Convert an ESM::Light to a SceneUtil::LightSource, and return it.
     /// @param esmLight The light definition coming from the game files containing radius, color, flicker, etc.